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 <pshete@xxxxxxxxxx>
> Signed-off-by: Petlozu Pravareshwar <petlozup@xxxxxxxxxx>
> ---
> 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
>
Attachment:
signature.asc
Description: PGP signature
