linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 8250_early for big-endian
@ 2012-10-12 13:05 Noam Camus
  2012-10-12 16:51 ` Alan Cox
  0 siblings, 1 reply; 19+ messages in thread
From: Noam Camus @ 2012-10-12 13:05 UTC (permalink / raw)
  To: linux-serial@vger.kernel.org

Hello

I am using 8250 console and wish to use the 8250_early as well.

I encounter endianess problem while using this driver for mmio32.

The problem is since my serial peripheral like my cpu is in BIG endian.

Driver through serial_in/serial_out uses readl/writel from asm-generic/io.h.

These macros assume peripheral is always in LITTLE endian.

Therefore all data needs a swab. 

In 8250 driver I can register replacement for default serial_in/serial_out and workaround the problem.
In 8250_early I had no choice but to change code with additional swab to workaround the problem.

I am looking for proper solution so it can be added to upstream.
I got several ideas if interest arise.

Waiting for your comments
Noam Camus 


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

* Re: 8250_early for big-endian
  2012-10-12 13:05 8250_early for big-endian Noam Camus
@ 2012-10-12 16:51 ` Alan Cox
  2012-10-12 18:41   ` Noam Camus
  0 siblings, 1 reply; 19+ messages in thread
From: Alan Cox @ 2012-10-12 16:51 UTC (permalink / raw)
  To: Noam Camus; +Cc: linux-serial@vger.kernel.org

> I am using 8250 console and wish to use the 8250_early as well.
> 
> I encounter endianess problem while using this driver for mmio32.
> 
> The problem is since my serial peripheral like my cpu is in BIG endian.
> 
> Driver through serial_in/serial_out uses readl/writel from asm-generic/io.h.
> 
> These macros assume peripheral is always in LITTLE endian.
> 
> Therefore all data needs a swab. 
> 
> In 8250 driver I can register replacement for default serial_in/serial_out and workaround the problem.
> In 8250_early I had no choice but to change code with additional swab to workaround the problem.
> 
> I am looking for proper solution so it can be added to upstream.
> I got several ideas if interest arise.

Given they are all byte registers so how can they need swapping ? They are
merely 3 further bytes offset.

Alan

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

* RE: 8250_early for big-endian
  2012-10-12 16:51 ` Alan Cox
@ 2012-10-12 18:41   ` Noam Camus
  2012-10-12 18:51     ` Alan Cox
  0 siblings, 1 reply; 19+ messages in thread
From: Noam Camus @ 2012-10-12 18:41 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial@vger.kernel.org

________________________________________
>From: Alan Cox [alan@lxorguk.ukuu.org.uk]
>Sent: Friday, October 12, 2012 6:51 PM
>To: Noam Camus
>Cc: linux-serial@vger.kernel.org
>Subject: Re: 8250_early for big-endian
>
>> I am using 8250 console and wish to use the 8250_early as well.
>>
>> I encounter endianess problem while using this driver for mmio32.
.>>
>> The problem is since my serial peripheral like my cpu is in BIG endian.
>>
>> Driver through serial_in/serial_out uses readl/writel from asm-generic/io.h.
>>
>> These macros assume peripheral is always in LITTLE endian.
>>
>> Therefore all data needs a swab.
>>
>> In 8250 driver I can register replacement for default serial_in/serial_out and workaround the problem.
>> In 8250_early I had no choice but to change code with additional swab to workaround the problem.
>>
>> I am looking for proper solution so it can be added to upstream.
>> I got several ideas if interest arise.
>
>Given they are all byte registers so how can they need swapping ? They are
>merely 3 further bytes offset.
>
>Alan

Of course swapping is a waste of cpu cycles.
Workaround can be 3 further bytes offset.
Thanks

Back to my problem.
How can 8250_early be enhanced to support big endian registers.
Is it configuation?
Maybe command line flag?
both? other?

Noam

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

* Re: 8250_early for big-endian
  2012-10-12 18:41   ` Noam Camus
@ 2012-10-12 18:51     ` Alan Cox
  2012-10-12 19:08       ` Noam Camus
  0 siblings, 1 reply; 19+ messages in thread
From: Alan Cox @ 2012-10-12 18:51 UTC (permalink / raw)
  To: Noam Camus; +Cc: linux-serial@vger.kernel.org

> Of course swapping is a waste of cpu cycles.
> Workaround can be 3 further bytes offset.
> Thanks
> 
> Back to my problem.
> How can 8250_early be enhanced to support big endian registers.

They are 8bit registers - I don't understand why you are asking the
question ?

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

* RE: 8250_early for big-endian
  2012-10-12 18:51     ` Alan Cox
@ 2012-10-12 19:08       ` Noam Camus
  2012-10-12 21:06         ` Alan Cox
  0 siblings, 1 reply; 19+ messages in thread
From: Noam Camus @ 2012-10-12 19:08 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial@vger.kernel.org

Sorry,but I wrote that I am using mmio32.

I thought it implies that I am using 32bit registers.

