serial: PL011: move interrupt clearing

PATCH New Linus Walleij 5 participants 10 messages Mar 21, 2012 – Apr 3, 2012

Subsystems (2)

TTY layer and serial drivers Arm primecell UART PL010 and PL011 drivers

lore

Share & tools

lore

Linus Walleij

author

2012-03-21 19:15 UTC #0 lore raw

PATCH 1/1 serial: PL011: move interrupt clearing

Commit 360f748b204275229f8398cb2f9f53955db1503b "serial: PL011: clear pending interrupts" attempts to clear interrupts by writing to a yet-unassigned memory address. This fixes the issue.

The breaking patch is marked for stable so should be carried along with the other patch.

Cc Shreshtha Kumar Sahu <[email protected]>

Cc Russell King <[email protected]>

Cc stable <[email protected]>

Reported-by Viresh Kumar <[email protected]>

Signed-off-by Linus Walleij <[email protected]>

---
 drivers/tty/serial/amba-pl011.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

drivers/tty/serial/amba-pl011.c +4 -4

index 7e01399..4ed35c5 100644
@@ -1930,10 +1930,6 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 		goto unmap;
 	}
 
-	/* Ensure interrupts from this UART are masked and cleared */
-	writew(0, uap->port.membase + UART011_IMSC);
-	writew(0xffff, uap->port.membase + UART011_ICR);
-
 	uap->vendor = vendor;
 	uap->lcrh_rx = vendor->lcrh_rx;
 	uap->lcrh_tx = vendor->lcrh_tx;

@@ -1951,6 +1947,10 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 	uap->port.line = i;
 	pl011_dma_probe(uap);
 
+	/* Ensure interrupts from this UART are masked and cleared */
+	writew(0, uap->port.membase + UART011_IMSC);
+	writew(0xffff, uap->port.membase + UART011_ICR);
+
 	snprintf(uap->type, sizeof(uap->type), "PL011 rev%u", amba_rev(dev));
 
 	amba_ports[i] = uap;

Signature · 1 line

1.7.7.6

viresh kumar

2012-03-22 01:04 UTC #1 lore raw

PATCH 1/1 serial: PL011: move interrupt clearing

Quoted 8 lines · Linus Walleij

Commit 360f748b204275229f8398cb2f9f53955db1503b "serial: PL011: clear pending interrupts" attempts to clear interrupts by writing to a yet-unassigned memory address. This fixes the issue.
The breaking patch is marked for stable so should be carried along with the other patch.

Quoted 8 lines

Cc Shreshtha Kumar Sahu <[email protected]>
Cc Russell King <[email protected]>
Cc stable <[email protected]>
Reported-by Viresh Kumar <[email protected]>
Signed-off-by Linus Walleij <[email protected]>
---
?drivers/tty/serial/amba-pl011.c | ? ?8 ++++----
?1 files changed, 4 insertions(+), 4 deletions(-)

You missed me in CC??

Quoted 17 lines

drivers/tty/serial/amba-pl011.c +0 -4

index 7e01399..4ed35c5 100644
@@ -1930,10 +1930,6 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 ? ? ? ? ? ? ? ?goto unmap;
 ? ? ? ?}
 
- ? ? ? /* Ensure interrupts from this UART are masked and cleared */
- ? ? ? writew(0, uap->port.membase + UART011_IMSC);
- ? ? ? writew(0xffff, uap->port.membase + UART011_ICR);
-
 ? ? ? ?uap->vendor = vendor;
 ? ? ? ?uap->lcrh_rx = vendor->lcrh_rx;
 ? ? ? ?uap->lcrh_tx = vendor->lcrh_tx;

drivers/tty/serial/amba-pl011.c static int pl011_probe(struct amba_device *dev, const struct amba_id *id) · line 1956

Quoted 10 lines

drivers/tty/serial/amba-pl011.c +4 -0

@@ -1951,6 +1947,10 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 ? ? ? ?uap->port.line = i;
 ? ? ? ?pl011_dma_probe(uap);
 
