[PATCH v2 01/16] dt-bindings: mvebu-uart: update documentation with extended UART

[Rob Herring]

On Wed, Oct 18, 2017 at 1:25 AM, Miquel RAYNAL
<miquel.raynal at free-electrons.com> wrote:
> Hi Rob,
>
> On Tue, 17 Oct 2017 17:00:22 -0500
> Rob Herring <robh at kernel.org> wrote:
>
>> On Fri, Oct 13, 2017 at 11:01:45AM +0200, Miquel Raynal wrote:
>> > Update the Device Tree binding documentation for the Marvell EBU
>> > UART, in order to allow describing the extended UART IP block, in
>> > addition to the already supported standard UART IP. This requires
>> > adding a new compatible string, the introduction of a clocks
>> > property, and extensions to the interrupts property.
>> >
>> > Signed-off-by: Miquel Raynal <miquel.raynal at free-electrons.com>
>> > Reviewed-by: Gregory CLEMENT <gregory.clement at free-electrons.com>
>> > ---
>> >  .../devicetree/bindings/serial/mvebu-uart.txt      | 49
>> > +++++++++++++++++++--- 1 file changed, 44 insertions(+), 5
>> > deletions(-)
>> >
>> > diff --git
>> > a/Documentation/devicetree/bindings/serial/mvebu-uart.txt
>> > b/Documentation/devicetree/bindings/serial/mvebu-uart.txt index
>> > d37fabe17bd1..3df3a3fab4bb 100644 ---
>> > a/Documentation/devicetree/bindings/serial/mvebu-uart.txt +++
>> > b/Documentation/devicetree/bindings/serial/mvebu-uart.txt @@ -1,13
>> > +1,52 @@ -* Marvell UART : Non standard UART used in some of
>> > Marvell EBU SoCs (e.g., Armada-3700) +* Marvell UART : Non standard
>> > UART used in some of Marvell EBU SoCs
>> > +                 e.g., Armada-3700.
>> >
>> >  Required properties:
>> > -- compatible: "marvell,armada-3700-uart"
>> > +- compatible:
>> > +    - "marvell,armada-3700-uart" for the standard variant of the
>> > UART
>> > +      (32 bytes FIFO, no DMA, level interrupts, 8-bit access to the
>> > +      FIFO, baudrate limited to 230400).
>> > +    - "marvell,armada-3700-uart-ext" for the extended variant of
>> > the
>> > +      UART (128 bytes FIFO, DMA, front interrupts, 8-bit or 32-bit
>> > +      accesses to the FIFO, baudrate unlimited by the dividers).
>>
>> What do you call the next extended version?
>> marvell,armada-3700-uart-ext-ext?
>
> I don't know what you mean by "next extended version"?

IP evolves on new chips with new features. Just trying to understand
how you are

>> This is different versions of UART on the same chip?
>
> Today in mainline there is support for the A3700 UART IP.
> This series add support for another IP, based on the A3700, but with
> extended features (explaining the -ext suffix).
>
> Can you precise what is bothering you?

Is this different versions of UART IP on 1 chip or a new version of
the UART IP on a new SoC? The latter should be a new compatible with
the new SoC. The former case does happen some, but is not common. I'm
just trying to understand which applies here.


>> >  - reg: offset and length of the register set for the device.
>> > -- interrupts: device interrupt
>> > +- clocks: UART reference clock used to derive the baudrate (only
>> > +      mandatory with "marvell,armada-3700-uart-ext" compatible).
>>
>> How is this optional? The freq is fixed if not present? If so, what
>> frequency?
>
> The "clocks" property should not be optional at all but that is how the
> bindings were handled before this series, so I can't tell now that
> this property is mandatory as it would break compatibility with older
> versions of the driver.

Okay. I think it should be mandatory with a note how missing property
is handled.

> When no clock is provided, the frequency is fixed by the bootloader
> and cannot be changed. There is no standard frequency for it but the
> one chosen by the bootloader often is 115200 as the UART is usually
> used as the serial console.
>
> Because the bootloader does only initialize the UART in use for the
> serial console, the clock is mandatory when using another port or it
> will not work at all.