This is why I keep asking.
________________________________________
From: Alan Cox [alan@lxorguk.ukuu.org.uk]
Sent: Friday, October 12, 2012 8:51 PM
To: Noam Camus
Cc: linux-serial@vger.kernel.org
Subject: Re: 8250_early for big-endian

> Of course swapping is a waste of cpu cycles.
> Workaround can be 3 further bytes offset.
> Thanks
>
> Back to my problem.
> How can 8250_early be enhanced to support big endian registers.

They are 8bit registers - I don't understand why you are asking the
question ?

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

* Re: 8250_early for big-endian
  2012-10-12 19:08       ` Noam Camus
@ 2012-10-12 21:06         ` Alan Cox
  2012-10-13  6:43           ` Noam Camus
  0 siblings, 1 reply; 19+ messages in thread
From: Alan Cox @ 2012-10-12 21:06 UTC (permalink / raw)
  To: Noam Camus; +Cc: linux-serial@vger.kernel.org

On Fri, 12 Oct 2012 21:08:15 +0200
Noam Camus <noamc@ezchip.com> wrote:

> Sorry,but I wrote that I am using mmio32.

Yes but you also keep talking about endian-ness. The underlying registers
are 8bit so they are not 'endian' at all. They might be at the address
"base + 4 * offset + 3" perhaps ?

If so they to use mmio32 presumably you just need to set the base address
properly (ie 0xXXXXXXX3 or similar).

Do the values have to be read with 32bit operations and do they have to
be read on 32bi alignment ?

Alan

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

* RE: 8250_early for big-endian
  2012-10-12 21:06         ` Alan Cox
@ 2012-10-13  6:43           ` Noam Camus
  2012-10-13 13:52             ` Alan Cox
  0 siblings, 1 reply; 19+ messages in thread
From: Noam Camus @ 2012-10-13  6:43 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial@vger.kernel.org

On Fri, 12 Oct 2012 21:11:06 +0200
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> Do the values have to be read with 32bit operations and do they have to
> be read on 32bi alignment ?

Yes I do need the 32bit alignment but I do not need 32bit operations.
So using mmio (instead of mmio32) with regshift equals 2 worked for me.

However 8250_early got no configurable regshift.
So it needs to be added.
Alternatively I can use regshift equals 0 and redefine locally all offsets from serial_reg.h e.g. UART_LST, UART_IER...

What do you think is the way to work this regshift?

BTW: no more endian-ness from my side.
________________________________________
From: Alan Cox [alan@lxorguk.ukuu.org.uk]
Sent: Friday, October 12, 2012 11:06 PM
To: Noam Camus
Cc: linux-serial@vger.kernel.org
Subject: Re: 8250_early for big-endian

On Fri, 12 Oct 2012 21:08:15 +0200
Noam Camus <noamc@ezchip.com> wrote:

> Sorry,but I wrote that I am using mmio32.

Yes but you also keep talking about endian-ness. The underlying registers
are 8bit so they are not 'endian' at all. They might be at the address
"base + 4 * offset + 3" perhaps ?

If so they to use mmio32 presumably you just need to set the base address
properly (ie 0xXXXXXXX3 or similar).

Do the values have to be read with 32bit operations and do they have to
be read on 32bi alignment ?

Alan

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

* Re: 8250_early for big-endian
  2012-10-13  6:43           ` Noam Camus
@ 2012-10-13 13:52             ` Alan Cox
  2012-10-14 10:11               ` Noam Camus
  0 siblings, 1 reply; 19+ messages in thread
From: Alan Cox @ 2012-10-13 13:52 UTC (permalink / raw)
  To: Noam Camus; +Cc: linux-serial@vger.kernel.org

On Sat, 13 Oct 2012 08:43:02 +0200
Noam Camus <noamc@ezchip.com> wrote:

> On Fri, 12 Oct 2012 21:11:06 +0200
> Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> > Do the values have to be read with 32bit operations and do they have to
> > be read on 32bi alignment ?
> 
> Yes I do need the 32bit alignment but I do not need 32bit operations.
> So using mmio (instead of mmio32) with regshift equals 2 worked for me.
> 
> However 8250_early got no configurable regshift.
> So it needs to be added.

I would favour adding the regshift and I see no problem in doing that.

Alan

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

* RE: 8250_early for big-endian
  2012-10-13 13:52             ` Alan Cox
@ 2012-10-14 10:11               ` Noam Camus
  2012-10-14 12:32                 ` Alan Cox
  0 siblings, 1 reply; 19+ messages in thread
From: Noam Camus @ 2012-10-14 10:11 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial@vger.kernel.org

Soon I will email you my patch for regshift addition.

In the meantime I discovered that one of my boards do need 32bit operations.

How to generalize the solution to cover this case as well?

Noam

-----Original Message-----
From: Alan Cox [mailto:alan@lxorguk.ukuu.org.uk] 
Sent: Saturday, October 13, 2012 3:52 PM
To: Noam Camus
Cc: linux-serial@vger.kernel.org
Subject: Re: 8250_early for big-endian