+ ? ? ? /* Ensure interrupts from this UART are masked and cleared */
+ ? ? ? writew(0, uap->port.membase + UART011_IMSC);
+ ? ? ? writew(0xffff, uap->port.membase + UART011_ICR);
+
 ? ? ? ?snprintf(uap->type, sizeof(uap->type), "PL011 rev%u", amba_rev(dev));
 
 ? ? ? ?amba_ports[i] = uap;

Reviewed-by Viresh Kumar <[email protected]>

--
viresh

Linus Walleij

author

2012-03-22 08:00 UTC #2 lore raw

PATCH 1/1 serial: PL011: move interrupt clearing

Quoted 10 lines · viresh kumar

Quoted 8 lines
Cc Shreshtha Kumar Sahu <[email protected]>
Cc Russell King <[email protected]>
Cc stable <[email protected]>
Reported-by Viresh Kumar <[email protected]>
Signed-off-by Linus Walleij <[email protected]>
---
?drivers/tty/serial/amba-pl011.c | ? ?8 ++++----
?1 files changed, 4 insertions(+), 4 deletions(-)
You missed me in CC??

I put you in Reported-by, then used --cc at the prompt because git send-email still does not realize it must put Reported-by: tags on the CC list.

Yours,
Linus Walleij

viresh kumar

2012-03-23 03:39 UTC #3 lore raw

PATCH 1/1 serial: PL011: move interrupt clearing

Quoted 5 lines · Linus Walleij

Quoted 1 line · viresh kumar
You missed me in CC??
I put you in Reported-by, then used --cc at the prompt because git send-email still does not realize it must put Reported-by: tags on the CC list.

That's strange. When i check my mail on @st.com id, i see myself in cc list, but on
gmail i see following cc list in your first mail.

linux-serial at vger.kernel.org, Greg Kroah-Hartman <[email protected]>, Russell King <[email protected]>, Jong-Sung Kim <[email protected]>, stable <[email protected]>, Chanho Min <[email protected]>,
linux-arm-kernel at lists.infradead.org, Shreshtha Kumar Sahu <[email protected]>

Why don't i see myself in cc here?

I have seen similar issues earlier too, with our ST internal and Mainline mailing lists.
Any ideas?

--
viresh

Linus Walleij

author

2012-03-23 08:17 UTC #4 lore raw

PATCH 1/1 serial: PL011: move interrupt clearing

Quoted 3 lines · viresh kumar

That's strange. When i check my mail on @st.com id, i see myself in cc list, but on
gmail i see following cc list in your first mail.

(...)

Quoted 2 lines

Why don't i see myself in cc here?

No clue :-(

On *my* gmail it looks like this:

Cc: linux-arm-kernel at lists.infradead.org,
Chanho Min <[email protected]>,
Jong-Sung Kim <[email protected]>,
Viresh Kumar <[email protected]>,
Linus Walleij <[email protected]>, Shreshtha Kumar Sahu <[email protected]>, Russell King <[email protected]>,
stable <[email protected]>

Trust no gmail...

Linus Walleij

Grant Likely

2012-03-29 20:49 UTC #5 lore raw

PATCH 1/1 serial: PL011: move interrupt clearing

In reply to Linus Walleij

Quoted 5 lines · Linus Walleij

Commit 360f748b204275229f8398cb2f9f53955db1503b "serial: PL011: clear pending interrupts" attempts to clear interrupts by writing to a yet-unassigned memory address. This fixes the issue.

Quoted 8 lines

The breaking patch is marked for stable so should be carried along with the other patch.
Cc Shreshtha Kumar Sahu <[email protected]>
Cc Russell King <[email protected]>
Cc stable <[email protected]>
Reported-by Viresh Kumar <[email protected]>
Signed-off-by Linus Walleij <[email protected]>

Ugh; the original patch is obviously broken. How did it get applied without testing?

Greg, can you get this out to Linus ASAP please? I have one comment below, but I don't think it should block merging this patch.

Tested-by Grant Likely <[email protected]>

Quoted 20 lines

---
?drivers/tty/serial/amba-pl011.c | ? ?8 ++++----
?1 files changed, 4 insertions(+), 4 deletions(-)

drivers/tty/serial/amba-pl011.c +0 -4

index 7e01399..4ed35c5 100644
@@ -1930,10 +1930,6 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 ? ? ? ? ? ? ? ?goto unmap;
 ? ? ? ?}
 
