| Message ID | 20200324004413.14355-1-marek.behun@nic.cz |
|---|---|
| State | New |
| Headers | show |
| Series | [pinctrl,REGRESSION] Revert "pinctrl: mvebu: armada-37xx: use use platform api" | expand |
On Tue, Mar 24, 2020 at 01:44:13AM +0100, Marek Behún wrote: > This reverts commit 06e26b75f5e613b400116fdb7ff6206a681ab271. > > This commit caused a regression on Armada 37xx. The pinctrl driver says > armada-37xx-pinctrl d0013800.pinctrl: invalid or no IRQ > armada-37xx-pinctrl d0018800.pinctrl: invalid or no IRQ > and afterwards other drivers cannot use GPIOs by this driver as IRQs. > > Fixes: 06e26b75f5e6 ("pinctrl: mvebu: armada-37xx: use use platform...") > Signed-off-by: Marek Behún <marek.behun@nic.cz> > Cc: Peng Fan <peng.fan@nxp.com> > --- > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > index 32f12a388b3c..5f125bd6279d 100644 > --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > @@ -15,6 +15,7 @@ > #include <linux/of.h> > #include <linux/of_address.h> > #include <linux/of_device.h> > +#include <linux/of_irq.h> > #include <linux/pinctrl/pinconf-generic.h> > #include <linux/pinctrl/pinconf.h> > #include <linux/pinctrl/pinctrl.h> > @@ -741,14 +742,7 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev, > return ret; > } > > - nr_irq_parent = platform_irq_count(pdev); Hi Marek Could you determine the value of nr_irq_parent(). Is it -EPROBE_DEFER? This patch has removed the handling of that. > - if (nr_irq_parent < 0) { > - if (nr_irq_parent != -EPROBE_DEFER) > - dev_err(dev, "Couldn't determine irq count: %pe\n", > - ERR_PTR(nr_irq_parent)); > - return nr_irq_parent; > - } Thanks Andrew
On Tuesday 24 March 2020 13:20:17 Andrew Lunn wrote: > On Tue, Mar 24, 2020 at 01:44:13AM +0100, Marek Behún wrote: > > This reverts commit 06e26b75f5e613b400116fdb7ff6206a681ab271. > > > > This commit caused a regression on Armada 37xx. The pinctrl driver says > > armada-37xx-pinctrl d0013800.pinctrl: invalid or no IRQ > > armada-37xx-pinctrl d0018800.pinctrl: invalid or no IRQ > > and afterwards other drivers cannot use GPIOs by this driver as IRQs. > > > > Fixes: 06e26b75f5e6 ("pinctrl: mvebu: armada-37xx: use use platform...") > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > > Cc: Peng Fan <peng.fan@nxp.com> > > --- > > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 12 +++--------- > > 1 file changed, 3 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > index 32f12a388b3c..5f125bd6279d 100644 > > --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > @@ -15,6 +15,7 @@ > > #include <linux/of.h> > > #include <linux/of_address.h> > > #include <linux/of_device.h> > > +#include <linux/of_irq.h> > > #include <linux/pinctrl/pinconf-generic.h> > > #include <linux/pinctrl/pinconf.h> > > #include <linux/pinctrl/pinctrl.h> > > @@ -741,14 +742,7 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev, > > return ret; > > } > > > > - nr_irq_parent = platform_irq_count(pdev); > > Hi Marek > > Could you determine the value of nr_irq_parent(). Is it -EPROBE_DEFER? Hello Andrew! I have tested it with 5.6 kernel and return value in nr_irq_parent is in both cases zero. So it is not -EPROBE_DEFER. And return value of of_irq_count(np) is 12 for d0013800.pinctrl and 5 for d0018800.pinctrl. So this is a clear regression introduced by that patch. > This patch has removed the handling of that. > > > - if (nr_irq_parent < 0) { > > - if (nr_irq_parent != -EPROBE_DEFER) > > - dev_err(dev, "Couldn't determine irq count: %pe\n", > > - ERR_PTR(nr_irq_parent)); > > - return nr_irq_parent; > > - } > > Thanks > Andrew
On Tuesday 07 April 2020 13:52:30 Pali Rohár wrote: > On Tuesday 24 March 2020 13:20:17 Andrew Lunn wrote: > > On Tue, Mar 24, 2020 at 01:44:13AM +0100, Marek Behún wrote: > > > This reverts commit 06e26b75f5e613b400116fdb7ff6206a681ab271. > > > > > > This commit caused a regression on Armada 37xx. The pinctrl driver says > > > armada-37xx-pinctrl d0013800.pinctrl: invalid or no IRQ > > > armada-37xx-pinctrl d0018800.pinctrl: invalid or no IRQ > > > and afterwards other drivers cannot use GPIOs by this driver as IRQs. > > > > > > Fixes: 06e26b75f5e6 ("pinctrl: mvebu: armada-37xx: use use platform...") > > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > > > Cc: Peng Fan <peng.fan@nxp.com> > > > --- > > > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 12 +++--------- > > > 1 file changed, 3 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > > index 32f12a388b3c..5f125bd6279d 100644 > > > --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > > +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > > @@ -15,6 +15,7 @@ > > > #include <linux/of.h> > > > #include <linux/of_address.h> > > > #include <linux/of_device.h> > > > +#include <linux/of_irq.h> > > > #include <linux/pinctrl/pinconf-generic.h> > > > #include <linux/pinctrl/pinconf.h> > > > #include <linux/pinctrl/pinctrl.h> > > > @@ -741,14 +742,7 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev, > > > return ret; > > > } > > > > > > - nr_irq_parent = platform_irq_count(pdev); > > > > Hi Marek > > > > Could you determine the value of nr_irq_parent(). Is it -EPROBE_DEFER? > > Hello Andrew! I have tested it with 5.6 kernel and return value in > nr_irq_parent is in both cases zero. So it is not -EPROBE_DEFER. And > return value of of_irq_count(np) is 12 for d0013800.pinctrl and 5 for > d0018800.pinctrl. Adding Jason, Gregory and Sebastian into the loop. Could you please look at this problem? If there is no easy solution, I would suggest to revert problematic commit as pinctrl in current state is broken and unusable. > So this is a clear regression introduced by that patch. > > > This patch has removed the handling of that. > > > > > - if (nr_irq_parent < 0) { > > > - if (nr_irq_parent != -EPROBE_DEFER) > > > - dev_err(dev, "Couldn't determine irq count: %pe\n", > > > - ERR_PTR(nr_irq_parent)); > > > - return nr_irq_parent; > > > - } > > > > Thanks > > Andrew
On Wed, 15 Apr 2020 11:53:07 +0200 Pali Rohár <pali@kernel.org> wrote: > On Tuesday 07 April 2020 13:52:30 Pali Rohár wrote: > > On Tuesday 24 March 2020 13:20:17 Andrew Lunn wrote: > > > On Tue, Mar 24, 2020 at 01:44:13AM +0100, Marek Behún wrote: > > > > This reverts commit 06e26b75f5e613b400116fdb7ff6206a681ab271. > > > > > > > > This commit caused a regression on Armada 37xx. The pinctrl driver says > > > > armada-37xx-pinctrl d0013800.pinctrl: invalid or no IRQ > > > > armada-37xx-pinctrl d0018800.pinctrl: invalid or no IRQ > > > > and afterwards other drivers cannot use GPIOs by this driver as IRQs. > > > > > > > > Fixes: 06e26b75f5e6 ("pinctrl: mvebu: armada-37xx: use use platform...") > > > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > > > > Cc: Peng Fan <peng.fan@nxp.com> > > > > --- > > > > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 12 +++--------- > > > > 1 file changed, 3 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > > > index 32f12a388b3c..5f125bd6279d 100644 > > > > --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > > > +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > > > @@ -15,6 +15,7 @@ > > > > #include <linux/of.h> > > > > #include <linux/of_address.h> > > > > #include <linux/of_device.h> > > > > +#include <linux/of_irq.h> > > > > #include <linux/pinctrl/pinconf-generic.h> > > > > #include <linux/pinctrl/pinconf.h> > > > > #include <linux/pinctrl/pinctrl.h> > > > > @@ -741,14 +742,7 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev, > > > > return ret; > > > > } > > > > > > > > - nr_irq_parent = platform_irq_count(pdev); > > > > > > Hi Marek > > > > > > Could you determine the value of nr_irq_parent(). Is it -EPROBE_DEFER? > > > > Hello Andrew! I have tested it with 5.6 kernel and return value in > > nr_irq_parent is in both cases zero. So it is not -EPROBE_DEFER. And > > return value of of_irq_count(np) is 12 for d0013800.pinctrl and 5 for > > d0018800.pinctrl. > > Adding Jason, Gregory and Sebastian into the loop. > > Could you please look at this problem? If there is no easy solution, > I would suggest to revert problematic commit as pinctrl in current state > is broken and unusable. Pali, the commit is already reverted in upstream.
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c index 32f12a388b3c..5f125bd6279d 100644 --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c @@ -15,6 +15,7 @@ #include <linux/of.h> #include <linux/of_address.h> #include <linux/of_device.h> +#include <linux/of_irq.h> #include <linux/pinctrl/pinconf-generic.h> #include <linux/pinctrl/pinconf.h> #include <linux/pinctrl/pinctrl.h> @@ -741,14 +742,7 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev, return ret; } - nr_irq_parent = platform_irq_count(pdev); - if (nr_irq_parent < 0) { - if (nr_irq_parent != -EPROBE_DEFER) - dev_err(dev, "Couldn't determine irq count: %pe\n", - ERR_PTR(nr_irq_parent)); - return nr_irq_parent; - } - + nr_irq_parent = of_irq_count(np); spin_lock_init(&info->irq_lock); if (!nr_irq_parent) { @@ -785,7 +779,7 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev, if (!girq->parents) return -ENOMEM; for (i = 0; i < nr_irq_parent; i++) { - int irq = platform_get_irq(pdev, i); + int irq = irq_of_parse_and_map(np, i); if (irq < 0) continue;
This reverts commit 06e26b75f5e613b400116fdb7ff6206a681ab271. This commit caused a regression on Armada 37xx. The pinctrl driver says armada-37xx-pinctrl d0013800.pinctrl: invalid or no IRQ armada-37xx-pinctrl d0018800.pinctrl: invalid or no IRQ and afterwards other drivers cannot use GPIOs by this driver as IRQs. Fixes: 06e26b75f5e6 ("pinctrl: mvebu: armada-37xx: use use platform...") Signed-off-by: Marek Behún <marek.behun@nic.cz> Cc: Peng Fan <peng.fan@nxp.com> --- drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)