On Sat, 13 Oct 2012 08:43:02 +0200
Noam Camus <noamc@ezchip.com> wrote:

> On Fri, 12 Oct 2012 21:11:06 +0200
> Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> > Do the values have to be read with 32bit operations and do they have 
> > to be read on 32bi alignment ?
> 
> Yes I do need the 32bit alignment but I do not need 32bit operations.
> So using mmio (instead of mmio32) with regshift equals 2 worked for me.
> 
> However 8250_early got no configurable regshift.
> So it needs to be added.

I would favour adding the regshift and I see no problem in doing that.

Alan

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

* Re: 8250_early for big-endian
  2012-10-14 10:11               ` Noam Camus
@ 2012-10-14 12:32                 ` Alan Cox
  2012-10-14 16:18                   ` Noam Camus
                                     ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Alan Cox @ 2012-10-14 12:32 UTC (permalink / raw)
  To: Noam Camus; +Cc: linux-serial@vger.kernel.org

On Sun, 14 Oct 2012 12:11:13 +0200
Noam Camus <noamc@ezchip.com> wrote:

> Soon I will email you my patch for regshift addition.
> 
> In the meantime I discovered that one of my boards do need 32bit operations.
> 
> How to generalize the solution to cover this case as well?

Then I guess if you need 32bit I/O and aligned we do actually need to to
support the shifts and you do end up needing to do MMIO32 and >> 24 for
your serial_in (or as you originally suggested a byte reverse if faster).

For 8250_early it may well be the right thing is to support similar
serial_in/serial_out methods.

Alan

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

* RE: 8250_early for big-endian
  2012-10-14 12:32                 ` Alan Cox
@ 2012-10-14 16:18                   ` Noam Camus
  2012-10-17  9:07                   ` Noam Camus
  2012-10-25  7:08                   ` Noam Camus
  2 siblings, 0 replies; 19+ messages in thread
From: Noam Camus @ 2012-10-14 16:18 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial@vger.kernel.org

From: Alan Cox [mailto:alan@lxorguk.ukuu.org.uk] 
Sent: Sunday, October 14, 2012 2:32 PM

> For 8250_early it may well be the right thing is to support similar serial_in/serial_out methods.

Is it acceptable solution to add attribute __weak (and remove static) from serial_in/serial_out methods?

Noam

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

* RE: 8250_early for big-endian
  2012-10-14 12:32                 ` Alan Cox
  2012-10-14 16:18                   ` Noam Camus
@ 2012-10-17  9:07                   ` Noam Camus
  2012-10-25  7:08                   ` Noam Camus
  2 siblings, 0 replies; 19+ messages in thread
From: Noam Camus @ 2012-10-17  9:07 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial@vger.kernel.org

________________________________________
From: Alan Cox [alan@lxorguk.ukuu.org.uk]
Sent: Sunday, October 14, 2012 2:32 PM

>For 8250_early it may well be the right thing is to support similar
serial_in/serial_out methods.

Hello Alan,
Here is my patch:

>From e819fbbf9690b72620c54a62fa34545c7b035f91 Mon Sep 17 00:00:00 2001
From: Noam Camus <noamc@ezchip.com>
Date: Wed, 3 Oct 2012 09:40:36 +0200
Subject: [PATCH] tty/8250_early: Make serial_in/serial_out be over-ridden

Now one can replace default methods with similar fit to its needs.
e.g. choose other regshift, register offset...

Signed-off-by: Noam Camus <noamc@ezchip.com>
CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 drivers/tty/serial/8250/8250_early.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index eaafb98..ac31b0c 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -48,7 +48,7 @@ struct early_serial8250_device {
 
 static struct early_serial8250_device early_device;
 
-static unsigned int __init serial_in(struct uart_port *port, int offset)
+unsigned int __weak __init serial_in(struct uart_port *port, int offset)
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
@@ -62,7 +62,7 @@ static unsigned int __init serial_in(struct uart_port *port, int offset)
 	}
 }
 
-static void __init serial_out(struct uart_port *port, int offset, int value)
+void __weak __init serial_out(struct uart_port *port, int offset, int value)
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
-- 
1.7.1


Noam

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

* RE: 8250_early for big-endian
  2012-10-14 12:32                 ` Alan Cox
  2012-10-14 16:18                   ` Noam Camus
  2012-10-17  9:07                   ` Noam Camus
@ 2012-10-25  7:08                   ` Noam Camus
  2012-10-25 11:18                     ` Alan Cox
  2 siblings, 1 reply; 19+ messages in thread
From: Noam Camus @ 2012-10-25  7:08 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial@vger.kernel.org, Vineet Gupta, Gilad Ben Yossef

________________________________________
From: Alan Cox [alan@lxorguk.ukuu.org.uk]
Sent: Sunday, October 14, 2012 2:32 PM

>For 8250_early it may well be the right thing is to support similar
serial_in/serial_out methods.