- ? ? ? /* Ensure interrupts from this UART are masked and cleared */
- ? ? ? writew(0, uap->port.membase + UART011_IMSC);
- ? ? ? writew(0xffff, uap->port.membase + UART011_ICR);
-
 ? ? ? ?uap->vendor = vendor;
 ? ? ? ?uap->lcrh_rx = vendor->lcrh_rx;
 ? ? ? ?uap->lcrh_tx = vendor->lcrh_tx;

drivers/tty/serial/amba-pl011.c static int pl011_probe(struct amba_device *dev, const struct amba_id *id) · line 1953

Quoted 7 lines

drivers/tty/serial/amba-pl011.c +4 -0

@@ -1951,6 +1947,10 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 ? ? ? ?uap->port.line = i;
 ? ? ? ?pl011_dma_probe(uap);
 
+ ? ? ? /* Ensure interrupts from this UART are masked and cleared */
+ ? ? ? writew(0, uap->port.membase + UART011_IMSC);
+ ? ? ? writew(0xffff, uap->port.membase + UART011_ICR);
+

Is it correct to move the interrupt clearing below the pl011_dma_probe() call? I've tested the fix with the interrupt clearing both above and below the pl011_dma_probe() call and versatile qemu boots in both cases.

Linus Walleij

author

2012-03-29 21:22 UTC #6 lore raw

PATCH 1/1 serial: PL011: move interrupt clearing

Quoted 7 lines · Grant Likely

Quoted 4 lines
Commit 360f748b204275229f8398cb2f9f53955db1503b "serial: PL011: clear pending interrupts" attempts to clear interrupts by writing to a yet-unassigned memory address. This fixes the issue.
Ugh; the original patch is obviously broken. ?How did it get applied without testing?

I've felt bad about that for a while, but to the best of my knowledge I did test it. Either I did some manual slip with the boot images or there was something else causing it to actually work as it looked... Chanho also had it working IIRC.

Quoted 4 lines

Greg, can you get this out to Linus ASAP please? ?I have one comment below, but I don't think it should block merging this patch.
Tested-by Grant Likely <[email protected]>

Thanks.

Quoted 11 lines

Quoted 11 lines
- ? ? ? /* Ensure interrupts from this UART are masked and cleared */
- ? ? ? writew(0, uap->port.membase + UART011_IMSC);
- ? ? ? writew(0xffff, uap->port.membase + UART011_ICR);
-
? ? ? ?uap->vendor = vendor;
? ? ? ?uap->lcrh_rx = vendor->lcrh_rx;
? ? ? ?uap->lcrh_tx = vendor->lcrh_tx;
(quoted diff) +0 -0
@@ -1951,6 +1947,10 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 ? ? ? ?uap->port.line = i;
 ? ? ? ?pl011_dma_probe(uap);
 

Quoted 9 lines

Review static int pl011_probe(struct amba_device *dev, const struct amba_id *id) · line 1953
Quoted 4 lines
(quoted diff) +4 -0
+ ? ? ? /* Ensure interrupts from this UART are masked and cleared */
+ ? ? ? writew(0, uap->port.membase + UART011_IMSC);
+ ? ? ? writew(0xffff, uap->port.membase + UART011_ICR);
+
Is it correct to move the interrupt clearing below the pl011_dma_probe() call? ?I've tested the fix with the interrupt clearing both above and below the pl011_dma_probe() call and versatile qemu boots in both cases.

pl011_dma_probe() basically just make logical setup so we do the clear-out as the last step of setup.

Yours,
Linus Walleij

Greg Kroah-Hartman

maintainer

2012-03-29 21:33 UTC #7 lore raw

PATCH 1/1 serial: PL011: move interrupt clearing

In reply to Grant Likely

Quoted 15 lines · Grant Likely

