During PMC resume, translate tier2 SC7 wake sources back into
irqs and do generic_handle_irq() to invoke the interrupt handlers
for edge triggered wake events such as sw-wake.
Signed-off-by: Prathamesh Shete <[email protected]>
Signed-off-by: Petlozu Pravareshwar <[email protected]>
---
drivers/soc/tegra/pmc.c | 44 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 8c7b46ac6ad6..f275af15f2d0 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -27,6 +27,7 @@
#include <linux/iopoll.h>
#include <linux/irqdomain.h>
#include <linux/irq.h>
+#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/of_address.h>
#include <linux/of_clk.h>
@@ -3181,6 +3182,40 @@ static void wke_clear_wake_status(void)
WAKE_AOWAKE_STATUS_W((i * 32) + wake));
}
}
+
+/* translate sc7 wake sources back into irqs to catch edge triggered wakeups */
+static void process_wake_event(int index, u32 status)
+{
+ int irq;
+ irq_hw_number_t hwirq;
+ int wake;
+ unsigned long flags;
+ struct irq_desc *desc;
+ unsigned long ulong_status = (unsigned long)status;
+
+ dev_info(pmc->dev, "Wake[%d:%d] status=0x%x\n", (index + 1) * 32,
+ index * 32, status);
+ for_each_set_bit(wake, &ulong_status, 32) {
+ hwirq = wake + 32 * index;
+
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+ irq = irq_find_mapping(pmc->domain, hwirq);
+#else
+ irq = hwirq;
+#endif
+ desc = irq_to_desc(irq);
+ if (!desc || !desc->action || !desc->action->name) {
+ dev_info(pmc->dev, "Resume caused by WAKE%d, irq %d\n",
+ (wake + 32 * index), irq);
+ continue;
+ }
+ dev_info(pmc->dev, "Resume caused by WAKE%d, %s\n",
+ (wake + 32 * index), desc->action->name);
+ local_irq_save(flags);
+ generic_handle_irq(irq);
+ local_irq_restore(flags);
+ }
+}
#endif /* CONFIG_ARM64 */
static int tegra_pmc_suspend(struct device *dev)
@@ -3219,6 +3254,15 @@ static int tegra_pmc_resume(struct device *dev)
struct tegra_pmc *pmc = dev_get_drvdata(dev);
tegra_pmc_writel(pmc, 0x0, PMC_SCRATCH41);
+#else /* CONFIG_ARM64 */
+ int i;
+ u32 status;
+
+ for (i = 0; i < WAKE_NR_VECTORS; i++) {
+ status = readl(pmc->wake + WAKE_AOWAKE_STATUS_R(i));
+ status = status & readl(pmc->wake + WAKE_AOWAKE_TIER2_ROUTING(i));
+ process_wake_event(i, status);
+ }
#endif
return 0;
--
2.17.1
On Wed, Sep 14, 2022 at 04:38:31AM +0000, Petlozu Pravareshwar wrote:
> During PMC resume, translate tier2 SC7 wake sources back into
> irqs and do generic_handle_irq() to invoke the interrupt handlers
> for edge triggered wake events such as sw-wake.
>
> Signed-off-by: Prathamesh Shete <[email protected]>
> Signed-off-by: Petlozu Pravareshwar <[email protected]>
> ---
> drivers/soc/tegra/pmc.c | 44 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 8c7b46ac6ad6..f275af15f2d0 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -27,6 +27,7 @@
> #include <linux/iopoll.h>
> #include <linux/irqdomain.h>
> #include <linux/irq.h>
> +#include <linux/interrupt.h>
This should be sorted alphabetically.
> #include <linux/kernel.h>
> #include <linux/of_address.h>
> #include <linux/of_clk.h>
> @@ -3181,6 +3182,40 @@ static void wke_clear_wake_status(void)
> WAKE_AOWAKE_STATUS_W((i * 32) + wake));
> }
> }
> +
> +/* translate sc7 wake sources back into irqs to catch edge triggered wakeups */
> +static void process_wake_event(int index, u32 status)
> +{
> + int irq;
Interrupts are usually unsigned int. The index parameter can also be
unsigned int because it will never be negative.
> + irq_hw_number_t hwirq;
> + int wake;
Can be unsigned int as well. Also, it's usually best to define variables
of the same time on a single line to make the code a bit shorter.
> + unsigned long flags;
> + struct irq_desc *desc;
> + unsigned long ulong_status = (unsigned long)status;
Why not just make the status parameter an unsigned long to avoid this
clumsy construct?
> +
> + dev_info(pmc->dev, "Wake[%d:%d] status=0x%x\n", (index + 1) * 32,
> + index * 32, status);
At most these should be dev_dbg().
> + for_each_set_bit(wake, &ulong_status, 32) {
> + hwirq = wake + 32 * index;
> +
> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
> + irq = irq_find_mapping(pmc->domain, hwirq);
We already use irq_domain_add_hierarchy(), and so the assumption is that
IRQ_DOMAIN_HIERARCHY will always be enabled. You can drop this check. We
may even want to make it explicit by selecting it from SOC_TEGRA_PMC.
> +#else
> + irq = hwirq;
> +#endif
> + desc = irq_to_desc(irq);
> + if (!desc || !desc->action || !desc->action->name) {
> + dev_info(pmc->dev, "Resume caused by WAKE%d, irq %d\n",
> + (wake + 32 * index), irq);
> + continue;
> + }
> + dev_info(pmc->dev, "Resume caused by WAKE%d, %s\n",
> + (wake + 32 * index), desc->action->name);
Same here.
> + local_irq_save(flags);
> + generic_handle_irq(irq);
> + local_irq_restore(flags);
> + }
> +}
> #endif /* CONFIG_ARM64 */
>
> static int tegra_pmc_suspend(struct device *dev)
> @@ -3219,6 +3254,15 @@ static int tegra_pmc_resume(struct device *dev)
> struct tegra_pmc *pmc = dev_get_drvdata(dev);
>
> tegra_pmc_writel(pmc, 0x0, PMC_SCRATCH41);
> +#else /* CONFIG_ARM64 */
> + int i;
unsigned int, please.
Thierry
> + u32 status;
> +
> + for (i = 0; i < WAKE_NR_VECTORS; i++) {
> + status = readl(pmc->wake + WAKE_AOWAKE_STATUS_R(i));
> + status = status & readl(pmc->wake + WAKE_AOWAKE_TIER2_ROUTING(i));
> + process_wake_event(i, status);
> + }
> #endif
>
> return 0;
> --
> 2.17.1
>
>
> On Wed, Sep 14, 2022 at 04:38:31AM +0000, Petlozu Pravareshwar wrote:
> > During PMC resume, translate tier2 SC7 wake sources back into irqs and
> > do generic_handle_irq() to invoke the interrupt handlers for edge
> > triggered wake events such as sw-wake.
> >
> > Signed-off-by: Prathamesh Shete <[email protected]>
> > Signed-off-by: Petlozu Pravareshwar <[email protected]>
> > ---
> > drivers/soc/tegra/pmc.c | 44
> > +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 44 insertions(+)
> >
> > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c index
> > 8c7b46ac6ad6..f275af15f2d0 100644
> > --- a/drivers/soc/tegra/pmc.c
> > +++ b/drivers/soc/tegra/pmc.c
> > @@ -27,6 +27,7 @@
> > #include <linux/iopoll.h>
> > #include <linux/irqdomain.h>
> > #include <linux/irq.h>
> > +#include <linux/interrupt.h>
>
> This should be sorted alphabetically.
>
> > #include <linux/kernel.h>
> > #include <linux/of_address.h>
> > #include <linux/of_clk.h>
> > @@ -3181,6 +3182,40 @@ static void wke_clear_wake_status(void)
> > WAKE_AOWAKE_STATUS_W((i * 32) +
> wake));
> > }
> > }
> > +
> > +/* translate sc7 wake sources back into irqs to catch edge triggered
> > +wakeups */ static void process_wake_event(int index, u32 status) {
> > + int irq;
>
> Interrupts are usually unsigned int. The index parameter can also be
> unsigned int because it will never be negative.
>
> > + irq_hw_number_t hwirq;
> > + int wake;
>
> Can be unsigned int as well. Also, it's usually best to define variables of the
> same time on a single line to make the code a bit shorter.
>
> > + unsigned long flags;
> > + struct irq_desc *desc;
> > + unsigned long ulong_status = (unsigned long)status;
>
> Why not just make the status parameter an unsigned long to avoid this
> clumsy construct?
>
> > +
> > + dev_info(pmc->dev, "Wake[%d:%d] status=0x%x\n", (index + 1) *
> 32,
> > + index * 32, status);
>
> At most these should be dev_dbg().
>
> > + for_each_set_bit(wake, &ulong_status, 32) {
> > + hwirq = wake + 32 * index;
> > +
> > +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
> > + irq = irq_find_mapping(pmc->domain, hwirq);
>
> We already use irq_domain_add_hierarchy(), and so the assumption is that
> IRQ_DOMAIN_HIERARCHY will always be enabled. You can drop this check.
> We may even want to make it explicit by selecting it from SOC_TEGRA_PMC.
>
> > +#else
> > + irq = hwirq;
> > +#endif
> > + desc = irq_to_desc(irq);
> > + if (!desc || !desc->action || !desc->action->name) {
> > + dev_info(pmc->dev, "Resume caused by WAKE%d,
> irq %d\n",
> > + (wake + 32 * index), irq);
> > + continue;
> > + }
> > + dev_info(pmc->dev, "Resume caused by WAKE%d, %s\n",
> > + (wake + 32 * index), desc->action->name);
>
> Same here.
>
> > + local_irq_save(flags);
> > + generic_handle_irq(irq);
> > + local_irq_restore(flags);
> > + }
> > +}
> > #endif /* CONFIG_ARM64 */
> >
> > static int tegra_pmc_suspend(struct device *dev) @@ -3219,6 +3254,15
> > @@ static int tegra_pmc_resume(struct device *dev)
> > struct tegra_pmc *pmc = dev_get_drvdata(dev);
> >
> > tegra_pmc_writel(pmc, 0x0, PMC_SCRATCH41);
> > +#else /* CONFIG_ARM64 */
> > + int i;
>
> unsigned int, please.
>
> Thierry
Agree to the comments. Will be updating the patch.
Thanks.
>
> > + u32 status;
> > +
> > + for (i = 0; i < WAKE_NR_VECTORS; i++) {
> > + status = readl(pmc->wake + WAKE_AOWAKE_STATUS_R(i));
> > + status = status & readl(pmc->wake +
> WAKE_AOWAKE_TIER2_ROUTING(i));
> > + process_wake_event(i, status);
> > + }
> > #endif
> >
> > return 0;
> > --
> > 2.17.1
> >