[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[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 <timur@codeaurora.org>
---
 drivers/tty/serial/amba-pl011.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index f6ad383..06f827a 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -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)
 		;
 }
 
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

[PATCH 2/2] tty: amba-pl011: use iotype instead of access_32b to track 32-bit I/O

[Timur Tabi]

Instead of defining a new field in the uart_amba_port structure, use the
existing iotype field of the uart_port structure, which is intended for
this purpose.  If we need to use 32-bit register access, we set iotype
to UPIO_MEM32, otherwise we set it to UPIO_MEM.

For early console, specify the "mmio32" option on the kernel command-line.
Example:

        earlycon=pl011,mmio32,0x3ced1000

Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
 Documentation/kernel-parameters.txt |  5 ++++-
 drivers/tty/serial/amba-pl011.c     | 16 +++++++++-------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 474ce69..ab11d5c 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -995,10 +995,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			unspecified, the h/w is not initialized.
 
 		pl011,<addr>
+		pl011,mmio32,<addr>
 			Start an early, polled-mode console on a pl011 serial
 			port at the specified address. The pl011 serial port
 			must already be setup and configured. Options are not
-			yet supported.
+			yet supported.  If 'mmio32' is specified, then only
+			the driver will use only 32-bit accessors to read/write
+			the device registers.
 
 		msm_serial,<addr>
 			Start an early, polled-mode console on an msm serial
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 06f827a..f2793c1 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -239,7 +239,6 @@ struct uart_amba_port {
 	unsigned int		fifosize;	/* vendor-specific */
 	unsigned int		old_cr;		/* state during shutdown */
 	bool			autorts;
-	bool			access_32b;
 	unsigned int		fixed_baud;	/* vendor-set fixed baud rate */
 	char			type[12];
 #ifdef CONFIG_DMA_ENGINE
@@ -263,7 +262,8 @@ static unsigned int pl011_read(const struct uart_amba_port *uap,
 {
 	void __iomem *addr = uap->port.membase + pl011_reg_to_offset(uap, reg);
 
-	return uap->access_32b ? readl_relaxed(addr) : readw_relaxed(addr);
+	return (uap->port.iotype == UPIO_MEM32) ?
+		readl_relaxed(addr) : readw_relaxed(addr);
 }
 
 static void pl011_write(unsigned int val, const struct uart_amba_port *uap,
@@ -271,7 +271,7 @@ static void pl011_write(unsigned int val, const struct uart_amba_port *uap,
 {
 	void __iomem *addr = uap->port.membase + pl011_reg_to_offset(uap, reg);
 
-	if (uap->access_32b)
+	if (uap->port.iotype == UPIO_MEM32)
 		writel_relaxed(val, addr);
 	else
 		writew_relaxed(val, addr);
@@ -2304,7 +2304,10 @@ static void pl011_putc(struct uart_port *port, int c)
 {
 	while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
 		;
-	writeb(c, port->membase + UART01x_DR);
+	if (port->iotype == UPIO_MEM32)
+		writel(c, port->membase + UART01x_DR);
+	else
+		writeb(c, port->membase + UART01x_DR);
 	while (readl(port->membase + UART01x_FR) & UART011_FR_BUSY)
 		;
 }
@@ -2417,7 +2420,6 @@ static int pl011_setup_port(struct device *dev, struct uart_amba_port *uap,
 	uap->port.dev = dev;
 	uap->port.mapbase = mmiobase->start;
 	uap->port.membase = base;
-	uap->port.iotype = UPIO_MEM;
 	uap->port.fifosize = uap->fifosize;
 	uap->port.flags = UPF_BOOT_AUTOCONF;
 	uap->port.line = index;
@@ -2471,9 +2473,9 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 		return PTR_ERR(uap->clk);
 
 	uap->reg_offset = vendor->reg_offset;
-	uap->access_32b = vendor->access_32b;
 	uap->vendor = vendor;
 	uap->fifosize = vendor->get_fifosize(dev);
+	uap->port.iotype = vendor->access_32b ? UPIO_MEM32 : UPIO_MEM;
 	uap->port.irq = dev->irq[0];
 	uap->port.ops = &amba_pl011_pops;
 
@@ -2552,9 +2554,9 @@ static int sbsa_uart_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	uap->reg_offset	= vendor_sbsa.reg_offset;
-	uap->access_32b = vendor_sbsa.access_32b;
 	uap->vendor	= &vendor_sbsa;
 	uap->fifosize	= 32;
+	uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
 	uap->port.irq	= platform_get_irq(pdev, 0);
 	uap->port.ops	= &sbsa_uart_pops;
 	uap->fixed_baud = baudrate;
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[Russell King - ARM Linux]

On Thu, Dec 24, 2015 at 09:49:48AM -0600, Timur Tabi wrote:
> 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 <timur@codeaurora.org>

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.

> ---
>  drivers/tty/serial/amba-pl011.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index f6ad383..06f827a 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -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)
>  		;
>  }
>  
> -- 
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project.
> 

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[Sudeep Holla]

Hi Russell,

On Thu, Dec 24, 2015 at 4:47 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Dec 24, 2015 at 09:49:48AM -0600, Timur Tabi wrote:
>> 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 <timur@codeaurora.org>
>
> 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.

Also since there's no logs on serial, it difficult to identify the
issue and better
to have some solution in place before many people start hitting this issue.

Regards,
Sudeep

[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[Russell King - ARM Linux]

On Tue, Jan 05, 2016 at 12:12:31PM +0000, Sudeep Holla wrote:
> Hi Russell,
> 
> On Thu, Dec 24, 2015 at 4:47 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Thu, Dec 24, 2015 at 09:49:48AM -0600, Timur Tabi wrote:
> >> 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 <timur@codeaurora.org>
> >
> > 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...

Greg's also been totally silent on the component helper changes, and I've
decided that I'm pushing those upstream myself irrespective of anything
else (and I really don't care at this point if Greg objects to this; he's
already had plenty of time to comment, and has chosen not to.)

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.

> 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.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[Sudeep Holla]

On 05/01/16 12:30, Russell King - ARM Linux wrote:
> On Tue, Jan 05, 2016 at 12:12:31PM +0000, Sudeep Holla wrote:
>> Hi Russell,
>>
>> On Thu, Dec 24, 2015 at 4:47 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>> On Thu, Dec 24, 2015 at 09:49:48AM -0600, Timur Tabi wrote:
>>>> 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 <timur@codeaurora.org>
>>>
>>> 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.
>

Since it's boot failure, it should be considered as bug fix and merged.

>> 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.

Yes, I observed that, but still mentioned it just to check if that was
the reason for holding this patch. Thanks for the clarification.

> 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.
>

Understood and thanks for the detailed explanation.

-- 
Regards,
Sudeep

[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[Greg Kroah-Hartman]

On Tue, Jan 05, 2016 at 12:30:19PM +0000, Russell King - ARM Linux wrote:
> On Tue, Jan 05, 2016 at 12:12:31PM +0000, Sudeep Holla wrote:
> > Hi Russell,
> > 
> > On Thu, Dec 24, 2015 at 4:47 PM, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:
> > > On Thu, Dec 24, 2015 at 09:49:48AM -0600, Timur Tabi wrote:
> > >> 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 <timur@codeaurora.org>
> > >
> > > 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...

"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.

> 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.

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.

greg k-h

[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[Russell King - ARM Linux]

On Tue, Jan 05, 2016 at 06:43:02PM -0800, Greg Kroah-Hartman wrote:
> On Tue, Jan 05, 2016 at 12:30:19PM +0000, Russell King - ARM Linux wrote:
> > On Tue, Jan 05, 2016 at 12:12:31PM +0000, Sudeep Holla wrote:
> > > Hi Russell,
> > > 
> > > On Thu, Dec 24, 2015 at 4:47 PM, Russell King - ARM Linux
> > > <linux@arm.linux.org.uk> wrote:
> > > > On Thu, Dec 24, 2015 at 09:49:48AM -0600, Timur Tabi wrote:
> > > >> 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 <timur@codeaurora.org>
> > > >
> > > > 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...
> 
> "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.

> 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...

> 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?

> 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".

The original patch set was found to be broken on the 3rd November, and
there's emails to prove it.

> 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 <linux@arm.linux.org.uk>
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.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[Greg Kroah-Hartman]

On Wed, Jan 06, 2016 at 10:07:00AM +0000, Russell King - ARM Linux wrote:
> On Tue, Jan 05, 2016 at 06:43:02PM -0800, Greg Kroah-Hartman wrote:
> > On Tue, Jan 05, 2016 at 12:30:19PM +0000, Russell King - ARM Linux wrote:
> > > On Tue, Jan 05, 2016 at 12:12:31PM +0000, Sudeep Holla wrote:
> > > > Hi Russell,
> > > > 
> > > > On Thu, Dec 24, 2015 at 4:47 PM, Russell King - ARM Linux
> > > > <linux@arm.linux.org.uk> wrote:
> > > > > On Thu, Dec 24, 2015 at 09:49:48AM -0600, Timur Tabi wrote:
> > > > >> 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 <timur@codeaurora.org>
> > > > >
> > > > > 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...
> > 
> > "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.
> 
> > 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...

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.

> > 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 :)

> > 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.

> 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 <linux@arm.linux.org.uk>
> 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.

> 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 :)

> 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

[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[Peter Hurley]

On 01/06/2016 09:17 PM, Greg Kroah-Hartman wrote:
> On Wed, Jan 06, 2016 at 10:07:00AM +0000, Russell King - ARM Linux wrote:
>> On Tue, Jan 05, 2016 at 06:43:02PM -0800, Greg Kroah-Hartman wrote:
>>> On Tue, Jan 05, 2016 at 12:30:19PM +0000, Russell King - ARM Linux wrote:
>>>> On Tue, Jan 05, 2016 at 12:12:31PM +0000, Sudeep Holla wrote:
>>>>> Hi Russell,
>>>>>
>>>>> On Thu, Dec 24, 2015 at 4:47 PM, Russell King - ARM Linux
>>>>> <linux@arm.linux.org.uk> wrote:
>>>>>> On Thu, Dec 24, 2015 at 09:49:48AM -0600, Timur Tabi wrote:
>>>>>>> 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 <timur@codeaurora.org>
>>>>>>
>>>>>> 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...
>>>
>>> "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.
>>
>>> 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...
> 
> 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.
> 
>>> 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 :)
> 
>>> 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.
> 
>> 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 <linux@arm.linux.org.uk>
>> 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.
> 
>> 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 :)
> 
>> 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 sad part about this whole mess is that it simply recapitulates
(in the biological sense) the state of this driver this summer requiring the
same earlycon fix which I wrote and sent Aug 10 [1]
*except that ZTE support still isn't working*.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-August/363369.html

[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[Huang Shijie]

On Thu, Dec 24, 2015 at 09:49:48AM -0600, Timur Tabi wrote:
> 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 <timur@codeaurora.org>
> ---
>  drivers/tty/serial/amba-pl011.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index f6ad383..06f827a 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -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)
We will meet the compiler error, since there is no UART011_FR_BUSY.
Please test it before you send it out :)

thanks
Huang Shijie
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[PATCH 1/2] tty: amba-pl011: fix earlycon register offsets

[Huang Shijie]

On Thu, Dec 24, 2015 at 09:49:48AM -0600, Timur Tabi wrote:
> 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 <timur@codeaurora.org>
> ---
>  drivers/tty/serial/amba-pl011.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index f6ad383..06f827a 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -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)
I changed the UART011_FR_BUSY with UART01x_FR_BUSY, the console is okay
now.

So after you change the UART011_FR_BUSY to UART01x_FR_BUSY in the next
version, please add my test-by :
      Tested-by: Huang Shijie <shijie.huang@arm.com>

thanks
Huang Shijie
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.