linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Bugfixes for Tegra memory controllers.
@ 2013-06-10  9:13 Tuomas Tynkkynen
  2013-06-10  9:13 ` [PATCH 1/2] memory: tegra20-mc: Fix hang in IRQ handler Tuomas Tynkkynen
  2013-06-10  9:13 ` [PATCH 2/2] memory: tegra30-mc: Fix IRQ handler bugs Tuomas Tynkkynen
  0 siblings, 2 replies; 6+ messages in thread
From: Tuomas Tynkkynen @ 2013-06-10  9:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-tegra, linux-arm-kernel, linux-kernel, Tuomas Tynkkynen

Hi,

Here's two simple bugfixes for the Tegra memory controllers.

Tuomas Tynkkynen (2):
  memory: tegra20-mc: Fix hang in IRQ handler.
  memory: tegra30-mc: Fix IRQ handler.

 drivers/memory/tegra20-mc.c | 5 ++++-
 drivers/memory/tegra30-mc.c | 9 ++++++---
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
1.8.1.5


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] memory: tegra20-mc: Fix hang in IRQ handler.
  2013-06-10  9:13 [PATCH 0/2] Bugfixes for Tegra memory controllers Tuomas Tynkkynen
@ 2013-06-10  9:13 ` Tuomas Tynkkynen
  2013-06-10 20:36   ` Thierry Reding
  2013-06-10  9:13 ` [PATCH 2/2] memory: tegra30-mc: Fix IRQ handler bugs Tuomas Tynkkynen
  1 sibling, 1 reply; 6+ messages in thread
From: Tuomas Tynkkynen @ 2013-06-10  9:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-tegra, linux-arm-kernel, linux-kernel, Tuomas Tynkkynen

In Tegra20 memory controller any MC interrupt would cause an
infinite loop in the IRQ handler.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
 drivers/memory/tegra20-mc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/memory/tegra20-mc.c b/drivers/memory/tegra20-mc.c
index 2ca5f28..bd5a553 100644
--- a/drivers/memory/tegra20-mc.c
+++ b/drivers/memory/tegra20-mc.c
@@ -193,8 +193,11 @@ static irqreturn_t tegra20_mc_isr(int irq, void *data)
 	mask &= stat;
 	if (!mask)
 		return IRQ_NONE;
-	while ((bit = ffs(mask)) != 0)
+	while ((bit = ffs(mask)) != 0) {
 		tegra20_mc_decode(mc, bit - 1);
+		mask &= BIT(bit);
+	}
+
 	mc_writel(mc, stat, MC_INTSTATUS);
 	return IRQ_HANDLED;
 }
-- 
1.8.1.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] memory: tegra30-mc: Fix IRQ handler bugs.
  2013-06-10  9:13 [PATCH 0/2] Bugfixes for Tegra memory controllers Tuomas Tynkkynen
  2013-06-10  9:13 ` [PATCH 1/2] memory: tegra20-mc: Fix hang in IRQ handler Tuomas Tynkkynen
@ 2013-06-10  9:13 ` Tuomas Tynkkynen
  2013-06-10 20:37   ` Thierry Reding
  1 sibling, 1 reply; 6+ messages in thread
From: Tuomas Tynkkynen @ 2013-06-10  9:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-tegra, linux-arm-kernel, linux-kernel, Tuomas Tynkkynen

In Tegra30 memory controller any MC interrupt would cause an infinite loop in
the IRQ handler. Additionally, a garbage pointer was used to read the MC
status registers, which causes wrong values to be printed if a MC error
occurred.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
 drivers/memory/tegra30-mc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/memory/tegra30-mc.c b/drivers/memory/tegra30-mc.c
index f4ae074..a6a05a2 100644
--- a/drivers/memory/tegra30-mc.c
+++ b/drivers/memory/tegra30-mc.c
@@ -218,7 +218,7 @@ static void tegra30_mc_decode(struct tegra30_mc *mc, int n)
 		return;
 	}
 
-	err = readl(mc + MC_ERR_STATUS);
+	err = mc_readl(mc, MC_ERR_STATUS);
 
 	type = (err & MC_ERR_TYPE_MASK) >> MC_ERR_TYPE_SHIFT;
 	perm = (err & MC_ERR_INVALID_SMMU_PAGE_MASK) >>
@@ -235,7 +235,7 @@ static void tegra30_mc_decode(struct tegra30_mc *mc, int n)
 	if (cid < ARRAY_SIZE(tegra30_mc_client))
 		client = tegra30_mc_client[cid];
 
-	addr = readl(mc + MC_ERR_ADR);
+	addr = mc_readl(mc, MC_ERR_ADR);
 
 	dev_err_ratelimited(mc->dev, "%s (0x%08x): 0x%08x %s (%s %s %s %s)\n",
 			   mc_int_err[idx], err, addr, client,
@@ -313,8 +313,11 @@ static irqreturn_t tegra30_mc_isr(int irq, void *data)
 	mask &= stat;
 	if (!mask)
 		return IRQ_NONE;
-	while ((bit = ffs(mask)) != 0)
+	while ((bit = ffs(mask)) != 0) {
 		tegra30_mc_decode(mc, bit - 1);
+		mask &= BIT(bit);
+	}
+
 	mc_writel(mc, stat, MC_INTSTATUS);
 	return IRQ_HANDLED;
 }