Hello Alan,
Below is my proposed patch could you apply it?

>From e819fbbf9690b72620c54a62fa34545c7b035f91 Mon Sep 17 00:00:00 2001
From: Noam Camus <noamc@ezchip.com>
Date: Wed, 3 Oct 2012 09:40:36 +0200
Subject: [PATCH] tty/8250_early: Make serial_in/serial_out be over-ridden

Now one can replace default methods with similar fit to its needs.
e.g. choose other regshift, register offset...

Signed-off-by: Noam Camus <noamc@ezchip.com>
CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 drivers/tty/serial/8250/8250_early.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index eaafb98..ac31b0c 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -48,7 +48,7 @@ struct early_serial8250_device {
 
 static struct early_serial8250_device early_device;
 
-static unsigned int __init serial_in(struct uart_port *port, int offset)
+unsigned int __weak __init serial_in(struct uart_port *port, int offset)
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
@@ -62,7 +62,7 @@ static unsigned int __init serial_in(struct uart_port *port, int offset)
 	}
 }
 
-static void __init serial_out(struct uart_port *port, int offset, int value)
+void __weak __init serial_out(struct uart_port *port, int offset, int value)
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
-- 
1.7.1


Noam

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

* Re: 8250_early for big-endian
  2012-10-25  7:08                   ` Noam Camus
@ 2012-10-25 11:18                     ` Alan Cox
  2012-10-29 15:38                       ` Noam Camus
  2012-11-11  4:34                       ` Noam Camus
  0 siblings, 2 replies; 19+ messages in thread
From: Alan Cox @ 2012-10-25 11:18 UTC (permalink / raw)
  To: Noam Camus; +Cc: linux-serial@vger.kernel.org, Vineet Gupta, Gilad Ben Yossef

On Thu, 25 Oct 2012 09:08:53 +0200
Noam Camus <noamc@ezchip.com> wrote:

