On 05/21/2015 06:13 AM, Andre Przywara wrote:
Hi Mark,
(please keep the people on CC:, I almost missed that mail)
On 05/13/2015 03:32 PM, Mark Langsdorf wrote:
On 05/12/2015 09:14 AM, Andre Przywara wrote:
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@xxxxxxx>
---
.../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
....
@@ -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);
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.
Since other people have said it:
AMD Seattle, booting with ACPI, connected via a serial console
and running Fedora. Systems boot just fine, but when you attempt
to log in, the first time you hit return it doesn't register
so you can't ask for your password. If you do hit return twice,
apparently both instances of the return are registered at the
same time and the system rejects your invalid password (less
sure of the mechanics here).
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.
Okay.
--Mark Langsdorf
which fixed the issue.
+ 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 =
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
