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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Biju.

On Fri, May 19, 2023 at 6:08 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > -----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;

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



-- 
Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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



[Index of Archives]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux