All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pl011: assert RTS signal in case the receiver uses flow control
@ 2013-09-02 14:51 Andre Przywara
  2013-09-02 15:53 ` Julien Grall
  2013-09-02 16:21 ` Tim Deegan
  0 siblings, 2 replies; 6+ messages in thread
From: Andre Przywara @ 2013-09-02 14:51 UTC (permalink / raw)
  To: julien.grall, Ian.Campbell; +Cc: xen-devel, stefano.stabellini

Although we do not support hardware flow control in the Xen driver
for the PL011 UART, the other end may be configured to use it.
In this case it waits in vain for the RTS signal to be asserted by
the host and will never transmit any characters.
This fixes the UART input on Calxeda Midway, which uses hardware
flow control for the serial-over-LAN functionality.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 xen/drivers/char/pl011.c         | 4 ++--
 xen/include/asm-arm/pl011-uart.h | 4 ++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 3ec6e10..e340961 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -120,8 +120,8 @@ static void __init pl011_init_preirq(struct serial_port *port)
     pl011_write(uart, IMSC, 0);
     pl011_write(uart, ICR, ALLI);
 
-    /* Enable the UART for RX and TX; no flow ctrl */
-    pl011_write(uart, CR, RXE | TXE | UARTEN);
+    /* Enable the UART for RX and TX; assert RTS in case the other end cares */
+    pl011_write(uart, CR, RTS | RXE | TXE | UARTEN);
 }
 
 static void __init pl011_init_postirq(struct serial_port *port)
diff --git a/xen/include/asm-arm/pl011-uart.h b/xen/include/asm-arm/pl011-uart.h
index 3332c51..123f477 100644
--- a/xen/include/asm-arm/pl011-uart.h
+++ b/xen/include/asm-arm/pl011-uart.h
@@ -38,6 +38,10 @@
 #define DMACR  (0x48)
 
 /* CR bits */
+#define CTSEN  (1<<15) /* automatic CTS hardware flow control */
+#define RTSEN  (1<<14) /* automatic RTS hardware flow control */
+#define RTS    (1<<11) /* RTS signal */
+#define DTR    (1<<10) /* DTR signal */
 #define RXE    (1<<9) /* Receive enable */
 #define TXE    (1<<8) /* Transmit enable */
 #define UARTEN (1<<0) /* UART enable */
-- 
1.7.12.1

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

* Re: [PATCH] pl011: assert RTS signal in case the receiver uses flow control
  2013-09-02 14:51 [PATCH] pl011: assert RTS signal in case the receiver uses flow control Andre Przywara
@ 2013-09-02 15:53 ` Julien Grall
  2013-09-03  6:07   ` Andre Przywara
  2013-09-02 16:21 ` Tim Deegan
  1 sibling, 1 reply; 6+ messages in thread
From: Julien Grall @ 2013-09-02 15:53 UTC (permalink / raw)
  To: Andre Przywara; +Cc: xen-devel, Ian.Campbell, stefano.stabellini

On 09/02/2013 03:51 PM, Andre Przywara wrote:
> Although we do not support hardware flow control in the Xen driver
> for the PL011 UART, the other end may be configured to use it.
> In this case it waits in vain for the RTS signal to be asserted by
> the host and will never transmit any characters.
> This fixes the UART input on Calxeda Midway, which uses hardware
> flow control for the serial-over-LAN functionality.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  xen/drivers/char/pl011.c         | 4 ++--
>  xen/include/asm-arm/pl011-uart.h | 4 ++++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
> index 3ec6e10..e340961 100644
> --- a/xen/drivers/char/pl011.c
> +++ b/xen/drivers/char/pl011.c
> @@ -120,8 +120,8 @@ static void __init pl011_init_preirq(struct serial_port *port)
>      pl011_write(uart, IMSC, 0);
>      pl011_write(uart, ICR, ALLI);
>  
> -    /* Enable the UART for RX and TX; no flow ctrl */
> -    pl011_write(uart, CR, RXE | TXE | UARTEN);
> +    /* Enable the UART for RX and TX; assert RTS in case the other end cares */
> +    pl011_write(uart, CR, RTS | RXE | TXE | UARTEN);
>  }

Instead of overriding CR can we read CR and enable the necessary bit?
It seems that Linux doesn't set RTS but keep the level from the previous
register value.

BTW, this patch works on the versatile express :).

Cheers,

-- 
Julien Grall

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

* Re: [PATCH] pl011: assert RTS signal in case the receiver uses flow control
  2013-09-02 14:51 [PATCH] pl011: assert RTS signal in case the receiver uses flow control Andre Przywara
  2013-09-02 15:53 ` Julien Grall
