| Message ID | 20201111140628.24067-2-linus.walleij@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] RFT: gpio: sifive: Set affinity callback to parent | expand |
On Wed, Nov 11, 2020 at 03:06:28PM +0100, Linus Walleij wrote: > This assigns the .irq_set_affinity to the parent callback. > I assume the Tegra186 is an SMP system so this would be > beneficial. > > I used the pattern making the hirerarchy tolerant for missing > parent as in Marc's earlier patch. > > Cc: Thierry Reding <treding@nvidia.com> > Cc: Vidya Sagar <vidyas@nvidia.com> > Suggested-by: Marc Zyngier <maz@kernel.org> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/gpio/gpio-tegra186.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c > index 9500074b1f1b..5060e81f5f49 100644 > --- a/drivers/gpio/gpio-tegra186.c > +++ b/drivers/gpio/gpio-tegra186.c > @@ -444,6 +444,16 @@ static int tegra186_irq_set_wake(struct irq_data *data, unsigned int on) > return 0; > } > > +static int tegra186_irq_set_affinity(struct irq_data *data, > + const struct cpumask *dest, > + bool force) > +{ > + if (data->parent_data) > + return irq_chip_set_affinity_parent(data, dest, force); > + > + return 0; > +} > + > static void tegra186_gpio_irq(struct irq_desc *desc) > { > struct tegra_gpio *gpio = irq_desc_get_handler_data(desc); > @@ -690,6 +700,7 @@ static int tegra186_gpio_probe(struct platform_device *pdev) > gpio->intc.irq_unmask = tegra186_irq_unmask; > gpio->intc.irq_set_type = tegra186_irq_set_type; > gpio->intc.irq_set_wake = tegra186_irq_set_wake; > + gpio->intc.irq_set_affinity = tegra186_irq_set_affinity; > > irq = &gpio->gpio.irq; > irq->chip = &gpio->intc; This does seem to mostly work, but I do get this new warning during boot: [ 6.406230] genirq: irq_chip gpio did not update eff. affinity mask of irq 70 Any idea what that's supposed to mean? I can probably dig in some more early next week to see if I can track down why exactly that happens. Thierry
Linus, Thierry, On 2020-11-13 21:44, Thierry Reding wrote: > On Wed, Nov 11, 2020 at 03:06:28PM +0100, Linus Walleij wrote: >> This assigns the .irq_set_affinity to the parent callback. >> I assume the Tegra186 is an SMP system so this would be >> beneficial. >> >> I used the pattern making the hirerarchy tolerant for missing >> parent as in Marc's earlier patch. >> >> Cc: Thierry Reding <treding@nvidia.com> >> Cc: Vidya Sagar <vidyas@nvidia.com> >> Suggested-by: Marc Zyngier <maz@kernel.org> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> >> --- >> drivers/gpio/gpio-tegra186.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/gpio/gpio-tegra186.c >> b/drivers/gpio/gpio-tegra186.c >> index 9500074b1f1b..5060e81f5f49 100644 >> --- a/drivers/gpio/gpio-tegra186.c >> +++ b/drivers/gpio/gpio-tegra186.c >> @@ -444,6 +444,16 @@ static int tegra186_irq_set_wake(struct irq_data >> *data, unsigned int on) >> return 0; >> } >> >> +static int tegra186_irq_set_affinity(struct irq_data *data, >> + const struct cpumask *dest, >> + bool force) >> +{ >> + if (data->parent_data) >> + return irq_chip_set_affinity_parent(data, dest, force); >> + >> + return 0; Having come to my senses, this should definitely return an error when there is no parent is present (i.e. we can't claim success when the affinity hasn't changed). >> +} >> + >> static void tegra186_gpio_irq(struct irq_desc *desc) >> { >> struct tegra_gpio *gpio = irq_desc_get_handler_data(desc); >> @@ -690,6 +700,7 @@ static int tegra186_gpio_probe(struct >> platform_device *pdev) >> gpio->intc.irq_unmask = tegra186_irq_unmask; >> gpio->intc.irq_set_type = tegra186_irq_set_type; >> gpio->intc.irq_set_wake = tegra186_irq_set_wake; >> + gpio->intc.irq_set_affinity = tegra186_irq_set_affinity; >> >> irq = &gpio->gpio.irq; >> irq->chip = &gpio->intc; > > This does seem to mostly work, but I do get this new warning during > boot: > > [ 6.406230] genirq: irq_chip gpio did not update eff. affinity > mask of irq 70 > > Any idea what that's supposed to mean? I can probably dig in some more > early next week to see if I can track down why exactly that happens. I can't however reproduce this on my Jetson-TX2. Can you post the content of /sys/kernel/debug/irq/irqs/70? Thanks, M. -- Jazz is not dead. It just smells funny...
On Mon, Nov 16, 2020 at 09:02:24AM +0000, Marc Zyngier wrote: > Linus, Thierry, > > On 2020-11-13 21:44, Thierry Reding wrote: > > On Wed, Nov 11, 2020 at 03:06:28PM +0100, Linus Walleij wrote: > > > This assigns the .irq_set_affinity to the parent callback. > > > I assume the Tegra186 is an SMP system so this would be > > > beneficial. > > > > > > I used the pattern making the hirerarchy tolerant for missing > > > parent as in Marc's earlier patch. > > > > > > Cc: Thierry Reding <treding@nvidia.com> > > > Cc: Vidya Sagar <vidyas@nvidia.com> > > > Suggested-by: Marc Zyngier <maz@kernel.org> > > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > > --- > > > drivers/gpio/gpio-tegra186.c | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/drivers/gpio/gpio-tegra186.c > > > b/drivers/gpio/gpio-tegra186.c > > > index 9500074b1f1b..5060e81f5f49 100644 > > > --- a/drivers/gpio/gpio-tegra186.c > > > +++ b/drivers/gpio/gpio-tegra186.c > > > @@ -444,6 +444,16 @@ static int tegra186_irq_set_wake(struct > > > irq_data *data, unsigned int on) > > > return 0; > > > } > > > > > > +static int tegra186_irq_set_affinity(struct irq_data *data, > > > + const struct cpumask *dest, > > > + bool force) > > > +{ > > > + if (data->parent_data) > > > + return irq_chip_set_affinity_parent(data, dest, force); > > > + > > > + return 0; > > Having come to my senses, this should definitely return an error when > there is no parent is present (i.e. we can't claim success when the > affinity hasn't changed). > > > > +} > > > + > > > static void tegra186_gpio_irq(struct irq_desc *desc) > > > { > > > struct tegra_gpio *gpio = irq_desc_get_handler_data(desc); > > > @@ -690,6 +700,7 @@ static int tegra186_gpio_probe(struct > > > platform_device *pdev) > > > gpio->intc.irq_unmask = tegra186_irq_unmask; > > > gpio->intc.irq_set_type = tegra186_irq_set_type; > > > gpio->intc.irq_set_wake = tegra186_irq_set_wake; > > > + gpio->intc.irq_set_affinity = tegra186_irq_set_affinity; > > > > > > irq = &gpio->gpio.irq; > > > irq->chip = &gpio->intc; > > > > This does seem to mostly work, but I do get this new warning during > > boot: > > > > [ 6.406230] genirq: irq_chip gpio did not update eff. affinity > > mask of irq 70 > > > > Any idea what that's supposed to mean? I can probably dig in some more > > early next week to see if I can track down why exactly that happens. > > I can't however reproduce this on my Jetson-TX2. Can you post the content > of /sys/kernel/debug/irq/irqs/70? Hi Marc, here's the debugfs output for IRQ 70: cat /sys/kernel/debug/irq/irqs/70 handler: handle_edge_irq device: (null) status: 0x00000001 istate: 0x00000020 IRQS_ONESHOT ddepth: 0 wdepth: 0 dstate: 0x02400201 IRQ_TYPE_EDGE_RISING IRQD_ACTIVATED IRQD_IRQ_STARTED IRQD_DEFAULT_TRIGGER_SET node: -1 affinity: 0-5 effectiv: domain: :gpio@2200000 hwirq: 0x7f chip: gpio flags: 0x0 This is with next-20201113 and a few suspend/resume fixes on top + Linus' patch for the affinity. I tried next-20201116 but that fails to boot. I can try again with vanilla linux-next just to be sure, but all the suspend/resume fixes are to very different areas of the kernel code, so I don't think they would influence this. Thierry
On Mon, Nov 16, 2020 at 09:02:24AM +0000, Marc Zyngier wrote: > Linus, Thierry, > > On 2020-11-13 21:44, Thierry Reding wrote: > > On Wed, Nov 11, 2020 at 03:06:28PM +0100, Linus Walleij wrote: > > > This assigns the .irq_set_affinity to the parent callback. > > > I assume the Tegra186 is an SMP system so this would be > > > beneficial. > > > > > > I used the pattern making the hirerarchy tolerant for missing > > > parent as in Marc's earlier patch. > > > > > > Cc: Thierry Reding <treding@nvidia.com> > > > Cc: Vidya Sagar <vidyas@nvidia.com> > > > Suggested-by: Marc Zyngier <maz@kernel.org> > > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > > --- > > > drivers/gpio/gpio-tegra186.c | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/drivers/gpio/gpio-tegra186.c > > > b/drivers/gpio/gpio-tegra186.c > > > index 9500074b1f1b..5060e81f5f49 100644 > > > --- a/drivers/gpio/gpio-tegra186.c > > > +++ b/drivers/gpio/gpio-tegra186.c > > > @@ -444,6 +444,16 @@ static int tegra186_irq_set_wake(struct > > > irq_data *data, unsigned int on) > > > return 0; > > > } > > > > > > +static int tegra186_irq_set_affinity(struct irq_data *data, > > > + const struct cpumask *dest, > > > + bool force) > > > +{ > > > + if (data->parent_data) > > > + return irq_chip_set_affinity_parent(data, dest, force); > > > + > > > + return 0; > > Having come to my senses, this should definitely return an error when > there is no parent is present (i.e. we can't claim success when the > affinity hasn't changed). FWIW, if I change the "return 0" to something like "return -EINVAL", I no longer see the warning in the kernel log. Thierry
diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c index 9500074b1f1b..5060e81f5f49 100644 --- a/drivers/gpio/gpio-tegra186.c +++ b/drivers/gpio/gpio-tegra186.c @@ -444,6 +444,16 @@ static int tegra186_irq_set_wake(struct irq_data *data, unsigned int on) return 0; } +static int tegra186_irq_set_affinity(struct irq_data *data, + const struct cpumask *dest, + bool force) +{ + if (data->parent_data) + return irq_chip_set_affinity_parent(data, dest, force); + + return 0; +} + static void tegra186_gpio_irq(struct irq_desc *desc) { struct tegra_gpio *gpio = irq_desc_get_handler_data(desc); @@ -690,6 +700,7 @@ static int tegra186_gpio_probe(struct platform_device *pdev) gpio->intc.irq_unmask = tegra186_irq_unmask; gpio->intc.irq_set_type = tegra186_irq_set_type; gpio->intc.irq_set_wake = tegra186_irq_set_wake; + gpio->intc.irq_set_affinity = tegra186_irq_set_affinity; irq = &gpio->gpio.irq; irq->chip = &gpio->intc;
This assigns the .irq_set_affinity to the parent callback. I assume the Tegra186 is an SMP system so this would be beneficial. I used the pattern making the hirerarchy tolerant for missing parent as in Marc's earlier patch. Cc: Thierry Reding <treding@nvidia.com> Cc: Vidya Sagar <vidyas@nvidia.com> Suggested-by: Marc Zyngier <maz@kernel.org> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/gpio/gpio-tegra186.c | 11 +++++++++++ 1 file changed, 11 insertions(+) -- 2.26.2