> ________________________________________
> From: Alan Cox [alan@lxorguk.ukuu.org.uk]
> Sent: Sunday, October 14, 2012 2:32 PM
> 
> >For 8250_early it may well be the right thing is to support similar
> serial_in/serial_out methods.
> 
> Hello Alan,
> Below is my proposed patch could you apply it?
> 
> >From e819fbbf9690b72620c54a62fa34545c7b035f91 Mon Sep 17 00:00:00 2001
> From: Noam Camus <noamc@ezchip.com>
> Date: Wed, 3 Oct 2012 09:40:36 +0200
> Subject: [PATCH] tty/8250_early: Make serial_in/serial_out be over-ridden
> 
> Now one can replace default methods with similar fit to its needs.
> e.g. choose other regshift, register offset...
> 
> Signed-off-by: Noam Camus <noamc@ezchip.com>
> CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
> ---
>  drivers/tty/serial/8250/8250_early.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
> index eaafb98..ac31b0c 100644
> --- a/drivers/tty/serial/8250/8250_early.c
> +++ b/drivers/tty/serial/8250/8250_early.c
> @@ -48,7 +48,7 @@ struct early_serial8250_device {
>  
>  static struct early_serial8250_device early_device;
>  
> -static unsigned int __init serial_in(struct uart_port *port, int offset)
> +unsigned int __weak __init serial_in(struct uart_port *port, int offset)

If you make the symbols global then they need a prefix to avoid clashes -
just rename them something like serial8250_early_in/serial8250_early_out

and of course they ought to be in the header files somewhere...

Alan

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

* RE: 8250_early for big-endian
  2012-10-25 11:18                     ` Alan Cox
@ 2012-10-29 15:38                       ` Noam Camus
  2012-11-11  4:34                       ` Noam Camus
  1 sibling, 0 replies; 19+ messages in thread
From: Noam Camus @ 2012-10-29 15:38 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial@vger.kernel.org, Vineet Gupta, Gilad Ben Yossef

>From: Alan Cox [mailto:alan@lxorguk.ukuu.org.uk] 
>Sent: Thursday, October 25, 2012 1:18 PM

>If you make the symbols global then they need a prefix to avoid clashes - just rename them something like serial8250_early_in/serial8250_early_out

> and of course they ought to be in the header files somewhere...

Thanks for your comments. I polished my patch and below is the updated.
Please comment on this patch.

>From 0ebe0d2d39d76a1d2983321599e138474f5e96d3 Mon Sep 17 00:00:00 2001
From: Noam Camus <noamc@ezchip.com>
Date: Mon, 29 Oct 2012 17:16:58 +0200
Subject: [PATCH] tty/8250_early: Turn serial_in/serial_out to weak symbols to allow platforms to override them.

Now one can replace default methods with similar fit to its needs.
e.g. choose other regshift, register offset, or any other manipulation.

This is useful for big endian platforms wishing to use the early serial driver.
Platform that needs to swab bytes before the register accesses.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/tty/serial/8250/8250_early.c |    4 ++--
 include/linux/serial_8250.h          |    2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index eaafb98..a359504 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -48,7 +48,7 @@ struct early_serial8250_device {

 static struct early_serial8250_device early_device;

-static unsigned int __init serial_in(struct uart_port *port, int offset)
+unsigned int __weak __init serial8250_early_in(struct uart_port *port, int offset)
 {
        switch (port->iotype) {
        case UPIO_MEM:
@@ -62,7 +62,7 @@ static unsigned int __init serial_in(struct uart_port *port, int offset)
        }
 }

-static void __init serial_out(struct uart_port *port, int offset, int value)
+void __weak __init serial8250_early_out(struct uart_port *port, int offset, int value)
 {
        switch (port->iotype) {
        case UPIO_MEM:
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index c174c90..c490d20 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -105,6 +105,8 @@ extern int early_serial_setup(struct uart_port *port);

 extern int serial8250_find_port(struct uart_port *p);
 extern int serial8250_find_port_for_earlycon(void);
+extern unsigned int serial8250_early_in(struct uart_port *port, int offset);
+extern void serial8250_early_out(struct uart_port *port, int offset, int value);
 extern int setup_early_serial8250_console(char *cmdline);
 extern void serial8250_do_set_termios(struct uart_port *port,
                struct ktermios *termios, struct ktermios *old);
-- 
1.7.1

Regards,
Noam

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

* RE: 8250_early for big-endian
  2012-10-25 11:18                     ` Alan Cox
  2012-10-29 15:38                       ` Noam Camus
@ 2012-11-11  4:34                       ` Noam Camus
  2012-11-14 12:20                         ` Alan Cox
  2012-11-16  1:11                         ` [PATCH] tty/8250_early: Turn serial_in/serial_out into weak symbols Greg KH
  1 sibling, 2 replies; 19+ messages in thread
From: Noam Camus @ 2012-11-11  4:34 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-serial@vger.kernel.org, Vineet Gupta, Gilad Ben Yossef

________________________________________
From: Alan Cox [alan@lxorguk.ukuu.org.uk]
Sent: Thursday, October 25, 2012 1:18 PM

>If you make the symbols global then they need a prefix to avoid clashes -
>just rename them something like serial8250_early_in/serial8250_early_out

>and of course they ought to be in the header files somewhere...

Hello Alan,

I will appreciate if you can review and comment on below patch.

>From e2dda56ab679d81bb0a021e102519185028deded Mon Sep 17 00:00:00 2001
From: Noam Camus <noamc@ezchip.com>
Date: Sun, 11 Nov 2012 05:49:39 +0200
Subject: [PATCH] tty/8250_early: Turn serial_in/serial_out into weak symbols.

Allows overriding default methods serial_in/serial_out.

In such platform specific replacement it is possible to use,
other regshift, biased register offset, any other manipulation
that is not covered with common default methods.

Overriding default methods may be useful for platforms which got
serial peripheral with registers represented in big endian.
In this situation and assuming that 32 bit operations / alignment
is required then it may be useful to swab words before/after
accessing the serial registers.

Signed-off-by: Noam Camus <noamc@ezchip.com>
CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 drivers/tty/serial/8250/8250_early.c |   42 +++++++++++++++++-----------------
 include/linux/serial_8250.h          |    2 +
 2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index eaafb98..eb88f6d 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -48,7 +48,7 @@ struct early_serial8250_device {
 
 static struct early_serial8250_device early_device;
 
-static unsigned int __init serial_in(struct uart_port *port, int offset)
+unsigned int __weak __init serial8250_early_in(struct uart_port *port, int offset)
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
@@ -62,7 +62,7 @@ static unsigned int __init serial_in(struct uart_port *port, int offset)
 	}
 }
 
