tty: amba-pl011: fix earlycon register offsets

The REG_x macros are indices into a table, not register offsets. Since earlycon does not have access to the vendor data, we can currently only support standard ARM PL011 devices.

Signed-off-by Timur Tabi <[email protected]>

---
 drivers/tty/serial/amba-pl011.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

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

index f6ad383..06f827a 100644
@@ -2302,10 +2302,10 @@ static struct console amba_console = {
 
 static void pl011_putc(struct uart_port *port, int c)
 {
-	while (readl(port->membase + REG_FR) & UART01x_FR_TXFF)
+	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
 		;
-	writeb(c, port->membase + REG_DR);
-	while (readl(port->membase + REG_FR) & UART01x_FR_BUSY)
+	writeb(c, port->membase + UART01x_DR);
+	while (readl(port->membase + UART01x_FR) & UART011_FR_BUSY)
 		;
 }
 

Signature · 3 lines

Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Quoted 5 lines · Timur Tabi

The REG_x macros are indices into a table, not register offsets. Since earlycon does not have access to the vendor data, we can currently only support standard ARM PL011 devices.

Signed-off-by Timur Tabi <[email protected]>

Please credit me with the change; this was obviously a change I made when I posted the updated patches, which Greg had failed to take instead of the original set. Thanks.

Hi Russell,

Quoted 7 lines · Russell King - ARM Linux

Quoted 5 lines · Timur Tabi

The REG_x macros are indices into a table, not register offsets. Since earlycon does not have access to the vendor data, we can currently only support standard ARM PL011 devices.

Signed-off-by Timur Tabi <[email protected]>

Quoted 4 lines

Please credit me with the change; this was obviously a change I made when I posted the updated patches, which Greg had failed to take instead of the original set. Thanks.

I don't see this patch in linux-next. Without this it fails to boot(panics) on ARM64 when earlycon is enabled.

Also I think this fix might not be correct
for ZTE pl011, though it works for ST and standard PL011.

Quoted 18 lines · Sudeep Holla

Hi Russell,

Quoted 11 lines · Russell King - ARM Linux

Quoted 5 lines · Timur Tabi

The REG_x macros are indices into a table, not register offsets. Since earlycon does not have access to the vendor data, we can currently only support standard ARM PL011 devices.

Signed-off-by Timur Tabi <[email protected]>

Please credit me with the change; this was obviously a change I made when I posted the updated patches, which Greg had failed to take instead of the original set. Thanks.

I don't see this patch in linux-next. Without this it fails to boot(panics) on ARM64 when earlycon is enabled.

As far as 4.4 goes, I think it's fate was sealed when Greg took the wrong set of patches. It's Greg's problem to sort out now.

Quoted 2 lines

Also I think this fix might not be correct
for ZTE pl011, though it works for ST and standard PL011.

Yes, earlycon won't work for ZTE PL011. This isn't a big problem at the moment, because the patch set doesn't wire up the ZTE PL011 itself.

The reason there is that I'm saying no to using a platform device; I really do not like drivers which end up with multiple different probe and remove methods. The AMBA PL011 driver is an AMBA primecell driver, and it sits on the AMBA primecell bus, not the platform bus.

What I want to see is some way of having the ZTE PL011 appearing on the AMBA bus, which means we need to come up with some way to deal with primecells which don't have an ID. That is an open issue, as is how to deal with earlycon.

Quoted 11 lines · Sudeep Holla

Hi Russell,

Quoted 7 lines · Russell King - ARM Linux

Quoted 5 lines · Timur Tabi

The REG_x macros are indices into a table, not register offsets. Since earlycon does not have access to the vendor data, we can currently only support standard ARM PL011 devices.

Signed-off-by Timur Tabi <[email protected]>

Quoted 7 lines

Quoted 4 lines

Please credit me with the change; this was obviously a change I made when I posted the updated patches, which Greg had failed to take instead of the original set. Thanks.

I don't see this patch in linux-next. Without this it fails to boot(panics) on ARM64 when earlycon is enabled.

I guess that's the way 4.4 is going to be then, because GregKH has not been anywhere near "responsive" during the last cycle, but he did say yesterday (in response to questions about driver model stuff) that he's closed his trees for the merge window last week.

All in all, this situation is entirely GregKH's making, as he took the wrong set of patches, and has yet to respond to _any_ of the resulting mails about it... I guess GregKH knows what he's doing as he's one of the top (and vocal) kernel developers far more than I do, so I guess he has his reasons for crapping up the AMBA PL011 driver...

I rather wish that I'd decided to do the same with the AMBA PL011 driver, because this seems to be the only sensible way of ensuring that the right set of patches get to Linus.

I had put the _right_ set of AMBA PL011 driver changes into linux-next just before Christmas, but Stephen Rothwell tells me that they merge without conflict with the set that Greg merged, and we still end up with the broken code. So there's nothing I can do about it at this point.

As far as 4.4 goes, I think it's fate was sealed when Greg took the wrong set of patches. It's Greg's problem to sort out now.

Quoted 25 lines · Russell King - ARM Linux

Quoted 11 lines · Sudeep Holla

Hi Russell,

Quoted 7 lines · Russell King - ARM Linux

Quoted 5 lines · Timur Tabi

The REG_x macros are indices into a table, not register offsets. Since earlycon does not have access to the vendor data, we can currently only support standard ARM PL011 devices.

Signed-off-by Timur Tabi <[email protected]>

Quoted 7 lines

Quoted 4 lines

Please credit me with the change; this was obviously a change I made when I posted the updated patches, which Greg had failed to take instead of the original set. Thanks.

I don't see this patch in linux-next. Without this it fails to boot(panics) on ARM64 when earlycon is enabled.

I guess that's the way 4.4 is going to be then, because GregKH has not been anywhere near "responsive" during the last cycle, but he did say yesterday (in response to questions about driver model stuff) that he's closed his trees for the merge window last week.

Quoted 5 lines

All in all, this situation is entirely GregKH's making, as he took the wrong set of patches, and has yet to respond to _any_ of the resulting mails about it... I guess GregKH knows what he's doing as he's one of the top (and vocal) kernel developers far more than I do, so I guess he has his reasons for crapping up the AMBA PL011 driver...

"plenty of time"? I see Timur's patches to fix this were sent on December 24th. Then fixed up and resent on January 4th. I see nothing in my todo queue that were sent earlier to resolve any of this horrid mess.

So yes, I haven't done anything with the Jan 04 patch, given that it's been 24 hours since it was sent, that's totally reasonable.

You all were throwing huge numbers of patches here for this tiny driver and digging through the mess was a major pain.

Turned out I guessed
wrong, and asked for a patch to fix up the mess once that was determined.

That didn't arrive until the yesterday in a format that might be acceptable. Quite a while after this all was determined to be "broken".

If you all think you could do better with the patch load you all were throwing at me, well, good for you. It's mighty easy to complain when it isn't your inbox... And I really don't care at all about this little driver, you all do, and yet you all can't agree what to do about it, so to somehow claim that I know better is a joke.

Quoted 32 lines · Greg Kroah-Hartman

Quoted 25 lines · Russell King - ARM Linux

Quoted 11 lines · Sudeep Holla

Hi Russell,

Quoted 7 lines · Russell King - ARM Linux

Quoted 5 lines · Timur Tabi

The REG_x macros are indices into a table, not register offsets. Since earlycon does not have access to the vendor data, we can currently only support standard ARM PL011 devices.

Signed-off-by Timur Tabi <[email protected]>

Quoted 7 lines

Quoted 4 lines

Please credit me with the change; this was obviously a change I made when I posted the updated patches, which Greg had failed to take instead of the original set. Thanks.

I don't see this patch in linux-next. Without this it fails to boot(panics) on ARM64 when earlycon is enabled.

I guess that's the way 4.4 is going to be then, because GregKH has not been anywhere near "responsive" during the last cycle, but he did say yesterday (in response to questions about driver model stuff) that he's closed his trees for the merge window last week.

Quoted 5 lines

All in all, this situation is entirely GregKH's making, as he took the wrong set of patches, and has yet to respond to _any_ of the resulting mails about it... I guess GregKH knows what he's doing as he's one of the top (and vocal) kernel developers far more than I do, so I guess he has his reasons for crapping up the AMBA PL011 driver...

Quoted 7 lines

"plenty of time"? I see Timur's patches to fix this were sent on December 24th. Then fixed up and resent on January 4th. I see nothing in my todo queue that were sent earlier to resolve any of this horrid mess.

So yes, I haven't done anything with the Jan 04 patch, given that it's been 24 hours since it was sent, that's totally reasonable.

The first series of 11 patches was sent on November 3rd. The problem at the root of this issue was discovered on the very same day, and is a part of the thread. People reviewed and tested it, and other comments were made.

These were addressed, and the next series of 12 patches were sent on the 16th November.

On the 12th November, you decided it was time to pick up the first series of patches and ignore the second series, despite the comments against the first series indicating that there were problems.

It was only realised on the 24th that you'd picked up the wrong set of patches.

Quoted 2 lines

You all were throwing huge numbers of patches here for this tiny driver and digging through the mess was a major pain.

That statement is doing nothing but trying to deflect the blame for this mistake on to other people.

11 or 12 patches is not "huge numbers of patches" and a driver of 2600 lines is not "tiny". In total, it's _only_ 11 + 12 patches. That is not "huge" by any sense of the word.

What would you prefer? One patch making multiple changes? Aren't we always telling people to split up their changes? I guess you're different because of your patch load...

Quoted 3 lines

Turned out I guessed
wrong, and asked for a patch to fix up the mess once that was determined.

I don't see why there should've been any guessing. One series of 11 patches posted on 3rd November vs a second series of 12 patches posted on the 16th November. Later date, one more patch. How could there have been any guessing?

Quoted 5 lines

If you all think you could do better with the patch load you all were throwing at me, well, good for you. It's mighty easy to complain when it isn't your inbox... And I really don't care at all about this little driver, you all do, and yet you all can't agree what to do about it, so to somehow claim that I know better is a joke.

Right, so what you're saying is that you're overloaded, and can't cope with the amount of work that you've chosen to take on.

The one thing that's missing from your message is to ask whether someone is willing to take on maintanence of the driver. Well, that's an interesting subject, because in theory, I _never_ delegated maintanence and patch handling to anyone else:

ARM PRIMECELL UART PL010 AND PL011 DRIVERS

M:      Russell King <[email protected]>
S:      Maintained
F:      drivers/tty/serial/amba-pl01*.c
F:      include/linux/amba/serial.h

but someone decided "I own drivers/tty..., so all patches _must_ come through me" which is a frequent pattern I've seen forming over the years that we've had the kernel in SCMs.

So, I guess the answer is for me to take back responsibility for merging patches for this driver and send pull requests for it directly to Linus.

Please merge what you have, and when it's merged, I'll resolve the differences between what you have merged and what _should_ have been merged. I'll send fixes directly to Linus, and from then on I'll be looking after this driver.

Quoted 62 lines · Russell King - ARM Linux

Quoted 32 lines · Greg Kroah-Hartman

Quoted 25 lines · Russell King - ARM Linux

Quoted 11 lines · Sudeep Holla

Hi Russell,

Quoted 7 lines · Russell King - ARM Linux

Quoted 5 lines · Timur Tabi

The REG_x macros are indices into a table, not register offsets. Since earlycon does not have access to the vendor data, we can currently only support standard ARM PL011 devices.

Signed-off-by Timur Tabi <[email protected]>

Quoted 7 lines

Quoted 4 lines

Please credit me with the change; this was obviously a change I made when I posted the updated patches, which Greg had failed to take instead of the original set. Thanks.

I don't see this patch in linux-next. Without this it fails to boot(panics) on ARM64 when earlycon is enabled.

I guess that's the way 4.4 is going to be then, because GregKH has not been anywhere near "responsive" during the last cycle, but he did say yesterday (in response to questions about driver model stuff) that he's closed his trees for the merge window last week.

Quoted 5 lines

All in all, this situation is entirely GregKH's making, as he took the wrong set of patches, and has yet to respond to _any_ of the resulting mails about it... I guess GregKH knows what he's doing as he's one of the top (and vocal) kernel developers far more than I do, so I guess he has his reasons for crapping up the AMBA PL011 driver...

Quoted 7 lines

"plenty of time"? I see Timur's patches to fix this were sent on December 24th. Then fixed up and resent on January 4th. I see nothing in my todo queue that were sent earlier to resolve any of this horrid mess.

So yes, I haven't done anything with the Jan 04 patch, given that it's been 24 hours since it was sent, that's totally reasonable.

The first series of 11 patches was sent on November 3rd. The problem at the root of this issue was discovered on the very same day, and is a part of the thread. People reviewed and tested it, and other comments were made.

These were addressed, and the next series of 12 patches were sent on the 16th November.

On the 12th November, you decided it was time to pick up the first series of patches and ignore the second series, despite the comments against the first series indicating that there were problems.

It was only realised on the 24th that you'd picked up the wrong set of patches.

Quoted 2 lines

You all were throwing huge numbers of patches here for this tiny driver and digging through the mess was a major pain.

That statement is doing nothing but trying to deflect the blame for this mistake on to other people.

Quoted 7 lines

11 or 12 patches is not "huge numbers of patches" and a driver of 2600 lines is not "tiny". In total, it's _only_ 11 + 12 patches. That is not "huge" by any sense of the word.

What would you prefer? One patch making multiple changes? Aren't we always telling people to split up their changes? I guess you're different because of your patch load...

There were more than just these two series of patches for this driver floating around in my todo queue, there were competing sets of patches from Timur and you and I thought I had applied the correct one. I get things wrong sometimes, but after I did it took a long time for a fix to show up. Yes, due to the hollidays, I get it, I've been very busy with non-kernel-stuff for the past months and I'm way behind as well.

And don't be snarky about splitting up patches, that wasn't the issue here.

Quoted 8 lines

Quoted 3 lines

Turned out I guessed
wrong, and asked for a patch to fix up the mess once that was determined.

I don't see why there should've been any guessing. One series of 11 patches posted on 3rd November vs a second series of 12 patches posted on the 16th November. Later date, one more patch. How could there have been any guessing?

I don't remember, sorry, but something confused me, I got it wrong, sorry. First time for 2015, not too bad :)

Quoted 8 lines

Quoted 5 lines

If you all think you could do better with the patch load you all were throwing at me, well, good for you. It's mighty easy to complain when it isn't your inbox... And I really don't care at all about this little driver, you all do, and yet you all can't agree what to do about it, so to somehow claim that I know better is a joke.

Right, so what you're saying is that you're overloaded, and can't cope with the amount of work that you've chosen to take on.

Again, I had real-world things keeping me from doing a lot of kernel patch review for the past few months, so I got behind, sorry.

Quoted 6 lines

The one thing that's missing from your message is to ask whether someone is willing to take on maintanence of the driver. Well, that's an interesting subject, because in theory, I _never_ delegated maintanence and patch handling to anyone else:

ARM PRIMECELL UART PL010 AND PL011 DRIVERS

Quoted 8 lines

M:      Russell King <[email protected]>
S:      Maintained
F:      drivers/tty/serial/amba-pl01*.c
F:      include/linux/amba/serial.h

but someone decided "I own drivers/tty..., so all patches _must_ come through me" which is a frequent pattern I've seen forming over the years that we've had the kernel in SCMs.

"decided"? Hah, as if, that happened many years ago, that's nothing new at all, you didn't suddenly realize this, again, stop it with the snark.

Quoted 3 lines

So, I guess the answer is for me to take back responsibility for merging patches for this driver and send pull requests for it directly to Linus.

That's not how kernel development works, sorry, you know this. I'll gladly just ignore all amba-pl01* patches except when you bundle them up and forward them on to me. In fact, I'll take pull requests as well, just send them on to me please whenever you feel they are ready for inclusion.

And good luck trying to get Linus to take pull requests for a single driver, that sure doesn't scale :)

Quoted 4 lines

Please merge what you have, and when it's merged, I'll resolve the differences between what you have merged and what _should_ have been merged. I'll send fixes directly to Linus, and from then on I'll be looking after this driver.

I've merged the fixes now, and only have one old patch from Timur about adding cpu_relax to the driver, and there's some random AMBA bus patches that touch it as well, which comments from the "ARM developers" have been asked for, but don't seem to be happening for some odd reason...

greg k-h

The REG_x macros are indices into a table, not register offsets. Since earlycon does not have access to the vendor data, we can currently only support standard ARM PL011 devices.

Signed-off-by Timur Tabi <[email protected]>

---
 drivers/tty/serial/amba-pl011.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

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

index f6ad383..06f827a 100644

The REG_x macros are indices into a table, not register offsets. Since earlycon does not have access to the vendor data, we can currently only support standard ARM PL011 devices.

Signed-off-by Timur Tabi <[email protected]>

---
 drivers/tty/serial/amba-pl011.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

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

index f6ad383..06f827a 100644