Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

[Bartosz Golaszewski]

On Thu, Dec 2, 2021 at 10:17 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> GPIO library does copy the of_node from the parent device of
> the GPIO chip, there is no need to repeat this in the individual
> drivers. Remove these assignment all at once.
>
> For the details one may look into the of_gpio_dev_init() implementation.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---

I have a bad feeling about this but I've gone through the drivers in
this patch and it seems like you don't update any of the drivers that
use multiple child OF nodes so I can't really point out any obvious
bug.

I have another change I'm working on that's related, let me send it shortly.

Bart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

[Andy Shevchenko]

On Fri, Dec 03, 2021 at 09:40:55AM +0100, Bartosz Golaszewski wrote:
> On Thu, Dec 2, 2021 at 10:17 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > GPIO library does copy the of_node from the parent device of
> > the GPIO chip, there is no need to repeat this in the individual
> > drivers. Remove these assignment all at once.
> >
> > For the details one may look into the of_gpio_dev_init() implementation.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> 
> I have a bad feeling about this but I've gone through the drivers in
> this patch and it seems like you don't update any of the drivers that
> use multiple child OF nodes so I can't really point out any obvious
> bug.

Yes, like I said it's just a series to kick off the conversion.
I left the corner cases to the last.

> I have another change I'm working on that's related, let me send it shortly.

Do you mean it should be attached to the series?

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

[Linus Walleij]

On Thu, Dec 2, 2021 at 10:17 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> GPIO library does copy the of_node from the parent device of
> the GPIO chip, there is no need to repeat this in the individual
> drivers. Remove these assignment all at once.
>
> For the details one may look into the of_gpio_dev_init() implementation.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

This is definitely a patch in the right direction, as Bart says
it can be a bit dangerous, the outliers are those drivers that
assign the .dev to something completely different than the
the dev where the of_node is copied from.

The idea was definitely always to only assign it in the core
*unless* there is a reason to have a completely different
of_node for some reason.

> +++ b/drivers/gpio/gpio-rda.c
> @@ -240,8 +240,6 @@ static int rda_gpio_probe(struct platform_device *pdev)
>         rda_gpio->chip.label = dev_name(dev);
>         rda_gpio->chip.ngpio = ngpios;
>         rda_gpio->chip.base = -1;
> -       rda_gpio->chip.parent = dev;
> -       rda_gpio->chip.of_node = np;

Mention in the commit message that in this driver
you also drop the the .parent assignment because the
core will handle it.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

[Andy Shevchenko]

On Sun, Dec 05, 2021 at 01:06:07AM +0100, Linus Walleij wrote:
> On Thu, Dec 2, 2021 at 10:17 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > GPIO library does copy the of_node from the parent device of
> > the GPIO chip, there is no need to repeat this in the individual
> > drivers. Remove these assignment all at once.
> >
> > For the details one may look into the of_gpio_dev_init() implementation.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> This is definitely a patch in the right direction, as Bart says
> it can be a bit dangerous, the outliers are those drivers that
> assign the .dev to something completely different than the
> the dev where the of_node is copied from.

I carefully checked these all and this patch series is only for the cases
when I'm sure it's the same device, which is used as parent, and its of_node
supplied.

> The idea was definitely always to only assign it in the core
> *unless* there is a reason to have a completely different
> of_node for some reason.
> 
> > +++ b/drivers/gpio/gpio-rda.c
> > @@ -240,8 +240,6 @@ static int rda_gpio_probe(struct platform_device *pdev)
> >         rda_gpio->chip.label = dev_name(dev);
> >         rda_gpio->chip.ngpio = ngpios;
> >         rda_gpio->chip.base = -1;
> > -       rda_gpio->chip.parent = dev;
> > -       rda_gpio->chip.of_node = np;
> 
> Mention in the commit message that in this driver
> you also drop the the .parent assignment because the
> core will handle it.

Okay, I will update it. Also I'll update to the last codebase (dunno if Bart
is going to pull the IB from Lee where one of the drivers is gone: da53cc634cea
("gpio: bd70528 Drop BD70528 support").

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

[Bartosz Golaszewski]

