Re: [PATCH 1/2] clk: vc5: Use i2c_get_match_data() instead of device_get_match_data()

[Luca Ceresoli <luca.ceresoli@bootlin.com>]

Hello Biju,

On Mon, 17 Jul 2023 07:46:34 +0000
Biju Das <biju.das.jz@bp.renesas.com> wrote:

> Hi Geert,
> 
> Thanks for the review.
> 
> > Subject: Re: [PATCH 1/2] clk: vc5: Use i2c_get_match_data() instead of
> > device_get_match_data()
> > 
> > Hi Biju,
> > 
> > On Sun, Jul 16, 2023 at 5:44 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:  
> > > The device_get_match_data(), is to get match data for firmware
> > > interfaces such as just OF/ACPI. This driver has I2C matching table as
> > > well. Use
> > > i2c_get_match_data() to get match data for I2C, ACPI and DT-based
> > > matching.

Good point, thanks!

> > > --- a/drivers/clk/clk-versaclock5.c
> > > +++ b/drivers/clk/clk-versaclock5.c
> > > @@ -956,7 +956,9 @@ static int vc5_probe(struct i2c_client *client)
> > >
> > >         i2c_set_clientdata(client, vc5);
> > >         vc5->client = client;
> > > -       vc5->chip_info = device_get_match_data(&client->dev);
> > > +       vc5->chip_info = i2c_get_match_data(client);
> > > +       if (!vc5->chip_info)
> > > +               return -ENODEV;  
> > 
> > Can this actually happen? All tables have data pointers.  
> 
> It is not needed. I just want to avoid people sending
> patches as this function can return NULL, so add a check.
> 
> Please let me know, whether I should remove this?
> I am happy to send V2 taking out this check.

I cannot foresee any sensible future use case for adding an entry
without a data pointer as the whole driver is now heavily based on this
data to handle so many variants. Also, the error checking did not exist
before and the i2c match table is not introducing anything new in terms
of .driver_data values.

Thus I vote for not adding any error checking here.

Otherwise looks good.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com