@ 2013-09-02 16:21 ` Tim Deegan
  2013-09-03  6:12   ` Andre Przywara
  1 sibling, 1 reply; 6+ messages in thread
From: Tim Deegan @ 2013-09-02 16:21 UTC (permalink / raw)
  To: Andre Przywara; +Cc: julien.grall, Ian.Campbell, stefano.stabellini, xen-devel

At 16:51 +0200 on 02 Sep (1378140705), Andre Przywara wrote:
> Although we do not support hardware flow control in the Xen driver
> for the PL011 UART, the other end may be configured to use it.
> In this case it waits in vain for the RTS signal to be asserted by
> the host and will never transmit any characters.
> This fixes the UART input on Calxeda Midway, which uses hardware
> flow control for the serial-over-LAN functionality.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  xen/drivers/char/pl011.c         | 4 ++--
>  xen/include/asm-arm/pl011-uart.h | 4 ++++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
> index 3ec6e10..e340961 100644
> --- a/xen/drivers/char/pl011.c
> +++ b/xen/drivers/char/pl011.c
> @@ -120,8 +120,8 @@ static void __init pl011_init_preirq(struct serial_port *port)
>      pl011_write(uart, IMSC, 0);
>      pl011_write(uart, ICR, ALLI);
>  
> -    /* Enable the UART for RX and TX; no flow ctrl */
> -    pl011_write(uart, CR, RXE | TXE | UARTEN);
> +    /* Enable the UART for RX and TX; assert RTS in case the other end cares */
> +    pl011_write(uart, CR, RTS | RXE | TXE | UARTEN);
>  }
>  
>  static void __init pl011_init_postirq(struct serial_port *port)
> diff --git a/xen/include/asm-arm/pl011-uart.h b/xen/include/asm-arm/pl011-uart.h
> index 3332c51..123f477 100644
> --- a/xen/include/asm-arm/pl011-uart.h
> +++ b/xen/include/asm-arm/pl011-uart.h
> @@ -38,6 +38,10 @@
>  #define DMACR  (0x48)
>  
>  /* CR bits */
> +#define CTSEN  (1<<15) /* automatic CTS hardware flow control */
> +#define RTSEN  (1<<14) /* automatic RTS hardware flow control */

Would this bit not be better than blindly setting RTS?

Tim.

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

* Re: [PATCH] pl011: assert RTS signal in case the receiver uses flow control
  2013-09-02 15:53 ` Julien Grall
@ 2013-09-03  6:07   ` Andre Przywara
  0 siblings, 0 replies; 6+ messages in thread
From: Andre Przywara @ 2013-09-03  6:07 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, Ian.Campbell, stefano.stabellini

