On Thu, May 21, 2015 at 4:43 PM, Andre Przywara [off-list ref] wrote:

Hi Mark,

(please keep the people on CC:, I almost missed that mail)

On 05/13/2015 03:32 PM, Mark Langsdorf wrote:

quoted

On 05/12/2015 09:14 AM, Andre Przywara wrote:

quoted

The ARM Server Base System Architecture[1] document describes a
generic UART which is a subset of the PL011 UART.
It lacks DMA support, baud rate control and modem status line
control, among other things.
The idea is to move the UART initialization and setup into the
firmware (which does this job today already) and let the kernel just
use the UART for sending and receiving characters.

We use the recent refactoring to build a new struct uart_ops
variable which points to some new functions avoiding access to the
missing registers. We reuse as much existing PL011 code as possible.

In contrast to the PL011 the SBSA UART does not define any AMBA or
PrimeCell relations, so we go with a pretty generic probe function
which only uses platform device functions.
A DT binding is provided with this patch, ACPI support is added in a
separate one.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
  .../devicetree/bindings/serial/arm_sbsa_uart.txt   |  10 ++
  drivers/tty/serial/amba-pl011.c                    | 168
+++++++++++++++++++++
  2 files changed, 178 insertions(+)
  create mode 100644
Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt

diff --git
a/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
new file mode 100644
index 0000000..4163e7e

--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt

@@ -0,0 +1,10 @@
+* ARM SBSA defined generic UART
+This UART uses a subset of the PL011 registers and consequently lives
+in the PL011 driver. It's baudrate and other communication parameters
+cannot be adjusted at runtime, so it lacks a clock specifier here.
+
+Required properties:
+- compatible: must be "arm,sbsa-uart"
+- reg: exactly one register range
+- interrupts: exactly one interrupt specifier
+- current-speed: the (fixed) baud rate set by the firmware

diff --git a/drivers/tty/serial/amba-pl011.c

b/drivers/tty/serial/amba-pl011.c
index 70e2958..cca93d9 100644

--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c

....

quoted

quoted

@@ -1872,6 +1915,24 @@ pl011_set_termios(struct uart_port *port,

struct ktermios *termios,
      spin_unlock_irqrestore(&port->lock, flags);
  }

+static void
+sbsa_uart_set_termios(struct uart_port *port, struct ktermios *termios,
+              struct ktermios *old)
+{
+    struct uart_amba_port *uap =
+        container_of(port, struct uart_amba_port, port);
+    unsigned long flags;
+
+    if (old)
+        tty_termios_copy_hw(termios, old);

This code prevented login via the serial console on our test hardware.

Mark Salter suggested the following patch:

-       if (old)
+       /*
+        * The first call to set_termios() comes when the console is
+        * registered via uart_add_one_port(). The serial core will
+        * pass in a dummy old termios rather than NULL. Check to make
+        * sure the old termios has reasonable info before copying from
+        * it.
+        */
+       if (old && old->c_cflag)
                tty_termios_copy_hw(termios, old);

Yes, this fixes the problem I was seeing with Fedora21 on ThunderX.

That seems like a kludge to me.
Regardless of the reason why uart_send_options() is passing a
dummy variable instead of a NULL pointer (which documentation explicitly
declares as legal) I saw that this unconditional copying of the old
value being common practise in various drivers, actually the default for
drivers not implementing set_termios at all.

Can you describe a setup where this issue shows up? I haven't observed
this issue in my testing.

But in fact I think we actually shouldn't care about the old value at
all. Instead we should filter the new termios settings to avoid userland
configuring for instance flow control or an differing byte format which
the SBSA UART does not support.
I will cook up a patch and send out a v5 ASAP.

Cheers,
Andre.

quoted

which fixed the issue.

quoted

+    tty_termios_encode_baud_rate(termios, uap->fixed_baud,
uap->fixed_baud);
+
+    spin_lock_irqsave(&port->lock, flags);
+    uart_update_timeout(port, CS8, uap->fixed_baud);
+    pl011_setup_status_masks(port, termios);
+    spin_unlock_irqrestore(&port->lock, flags);
+}
+
  static const char *pl011_type(struct uart_port *port)
  {
      struct uart_amba_port *uap =

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
"For things to change, we must change"
-Naresh Bhat