Re: [PATCH 10/10] drivers: PL011: add support for the ARM SBSA generic UART

[Mark Rutland <mark.rutland@arm.com>]

On Fri, Jan 16, 2015 at 05:23:06PM +0000, 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 the 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 a pretty generic probe function
> which only uses platform device functions.
> A DT binding is provided, but other systems can easily attach to it,
> too (hint, hint!).
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../devicetree/bindings/serial/arm_sbsa_uart.txt   |    9 ++
>  drivers/tty/serial/amba-pl011.c                    |  154 ++++++++++++++++++++
>  2 files changed, 163 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..21d211f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
> @@ -0,0 +1,9 @@
> +* 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
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index a1c929f..596e641 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -101,6 +101,14 @@ static struct vendor_data vendor_arm = {
>  	.get_fifosize		= get_fifosize_arm,
>  };
>  
> +static struct vendor_data vendor_sbsa = {
> +	.oversampling		= false,
> +	.dma_threshold		= false,
> +	.cts_event_workaround	= false,
> +	.always_enabled		= true,
> +	.fixed_options		= "115200n8",
> +};

Is this configuration mandated by the SBSA? If so, please mandate it in
the binding document.

If the rate and so on are not mandated, they should probably be
described by the binding so software has a chance of getting the real
configuration details.

Thanks,
Mark.