Hi!
2023-07-17 at 15:48, Biju Das wrote:
> Drop enum ltc_type and split the array chips[] as individual
> variables, and make lines shorter by referring to e.g. <c_4305_chip
What do you mean "make lines shorter"???
By a few percent or something (2 chars). When your previous patch made
those same lines more than 2.5 times as long! I have to say, going
from 24 to 62 isn't exactly making lines shorter...
Not that I care deeply about line length well below 80, but if one of
the selling points is line length you should present a patch series
that actually make lines shorter instead of longer. However, the need
to sell a patch on shorter line length when the lines are actually
made longer by the series is a sign that something is not right.
So, again, I fail to see the point of patches like this.
Also again, I think it is bad to have a named field for .driver_data
but not for the other member of the struct i2c_device_id inits. Not
that it matters much when the whole exercise is pointless.
Also again, what you need to present to get me on board for patches
like this is to somehow make it *one* table with driver data. Then
it makes sense to force all driver data to have the same value. But
as I see no road map for unifying the tables for driver data there
is simply no point to the churn...
Cheers,
Peter
> instead of &chips[ltc_4305].
>
> Suggested-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> v2:
> * New patch
> ---
> drivers/i2c/muxes/i2c-mux-ltc4306.c | 28 +++++++++++-----------------
> 1 file changed, 11 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-ltc4306.c b/drivers/i2c/muxes/i2c-mux-ltc4306.c
> index c7dfd5eba413..c4f090e8d6db 100644
> --- a/drivers/i2c/muxes/i2c-mux-ltc4306.c
> +++ b/drivers/i2c/muxes/i2c-mux-ltc4306.c
> @@ -34,11 +34,6 @@
> #define LTC_GPIO_ALL_INPUT 0xC0
> #define LTC_SWITCH_MASK 0xF0
>
> -enum ltc_type {
> - ltc_4305,
> - ltc_4306,
> -};
> -
> struct chip_desc {
> u8 nchans;
> u8 num_gpios;
> @@ -50,14 +45,13 @@ struct ltc4306 {
> const struct chip_desc *chip;
> };
>
> -static const struct chip_desc chips[] = {
> - [ltc_4305] = {
> - .nchans = LTC4305_MAX_NCHANS,
> - },
> - [ltc_4306] = {
> - .nchans = LTC4306_MAX_NCHANS,
> - .num_gpios = 2,
> - },
> +static const struct chip_desc ltc_4305_chip = {
> + .nchans = LTC4305_MAX_NCHANS
> +};
> +
> +static const struct chip_desc ltc_4306_chip = {
> + .nchans = LTC4306_MAX_NCHANS,
> + .num_gpios = 2
> };
>
> static bool ltc4306_is_volatile_reg(struct device *dev, unsigned int reg)
> @@ -192,15 +186,15 @@ static int ltc4306_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
> }
>
> static const struct i2c_device_id ltc4306_id[] = {
> - { "ltc4305", .driver_data = (kernel_ulong_t)&chips[ltc_4305] },
> - { "ltc4306", .driver_data = (kernel_ulong_t)&chips[ltc_4306] },
> + { "ltc4305", .driver_data = (kernel_ulong_t)<c_4305_chip },
> + { "ltc4306", .driver_data = (kernel_ulong_t)<c_4306_chip },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, ltc4306_id);
>
> static const struct of_device_id ltc4306_of_match[] = {
> - { .compatible = "lltc,ltc4305", .data = &chips[ltc_4305] },
> - { .compatible = "lltc,ltc4306", .data = &chips[ltc_4306] },
> + { .compatible = "lltc,ltc4305", .data = <c_4305_chip },
> + { .compatible = "lltc,ltc4306", .data = <c_4306_chip },
> { }
> };
> MODULE_DEVICE_TABLE(of, ltc4306_of_match);