On 09/02/2013 05:53 PM, Julien Grall wrote:
> On 09/02/2013 03:51 PM, Andre Przywara wrote:
>> Although we do not support hardware flow control in the Xen driver
>> for the PL011 UART, the other end may be configured to use it.
>> In this case it waits in vain for the RTS signal to be asserted by
>> the host and will never transmit any characters.
>> This fixes the UART input on Calxeda Midway, which uses hardware
>> flow control for the serial-over-LAN functionality.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>>   xen/drivers/char/pl011.c         | 4 ++--
>>   xen/include/asm-arm/pl011-uart.h | 4 ++++
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
>> index 3ec6e10..e340961 100644
>> --- a/xen/drivers/char/pl011.c
>> +++ b/xen/drivers/char/pl011.c
>> @@ -120,8 +120,8 @@ static void __init pl011_init_preirq(struct serial_port *port)
>>       pl011_write(uart, IMSC, 0);
>>       pl011_write(uart, ICR, ALLI);
>>
>> -    /* Enable the UART for RX and TX; no flow ctrl */
>> -    pl011_write(uart, CR, RXE | TXE | UARTEN);
>> +    /* Enable the UART for RX and TX; assert RTS in case the other end cares */
>> +    pl011_write(uart, CR, RTS | RXE | TXE | UARTEN);
>>   }
>
> Instead of overriding CR can we read CR and enable the necessary bit?
> It seems that Linux doesn't set RTS but keep the level from the previous
> register value.

Right, that is the way to do it. In fact u-boot already cares about 
this: 
http://git.denx.de/?p=u-boot.git;a=commit;h=10501df05e2d2eef501c92483c134d5f7c9da150

Will send a new version.

> BTW, this patch works on the versatile express :).

Nice, thanks for testing.

Andre.

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

* Re: [PATCH] pl011: assert RTS signal in case the receiver uses flow control
  2013-09-02 16:21 ` Tim Deegan
@ 2013-09-03  6:12   ` Andre Przywara
  2013-09-03  6:40     ` Tim Deegan
  0 siblings, 1 reply; 6+ messages in thread
From: Andre Przywara @ 2013-09-03  6:12 UTC (permalink / raw)
  To: Tim Deegan; +Cc: julien.grall, Ian.Campbell, stefano.stabellini, xen-devel

On 09/02/2013 06:21 PM, Tim Deegan wrote:
> At 16:51 +0200 on 02 Sep (1378140705), Andre Przywara wrote:
>> Although we do not support hardware flow control in the Xen driver
>> for the PL011 UART, the other end may be configured to use it.
>> In this case it waits in vain for the RTS signal to be asserted by
>> the host and will never transmit any characters.
>> This fixes the UART input on Calxeda Midway, which uses hardware
>> flow control for the serial-over-LAN functionality.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>>   xen/drivers/char/pl011.c         | 4 ++--
>>   xen/include/asm-arm/pl011-uart.h | 4 ++++
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
>> index 3ec6e10..e340961 100644
>> --- a/xen/drivers/char/pl011.c
>> +++ b/xen/drivers/char/pl011.c
>> @@ -120,8 +120,8 @@ static void __init pl011_init_preirq(struct serial_port *port)
>>       pl011_write(uart, IMSC, 0);
>>       pl011_write(uart, ICR, ALLI);
>>
>> -    /* Enable the UART for RX and TX; no flow ctrl */
>> -    pl011_write(uart, CR, RXE | TXE | UARTEN);
>> +    /* Enable the UART for RX and TX; assert RTS in case the other end cares */
>> +    pl011_write(uart, CR, RTS | RXE | TXE | UARTEN);
>>   }
>>
>>   static void __init pl011_init_postirq(struct serial_port *port)
>> diff --git a/xen/include/asm-arm/pl011-uart.h b/xen/include/asm-arm/pl011-uart.h
>> index 3332c51..123f477 100644
>> --- a/xen/include/asm-arm/pl011-uart.h
>> +++ b/xen/include/asm-arm/pl011-uart.h
>> @@ -38,6 +38,10 @@
>>   #define DMACR  (0x48)
>>
>>   /* CR bits */
>> +#define CTSEN  (1<<15) /* automatic CTS hardware flow control */
>> +#define RTSEN  (1<<14) /* automatic RTS hardware flow control */
>
> Would this bit not be better than blindly setting RTS?

I don't think so. This sets and clears RTS according to the FIFO fill 
level. Since we don't claim to support h/w flow control, we better leave 
this disabled.
Just asserting RTS all of the time should have no influence on 
non-prepared devices[1]. So I will go with simply ORing in our requested 
bits and leave the flow-control bits as they have been setup before.

Regards,
Andre.

[1] 
http://git.denx.de/?p=u-boot.git;a=commit;h=10501df05e2d2eef501c92483c134d5f7c9da150 

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

* Re: [PATCH] pl011: assert RTS signal in case the receiver uses flow control
  2013-09-03  6:12   ` Andre Przywara
