Thread (33 messages) 33 messages, 5 authors, 2017-05-31

[V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support

From: Dong Aisheng <hidden>
Date: 2017-05-31 08:07:40
Also in: linux-serial, lkml

Hi Nikita,

On Tue, May 23, 2017 at 08:24:18AM +0300, Nikita Yushchenko wrote:
Hi,
quoted
quoted
quoted
quoted
Alternative solution could be - have separate write path for earlycon.
It looks to me having the same issue with a separate write patch
for earlycon as we still need distinguish Little or Big endian
for Layerscape and IMX.
quoted
At a glance, it is dozen lines of code.
Would you please show some sample code?
Do not reuse lpuart32_console_putchar() in earlycon code.

Have two sets of early_setup/early_write/putchar - for BE and
defaut-endian earlycon. And in these putchar's do not use
lpuart_(read|write).
Isn't that introducing another consistency break after fix one
consistency break?

If doing that, we then have two register read/write APIs.
One for normal driver operation by dynamically checking lpuart_is_be
property to distinguish the endian difference problem.
Another is specifically implemented for only early console read/write
and use hardcoded way to read/write register directly instead of using
the standard API lpuart32_read/write, like follows:
e.g.
lpuart32_le_console_write() {
	 writel();
}

lpuart32_be_console_write() {
	 iowrite32be()
}
This also makes the driver a bit strange and ugly.

It looks to me both way are trade offs and the later one seems sacrifice
more. And i doubt if it's really necessary for probably a no real gain
purpose as the FPGA you mentioned is a theoretical case and less
possibility to exist.

I'm still wondering how about keep using the exist way and adding more
information in code to explain why use a global var?
I've checked other driver under drivers/tty/serial/, for examples of
similar cases.

Please look at serial8250_early_in() / serial8250_early_out() ?
These do handle different endian, via port->iotype
Well, that does inspire me.

With struct uart_port's iotype member, the global lpuart_is_be can be
gone. We can update lpuart32_read/write API to take the reference of
of structure uart_port, then the API knows the endian information and
can use the correct further IO accessor accordingly.

And most importantly, it also works with earlycon.
Another example is drivers/tty/serial/samsung.c, where
port->private_data is initialized and used.
If using iotype, seems no need private_data anymore.
Will try and send the new series later with you CCed to help review.

Thanks for the advice.

Regards
Dong Aisheng
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help