[PATCH 0/2] pinctrl: ocelot: Add fixes for ocelot driver

[PATCH 0/2] pinctrl: ocelot: Add fixes for ocelot driver

[Horatiu Vultur]

This contains two fixes for the ocelot pinctrl:
- first fixes the memory resource index for pincfg
- second fixes the interrupt parsing

Horatiu Vultur (2):
  pinctrl: ocelot: Fix the pincfg resource.
  pinctrl: ocelot: Fix interrupt parsing

 drivers/pinctrl/pinctrl-ocelot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.33.0

[PATCH 1/2] pinctrl: ocelot: Fix the pincfg resource.

[Horatiu Vultur]

The pincfg resources are in the second memory resource. But the driver
still tries to access the first memory resource to get the pincfg. This
is wrong therefore fix to access the second memory resource.

Fixes: ad96111e658a95 ("pinctrl: ocelot: combine get resource and ioremap into single call")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/pinctrl/pinctrl-ocelot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c
index 685c79e08d40..a859fbcb09af 100644
--- a/drivers/pinctrl/pinctrl-ocelot.c
+++ b/drivers/pinctrl/pinctrl-ocelot.c
@@ -1892,7 +1892,7 @@ static struct regmap *ocelot_pinctrl_create_pincfg(struct platform_device *pdev)
 		.max_register = 32,
 	};
 
-	base = devm_platform_ioremap_resource(pdev, 0);
+	base = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(base)) {
 		dev_dbg(&pdev->dev, "Failed to ioremap config registers (no extended pinconf)\n");
 		return NULL;
-- 
2.33.0

Re: [PATCH 1/2] pinctrl: ocelot: Fix the pincfg resource.

[Colin Foster]

Hi Horatiu,

On Thu, Mar 03, 2022 at 09:37:15PM +0100, Horatiu Vultur wrote:
> The pincfg resources are in the second memory resource. But the driver
> still tries to access the first memory resource to get the pincfg. This
> is wrong therefore fix to access the second memory resource.
> 
> Fixes: ad96111e658a95 ("pinctrl: ocelot: combine get resource and ioremap into single call")

Sorry for this bug, but thanks for finding / fixing it. Digging through
my commit log to make sure I didn't make the same mistake a _third_ time
:(

For what it's worth:

Reviewed-by: Colin Foster <colin.foster@in-advantage.com>

> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  drivers/pinctrl/pinctrl-ocelot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c
> index 685c79e08d40..a859fbcb09af 100644
> --- a/drivers/pinctrl/pinctrl-ocelot.c
> +++ b/drivers/pinctrl/pinctrl-ocelot.c
> @@ -1892,7 +1892,7 @@ static struct regmap *ocelot_pinctrl_create_pincfg(struct platform_device *pdev)
>  		.max_register = 32,
>  	};
>  
> -	base = devm_platform_ioremap_resource(pdev, 0);
> +	base = devm_platform_ioremap_resource(pdev, 1);
>  	if (IS_ERR(base)) {
>  		dev_dbg(&pdev->dev, "Failed to ioremap config registers (no extended pinconf)\n");
>  		return NULL;
> -- 
> 2.33.0
> 

[PATCH 2/2] pinctrl: ocelot: Fix interrupt parsing

[Horatiu Vultur]

In the blamed commit, it removes the duplicate of_node assignment in the
driver. But the driver uses this before calling into of_gpio_dev_init to
determine if it needs to assign an IRQ chip to the GPIO. The fixes
consists in using of_node from dev.

Fixes: 8a8d6bbe1d3bc7 ("pinctrl: Get rid of duplicate of_node assignment in the drivers")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/pinctrl/pinctrl-ocelot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c
index a859fbcb09af..a0f00380e700 100644
--- a/drivers/pinctrl/pinctrl-ocelot.c
+++ b/drivers/pinctrl/pinctrl-ocelot.c
@@ -1851,7 +1851,7 @@ static int ocelot_gpiochip_register(struct platform_device *pdev,
 	gc->base = -1;
 	gc->label = "ocelot-gpio";
 
-	irq = irq_of_parse_and_map(gc->of_node, 0);
+	irq = irq_of_parse_and_map(info->dev->of_node, 0);
 	if (irq) {
 		girq = &gc->irq;
 		girq->chip = &ocelot_irqchip;
-- 
2.33.0

Re: [PATCH 2/2] pinctrl: ocelot: Fix interrupt parsing

[Andy Shevchenko]

On Thu, Mar 03, 2022 at 09:37:16PM +0100, Horatiu Vultur wrote:
> In the blamed commit, it removes the duplicate of_node assignment in the
> driver. But the driver uses this before calling into of_gpio_dev_init to
> determine if it needs to assign an IRQ chip to the GPIO. The fixes
> consists in using of_node from dev.

...

> -	irq = irq_of_parse_and_map(gc->of_node, 0);
> +	irq = irq_of_parse_and_map(info->dev->of_node, 0);

Why platform_get_irq() can't be used?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] pinctrl: ocelot: Fix interrupt parsing

[Andy Shevchenko]

On Fri, Mar 04, 2022 at 02:40:38PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 03, 2022 at 09:37:16PM +0100, Horatiu Vultur wrote:
> > In the blamed commit, it removes the duplicate of_node assignment in the
> > driver. But the driver uses this before calling into of_gpio_dev_init to
> > determine if it needs to assign an IRQ chip to the GPIO. The fixes
> > consists in using of_node from dev.
> 
> ...
> 
> > -	irq = irq_of_parse_and_map(gc->of_node, 0);
> > +	irq = irq_of_parse_and_map(info->dev->of_node, 0);
> 
> Why platform_get_irq() can't be used?

Or actually _optional() variant of it?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] pinctrl: ocelot: Fix interrupt parsing

[Horatiu Vultur]

The 03/04/2022 14:41, Andy Shevchenko wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Mar 04, 2022 at 02:40:38PM +0200, Andy Shevchenko wrote:
> > On Thu, Mar 03, 2022 at 09:37:16PM +0100, Horatiu Vultur wrote:
> > > In the blamed commit, it removes the duplicate of_node assignment in the
> > > driver. But the driver uses this before calling into of_gpio_dev_init to
> > > determine if it needs to assign an IRQ chip to the GPIO. The fixes
> > > consists in using of_node from dev.
> >
> > ...
> >
> > > -   irq = irq_of_parse_and_map(gc->of_node, 0);
> > > +   irq = irq_of_parse_and_map(info->dev->of_node, 0);
> >
> > Why platform_get_irq() can't be used?
> 
> Or actually _optional() variant of it?

It can be used. I will update this in the next series.

> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

-- 
/Horatiu