On Mon, Dec 6, 2021 at 2:13 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Sun, Dec 05, 2021 at 01:06:07AM +0100, Linus Walleij wrote:
> > On Thu, Dec 2, 2021 at 10:17 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> >
> > > GPIO library does copy the of_node from the parent device of
> > > the GPIO chip, there is no need to repeat this in the individual
> > > drivers. Remove these assignment all at once.
> > >
> > > For the details one may look into the of_gpio_dev_init() implementation.
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > This is definitely a patch in the right direction, as Bart says
> > it can be a bit dangerous, the outliers are those drivers that
> > assign the .dev to something completely different than the
> > the dev where the of_node is copied from.
>
> I carefully checked these all and this patch series is only for the cases
> when I'm sure it's the same device, which is used as parent, and its of_node
> supplied.
>
> > The idea was definitely always to only assign it in the core
> > *unless* there is a reason to have a completely different
> > of_node for some reason.
> >
> > > +++ b/drivers/gpio/gpio-rda.c
> > > @@ -240,8 +240,6 @@ static int rda_gpio_probe(struct platform_device *pdev)
> > >         rda_gpio->chip.label = dev_name(dev);
> > >         rda_gpio->chip.ngpio = ngpios;
> > >         rda_gpio->chip.base = -1;
> > > -       rda_gpio->chip.parent = dev;
> > > -       rda_gpio->chip.of_node = np;
> >
> > Mention in the commit message that in this driver
> > you also drop the the .parent assignment because the
> > core will handle it.
>
> Okay, I will update it. Also I'll update to the last codebase (dunno if Bart
> is going to pull the IB from Lee where one of the drivers is gone: da53cc634cea
> ("gpio: bd70528 Drop BD70528 support").
>

I didn't plan to, just drop it from your patch.

Bart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 384 bytes --]

Hello Andy,

you Cc: linux-pwm and the pwm maintainers in this series. I don't spot
anything pwm specific here (apart from touching gpio-mvebu which also
contains a PWM driver). Do I miss something?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

[Baruch Siach]

Hi Uwe,

On Tue, Dec 07 2021, Uwe Kleine-König wrote:
> Hello Andy,
>
> you Cc: linux-pwm and the pwm maintainers in this series. I don't spot
> anything pwm specific here (apart from touching gpio-mvebu which also
> contains a PWM driver). Do I miss something?

That's probably because of drivers/gpio/gpio-mvebu.c that appears in the
MAINTAINERS PWM entry.

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

[Andy Shevchenko]

On Tue, Dec 07, 2021 at 10:05:08AM +0200, Baruch Siach wrote:
> On Tue, Dec 07 2021, Uwe Kleine-König wrote:
> > Hello Andy,
> >
> > you Cc: linux-pwm and the pwm maintainers in this series. I don't spot
> > anything pwm specific here (apart from touching gpio-mvebu which also
> > contains a PWM driver). Do I miss something?

Thanks for your eagle eye! For the record I don't do this list manually,
I used the get_maintainer.pl's heuristics [1]. Usually it gives me more
than 95% correct prediction, so I believe here is no smoke without flame
(and it seems what Baruch discovered as well).

> That's probably because of drivers/gpio/gpio-mvebu.c that appears in the
> MAINTAINERS PWM entry.

[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

[Bartosz Golaszewski]

On Thu, Dec 2, 2021 at 10:17 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> GPIO library does copy the of_node from the parent device of
> the GPIO chip, there is no need to repeat this in the individual
> drivers. Remove these assignment all at once.
>
> For the details one may look into the of_gpio_dev_init() implementation.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---

If there are no objections, I am willing to apply this to give it some
time in next and get the work on dropping the OF APIs from gpiolib
going.

Bart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

[Andy Shevchenko]

On Mon, Dec 13, 2021 at 02:50:57PM +0100, Bartosz Golaszewski wrote:
> On Thu, Dec 2, 2021 at 10:17 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > GPIO library does copy the of_node from the parent device of
> > the GPIO chip, there is no need to repeat this in the individual
> > drivers. Remove these assignment all at once.
> >
> > For the details one may look into the of_gpio_dev_init() implementation.

> If there are no objections, I am willing to apply this to give it some
> time in next and get the work on dropping the OF APIs from gpiolib
> going.

There was a v2 sent out.

  Message-Id: <20211206131852.74746-1-andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/3] gpio: Get rid of duplicate of_node assignment in the drivers

[Bartosz Golaszewski]

On Thu, Dec 2, 2021 at 10:17 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> GPIO library does copy the of_node from the parent device of
> the GPIO chip, there is no need to repeat this in the individual
> drivers. Remove these assignment all at once.
>
> For the details one may look into the of_gpio_dev_init() implementation.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Queued the series for next, thanks!

Bart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel