[PATCH 1/2] soc/tegra: pmc: Fail to allocate more than one wake IRQ

[PATCH 1/2] soc/tegra: pmc: Fail to allocate more than one wake IRQ

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The code currently doesn't support allocating more than one wake IRQ at
a time. Detect this situation and error out. Also make sure to output a
warning when that happens to help track down callers.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 5648e5c09ef5..653fe2c466f6 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -1863,6 +1863,9 @@ static int tegra_pmc_irq_alloc(struct irq_domain *domain, unsigned int virq,
 	unsigned int i;
 	int err = 0;
 
+	if (WARN_ON(num_irqs > 1))
+		return -EINVAL;
+
 	for (i = 0; i < soc->num_wake_events; i++) {
 		const struct tegra_wake_event *event = &soc->wake_events[i];
 
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] soc/tegra: pmc: Avoid crash for non-wake IRQs

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

For interrupts that are not wakeup sources but that may end up getting
mapped through the PMC as interrupt parent (this can happen for GPIOs),
return early in order to avoid a subsequent crash from an out-of-bounds
access to the register region.

Reported-by: Bitan Biswas <bbiswas@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 653fe2c466f6..6e66b5e293be 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -1924,6 +1924,9 @@ static int tegra_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
 	unsigned int offset, bit;
 	u32 value;
 
+	if (WARN_ON(data->hwirq == ULONG_MAX))
+		return 0;
+
 	offset = data->hwirq / 32;
 	bit = data->hwirq % 32;
 
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] soc/tegra: pmc: Avoid crash for non-wake IRQs

[Jon Hunter]

On 29/05/2019 11:26, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> For interrupts that are not wakeup sources but that may end up getting
> mapped through the PMC as interrupt parent (this can happen for GPIOs),
> return early in order to avoid a subsequent crash from an out-of-bounds
> access to the register region.

Maybe worth clarifying here what you mean by 'not wakeup sources'
because the Tegra GPIO driver does have a set_wake() API to enable
wakeup at the LIC IIRC. So maybe GPIOs that are not wakeup sources for
what level of suspend?

> Reported-by: Bitan Biswas <bbiswas@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/soc/tegra/pmc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 653fe2c466f6..6e66b5e293be 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -1924,6 +1924,9 @@ static int tegra_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>  	unsigned int offset, bit;
>  	u32 value;
>  
> +	if (WARN_ON(data->hwirq == ULONG_MAX))
> +		return 0;
> +
>  	offset = data->hwirq / 32;
>  	bit = data->hwirq % 32;

Otherwise ...

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon


-- 
nvpublic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] soc/tegra: pmc: Avoid crash for non-wake IRQs

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1848 bytes --]

On Fri, May 31, 2019 at 10:32:40AM +0100, Jon Hunter wrote:
> 
> 
> On 29/05/2019 11:26, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > For interrupts that are not wakeup sources but that may end up getting
> > mapped through the PMC as interrupt parent (this can happen for GPIOs),
> > return early in order to avoid a subsequent crash from an out-of-bounds
> > access to the register region.
> 
> Maybe worth clarifying here what you mean by 'not wakeup sources'
> because the Tegra GPIO driver does have a set_wake() API to enable
> wakeup at the LIC IIRC. So maybe GPIOs that are not wakeup sources for
> what level of suspend?

Wakeup sources in the context of PMC is always LP0 wakeup sources. At
that point I don't think LIC is enabled anymore. So LIC is to wake up
from LP1 (and perhaps LP2), while PMC wakeup sources need to be
configured in order to wake up from LP0.

Adding Peter to confirm, I think he's more familiar with the power
states on earlier chips than I am.

Thierry

> 
> > Reported-by: Bitan Biswas <bbiswas@nvidia.com>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/soc/tegra/pmc.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> > index 653fe2c466f6..6e66b5e293be 100644
> > --- a/drivers/soc/tegra/pmc.c
> > +++ b/drivers/soc/tegra/pmc.c
> > @@ -1924,6 +1924,9 @@ static int tegra_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
> >  	unsigned int offset, bit;
> >  	u32 value;
> >  
> > +	if (WARN_ON(data->hwirq == ULONG_MAX))
> > +		return 0;
> > +
> >  	offset = data->hwirq / 32;
> >  	bit = data->hwirq % 32;
> 
> Otherwise ...
> 
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> 
> Cheers
> Jon
> 
> 
> -- 
> nvpublic

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] soc/tegra: pmc: Avoid crash for non-wake IRQs

[Peter De Schrijver]

On Fri, May 31, 2019 at 12:28:58PM +0200, Thierry Reding wrote:
> On Fri, May 31, 2019 at 10:32:40AM +0100, Jon Hunter wrote:
> > 
> > 
> > On 29/05/2019 11:26, Thierry Reding wrote:
> > > From: Thierry Reding <treding@nvidia.com>
> > > 
> > > For interrupts that are not wakeup sources but that may end up getting
> > > mapped through the PMC as interrupt parent (this can happen for GPIOs),
> > > return early in order to avoid a subsequent crash from an out-of-bounds
> > > access to the register region.
> > 
> > Maybe worth clarifying here what you mean by 'not wakeup sources'
> > because the Tegra GPIO driver does have a set_wake() API to enable
> > wakeup at the LIC IIRC. So maybe GPIOs that are not wakeup sources for
> > what level of suspend?
> 
> Wakeup sources in the context of PMC is always LP0 wakeup sources. At
> that point I don't think LIC is enabled anymore. So LIC is to wake up
> from LP1 (and perhaps LP2), while PMC wakeup sources need to be
> configured in order to wake up from LP0.
> 
> Adding Peter to confirm, I think he's more familiar with the power
> states on earlier chips than I am.

Yes. LIC is in a domain which is off during SC7 so it can't trigger a
wakeup.

Peter.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] soc/tegra: pmc: Fail to allocate more than one wake IRQ

[Jon Hunter]

On 29/05/2019 11:26, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> The code currently doesn't support allocating more than one wake IRQ at
> a time. Detect this situation and error out. Also make sure to output a
> warning when that happens to help track down callers.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/soc/tegra/pmc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 5648e5c09ef5..653fe2c466f6 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -1863,6 +1863,9 @@ static int tegra_pmc_irq_alloc(struct irq_domain *domain, unsigned int virq,
>  	unsigned int i;
>  	int err = 0;
>  
> +	if (WARN_ON(num_irqs > 1))
> +		return -EINVAL;
> +
>  	for (i = 0; i < soc->num_wake_events; i++) {
>  		const struct tegra_wake_event *event = &soc->wake_events[i];

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel