[PATCH 0/2] Use i2c_get_match_data()

[PATCH 0/2] Use i2c_get_match_data()

[Biju Das]

Use i2c_get_match_data() to get match data for I2C, ACPI and
DT-based matching instead of device_get_match_data() the one for
ACPI/DT-based matching.

This patch series is only compile tested.

Biju Das (2):
  clk: vc5: Use i2c_get_match_data() instead of device_get_match_data()
  clk: vc7: Use i2c_get_match_data() instead of device_get_match_data()

 drivers/clk/clk-versaclock5.c | 4 +++-
 drivers/clk/clk-versaclock7.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.25.1

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

[Biju Das]

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.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
Note:
This patch is based on the work done for rtc-isl1208 and is compile
tested.
---
 drivers/clk/clk-versaclock5.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
index 8bc54176f325..4197991803ba 100644
--- 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;
 
 	vc5->pin_xin = devm_clk_get(&client->dev, "xin");
 	if (PTR_ERR(vc5->pin_xin) == -EPROBE_DEFER)
-- 
2.25.1

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

[Marek Vasut]

On 7/16/23 17:44, Biju Das 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.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Marek Vasut <marek.vasut+renesas@mailbox.org>

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

[Geert Uytterhoeven]

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.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

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

> --- 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.

>         vc5->pin_xin = devm_clk_get(&client->dev, "xin");
>         if (PTR_ERR(vc5->pin_xin) == -EPROBE_DEFER)

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

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

[Biju Das]

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.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > --- 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.

Cheers,
Biju

> 
> >         vc5->pin_xin = devm_clk_get(&client->dev, "xin");
> >         if (PTR_ERR(vc5->pin_xin) == -EPROBE_DEFER)
> 
> 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

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

[Luca Ceresoli]

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

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

[Biju Das]

Hi Luca,

Thanks for the feedback.

> -----Original Message-----
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Sent: Monday, July 17, 2023 1:56 PM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>; Michael Turquette
> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; linux-
> clk@vger.kernel.org; Geert Uytterhoeven <geert+renesas@glider.be>; Marek
> Vasut <marex@denx.de>; Alexander Helms <alexander.helms.jy@renesas.com>;
> Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; linux-
> renesas-soc@vger.kernel.org
> Subject: Re: [PATCH 1/2] clk: vc5: Use i2c_get_match_data() instead of
> device_get_match_data()
> 
> 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.

OK will remove error checking in next version.

Cheers,
Biju

[PATCH 2/2] clk: vc7: Use i2c_get_match_data() instead of device_get_match_data()

[Biju Das]

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.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
Note:
This patch is based on the work done for rtc-isl1208 and is compile
tested.
---
 drivers/clk/clk-versaclock7.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-versaclock7.c b/drivers/clk/clk-versaclock7.c
index 9babb7913c1c..be91aeada81c 100644
--- a/drivers/clk/clk-versaclock7.c
+++ b/drivers/clk/clk-versaclock7.c
@@ -1109,7 +1109,9 @@ static int vc7_probe(struct i2c_client *client)
 
 	i2c_set_clientdata(client, vc7);
 	vc7->client = client;
-	vc7->chip_info = device_get_match_data(&client->dev);
+	vc7->chip_info = i2c_get_match_data(client);
+	if (!vc7->chip_info)
+		return -ENODEV;
 
 	vc7->pin_xin = devm_clk_get(&client->dev, "xin");
 	if (PTR_ERR(vc7->pin_xin) == -EPROBE_DEFER) {
-- 
2.25.1

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

[Marek Vasut]

On 7/16/23 17:44, Biju Das 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.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Marek Vasut <marek.vasut+renesas@mailbox.org>

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

[Geert Uytterhoeven]

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.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

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

> --- a/drivers/clk/clk-versaclock7.c
> +++ b/drivers/clk/clk-versaclock7.c
> @@ -1109,7 +1109,9 @@ static int vc7_probe(struct i2c_client *client)
>
>         i2c_set_clientdata(client, vc7);
>         vc7->client = client;
> -       vc7->chip_info = device_get_match_data(&client->dev);
> +       vc7->chip_info = i2c_get_match_data(client);
> +       if (!vc7->chip_info)
> +               return -ENODEV;

Can this actually fail? All tables have data pointers.

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

RE: [PATCH 2/2] clk: vc7: Use i2c_get_match_data() instead of device_get_match_data()

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 2/2] clk: vc7: 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.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > --- a/drivers/clk/clk-versaclock7.c
> > +++ b/drivers/clk/clk-versaclock7.c
> > @@ -1109,7 +1109,9 @@ static int vc7_probe(struct i2c_client *client)
> >
> >         i2c_set_clientdata(client, vc7);
> >         vc7->client = client;
> > -       vc7->chip_info = device_get_match_data(&client->dev);
> > +       vc7->chip_info = i2c_get_match_data(client);
> > +       if (!vc7->chip_info)
> > +               return -ENODEV;
> 
> Can this actually fail? 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.

Cheers,
Biju