* Re: [PATCH 01/10] leds: pca953x: Use of_device_get_match_data()
2019-10-04 21:43 ` [PATCH 01/10] leds: pca953x: Use of_device_get_match_data() Stephen Boyd
@ 2019-10-06 12:15 ` Jacek Anaszewski
2019-10-07 8:01 ` Geert Uytterhoeven
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Jacek Anaszewski @ 2019-10-06 12:15 UTC (permalink / raw)
To: Stephen Boyd, linux-kernel
Cc: Arnd Bergmann, Geert Uytterhoeven, Riku Voipio, Rob Herring,
Frank Rowand, Pavel Machek, Dan Murphy, linux-leds
On 10/4/19 11:43 PM, Stephen Boyd wrote:
> This driver can use the of_device_get_match_data() API to simplify the
> code. Replace calls to of_match_device() with this newer API under the
> assumption that where it is called will be when we know the device is
> backed by a DT node. This nicely avoids referencing the match table when
> it is undefined with configurations where CONFIG_OF=n.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Riku Voipio <riku.voipio@iki.fi>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: <linux-leds@vger.kernel.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
> Please ack or pick for immediate merge so the last patch can be merged.
>
> drivers/leds/leds-pca9532.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
> index c7c7199e8ebd..7d515d5e57bd 100644
> --- a/drivers/leds/leds-pca9532.c
> +++ b/drivers/leds/leds-pca9532.c
> @@ -467,16 +467,11 @@ pca9532_of_populate_pdata(struct device *dev, struct device_node *np)
> {
> struct pca9532_platform_data *pdata;
> struct device_node *child;
> - const struct of_device_id *match;
> int devid, maxleds;
> int i = 0;
> const char *state;
>
> - match = of_match_device(of_pca9532_leds_match, dev);
> - if (!match)
> - return ERR_PTR(-ENODEV);
> -
> - devid = (int)(uintptr_t)match->data;
> + devid = (int)(uintptr_t)of_device_get_match_data(dev);
> maxleds = pca9532_chip_info_tbl[devid].num_leds;
>
> pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> @@ -509,7 +504,6 @@ static int pca9532_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> int devid;
> - const struct of_device_id *of_id;
> struct pca9532_data *data = i2c_get_clientdata(client);
> struct pca9532_platform_data *pca9532_pdata =
> dev_get_platdata(&client->dev);
> @@ -525,11 +519,7 @@ static int pca9532_probe(struct i2c_client *client,
> dev_err(&client->dev, "no platform data\n");
> return -EINVAL;
> }
> - of_id = of_match_device(of_pca9532_leds_match,
> - &client->dev);
> - if (unlikely(!of_id))
> - return -EINVAL;
> - devid = (int)(uintptr_t) of_id->data;
> + devid = (int)(uintptr_t)of_device_get_match_data(&client->dev);
> } else {
> devid = id->driver_data;
> }
>
Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 01/10] leds: pca953x: Use of_device_get_match_data()
2019-10-04 21:43 ` [PATCH 01/10] leds: pca953x: Use of_device_get_match_data() Stephen Boyd
2019-10-06 12:15 ` Jacek Anaszewski
@ 2019-10-07 8:01 ` Geert Uytterhoeven
2019-10-13 12:10 ` Pavel Machek
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2019-10-07 8:01 UTC (permalink / raw)
To: Stephen Boyd
Cc: Linux Kernel Mailing List, Arnd Bergmann, Riku Voipio,
Rob Herring, Frank Rowand, Jacek Anaszewski, Pavel Machek,
Dan Murphy, linux-leds
On Fri, Oct 4, 2019 at 11:43 PM Stephen Boyd <swboyd@chromium.org> wrote:
> This driver can use the of_device_get_match_data() API to simplify the
> code. Replace calls to of_match_device() with this newer API under the
> assumption that where it is called will be when we know the device is
> backed by a DT node. This nicely avoids referencing the match table when
> it is undefined with configurations where CONFIG_OF=n.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Riku Voipio <riku.voipio@iki.fi>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: <linux-leds@vger.kernel.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 01/10] leds: pca953x: Use of_device_get_match_data()
2019-10-04 21:43 ` [PATCH 01/10] leds: pca953x: Use of_device_get_match_data() Stephen Boyd
2019-10-06 12:15 ` Jacek Anaszewski
2019-10-07 8:01 ` Geert Uytterhoeven
@ 2019-10-13 12:10 ` Pavel Machek
2019-10-14 17:50 ` Andy Shevchenko
2019-11-04 9:09 ` Pavel Machek
4 siblings, 0 replies; 11+ messages in thread
From: Pavel Machek @ 2019-10-13 12:10 UTC (permalink / raw)
To: Stephen Boyd
Cc: linux-kernel, Arnd Bergmann, Geert Uytterhoeven, Riku Voipio,
Rob Herring, Frank Rowand, Jacek Anaszewski, Dan Murphy,
linux-leds
[-- Attachment #1: Type: text/plain, Size: 2789 bytes --]
On Fri 2019-10-04 14:43:25, Stephen Boyd wrote:
> This driver can use the of_device_get_match_data() API to simplify the
> code. Replace calls to of_match_device() with this newer API under the
> assumption that where it is called will be when we know the device is
> backed by a DT node. This nicely avoids referencing the match table when
> it is undefined with configurations where CONFIG_OF=n.
Thanks, applied.
Pavel
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Riku Voipio <riku.voipio@iki.fi>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: <linux-leds@vger.kernel.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
> Please ack or pick for immediate merge so the last patch can be merged.
>
> drivers/leds/leds-pca9532.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
> index c7c7199e8ebd..7d515d5e57bd 100644
> --- a/drivers/leds/leds-pca9532.c
> +++ b/drivers/leds/leds-pca9532.c
> @@ -467,16 +467,11 @@ pca9532_of_populate_pdata(struct device *dev, struct device_node *np)
> {
> struct pca9532_platform_data *pdata;
> struct device_node *child;
> - const struct of_device_id *match;
> int devid, maxleds;
> int i = 0;
> const char *state;
>
> - match = of_match_device(of_pca9532_leds_match, dev);
> - if (!match)
> - return ERR_PTR(-ENODEV);
> -
> - devid = (int)(uintptr_t)match->data;
> + devid = (int)(uintptr_t)of_device_get_match_data(dev);
> maxleds = pca9532_chip_info_tbl[devid].num_leds;
>
> pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> @@ -509,7 +504,6 @@ static int pca9532_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> int devid;
> - const struct of_device_id *of_id;
> struct pca9532_data *data = i2c_get_clientdata(client);
> struct pca9532_platform_data *pca9532_pdata =
> dev_get_platdata(&client->dev);
> @@ -525,11 +519,7 @@ static int pca9532_probe(struct i2c_client *client,
> dev_err(&client->dev, "no platform data\n");
> return -EINVAL;
> }
> - of_id = of_match_device(of_pca9532_leds_match,
> - &client->dev);
> - if (unlikely(!of_id))
> - return -EINVAL;
> - devid = (int)(uintptr_t) of_id->data;
> + devid = (int)(uintptr_t)of_device_get_match_data(&client->dev);
> } else {
> devid = id->driver_data;
> }
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 01/10] leds: pca953x: Use of_device_get_match_data()
2019-10-04 21:43 ` [PATCH 01/10] leds: pca953x: Use of_device_get_match_data() Stephen Boyd
` (2 preceding siblings ...)
2019-10-13 12:10 ` Pavel Machek
@ 2019-10-14 17:50 ` Andy Shevchenko
2019-10-14 20:54 ` Stephen Boyd
2019-11-04 9:09 ` Pavel Machek
4 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2019-10-14 17:50 UTC (permalink / raw)
To: Stephen Boyd, Wolfram Sang
Cc: Linux Kernel Mailing List, Arnd Bergmann, Geert Uytterhoeven,
Riku Voipio, Rob Herring, Frank Rowand, Jacek Anaszewski,
Pavel Machek, Dan Murphy, Linux LED Subsystem
On Sat, Oct 5, 2019 at 12:47 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> This driver can use the of_device_get_match_data() API to simplify the
> code. Replace calls to of_match_device() with this newer API under the
> assumption that where it is called will be when we know the device is
> backed by a DT node. This nicely avoids referencing the match table when
> it is undefined with configurations where CONFIG_OF=n.
> + devid = (int)(uintptr_t)of_device_get_match_data(dev);
> + devid = (int)(uintptr_t)of_device_get_match_data(&client->dev);
This still leaves it OF-centric.
Better to use device_get_match_data().
Also, I'm thinking that following may help to clean a lot of the i2c
client drivers
static inline // perhaps no
const void *i2c_device_get_match_data(struct i2c_client *client, const
struct i2c_device_id *id)
{
if (id)
return (const void *)id->driver_data;
return device_get_match_data(&client->dev);
}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 01/10] leds: pca953x: Use of_device_get_match_data()
2019-10-14 17:50 ` Andy Shevchenko
@ 2019-10-14 20:54 ` Stephen Boyd
2019-10-15 9:02 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Stephen Boyd @ 2019-10-14 20:54 UTC (permalink / raw)
To: Andy Shevchenko, Wolfram Sang
Cc: Linux Kernel Mailing List, Arnd Bergmann, Geert Uytterhoeven,
Riku Voipio, Rob Herring, Frank Rowand, Jacek Anaszewski,
Pavel Machek, Dan Murphy, Linux LED Subsystem
Quoting Andy Shevchenko (2019-10-14 10:50:06)
> On Sat, Oct 5, 2019 at 12:47 AM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > This driver can use the of_device_get_match_data() API to simplify the
> > code. Replace calls to of_match_device() with this newer API under the
> > assumption that where it is called will be when we know the device is
> > backed by a DT node. This nicely avoids referencing the match table when
> > it is undefined with configurations where CONFIG_OF=n.
>
> > + devid = (int)(uintptr_t)of_device_get_match_data(dev);
>
> > + devid = (int)(uintptr_t)of_device_get_match_data(&client->dev);
>
> This still leaves it OF-centric.
> Better to use device_get_match_data().
>
> Also, I'm thinking that following may help to clean a lot of the i2c
> client drivers
>
> static inline // perhaps no
> const void *i2c_device_get_match_data(struct i2c_client *client, const
> struct i2c_device_id *id)
> {
> if (id)
> return (const void *)id->driver_data;
> return device_get_match_data(&client->dev);
> }
>
Looks alright to me. Maybe device_get_match_data() can look at the bus
and call some bus op if the firmware match isn't present? Then we can
replace a bunch of these calls with device_get_match_data() and it will
"do the right thing" regardless of what bus or firmware the device is
running on.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 01/10] leds: pca953x: Use of_device_get_match_data()
2019-10-14 20:54 ` Stephen Boyd
@ 2019-10-15 9:02 ` Andy Shevchenko
2019-10-15 22:41 ` Stephen Boyd
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2019-10-15 9:02 UTC (permalink / raw)
To: Stephen Boyd
Cc: Wolfram Sang, Linux Kernel Mailing List, Arnd Bergmann,
Geert Uytterhoeven, Riku Voipio, Rob Herring, Frank Rowand,
Jacek Anaszewski, Pavel Machek, Dan Murphy, Linux LED Subsystem
On Mon, Oct 14, 2019 at 11:54 PM Stephen Boyd <swboyd@chromium.org> wrote:
> Quoting Andy Shevchenko (2019-10-14 10:50:06)
> > On Sat, Oct 5, 2019 at 12:47 AM Stephen Boyd <swboyd@chromium.org> wrote:
> > >
> > > This driver can use the of_device_get_match_data() API to simplify the
> > > code. Replace calls to of_match_device() with this newer API under the
> > > assumption that where it is called will be when we know the device is
> > > backed by a DT node. This nicely avoids referencing the match table when
> > > it is undefined with configurations where CONFIG_OF=n.
> >
> > > + devid = (int)(uintptr_t)of_device_get_match_data(dev);
> >
> > > + devid = (int)(uintptr_t)of_device_get_match_data(&client->dev);
> >
> > This still leaves it OF-centric.
> > Better to use device_get_match_data().
> >
> > Also, I'm thinking that following may help to clean a lot of the i2c
> > client drivers
> >
> > static inline // perhaps no
> > const void *i2c_device_get_match_data(struct i2c_client *client, const
> > struct i2c_device_id *id)
> > {
> > if (id)
> > return (const void *)id->driver_data;
> > return device_get_match_data(&client->dev);
> > }
> >
>
> Looks alright to me. Maybe device_get_match_data() can look at the bus
> and call some bus op if the firmware match isn't present? Then we can
> replace a bunch of these calls with device_get_match_data() and it will
> "do the right thing" regardless of what bus or firmware the device is
> running on.
It will be something ugly like
buses {
#ifdef I2C
&i2c_bus_type,
#endif
...
}
in the code. I won't do this.
See generic_match_buses[] for example.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 01/10] leds: pca953x: Use of_device_get_match_data()
2019-10-15 9:02 ` Andy Shevchenko
@ 2019-10-15 22:41 ` Stephen Boyd
2019-10-16 13:46 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Stephen Boyd @ 2019-10-15 22:41 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wolfram Sang, Linux Kernel Mailing List, Arnd Bergmann,
Geert Uytterhoeven, Riku Voipio, Rob Herring, Frank Rowand,
Jacek Anaszewski, Pavel Machek, Dan Murphy, Linux LED Subsystem
Quoting Andy Shevchenko (2019-10-15 02:02:01)
> On Mon, Oct 14, 2019 at 11:54 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > Quoting Andy Shevchenko (2019-10-14 10:50:06)
> > > On Sat, Oct 5, 2019 at 12:47 AM Stephen Boyd <swboyd@chromium.org> wrote:
> > > >
> > > > This driver can use the of_device_get_match_data() API to simplify the
> > > > code. Replace calls to of_match_device() with this newer API under the
> > > > assumption that where it is called will be when we know the device is
> > > > backed by a DT node. This nicely avoids referencing the match table when
> > > > it is undefined with configurations where CONFIG_OF=n.
> > >
> > > > + devid = (int)(uintptr_t)of_device_get_match_data(dev);
> > >
> > > > + devid = (int)(uintptr_t)of_device_get_match_data(&client->dev);
> > >
> > > This still leaves it OF-centric.
> > > Better to use device_get_match_data().
> > >
> > > Also, I'm thinking that following may help to clean a lot of the i2c
> > > client drivers
> > >
> > > static inline // perhaps no
> > > const void *i2c_device_get_match_data(struct i2c_client *client, const
> > > struct i2c_device_id *id)
> > > {
> > > if (id)
> > > return (const void *)id->driver_data;
> > > return device_get_match_data(&client->dev);
> > > }
> > >
> >
> > Looks alright to me. Maybe device_get_match_data() can look at the bus
> > and call some bus op if the firmware match isn't present? Then we can
> > replace a bunch of these calls with device_get_match_data() and it will
> > "do the right thing" regardless of what bus or firmware the device is
> > running on.
>
> It will be something ugly like
>
> buses {
> #ifdef I2C
> &i2c_bus_type,
> #endif
> ...
> }
>
> in the code. I won't do this.
>
> See generic_match_buses[] for example.
Why is it like generic_match_buses[]? I thought it would look at
struct device::of_node or struct device::fw_node and try to extract
device data out that and if that fails it would fallback to some new
function like struct bus_type::get_match_data() that does the right
thing for the bus. In the case of i2c it would extract the i2c_client's
i2c_device_id pointer and return it onwards.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 01/10] leds: pca953x: Use of_device_get_match_data()
2019-10-15 22:41 ` Stephen Boyd
@ 2019-10-16 13:46 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2019-10-16 13:46 UTC (permalink / raw)
To: Stephen Boyd
Cc: Wolfram Sang, Linux Kernel Mailing List, Arnd Bergmann,
Geert Uytterhoeven, Riku Voipio, Rob Herring, Frank Rowand,
Jacek Anaszewski, Pavel Machek, Dan Murphy, Linux LED Subsystem
On Wed, Oct 16, 2019 at 1:42 AM Stephen Boyd <swboyd@chromium.org> wrote:
> Quoting Andy Shevchenko (2019-10-15 02:02:01)
> > On Mon, Oct 14, 2019 at 11:54 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > > Quoting Andy Shevchenko (2019-10-14 10:50:06)
> > > > On Sat, Oct 5, 2019 at 12:47 AM Stephen Boyd <swboyd@chromium.org> wrote:
> > > > Also, I'm thinking that following may help to clean a lot of the i2c
> > > > client drivers
> > > >
> > > > static inline // perhaps no
> > > > const void *i2c_device_get_match_data(struct i2c_client *client, const
> > > > struct i2c_device_id *id)
> > > > {
> > > > if (id)
> > > > return (const void *)id->driver_data;
> > > > return device_get_match_data(&client->dev);
> > > > }
> > > >
> > >
> > > Looks alright to me. Maybe device_get_match_data() can look at the bus
> > > and call some bus op if the firmware match isn't present? Then we can
> > > replace a bunch of these calls with device_get_match_data() and it will
> > > "do the right thing" regardless of what bus or firmware the device is
> > > running on.
> >
> > It will be something ugly like
> >
> > buses {
> > #ifdef I2C
> > &i2c_bus_type,
> > #endif
> > ...
> > }
> >
> > in the code. I won't do this.
> >
> > See generic_match_buses[] for example.
>
> Why is it like generic_match_buses[]? I thought it would look at
> struct device::of_node or struct device::fw_node and try to extract
> device data out that and if that fails it would fallback to some new
> function like struct bus_type::get_match_data() that does the right
> thing for the bus. In the case of i2c it would extract the i2c_client's
> i2c_device_id pointer and return it onwards.
Can you send a patch for review?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 01/10] leds: pca953x: Use of_device_get_match_data()
2019-10-04 21:43 ` [PATCH 01/10] leds: pca953x: Use of_device_get_match_data() Stephen Boyd
` (3 preceding siblings ...)
2019-10-14 17:50 ` Andy Shevchenko
@ 2019-11-04 9:09 ` Pavel Machek
4 siblings, 0 replies; 11+ messages in thread
From: Pavel Machek @ 2019-11-04 9:09 UTC (permalink / raw)
To: Stephen Boyd
Cc: linux-kernel, Arnd Bergmann, Geert Uytterhoeven, Riku Voipio,
Rob Herring, Frank Rowand, Jacek Anaszewski, Dan Murphy,
linux-leds
[-- Attachment #1: Type: text/plain, Size: 751 bytes --]
Hi!
> This driver can use the of_device_get_match_data() API to simplify the
> code. Replace calls to of_match_device() with this newer API under the
> assumption that where it is called will be when we know the device is
> backed by a DT node. This nicely avoids referencing the match table when
> it is undefined with configurations where CONFIG_OF=n.
> Please ack or pick for immediate merge so the last patch can be
merged.
I see nothing obviously wrong, so...
Acked-by: Pavel Machek <pavel@ucw.cz>
... but it did not apply on top of leds-next tree.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread