[PATCH 0/2] bus: omap_l3_noc: Add resume hook to restore mask registers

[PATCH 0/2] bus: omap_l3_noc: Add resume hook to restore mask registers

[Keerthy]

l3_noc module loses context during both DS0 and RTC+DDR in self refresh modes.
This causes the previously masked l3 interrupts to trigger and the software
book keeping assumes the interrupts are masked and does nothing to mask.
This software/hardware out of sync leads to back to back l3 interrupts
and eventually causes a hang. Hence adding resume hook to restore the
mask registers based on the book keeping variables

Keerthy (2):
  bus: omap_l3_noc: Add resume hook to restore context
  bus: omap_l3_noc: Correct returning IRQ_HANDLED unconditionally in
    the irq handler

 drivers/bus/omap_l3_noc.c |   63 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 61 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] bus: omap_l3_noc: Add resume hook to restore context

[Keerthy]

On certain SoCs such as AM437x SoC, L3_noc error registers are
maintained in power domain such as per domain which looses context as part
of low power state such as RTC+DDR mode. On these platforms when we
mask interrupts which we cannot handle, the source of these interrupts
still remain on resume, however, the flag mux registers now contain
their reset value (unmasked) - this breaks the system with infinite
interrupts since we do not these interrupts to take place ever again.

To handle this: restore the masking of interrupts which we have
already recorded in the system as ones we cannot handle.

Fixes: 654fa7979b5db9a44b8 ("bus: omap_l3_noc: ignore masked out unclearable targets")
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/bus/omap_l3_noc.c |   55 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/bus/omap_l3_noc.c b/drivers/bus/omap_l3_noc.c
index 531ae59..b5eac29 100644
--- a/drivers/bus/omap_l3_noc.c
+++ b/drivers/bus/omap_l3_noc.c
@@ -296,11 +296,66 @@ static int omap_l3_probe(struct platform_device *pdev)
 	return ret;
 }
 
+#ifdef	CONFIG_PM
+
+/**
+ * l3_resume_noirq() - resume function for l3_noc
+ * @dev:	pointer to l3_noc device structure
+ *
+ * We only have the resume handler only since we
+ * have already maintained the delta register
+ * configuration as part of configuring the system
+ */
+static int l3_resume_noirq(struct device *dev)
+{
+	struct omap_l3 *l3 = dev_get_drvdata(dev);
+	int i;
+	struct l3_flagmux_data *flag_mux;
+	void __iomem *base, *mask_regx = NULL;
+	u32 mask_val;
+
+	for (i = 0; i < l3->num_modules; i++) {
+		base = l3->l3_base[i];
+		flag_mux = l3->l3_flagmux[i];
+		if (!flag_mux->mask_app_bits && !flag_mux->mask_dbg_bits)
+			continue;
+
+		mask_regx = base + flag_mux->offset + L3_FLAGMUX_MASK0 +
+			   (L3_APPLICATION_ERROR << 3);
+		mask_val = readl_relaxed(mask_regx);
+		mask_val &= ~(flag_mux->mask_app_bits);
+
+		writel_relaxed(mask_val, mask_regx);
+		mask_regx = base + flag_mux->offset + L3_FLAGMUX_MASK0 +
+			   (L3_DEBUG_ERROR << 3);
+		mask_val = readl_relaxed(mask_regx);
+		mask_val &= ~(flag_mux->mask_dbg_bits);
+
+		writel_relaxed(mask_val, mask_regx);
+	}
+
+	/* Dummy read to force OCP barrier */
+	if (mask_regx)
+		(void)readl(mask_regx);
+
+	return 0;
+}
+
+static const struct dev_pm_ops l3_dev_pm_ops = {
+	.resume_noirq		= l3_resume_noirq,
+};
+
+#define L3_DEV_PM_OPS (&l3_dev_pm_ops)
+#else
+#define L3_DEV_PM_OPS NULL
+#endif
+
 static struct platform_driver omap_l3_driver = {
 	.probe		= omap_l3_probe,
 	.driver		= {
 		.name		= "omap_l3_noc",
 		.owner		= THIS_MODULE,
+		.pm		= L3_DEV_PM_OPS,
 		.of_match_table = of_match_ptr(l3_noc_match),
 	},
 };