-- 
1.8.1.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] memory: tegra20-mc: Fix hang in IRQ handler.
  2013-06-10  9:13 ` [PATCH 1/2] memory: tegra20-mc: Fix hang in IRQ handler Tuomas Tynkkynen
@ 2013-06-10 20:36   ` Thierry Reding
  2013-06-11 10:09     ` Tuomas Tynkkynen
  0 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2013-06-10 20:36 UTC (permalink / raw)
  To: Tuomas Tynkkynen; +Cc: gregkh, linux-tegra, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 972 bytes --]

On Mon, Jun 10, 2013 at 12:13:43PM +0300, Tuomas Tynkkynen wrote:
> In Tegra20 memory controller any MC interrupt would cause an
> infinite loop in the IRQ handler.
> 
> Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
> ---
>  drivers/memory/tegra20-mc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/memory/tegra20-mc.c b/drivers/memory/tegra20-mc.c
> index 2ca5f28..bd5a553 100644
> --- a/drivers/memory/tegra20-mc.c
> +++ b/drivers/memory/tegra20-mc.c
> @@ -193,8 +193,11 @@ static irqreturn_t tegra20_mc_isr(int irq, void *data)
>  	mask &= stat;
>  	if (!mask)
>  		return IRQ_NONE;
> -	while ((bit = ffs(mask)) != 0)
> +	while ((bit = ffs(mask)) != 0) {
>  		tegra20_mc_decode(mc, bit - 1);
> +		mask &= BIT(bit);

Shouldn't this be "mask &= ~BIT(bit);"? The intent of the code is to
clear the bit which was handled by the loop body, right? The above
clears all other bits instead.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] memory: tegra30-mc: Fix IRQ handler bugs.
  2013-06-10  9:13 ` [PATCH 2/2] memory: tegra30-mc: Fix IRQ handler bugs Tuomas Tynkkynen
@ 2013-06-10 20:37   ` Thierry Reding
  0 siblings, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2013-06-10 20:37 UTC (permalink / raw)
  To: Tuomas Tynkkynen; +Cc: gregkh, linux-tegra, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

On Mon, Jun 10, 2013 at 12:13:44PM +0300, Tuomas Tynkkynen wrote:
[...]
> diff --git a/drivers/memory/tegra30-mc.c b/drivers/memory/tegra30-mc.c
[...]
> @@ -313,8 +313,11 @@ static irqreturn_t tegra30_mc_isr(int irq, void *data)
>  	mask &= stat;
>  	if (!mask)
>  		return IRQ_NONE;
> -	while ((bit = ffs(mask)) != 0)
> +	while ((bit = ffs(mask)) != 0) {
>  		tegra30_mc_decode(mc, bit - 1);
> +		mask &= BIT(bit);

Same comment as for patch 1/2.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] memory: tegra20-mc: Fix hang in IRQ handler.
  2013-06-10 20:36   ` Thierry Reding
@ 2013-06-11 10:09     ` Tuomas Tynkkynen
  0 siblings, 0 replies; 6+ messages in thread
From: Tuomas Tynkkynen @ 2013-06-11 10:09 UTC (permalink / raw)
  To: Thierry Reding; +Cc: gregkh, linux-tegra, linux-arm-kernel, linux-kernel

On 06/10/2013 11:36 PM, Thierry Reding wrote:
> On Mon, Jun 10, 2013 at 12:13:43PM +0300, Tuomas Tynkkynen wrote:
>> In Tegra20 memory controller any MC interrupt would cause an
>> infinite loop in the IRQ handler.
>>
>> Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
>> ---
>>  drivers/memory/tegra20-mc.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/memory/tegra20-mc.c b/drivers/memory/tegra20-mc.c
>> index 2ca5f28..bd5a553 100644
>> --- a/drivers/memory/tegra20-mc.c
>> +++ b/drivers/memory/tegra20-mc.c
>> @@ -193,8 +193,11 @@ static irqreturn_t tegra20_mc_isr(int irq, void *data)
>>  	mask &= stat;
>>  	if (!mask)
>>  		return IRQ_NONE;
>> -	while ((bit = ffs(mask)) != 0)
>> +	while ((bit = ffs(mask)) != 0) {
>>  		tegra20_mc_decode(mc, bit - 1);
>> +		mask &= BIT(bit);
> 
> Shouldn't this be "mask &= ~BIT(bit);"? The intent of the code is to
> clear the bit which was handled by the loop body, right? The above
> clears all other bits instead.
> 
> Thierry

Whoops, yes it should be clearing just one bit. And since ffs() returned
a one-based bit-index, it seemed to work in practice.

I'll fix those & resend.

- Tuomas

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-06-11 10:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-10  9:13 [PATCH 0/2] Bugfixes for Tegra memory controllers Tuomas Tynkkynen
2013-06-10  9:13 ` [PATCH 1/2] memory: tegra20-mc: Fix hang in IRQ handler Tuomas Tynkkynen
2013-06-10 20:36   ` Thierry Reding
2013-06-11 10:09     ` Tuomas Tynkkynen
2013-06-10  9:13 ` [PATCH 2/2] memory: tegra30-mc: Fix IRQ handler bugs Tuomas Tynkkynen
2013-06-10 20:37   ` Thierry Reding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).