[PATCH 1/2] RFT: gpio: sifive: Set affinity callback to parent

[PATCH 1/2] RFT: gpio: sifive: Set affinity callback to parent

[Linus Walleij]

This assigns the .irq_set_affinity to the parent callback.
I assume the sifive GPIO can be used in systems with
SMP.

I used the pattern making the hirerarchy tolerant for missing
parent as in Marc's earlier patches.

Cc: Yash Shah <yash.shah@sifive.com>
Cc: Wesley W. Terpstra <wesley@sifive.com>
Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpio-sifive.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpio/gpio-sifive.c b/drivers/gpio/gpio-sifive.c
index c54dd08f2cbf..e4c190562132 100644
--- a/drivers/gpio/gpio-sifive.c
+++ b/drivers/gpio/gpio-sifive.c
@@ -128,6 +128,16 @@ static void sifive_gpio_irq_eoi(struct irq_data *d)
 	irq_chip_eoi_parent(d);
 }
 
+static int sifive_gpio_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 struct irq_chip sifive_gpio_irqchip = {
 	.name		= "sifive-gpio",
 	.irq_set_type	= sifive_gpio_irq_set_type,
@@ -136,6 +146,7 @@ static struct irq_chip sifive_gpio_irqchip = {
 	.irq_enable	= sifive_gpio_irq_enable,
 	.irq_disable	= sifive_gpio_irq_disable,
 	.irq_eoi	= sifive_gpio_irq_eoi,
+	.irq_set_affinity = sifive_gpio_irq_set_affinity,
 };
 
 static int sifive_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
-- 
2.26.2

[PATCH 2/2] RFT: gpio: tegra186: Set affinity callback to parent

[Linus Walleij]

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;
-- 
2.26.2

Re: [PATCH 2/2] RFT: gpio: tegra186: Set affinity callback to parent

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]

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

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

Re: [PATCH 2/2] RFT: gpio: tegra186: Set affinity callback to parent

[Marc Zyngier]

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...

Re: [PATCH 2/2] RFT: gpio: tegra186: Set affinity callback to parent

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3467 bytes --]

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

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

Re: [PATCH 2/2] RFT: gpio: tegra186: Set affinity callback to parent

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1742 bytes --]

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

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