Quoted 5 lines · Linus Walleij
Commit 360f748b204275229f8398cb2f9f53955db1503b "serial: PL011: clear pending interrupts" attempts to clear interrupts by writing to a yet-unassigned memory address. This fixes the issue.
Quoted 8 lines
The breaking patch is marked for stable so should be carried along with the other patch.
Cc Shreshtha Kumar Sahu <[email protected]>
Cc Russell King <[email protected]>
Cc stable <[email protected]>
Reported-by Viresh Kumar <[email protected]>
Signed-off-by Linus Walleij <[email protected]>

Quoted 7 lines

Ugh; the original patch is obviously broken. How did it get applied without testing?
Greg, can you get this out to Linus ASAP please? I have one comment below, but I don't think it should block merging this patch.
Tested-by Grant Likely <[email protected]>

Thanks, I'll get it to him after 3.4-rc1 is out, in a few days, along with other tty fixes being queued up.

greg k-h

Paul Gortmaker

2012-04-03 15:36 UTC #8 lore raw

PATCH 1/1 serial: PL011: move interrupt clearing

Quoted 19 lines · Greg Kroah-Hartman

Quoted 18 lines · Grant Likely
Quoted 5 lines · Linus Walleij
Commit 360f748b204275229f8398cb2f9f53955db1503b "serial: PL011: clear pending interrupts" attempts to clear interrupts by writing to a yet-unassigned memory address. This fixes the issue.
Quoted 8 lines
The breaking patch is marked for stable so should be carried along with the other patch.
Cc Shreshtha Kumar Sahu <[email protected]>
Cc Russell King <[email protected]>
Cc stable <[email protected]>
Reported-by Viresh Kumar <[email protected]>
Signed-off-by Linus Walleij <[email protected]>
Ugh; the original patch is obviously broken. ?How did it get applied without testing?

Quoted 7 lines

Quoted 4 lines
Greg, can you get this out to Linus ASAP please? ?I have one comment below, but I don't think it should block merging this patch.
Tested-by Grant Likely <[email protected]>
Thanks, I'll get it to him after 3.4-rc1 is out, in a few days, along with other tty fixes being queued up.

Hi Greg,

Just wanted to check this didn't fall through the cracks, as I don't see it in your tty-next or tty-linus branches.

The original bad commit causes Silent Boot Death on qemu ARM versatile, which is kind of nasty. It pretty much forces a bisection, which I did, which then led me here.

Thanks,
Paul.
--

Quoted 6 lines

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html

Greg Kroah-Hartman

maintainer

2012-04-03 16:19 UTC #9 lore raw

PATCH 1/1 serial: PL011: move interrupt clearing

Quoted 29 lines · Paul Gortmaker

Quoted 19 lines · Greg Kroah-Hartman
Quoted 18 lines · Grant Likely
Quoted 5 lines · Linus Walleij
Commit 360f748b204275229f8398cb2f9f53955db1503b "serial: PL011: clear pending interrupts" attempts to clear interrupts by writing to a yet-unassigned memory address. This fixes the issue.
Quoted 8 lines
The breaking patch is marked for stable so should be carried along with the other patch.
Cc Shreshtha Kumar Sahu <[email protected]>
Cc Russell King <[email protected]>
Cc stable <[email protected]>
Reported-by Viresh Kumar <[email protected]>
Signed-off-by Linus Walleij <[email protected]>
Ugh; the original patch is obviously broken. ?How did it get applied without testing?
Quoted 7 lines
Quoted 4 lines
Greg, can you get this out to Linus ASAP please? ?I have one comment below, but I don't think it should block merging this patch.
Tested-by Grant Likely <[email protected]>
Thanks, I'll get it to him after 3.4-rc1 is out, in a few days, along with other tty fixes being queued up.

Quoted 8 lines

Hi Greg,
Just wanted to check this didn't fall through the cracks, as I don't see it in your tty-next or tty-linus branches.
The original bad commit causes Silent Boot Death on qemu ARM versatile, which is kind of nasty. It pretty much forces a bisection, which I did, which then led me here.

It's still in my "to-apply" queue, don't worry, it's not lost.

greg k-h