| Message ID | 20190529102654.14665-2-thierry.reding@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] soc/tegra: pmc: Fail to allocate more than one wake IRQ | expand |
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
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
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.
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;