[PATCH v1 1/1] gpio: altera: Add missed base and label initialisations

[PATCH v1 1/1] gpio: altera: Add missed base and label initialisations

[Andy Shevchenko]

During conversion driver to modern APIs the base field initial value
of the GPIO chip was moved from -1 to 0, which triggers a warning.
Add missed base initialisation as it was in the original code.

Also initialise the GPIO chip label correctly.

Fixes: 50dded8d9d62 ("gpio: altera: Drop legacy-of-mm-gpiochip.h header")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

Can be folded into the mentioned change, up to you and sorry for missing
these in the first place.

 drivers/gpio/gpio-altera.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
index 039fef26546e..73e660c5e38a 100644
--- a/drivers/gpio/gpio-altera.c
+++ b/drivers/gpio/gpio-altera.c
@@ -261,6 +261,11 @@ static int altera_gpio_probe(struct platform_device *pdev)
 	altera_gc->gc.set		= altera_gpio_set;
 	altera_gc->gc.owner		= THIS_MODULE;
 	altera_gc->gc.parent		= &pdev->dev;
+	altera_gc->gc.base		= -1;
+
+	altera_gc->gc.label = devm_kasprintf(dev, GFP_KERNEL, "%pfw", dev_fwnode(dev));
+	if (!altera_gc->gc.label)
+		return -ENOMEM;
 
 	altera_gc->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(altera_gc->regs))
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 1/1] gpio: altera: Add missed base and label initialisations

[Bartosz Golaszewski]

On Fri, Nov 15, 2024 at 1:30 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> During conversion driver to modern APIs the base field initial value
> of the GPIO chip was moved from -1 to 0, which triggers a warning.
> Add missed base initialisation as it was in the original code.
>
> Also initialise the GPIO chip label correctly.
>
> Fixes: 50dded8d9d62 ("gpio: altera: Drop legacy-of-mm-gpiochip.h header")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>
> Can be folded into the mentioned change, up to you and sorry for missing
> these in the first place.
>
>  drivers/gpio/gpio-altera.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
> index 039fef26546e..73e660c5e38a 100644
> --- a/drivers/gpio/gpio-altera.c
> +++ b/drivers/gpio/gpio-altera.c
> @@ -261,6 +261,11 @@ static int altera_gpio_probe(struct platform_device *pdev)
>         altera_gc->gc.set               = altera_gpio_set;
>         altera_gc->gc.owner             = THIS_MODULE;
>         altera_gc->gc.parent            = &pdev->dev;
> +       altera_gc->gc.base              = -1;

This does in fact fix commit 50dded8d9d62 ("gpio: altera: Drop
legacy-of-mm-gpiochip.h header") and should go together with it
upstream.

> +
> +       altera_gc->gc.label = devm_kasprintf(dev, GFP_KERNEL, "%pfw", dev_fwnode(dev));
> +       if (!altera_gc->gc.label)
> +               return -ENOMEM;

This is a separate thing and I would argue it's not even a fix per se.
Lack of label doesn't make it entirely impossible to use the chip. I'm
willing to send it as a fix but please make it a separate commit as it
doesn't fix anything from 50dded8d9d62.

The commit message for it could also use some explanation why it's
better to set label (so that the chip will not figure as "unknown" in
the kernel).

Bart

>
>         altera_gc->regs = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(altera_gc->regs))
> --
> 2.43.0.rc1.1336.g36b5255a03ac
>

Re: [PATCH v1 1/1] gpio: altera: Add missed base and label initialisations

[Bartosz Golaszewski]

On Sat, Nov 16, 2024 at 3:12 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Fri, Nov 15, 2024 at 1:30 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > During conversion driver to modern APIs the base field initial value
> > of the GPIO chip was moved from -1 to 0, which triggers a warning.
> > Add missed base initialisation as it was in the original code.
> >
> > Also initialise the GPIO chip label correctly.
> >
> > Fixes: 50dded8d9d62 ("gpio: altera: Drop legacy-of-mm-gpiochip.h header")
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >
> > Can be folded into the mentioned change, up to you and sorry for missing
> > these in the first place.
> >
> >  drivers/gpio/gpio-altera.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
> > index 039fef26546e..73e660c5e38a 100644
> > --- a/drivers/gpio/gpio-altera.c
> > +++ b/drivers/gpio/gpio-altera.c
> > @@ -261,6 +261,11 @@ static int altera_gpio_probe(struct platform_device *pdev)
> >         altera_gc->gc.set               = altera_gpio_set;
> >         altera_gc->gc.owner             = THIS_MODULE;
> >         altera_gc->gc.parent            = &pdev->dev;
> > +       altera_gc->gc.base              = -1;
>
> This does in fact fix commit 50dded8d9d62 ("gpio: altera: Drop
> legacy-of-mm-gpiochip.h header") and should go together with it
> upstream.
>
> > +
> > +       altera_gc->gc.label = devm_kasprintf(dev, GFP_KERNEL, "%pfw", dev_fwnode(dev));
> > +       if (!altera_gc->gc.label)
> > +               return -ENOMEM;
>
> This is a separate thing and I would argue it's not even a fix per se.
> Lack of label doesn't make it entirely impossible to use the chip. I'm
> willing to send it as a fix but please make it a separate commit as it
> doesn't fix anything from 50dded8d9d62.
>
> The commit message for it could also use some explanation why it's
> better to set label (so that the chip will not figure as "unknown" in
> the kernel).
>
> Bart

Ah scratch that... of_mm_gpiochip_add_data() would set the label from
the OF node's name.

In that case this can be a single commit but please explain it better
in the commit message to avoid confusion. Say what happened and why we
need to set the label in the driver now.

Bart

>
> >
> >         altera_gc->regs = devm_platform_ioremap_resource(pdev, 0);
> >         if (IS_ERR(altera_gc->regs))
> > --
> > 2.43.0.rc1.1336.g36b5255a03ac
> >