lkml.org 
[lkml]   [2014]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC PATCH 1/1] drivers: introduce ARM SBSA generic UART driver
    Hi Rob,

    thanks for looking at this.

    On 02/09/14 04:06, Rob Herring wrote:
    > On Fri, Aug 29, 2014 at 11:13 AM, Andre Przywara <andre.przywara@arm.com> wrote:
    >> The ARM Server Base System Architecture (SBSA) describes a generic
    >> UART which all compliant level 1 systems should implement. This is
    >> actually a PL011 subset, so a full PL011 implementation will satisfy
    >> this requirement.
    >> However if a system does not have a PL011, a very stripped down
    >> implementation complying to the SBSA defined specification will
    >> suffice. The Linux PL011 driver is not guaranteed to drive this
    >> limited device (and indeed the fast model implentation hangs the
    >> kernel if driven by the PL011 driver).
    >> So introduce a new driver just implementing the part specified by the
    >> SBSA (which lacks DMA, the modem control signals and many of the
    >> registers including baud rate control). This driver has been derived
    >> by the actual PL011 one, removing all unnecessary code.
    >>
    >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
    >> ---
    >> .../devicetree/bindings/serial/arm_sbsa_uart.txt | 6 +
    >> drivers/tty/serial/Kconfig | 28 +
    >> drivers/tty/serial/Makefile | 1 +
    >> drivers/tty/serial/sbsa_uart.c | 793 ++++++++++++++++++++
    >> include/uapi/linux/serial_core.h | 1 +
    >
    > Sorry, but I think this is all wrong. We've now just duplicated some
    > subset of the pl011 driver leaving out the parts like setting baudrate
    > which can never be added since those things could be different for
    > every vendor.
    >
    > The original intent of the SBSA uart was to provide a common early
    > debug uart. It was not to have a full fledged driver. I think the SBSA
    > has failed in this area and created the potential to create a mess of
    > serial drivers different for every vendor. Reality will hopefully not
    > be that extreme and most vendors will just use the pl011 and create
    > their value add somewhere besides the uart. For the purpose of debug
    > output, we already support that as the pl011 earlycon only touches
    > SBSA compatible registers.

    I see your point (and was actually looking for those kind of comments
    when posting this).
    I agree to that debug aspect and understand that earlycon already does
    this, but I think we need some support beyond earlycon, to be able to
    login and use it as a console (which is not possible with earlycon,
    right?) This is probably still for debugging or emergency access to the
    system only, but maybe also for logging - actually quite similar to how
    UARTs are used on today's x86 servers.
    So after having written three incarnations of this driver (goldfish
    based, PL010 based, PL011 based) I wonder if supporting the SBSA subset
    in the real PL011 driver is an option. Either this would be enabled by a
    new explicit DT property or preferably by a clever compatible string.
    Ideally we would just provide a different set of "struct uart_ops"
    members, with some pointing to generic PL011 routines, some to SBSA UART
    specific ones.
    Maybe we make the full featured PL011 support a config option
    (defaulting to y), allowing people to only use the SBSA subset in their
    kernel?

    Does that make more sense? (for a general SBSA h/w rationale see below)

    >> 5 files changed, 829 insertions(+)
    >> create mode 100644 Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
    >> create mode 100644 drivers/tty/serial/sbsa_uart.c
    >>
    >> 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..8e2c5d6
    >> --- /dev/null
    >> +++ b/Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt
    >> @@ -0,0 +1,6 @@
    >> +* ARM SBSA defined generic UART
    >> +
    >> +Required properties:
    >> +- compatible: must be "arm,sbsa-uart"
    >
    > This alone is not okay. There is no such implementation of hardware.

    But the SBSA explicitly allows this. I don't know of any vendor who just
    implements the subset, but I've been told that this has been asked for.

    > The DT must specify the implementation such as pl011.

    If it is a full featured PL011: sure. Then we don't need this driver at
    all and just use the SBSA UART spec as a guideline for our earlycon
    implementation.
    I will try to learn if there is someone actually implementing only the
    subset.

    Cheers,
    Andre.


    \
     
     \ /
      Last update: 2014-09-02 12:21    [W:3.083 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site