@ 2013-09-03  6:40     ` Tim Deegan
  0 siblings, 0 replies; 6+ messages in thread
From: Tim Deegan @ 2013-09-03  6:40 UTC (permalink / raw)
  To: Andre Przywara; +Cc: julien.grall, Ian.Campbell, stefano.stabellini, xen-devel

At 08:12 +0200 on 03 Sep (1378195944), Andre Przywara wrote:
> On 09/02/2013 06:21 PM, Tim Deegan wrote:
> >At 16:51 +0200 on 02 Sep (1378140705), Andre Przywara wrote:
> >>Although we do not support hardware flow control in the Xen driver
> >>for the PL011 UART, the other end may be configured to use it.
> >>In this case it waits in vain for the RTS signal to be asserted by
> >>the host and will never transmit any characters.
> >>This fixes the UART input on Calxeda Midway, which uses hardware
> >>flow control for the serial-over-LAN functionality.
> >>
> >>Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> >>---
> >>  xen/drivers/char/pl011.c         | 4 ++--
> >>  xen/include/asm-arm/pl011-uart.h | 4 ++++
> >>  2 files changed, 6 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
> >>index 3ec6e10..e340961 100644
> >>--- a/xen/drivers/char/pl011.c
> >>+++ b/xen/drivers/char/pl011.c
> >>@@ -120,8 +120,8 @@ static void __init pl011_init_preirq(struct 
> >>serial_port *port)
> >>      pl011_write(uart, IMSC, 0);
> >>      pl011_write(uart, ICR, ALLI);
> >>
> >>-    /* Enable the UART for RX and TX; no flow ctrl */
> >>-    pl011_write(uart, CR, RXE | TXE | UARTEN);
> >>+    /* Enable the UART for RX and TX; assert RTS in case the other end 
> >>cares */
> >>+    pl011_write(uart, CR, RTS | RXE | TXE | UARTEN);
> >>  }
> >>
> >>  static void __init pl011_init_postirq(struct serial_port *port)
> >>diff --git a/xen/include/asm-arm/pl011-uart.h 
> >>b/xen/include/asm-arm/pl011-uart.h
> >>index 3332c51..123f477 100644
> >>--- a/xen/include/asm-arm/pl011-uart.h
> >>+++ b/xen/include/asm-arm/pl011-uart.h
> >>@@ -38,6 +38,10 @@
> >>  #define DMACR  (0x48)
> >>
> >>  /* CR bits */
> >>+#define CTSEN  (1<<15) /* automatic CTS hardware flow control */
> >>+#define RTSEN  (1<<14) /* automatic RTS hardware flow control */
> >
> >Would this bit not be better than blindly setting RTS?
> 
> I don't think so. This sets and clears RTS according to the FIFO fill 
> level. Since we don't claim to support h/w flow control, we better leave 
> this disabled.
> Just asserting RTS all of the time should have no influence on 
> non-prepared devices

Sure, but frobbing RTS up and down won't have any effect on devices that
ignore it either, and if the hardware will actually DTRT when the other
end _wants_ flow control, why not?

But it's no big deal either way; asserting RTS is harmless and better
than what we have.

> [1]. So I will go with simply ORing in our requested 
> bits and leave the flow-control bits as they have been setup before.

That's fine too, at least in cases where we're alreay taking the baud
settings from the existing setup (which is all the time now?).

Tim.

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

end of thread, other threads:[~2013-09-03  6:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-02 14:51 [PATCH] pl011: assert RTS signal in case the receiver uses flow control Andre Przywara
2013-09-02 15:53 ` Julien Grall
2013-09-03  6:07   ` Andre Przywara
2013-09-02 16:21 ` Tim Deegan
2013-09-03  6:12   ` Andre Przywara
2013-09-03  6:40     ` Tim Deegan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.