diff mbox series

i2c: mux: ltc4306: Simplify probe()

Message ID 20230716185108.283447-1-biju.das.jz@bp.renesas.com
State Superseded
Headers show
Series i2c: mux: ltc4306: Simplify probe() | expand

Commit Message

Biju Das July 16, 2023, 6:51 p.m. UTC
The ltc4306_id[].driver_data could store a pointer to the chips,
like for DT-based matching, making I2C and DT-based matching
more similar.

After that, we can simplify the probe() by replacing of_device_get_
match_data() and i2c_match_id() by i2c_get_match_data() as we have
similar I2C and DT-based matching table.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/i2c/muxes/i2c-mux-ltc4306.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Geert Uytterhoeven July 17, 2023, 9:16 a.m. UTC | #1
Hi Biju,

On Sun, Jul 16, 2023 at 8:51 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> The ltc4306_id[].driver_data could store a pointer to the chips,
> like for DT-based matching, making I2C and DT-based matching
> more similar.
>
> After that, we can simplify the probe() by replacing of_device_get_
> match_data() and i2c_match_id() by i2c_get_match_data() as we have
> similar I2C and DT-based matching table.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

A suggestion for improvement (which can be a separate patch, as it would
touch ltc4306_of_match[] too) below.

> --- a/drivers/i2c/muxes/i2c-mux-ltc4306.c
> +++ b/drivers/i2c/muxes/i2c-mux-ltc4306.c
> @@ -192,8 +192,8 @@ static int ltc4306_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
>  }
>
>  static const struct i2c_device_id ltc4306_id[] = {
> -       { "ltc4305", ltc_4305 },
> -       { "ltc4306", ltc_4306 },
> +       { "ltc4305", .driver_data = (kernel_ulong_t)&chips[ltc_4305] },
> +       { "ltc4306", .driver_data = (kernel_ulong_t)&chips[ltc_4306] },

As after this the ltc_type enum values are used only for hardcoded
indexing inside the chips array, you can get rid of them by splitting
the array in individual variables, shortening the lines above
(and in ltc4306_of_match[] below) in the process.

>         { }
>  };
>  MODULE_DEVICE_TABLE(i2c, ltc4306_id);

Gr{oetje,eeting}s,

                        Geert
Biju Das July 17, 2023, 9:20 a.m. UTC | #2
Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Monday, July 17, 2023 10:17 AM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Peter Rosin <peda@axentia.se>; Michael Hennerich
> <michael.hennerich@analog.com>; linux-i2c@vger.kernel.org; Prabhakar
> Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; linux-renesas-
> soc@vger.kernel.org
> Subject: Re: [PATCH] i2c: mux: ltc4306: Simplify probe()
> 
> Hi Biju,
> 
> On Sun, Jul 16, 2023 at 8:51 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > The ltc4306_id[].driver_data could store a pointer to the chips, like
> > for DT-based matching, making I2C and DT-based matching more similar.
> >
> > After that, we can simplify the probe() by replacing of_device_get_
> > match_data() and i2c_match_id() by i2c_get_match_data() as we have
> > similar I2C and DT-based matching table.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> A suggestion for improvement (which can be a separate patch, as it would
> touch ltc4306_of_match[] too) below.

OK.

> 
> > --- a/drivers/i2c/muxes/i2c-mux-ltc4306.c
> > +++ b/drivers/i2c/muxes/i2c-mux-ltc4306.c
> > @@ -192,8 +192,8 @@ static int ltc4306_deselect_mux(struct
> > i2c_mux_core *muxc, u32 chan)  }
> >
> >  static const struct i2c_device_id ltc4306_id[] = {
> > -       { "ltc4305", ltc_4305 },
> > -       { "ltc4306", ltc_4306 },
> > +       { "ltc4305", .driver_data = (kernel_ulong_t)&chips[ltc_4305]
> },
> > +       { "ltc4306", .driver_data = (kernel_ulong_t)&chips[ltc_4306]
> > + },
> 
> As after this the ltc_type enum values are used only for hardcoded
> indexing inside the chips array, you can get rid of them by splitting
> the array in individual variables, shortening the lines above (and in
> ltc4306_of_match[] below) in the process.

Agreed. Will send separate patch for this.

Cheers,
Biju
diff mbox series

Patch

diff --git a/drivers/i2c/muxes/i2c-mux-ltc4306.c b/drivers/i2c/muxes/i2c-mux-ltc4306.c
index 5a03031519be..c7dfd5eba413 100644
--- a/drivers/i2c/muxes/i2c-mux-ltc4306.c
+++ b/drivers/i2c/muxes/i2c-mux-ltc4306.c
@@ -192,8 +192,8 @@  static int ltc4306_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
 }
 
 static const struct i2c_device_id ltc4306_id[] = {
-	{ "ltc4305", ltc_4305 },
-	{ "ltc4306", ltc_4306 },
+	{ "ltc4305", .driver_data = (kernel_ulong_t)&chips[ltc_4305] },
+	{ "ltc4306", .driver_data = (kernel_ulong_t)&chips[ltc_4306] },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ltc4306_id);
@@ -216,10 +216,9 @@  static int ltc4306_probe(struct i2c_client *client)
 	unsigned int val = 0;
 	int num, ret;
 
-	chip = of_device_get_match_data(&client->dev);
-
+	chip = i2c_get_match_data(client);
 	if (!chip)
-		chip = &chips[i2c_match_id(ltc4306_id, client)->driver_data];
+		return -ENODEV;
 
 	idle_disc = device_property_read_bool(&client->dev,
 					      "i2c-mux-idle-disconnect");