-static void __init serial_out(struct uart_port *port, int offset, int value)
+void __weak __init serial8250_early_out(struct uart_port *port, int offset, int value)
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
@@ -84,7 +84,7 @@ static void __init wait_for_xmitr(struct uart_port *port)
 	unsigned int status;
 
 	for (;;) {
-		status = serial_in(port, UART_LSR);
+		status = serial8250_early_in(port, UART_LSR);
 		if ((status & BOTH_EMPTY) == BOTH_EMPTY)
 			return;
 		cpu_relax();
@@ -94,7 +94,7 @@ static void __init wait_for_xmitr(struct uart_port *port)
 static void __init serial_putc(struct uart_port *port, int c)
 {
 	wait_for_xmitr(port);
-	serial_out(port, UART_TX, c);
+	serial8250_early_out(port, UART_TX, c);
 }
 
 static void __init early_serial8250_write(struct console *console,
@@ -104,14 +104,14 @@ static void __init early_serial8250_write(struct console *console,
 	unsigned int ier;
 
 	/* Save the IER and disable interrupts */
-	ier = serial_in(port, UART_IER);
-	serial_out(port, UART_IER, 0);
+	ier = serial8250_early_in(port, UART_IER);
+	serial8250_early_out(port, UART_IER, 0);
 
 	uart_console_write(port, s, count, serial_putc);
 
 	/* Wait for transmitter to become empty and restore the IER */
 	wait_for_xmitr(port);
-	serial_out(port, UART_IER, ier);
+	serial8250_early_out(port, UART_IER, ier);
 }
 
 static unsigned int __init probe_baud(struct uart_port *port)
@@ -119,11 +119,11 @@ static unsigned int __init probe_baud(struct uart_port *port)
 	unsigned char lcr, dll, dlm;
 	unsigned int quot;
 
-	lcr = serial_in(port, UART_LCR);
-	serial_out(port, UART_LCR, lcr | UART_LCR_DLAB);
-	dll = serial_in(port, UART_DLL);
-	dlm = serial_in(port, UART_DLM);
-	serial_out(port, UART_LCR, lcr);
+	lcr = serial8250_early_in(port, UART_LCR);
+	serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
+	dll = serial8250_early_in(port, UART_DLL);
+	dlm = serial8250_early_in(port, UART_DLM);
+	serial8250_early_out(port, UART_LCR, lcr);
 
 	quot = (dlm << 8) | dll;
 	return (port->uartclk / 16) / quot;
@@ -135,17 +135,17 @@ static void __init init_port(struct early_serial8250_device *device)
 	unsigned int divisor;
 	unsigned char c;
 
-	serial_out(port, UART_LCR, 0x3);	/* 8n1 */
-	serial_out(port, UART_IER, 0);		/* no interrupt */
-	serial_out(port, UART_FCR, 0);		/* no fifo */
-	serial_out(port, UART_MCR, 0x3);	/* DTR + RTS */
+	serial8250_early_out(port, UART_LCR, 0x3);	/* 8n1 */
+	serial8250_early_out(port, UART_IER, 0);		/* no interrupt */
+	serial8250_early_out(port, UART_FCR, 0);		/* no fifo */
+	serial8250_early_out(port, UART_MCR, 0x3);	/* DTR + RTS */
 
 	divisor = port->uartclk / (16 * device->baud);
-	c = serial_in(port, UART_LCR);
-	serial_out(port, UART_LCR, c | UART_LCR_DLAB);
-	serial_out(port, UART_DLL, divisor & 0xff);
-	serial_out(port, UART_DLM, (divisor >> 8) & 0xff);
-	serial_out(port, UART_LCR, c & ~UART_LCR_DLAB);
+	c = serial8250_early_in(port, UART_LCR);
+	serial8250_early_out(port, UART_LCR, c | UART_LCR_DLAB);
+	serial8250_early_out(port, UART_DLL, divisor & 0xff);
+	serial8250_early_out(port, UART_DLM, (divisor >> 8) & 0xff);
+	serial8250_early_out(port, UART_LCR, c & ~UART_LCR_DLAB);
 }
 
 static int __init parse_options(struct early_serial8250_device *device,
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index c174c90..c490d20 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -105,6 +105,8 @@ extern int early_serial_setup(struct uart_port *port);
 
 extern int serial8250_find_port(struct uart_port *p);
 extern int serial8250_find_port_for_earlycon(void);
+extern unsigned int serial8250_early_in(struct uart_port *port, int offset);
+extern void serial8250_early_out(struct uart_port *port, int offset, int value);
 extern int setup_early_serial8250_console(char *cmdline);
 extern void serial8250_do_set_termios(struct uart_port *port,
 		struct ktermios *termios, struct ktermios *old);
-- 
1.7.1

regards,
Noam

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

* Re: 8250_early for big-endian
  2012-11-11  4:34                       ` Noam Camus
@ 2012-11-14 12:20                         ` Alan Cox
  2012-11-16  1:11                         ` [PATCH] tty/8250_early: Turn serial_in/serial_out into weak symbols Greg KH
  1 sibling, 0 replies; 19+ messages in thread
From: Alan Cox @ 2012-11-14 12:20 UTC (permalink / raw)
  To: Noam Camus; +Cc: linux-serial@vger.kernel.org, Vineet Gupta, Gilad Ben Yossef

>From e2dda56ab679d81bb0a021e102519185028deded Mon Sep 17 00:00:00 2001
> From: Noam Camus <noamc@ezchip.com>
> Date: Sun, 11 Nov 2012 05:49:39 +0200
> Subject: [PATCH] tty/8250_early: Turn serial_in/serial_out into weak symbols.
> 
> Allows overriding default methods serial_in/serial_out.
> 
> In such platform specific replacement it is possible to use,
> other regshift, biased register offset, any other manipulation
> that is not covered with common default methods.
> 
> Overriding default methods may be useful for platforms which got
> serial peripheral with registers represented in big endian.
> In this situation and assuming that 32 bit operations / alignment
> is required then it may be useful to swab words before/after
> accessing the serial registers.


Looks ok to me.

Acked-by: Alan Cox <alan@linux.intel.com>

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

* Re: [PATCH] tty/8250_early: Turn serial_in/serial_out into weak symbols.
  2012-11-11  4:34                       ` Noam Camus
  2012-11-14 12:20                         ` Alan Cox
@ 2012-11-16  1:11                         ` Greg KH
  2012-11-16  5:03                           ` Noam Camus
  1 sibling, 1 reply; 19+ messages in thread
From: Greg KH @ 2012-11-16  1:11 UTC (permalink / raw)
  To: Noam Camus
  Cc: Alan Cox, linux-serial@vger.kernel.org, Vineet Gupta,
	Gilad Ben Yossef

On Sun, Nov 11, 2012 at 06:34:30AM +0200, Noam Camus wrote:
> From: Noam Camus <noamc@ezchip.com>
> 
> Allows overriding default methods serial_in/serial_out.
> 
> In such platform specific replacement it is possible to use,
> other regshift, biased register offset, any other manipulation
> that is not covered with common default methods.
> 
> Overriding default methods may be useful for platforms which got
> serial peripheral with registers represented in big endian.
> In this situation and assuming that 32 bit operations / alignment
> is required then it may be useful to swab words before/after
> accessing the serial registers.
> 
> Signed-off-by: Noam Camus <noamc@ezchip.com>
> Acked-by: Alan Cox <alan@linux.intel.com>
> ---
>  drivers/tty/serial/8250/8250_early.c |   42 +++++++++++++++++-----------------
>  include/linux/serial_8250.h          |    2 +
>  2 files changed, 23 insertions(+), 21 deletions(-)

This patch no longer applies to my tree.  Can you please refresh it
against the tty-next tree and resend it, with Alan's ack kept on it, so
I remember it is there?

thanks,

greg k-h

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

* RE: [PATCH] tty/8250_early: Turn serial_in/serial_out into weak symbols.
  2012-11-16  1:11                         ` [PATCH] tty/8250_early: Turn serial_in/serial_out into weak symbols Greg KH
@ 2012-11-16  5:03                           ` Noam Camus
  0 siblings, 0 replies; 19+ messages in thread
From: Noam Camus @ 2012-11-16  5:03 UTC (permalink / raw)
  To: Greg KH
  Cc: Alan Cox, linux-serial@vger.kernel.org, Vineet Gupta,
	Gilad Ben Yossef


>From: Greg KH [gregkh@linuxfoundation.org]
>Sent: Friday, November 16, 2012 3:11 AM

>This patch no longer applies to my tree.  Can you please refresh it
>against the tty-next tree and resend it, with Alan's ack kept on it, so
>I remember it is there?

Hello Greg,
Here is my refreshed patch:

>From b1f527ea26ce6ef2bbe3efcb7872aa4a4e6b77d5 Mon Sep 17 00:00:00 2001
From: Noam Camus <noamc@ezchip.com>
Date: Fri, 16 Nov 2012 06:42:53 +0200
Subject: [PATCH] tty/8250_early: Turn serial_in/serial_out into weak symbols.

Allows overriding default methods serial_in/serial_out.

In such platform specific replacement it is possible to use
other regshift, biased register offset, any other manipulation
that is not covered with common default methods.

Overriding default methods may be useful for platforms which got
serial peripheral with registers represented in big endian.
In this situation and assuming that 32 bit operations / alignment
is required then it may be useful to swab words before/after
accessing the serial registers.

Signed-off-by: Noam Camus <noamc@ezchip.com>
Acked-by: Alan Cox <alan@linux.intel.com>
---
 drivers/tty/serial/8250/8250_early.c |   42 +++++++++++++++++-----------------
 include/linux/serial_8250.h          |    2 +
 2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index 843a150..f53a7db 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -48,7 +48,7 @@ struct early_serial8250_device {
 
 static struct early_serial8250_device early_device;
 
-static unsigned int __init serial_in(struct uart_port *port, int offset)
+unsigned int __weak __init serial8250_early_in(struct uart_port *port, int offset)
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
@@ -62,7 +62,7 @@ static unsigned int __init serial_in(struct uart_port *port, int offset)
 	}
 }
 
-static void __init serial_out(struct uart_port *port, int offset, int value)
+void __weak __init serial8250_early_out(struct uart_port *port, int offset, int value)
 {
 	switch (port->iotype) {
 	case UPIO_MEM:
@@ -84,7 +84,7 @@ static void __init wait_for_xmitr(struct uart_port *port)
 	unsigned int status;
 
 	for (;;) {
-		status = serial_in(port, UART_LSR);
+		status = serial8250_early_in(port, UART_LSR);
 		if ((status & BOTH_EMPTY) == BOTH_EMPTY)
 			return;
 		cpu_relax();
@@ -94,7 +94,7 @@ static void __init wait_for_xmitr(struct uart_port *port)
 static void __init serial_putc(struct uart_port *port, int c)
 {
 	wait_for_xmitr(port);
-	serial_out(port, UART_TX, c);
+	serial8250_early_out(port, UART_TX, c);
 }
 
 static void __init early_serial8250_write(struct console *console,
@@ -104,14 +104,14 @@ static void __init early_serial8250_write(struct console *console,
 	unsigned int ier;
 
 	/* Save the IER and disable interrupts */
-	ier = serial_in(port, UART_IER);
-	serial_out(port, UART_IER, 0);
+	ier = serial8250_early_in(port, UART_IER);
+	serial8250_early_out(port, UART_IER, 0);
 
 	uart_console_write(port, s, count, serial_putc);
 
 	/* Wait for transmitter to become empty and restore the IER */
 	wait_for_xmitr(port);
-	serial_out(port, UART_IER, ier);
+	serial8250_early_out(port, UART_IER, ier);
 }
 
 static unsigned int __init probe_baud(struct uart_port *port)
@@ -119,11 +119,11 @@ static unsigned int __init probe_baud(struct uart_port *port)
 	unsigned char lcr, dll, dlm;
 	unsigned int quot;
 
-	lcr = serial_in(port, UART_LCR);
-	serial_out(port, UART_LCR, lcr | UART_LCR_DLAB);
-	dll = serial_in(port, UART_DLL);
-	dlm = serial_in(port, UART_DLM);
-	serial_out(port, UART_LCR, lcr);
+	lcr = serial8250_early_in(port, UART_LCR);
+	serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
+	dll = serial8250_early_in(port, UART_DLL);
+	dlm = serial8250_early_in(port, UART_DLM);
+	serial8250_early_out(port, UART_LCR, lcr);
 
 	quot = (dlm << 8) | dll;
 	return (port->uartclk / 16) / quot;
@@ -135,17 +135,17 @@ static void __init init_port(struct early_serial8250_device *device)
 	unsigned int divisor;
 	unsigned char c;
 
-	serial_out(port, UART_LCR, 0x3);	/* 8n1 */
-	serial_out(port, UART_IER, 0);		/* no interrupt */
-	serial_out(port, UART_FCR, 0);		/* no fifo */
-	serial_out(port, UART_MCR, 0x3);	/* DTR + RTS */
+	serial8250_early_out(port, UART_LCR, 0x3);	/* 8n1 */
+	serial8250_early_out(port, UART_IER, 0);	/* no interrupt */
+	serial8250_early_out(port, UART_FCR, 0);	/* no fifo */
+	serial8250_early_out(port, UART_MCR, 0x3);	/* DTR + RTS */
 
 	divisor = DIV_ROUND_CLOSEST(port->uartclk, 16 * device->baud);
-	c = serial_in(port, UART_LCR);
-	serial_out(port, UART_LCR, c | UART_LCR_DLAB);
-	serial_out(port, UART_DLL, divisor & 0xff);
-	serial_out(port, UART_DLM, (divisor >> 8) & 0xff);
-	serial_out(port, UART_LCR, c & ~UART_LCR_DLAB);
+	c = serial8250_early_in(port, UART_LCR);
+	serial8250_early_out(port, UART_LCR, c | UART_LCR_DLAB);
+	serial8250_early_out(port, UART_DLL, divisor & 0xff);
+	serial8250_early_out(port, UART_DLM, (divisor >> 8) & 0xff);
+	serial8250_early_out(port, UART_LCR, c & ~UART_LCR_DLAB);
 }
 
 static int __init parse_options(struct early_serial8250_device *device,
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index c174c90..c490d20 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -105,6 +105,8 @@ extern int early_serial_setup(struct uart_port *port);
 
 extern int serial8250_find_port(struct uart_port *p);
 extern int serial8250_find_port_for_earlycon(void);
+extern unsigned int serial8250_early_in(struct uart_port *port, int offset);
+extern void serial8250_early_out(struct uart_port *port, int offset, int value);
 extern int setup_early_serial8250_console(char *cmdline);
 extern void serial8250_do_set_termios(struct uart_port *port,
 		struct ktermios *termios, struct ktermios *old);
-- 
1.7.1

Noam Camus

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

end of thread, other threads:[~2012-11-16  5:03 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-12 13:05 8250_early for big-endian Noam Camus
2012-10-12 16:51 ` Alan Cox
2012-10-12 18:41   ` Noam Camus
2012-10-12 18:51     ` Alan Cox
2012-10-12 19:08       ` Noam Camus
2012-10-12 21:06         ` Alan Cox
2012-10-13  6:43           ` Noam Camus
2012-10-13 13:52             ` Alan Cox
2012-10-14 10:11               ` Noam Camus
2012-10-14 12:32                 ` Alan Cox
2012-10-14 16:18                   ` Noam Camus
2012-10-17  9:07                   ` Noam Camus
2012-10-25  7:08                   ` Noam Camus
2012-10-25 11:18                     ` Alan Cox
2012-10-29 15:38                       ` Noam Camus
2012-11-11  4:34                       ` Noam Camus
2012-11-14 12:20                         ` Alan Cox
2012-11-16  1:11                         ` [PATCH] tty/8250_early: Turn serial_in/serial_out into weak symbols Greg KH
2012-11-16  5:03                           ` Noam Camus

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