-- 
1.7.9.5

Re: [PATCH 1/2] bus: omap_l3_noc: Add resume hook to restore context

[Nishanth Menon]

On 11/09/2014 11:03 PM, Keerthy wrote:
> On certain SoCs such as AM437x SoC, L3_noc error registers are
> maintained in power domain such as per domain which looses context as part
> of low power state such as RTC+DDR mode. On these platforms when we
> mask interrupts which we cannot handle, the source of these interrupts
> still remain on resume, however, the flag mux registers now contain
> their reset value (unmasked) - this breaks the system with infinite
> interrupts since we do not these interrupts to take place ever again.
> 
> To handle this: restore the masking of interrupts which we have
> already recorded in the system as ones we cannot handle.
> 
> Fixes: 654fa7979b5db9a44b8 ("bus: omap_l3_noc: ignore masked out unclearable targets")
please use instead:
Fixes: 2100b595b7 ("bus: omap_l3_noc: ignore masked out unclearable
targets")

?


> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  drivers/bus/omap_l3_noc.c |   55 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/bus/omap_l3_noc.c b/drivers/bus/omap_l3_noc.c
> index 531ae59..b5eac29 100644
> --- a/drivers/bus/omap_l3_noc.c
> +++ b/drivers/bus/omap_l3_noc.c
> @@ -296,11 +296,66 @@ static int omap_l3_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> +#ifdef	CONFIG_PM
> +
> +/**
> + * l3_resume_noirq() - resume function for l3_noc
> + * @dev:	pointer to l3_noc device structure
> + *
> + * We only have the resume handler only since we
> + * have already maintained the delta register
> + * configuration as part of configuring the system
> + */
> +static int l3_resume_noirq(struct device *dev)
> +{
> +	struct omap_l3 *l3 = dev_get_drvdata(dev);
> +	int i;
> +	struct l3_flagmux_data *flag_mux;
> +	void __iomem *base, *mask_regx = NULL;
> +	u32 mask_val;
> +
> +	for (i = 0; i < l3->num_modules; i++) {
> +		base = l3->l3_base[i];
> +		flag_mux = l3->l3_flagmux[i];
> +		if (!flag_mux->mask_app_bits && !flag_mux->mask_dbg_bits)
> +			continue;
> +
> +		mask_regx = base + flag_mux->offset + L3_FLAGMUX_MASK0 +
> +			   (L3_APPLICATION_ERROR << 3);
> +		mask_val = readl_relaxed(mask_regx);
> +		mask_val &= ~(flag_mux->mask_app_bits);
> +
> +		writel_relaxed(mask_val, mask_regx);
> +		mask_regx = base + flag_mux->offset + L3_FLAGMUX_MASK0 +
> +			   (L3_DEBUG_ERROR << 3);
> +		mask_val = readl_relaxed(mask_regx);
> +		mask_val &= ~(flag_mux->mask_dbg_bits);
> +
> +		writel_relaxed(mask_val, mask_regx);
> +	}
> +
> +	/* Dummy read to force OCP barrier */
> +	if (mask_regx)
> +		(void)readl(mask_regx);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops l3_dev_pm_ops = {
> +	.resume_noirq		= l3_resume_noirq,
> +};
> +
> +#define L3_DEV_PM_OPS (&l3_dev_pm_ops)
> +#else
> +#define L3_DEV_PM_OPS NULL
> +#endif
> +
>  static struct platform_driver omap_l3_driver = {
>  	.probe		= omap_l3_probe,
>  	.driver		= {
>  		.name		= "omap_l3_noc",
>  		.owner		= THIS_MODULE,
> +		.pm		= L3_DEV_PM_OPS,
>  		.of_match_table = of_match_ptr(l3_noc_match),
>  	},
>  };
> 


-- 
Regards,
Nishanth Menon

[PATCH 2/2] bus: omap_l3_noc: Correct returning IRQ_HANDLED unconditionally in the irq handler

[Keerthy]

Correct returning IRQ_HANDLED unconditionally in the irq handler.
Return IRQ_NONE for some interrupt which we do not expect to be
handled in this handler. This prevents kernel stalling with back
to back spurious interrupts.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/bus/omap_l3_noc.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/omap_l3_noc.c b/drivers/bus/omap_l3_noc.c
index b5eac29..17d8659 100644
--- a/drivers/bus/omap_l3_noc.c
+++ b/drivers/bus/omap_l3_noc.c
@@ -222,10 +222,14 @@ static irqreturn_t l3_interrupt_handler(int irq, void *_l3)
 			}
 
 			/* Error found so break the for loop */
-			break;
+			return IRQ_HANDLED;
 		}
 	}
