From: Andre Przywara <andre.przywara@arm.com>
To: Rob Herring <robherring2@gmail.com>
Cc: "linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jslaby@suse.cz>,
Russell King <linux@arm.linux.org.uk>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH 1/1] drivers: introduce ARM SBSA generic UART driver
Date: Tue, 02 Sep 2014 11:06:30 +0100 [thread overview]
Message-ID: <540596A6.8000608@arm.com> (raw)
In-Reply-To: <CAL_JsqJbAWRhduzMwdQMh8sDdVqTWVK2Ej9LQjx4gp2ifwxe6Q@mail.gmail.com>
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.
next parent reply other threads:[~2014-09-02 10:07 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1409328803-1953-1-git-send-email-andre.przywara@arm.com>
[not found] ` <1409328803-1953-2-git-send-email-andre.przywara@arm.com>
[not found] ` <CAL_JsqJbAWRhduzMwdQMh8sDdVqTWVK2Ej9LQjx4gp2ifwxe6Q@mail.gmail.com>
2014-09-02 10:06 ` Andre Przywara [this message]
2014-09-02 10:46 ` [RFC PATCH 1/1] drivers: introduce ARM SBSA generic UART driver Mark Rutland
2014-09-02 13:20 ` Rob Herring
2014-09-02 13:48 ` Arnd Bergmann
2014-09-02 17:38 ` Rob Herring
2014-09-02 19:34 ` Arnd Bergmann
2014-09-05 14:27 ` Andre Przywara
2014-09-05 14:37 ` Andre Przywara
[not found] ` <8453809.d9xRztN9Sq@wuerfel>
[not found] ` <5401086F.1050601@arm.com>
[not found] ` <3427461.cOVRFPXkyG@wuerfel>
2014-09-05 14:11 ` Andre Przywara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=540596A6.8000608@arm.com \
--to=andre.przywara@arm.com \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.cz \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=robherring2@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).