| Message ID | 20230518113643.420806-6-biju.das.jz@bp.renesas.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add Renesas PMIC RAA215300 and built-in RTC support | expand |
On Thu, May 18, 2023 at 1:37 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > The isl1208_id[].driver_data could store a pointer to the config, > like for DT-based matching, making I2C and DT-based matching > more similar. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v4: > * New patch. Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > @@ -822,9 +822,9 @@ isl1208_probe(struct i2c_client *client) > } else { > const struct i2c_device_id *id = i2c_match_id(isl1208_id, client); > > - if (id->driver_data >= ISL_LAST_ID) > + if (!id) > return -ENODEV; > - isl1208->config = &isl1208_configs[id->driver_data]; > + isl1208->config = (struct isl1208_config *)id->driver_data; It's a pity there's no i2c_get_match_data() yet... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: Friday, May 19, 2023 1:39 PM > To: Biju Das <biju.das.jz@bp.renesas.com> > Cc: Alessandro Zummo <a.zummo@towertech.it>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; linux-rtc@vger.kernel.org; Fabrizio > Castro <fabrizio.castro.jz@renesas.com>; linux-renesas- > soc@vger.kernel.org > Subject: Re: [PATCH v4 05/11] rtc: isl1208: Make similar I2C and DT- > based matching table > > On Thu, May 18, 2023 at 1:37 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > The isl1208_id[].driver_data could store a pointer to the config, like > > for DT-based matching, making I2C and DT-based matching more similar. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v4: > > * New patch. > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > @@ -822,9 +822,9 @@ isl1208_probe(struct i2c_client *client) > > } else { > > const struct i2c_device_id *id = > > i2c_match_id(isl1208_id, client); > > > > - if (id->driver_data >= ISL_LAST_ID) > > + if (!id) > > return -ENODEV; > > - isl1208->config = &isl1208_configs[id->driver_data]; > > + isl1208->config = (struct isl1208_config > > + *)id->driver_data; > > It's a pity there's no i2c_get_match_data() yet... You mean, introduce [1] and optimize ?? if (client->dev.of_node) isl1208->config = of_device_get_match_data(&client->dev); else isl1208->config = i2c_get_match_data(isl1208_id, client); if (!isl1208->config) return -ENODEV; [1] const void * i2c_get_match_data(const struct i2c_device_id *id, const struct i2c_client *client) { if (!(id && client)) return NULL; while (id->name[0]) { if (strcmp(client->name, id->name) == 0) return id->driver_data; id++; } return NULL; } EXPORT_SYMBOL(i2c_get_match_data); Cheers, Biju
+ Wolfram > -----Original Message----- > From: Biju Das <biju.das.jz@bp.renesas.com> > Sent: Friday, May 19, 2023 5:09 PM > To: Geert Uytterhoeven <geert@linux-m68k.org> > Cc: Alessandro Zummo <a.zummo@towertech.it>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; linux-rtc@vger.kernel.org; Fabrizio > Castro <fabrizio.castro.jz@renesas.com>; linux-renesas- > soc@vger.kernel.org > Subject: RE: [PATCH v4 05/11] rtc: isl1208: Make similar I2C and DT- > based matching table > > Hi Geert, > > Thanks for the feedback. > > > -----Original Message----- > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > Sent: Friday, May 19, 2023 1:39 PM > > To: Biju Das <biju.das.jz@bp.renesas.com> > > Cc: Alessandro Zummo <a.zummo@towertech.it>; Alexandre Belloni > > <alexandre.belloni@bootlin.com>; linux-rtc@vger.kernel.org; Fabrizio > > Castro <fabrizio.castro.jz@renesas.com>; linux-renesas- > > soc@vger.kernel.org > > Subject: Re: [PATCH v4 05/11] rtc: isl1208: Make similar I2C and DT- > > based matching table > > > > On Thu, May 18, 2023 at 1:37 PM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > The isl1208_id[].driver_data could store a pointer to the config, > > > like for DT-based matching, making I2C and DT-based matching more > similar. > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- > > > v4: > > > * New patch. > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > @@ -822,9 +822,9 @@ isl1208_probe(struct i2c_client *client) > > > } else { > > > const struct i2c_device_id *id = > > > i2c_match_id(isl1208_id, client); > > > > > > - if (id->driver_data >= ISL_LAST_ID) > > > + if (!id) > > > return -ENODEV; > > > - isl1208->config = &isl1208_configs[id->driver_data]; > > > + isl1208->config = (struct isl1208_config > > > + *)id->driver_data; > > > > It's a pity there's no i2c_get_match_data() yet... > > You mean, introduce [1] and optimize ?? > > if (client->dev.of_node) > isl1208->config = of_device_get_match_data(&client->dev); > else > isl1208->config = i2c_get_match_data(isl1208_id, client); > > if (!isl1208->config) > return -ENODEV; > > [1] > const void * i2c_get_match_data(const struct i2c_device_id *id, const > struct i2c_client *client) { > if (!(id && client)) > return NULL; > > while (id->name[0]) { > if (strcmp(client->name, id->name) == 0) > return id->driver_data; > id++; > } > return NULL; > } > EXPORT_SYMBOL(i2c_get_match_data); > > Cheers, > Biju
Hi Biju. On Fri, May 19, 2023 at 6:08 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > -----Original Message----- > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > Sent: Friday, May 19, 2023 1:39 PM > > To: Biju Das <biju.das.jz@bp.renesas.com> > > Cc: Alessandro Zummo <a.zummo@towertech.it>; Alexandre Belloni > > <alexandre.belloni@bootlin.com>; linux-rtc@vger.kernel.org; Fabrizio > > Castro <fabrizio.castro.jz@renesas.com>; linux-renesas- > > soc@vger.kernel.org > > Subject: Re: [PATCH v4 05/11] rtc: isl1208: Make similar I2C and DT- > > based matching table > > > > On Thu, May 18, 2023 at 1:37 PM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > The isl1208_id[].driver_data could store a pointer to the config, like > > > for DT-based matching, making I2C and DT-based matching more similar. > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- > > > v4: > > > * New patch. > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > @@ -822,9 +822,9 @@ isl1208_probe(struct i2c_client *client) > > > } else { > > > const struct i2c_device_id *id = > > > i2c_match_id(isl1208_id, client); > > > > > > - if (id->driver_data >= ISL_LAST_ID) > > > + if (!id) > > > return -ENODEV; > > > - isl1208->config = &isl1208_configs[id->driver_data]; > > > + isl1208->config = (struct isl1208_config > > > + *)id->driver_data; > > > > It's a pity there's no i2c_get_match_data() yet... > > You mean, introduce [1] and optimize ?? > > if (client->dev.of_node) > isl1208->config = of_device_get_match_data(&client->dev); > else > isl1208->config = i2c_get_match_data(isl1208_id, client); > > if (!isl1208->config) > return -ENODEV; Exactly. Might be better to do that later, to avoid stalling this series. > > [1] > const void * i2c_get_match_data(const struct i2c_device_id *id, const struct i2c_client *client) > { > if (!(id && client)) > return NULL; > > while (id->name[0]) { > if (strcmp(client->name, id->name) == 0) > return id->driver_data; > id++; > } > return NULL; Please reuse the existing i2c_match_id(), just like of_device_get_match_data() reuses of_match_device(). > } > EXPORT_SYMBOL(i2c_get_match_data); > > Cheers, > Biju
Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: Friday, May 19, 2023 8:43 PM > To: Biju Das <biju.das.jz@bp.renesas.com> > Cc: Alessandro Zummo <a.zummo@towertech.it>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; linux-rtc@vger.kernel.org; Fabrizio > Castro <fabrizio.castro.jz@renesas.com>; linux-renesas- > soc@vger.kernel.org > Subject: Re: [PATCH v4 05/11] rtc: isl1208: Make similar I2C and DT- > based matching table > > Hi Biju. > > On Fri, May 19, 2023 at 6:08 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > > -----Original Message----- > > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > > Sent: Friday, May 19, 2023 1:39 PM > > > To: Biju Das <biju.das.jz@bp.renesas.com> > > > Cc: Alessandro Zummo <a.zummo@towertech.it>; Alexandre Belloni > > > <alexandre.belloni@bootlin.com>; linux-rtc@vger.kernel.org; Fabrizio > > > Castro <fabrizio.castro.jz@renesas.com>; linux-renesas- > > > soc@vger.kernel.org > > > Subject: Re: [PATCH v4 05/11] rtc: isl1208: Make similar I2C and DT- > > > based matching table > > > > > > On Thu, May 18, 2023 at 1:37 PM Biju Das > > > <biju.das.jz@bp.renesas.com> > > > wrote: > > > > The isl1208_id[].driver_data could store a pointer to the config, > > > > like for DT-based matching, making I2C and DT-based matching more > similar. > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > --- > > > > v4: > > > > * New patch. > > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > > @@ -822,9 +822,9 @@ isl1208_probe(struct i2c_client *client) > > > > } else { > > > > const struct i2c_device_id *id = > > > > i2c_match_id(isl1208_id, client); > > > > > > > > - if (id->driver_data >= ISL_LAST_ID) > > > > + if (!id) > > > > return -ENODEV; > > > > - isl1208->config = &isl1208_configs[id- > >driver_data]; > > > > + isl1208->config = (struct isl1208_config > > > > + *)id->driver_data; > > > > > > It's a pity there's no i2c_get_match_data() yet... > > > > You mean, introduce [1] and optimize ?? > > > > if (client->dev.of_node) > > isl1208->config = of_device_get_match_data(&client- > >dev); > > else > > isl1208->config = i2c_get_match_data(isl1208_id, > > client); > > > > if (!isl1208->config) > > return -ENODEV; > > Exactly. Might be better to do that later, to avoid stalling this > series. OK, will do it later. > > > > > [1] > > const void * i2c_get_match_data(const struct i2c_device_id *id, const > > struct i2c_client *client) { > > if (!(id && client)) > > return NULL; > > > > while (id->name[0]) { > > if (strcmp(client->name, id->name) == 0) > > return id->driver_data; > > id++; > > } > > return NULL; > > Please reuse the existing i2c_match_id(), just like > of_device_get_match_data() reuses of_match_device(). OK, Will send this as standalone patch, as it is enhancement. Cheers, Biju > > > } > > EXPORT_SYMBOL(i2c_get_match_data); > > > > Cheers, > > Biju > > > > -- > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > In personal conversations with technical people, I call myself a hacker. > But when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds
diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c index a73eb78b8a40..a6a133f719df 100644 --- a/drivers/rtc/rtc-isl1208.c +++ b/drivers/rtc/rtc-isl1208.c @@ -90,10 +90,10 @@ static const struct isl1208_config { }; static const struct i2c_device_id isl1208_id[] = { - { "isl1208", TYPE_ISL1208 }, - { "isl1209", TYPE_ISL1209 }, - { "isl1218", TYPE_ISL1218 }, - { "isl1219", TYPE_ISL1219 }, + { "isl1208", .driver_data = (unsigned long)&isl1208_configs[TYPE_ISL1208] }, + { "isl1209", .driver_data = (unsigned long)&isl1208_configs[TYPE_ISL1209] }, + { "isl1218", .driver_data = (unsigned long)&isl1208_configs[TYPE_ISL1218] }, + { "isl1219", .driver_data = (unsigned long)&isl1208_configs[TYPE_ISL1219] }, { } }; MODULE_DEVICE_TABLE(i2c, isl1208_id); @@ -822,9 +822,9 @@ isl1208_probe(struct i2c_client *client) } else { const struct i2c_device_id *id = i2c_match_id(isl1208_id, client); - if (id->driver_data >= ISL_LAST_ID) + if (!id) return -ENODEV; - isl1208->config = &isl1208_configs[id->driver_data]; + isl1208->config = (struct isl1208_config *)id->driver_data; } isl1208->rtc = devm_rtc_allocate_device(&client->dev);
The isl1208_id[].driver_data could store a pointer to the config, like for DT-based matching, making I2C and DT-based matching more similar. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v4: * New patch. --- drivers/rtc/rtc-isl1208.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)