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

From: Thierry Reding <thierry.reding@gmail.com>
To: Jon Hunter <jonathanh@nvidia.com>
Cc: linux-tegra@vger.kernel.org,
	Peter De Schrijver <pdeschrijver@nvidia.com>,
	linux-arm-kernel@lists.infradead.org,
	Bitan Biswas <bbiswas@nvidia.com>
Subject: Re: [PATCH 2/2] soc/tegra: pmc: Avoid crash for non-wake IRQs
Date: Fri, 31 May 2019 12:28:58 +0200	[thread overview]
Message-ID: <20190531102858.GA21355@ulmo> (raw)
In-Reply-To: <27ffbfe6-1c88-63a1-aee1-7fe6ce252e54@nvidia.com>

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