-	return IRQ_HANDLED;
+
+	dev_err(l3->dev, "L3 %s IRQ not handled!!\n",
+		inttype ? "debug" : "application");
+
+	return IRQ_NONE;
 }
 
 static const struct of_device_id l3_noc_match[] = {
-- 
1.7.9.5

Re: [PATCH 2/2] bus: omap_l3_noc: Correct returning IRQ_HANDLED unconditionally in the irq handler

[Nishanth Menon]

On 11/09/2014 11:03 PM, Keerthy wrote:
> Correct returning IRQ_HANDLED unconditionally in the irq handler.
> Return IRQ_NONE for some interrupt which we do not expect to be
> handled in this handler. This prevents kernel stalling with back
> to back spurious interrupts.
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>

Consider providing an appropriate fixes comment here as well?

> ---
>  drivers/bus/omap_l3_noc.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bus/omap_l3_noc.c b/drivers/bus/omap_l3_noc.c
> index b5eac29..17d8659 100644
> --- a/drivers/bus/omap_l3_noc.c
> +++ b/drivers/bus/omap_l3_noc.c
> @@ -222,10 +222,14 @@ static irqreturn_t l3_interrupt_handler(int irq, void *_l3)
>  			}
>  
>  			/* Error found so break the for loop */
> -			break;
> +			return IRQ_HANDLED;
>  		}
>  	}
> -	return IRQ_HANDLED;
> +
> +	dev_err(l3->dev, "L3 %s IRQ not handled!!\n",
> +		inttype ? "debug" : "application");
> +
> +	return IRQ_NONE;
>  }
>  
>  static const struct of_device_id l3_noc_match[] = {
> 


-- 
Regards,
Nishanth Menon

Re: [PATCH 0/2] bus: omap_l3_noc: Add resume hook to restore mask registers

[Nishanth Menon]

On 11/09/2014 11:03 PM, Keerthy wrote:
> l3_noc module loses context during both DS0 and RTC+DDR in self refresh modes.
> This causes the previously masked l3 interrupts to trigger and the software
> book keeping assumes the interrupts are masked and does nothing to mask.
> This software/hardware out of sync leads to back to back l3 interrupts
> and eventually causes a hang. Hence adding resume hook to restore the
> mask registers based on the book keeping variables
> 
> Keerthy (2):
>   bus: omap_l3_noc: Add resume hook to restore context
minor comment on this patch,

>   bus: omap_l3_noc: Correct returning IRQ_HANDLED unconditionally in
>     the irq handler
> 
>  drivers/bus/omap_l3_noc.c |   63 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 61 insertions(+), 2 deletions(-)
> 
Otherwise:
Acked-by: Nishanth Menon <nm@ti.com>

yep, AM437x did bring out some interesting surprises :(... thanks for
catching them.

-- 
Regards,
Nishanth Menon