RE: [PATCH v4 05/11] rtc: isl1208: Make similar I2C and DT-based matching table

[Biju Das <biju.das.jz@xxxxxxxxxxxxxx>]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Friday, May 19, 2023 1:39 PM
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Cc: Alessandro Zummo <a.zummo@xxxxxxxxxxxx>; Alexandre Belloni
> <alexandre.belloni@xxxxxxxxxxx>; linux-rtc@xxxxxxxxxxxxxxx; Fabrizio
> Castro <fabrizio.castro.jz@xxxxxxxxxxx>; linux-renesas-
> soc@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
> > ---
> > v4:
> >  * New patch.
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> 
> > @@ -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