Re: [PATCH] dt-bindings: serial: sh-sci: Convert to json-schema

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 23, 2020 at 8:38 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Rob,
>
> Thanks a lot for your comments!
>
> On Thu, Mar 12, 2020 at 9:05 PM Rob Herring <robh@xxxxxxxxxx> wrote:
> > On Fri, Mar 06, 2020 at 10:07:20AM +0100, Geert Uytterhoeven wrote:
> > > Convert the Renesas Serial Communication Interface ((H)SCI(F)(A|B))
> > > Device Tree binding documentation to json-schema.
> > >
> > > Split the bindings in 5 files, one per major type, to ease expressing
> > > constraints.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
>
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/serial/renesas,sci.yaml
>
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - items:
> > > +          - const: renesas,sci
> >
> > Do you plan to add to this? It can be simplified to just
>
> Not really. I just used the same construct for consistency with the other
> SCI variants.
>
> > 'const: renesas,sci'.
>
> OK.  I guess no other H8 and SuperH variants will pop up anytime soon.
> Oops, RZ/A1 has SCI, and RZ/A2 has SCIg (they keep on inventing new
> names, to be seen how compatible).
>
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> > > @@ -0,0 +1,168 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: "http://devicetree.org/schemas/serial/renesas,scif.yaml#";
> > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#";
> > > +
> > > +title: Renesas Serial Communication Interface with FIFO (SCIF)
> > > +
> > > +maintainers:
> > > +  - Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > > +
> > > +description:
> > > +  Each enabled UART may have an optional "serialN" alias in the "aliases" node,
> > > +  where N is the port number (non-negative decimal integer) as printed on the
> > > +  label next to the physical port.
> >
> > That's every serial port...
>
> So you suggest to just remove this paragraph from all files?

Yes.

> Shall I add it to serial.yaml instead?

Sure. I'd really like to get to having a registry of alias names we can check.

>
> > > +  interrupts:
> > > +    description: |
> > > +      Must contain one or more interrupt-specifiers for the serial interface.
> > > +      If a single interrupt is expressed, then all events are
> > > +      multiplexed into this single interrupt.
> > > +
> > > +      If multiple interrupts are provided by the hardware, the order
> > > +      in which the interrupts are listed must match order below. Note
> > > +      that some HW interrupt events may be muxed together resulting
> > > +      in duplicate entries.
> > > +    minItems: 1
> > > +    maxItems: 6
> >
> > This allows 2, 3, 4, or 5 interrupts. Is that valid? If not, then you
>
> 1, 4, and 6 are valid.
>
> > should do something like this:
> >
> > oneOf:
> >   - items:
> >       description: A combined interrupt
> >   - items:
> >       - description: Error interrupt
> >       - ...
>
> So I tried:
>
>   interrupts:
>     oneOf:
>       - items:
>           description: A combined interrupt

You're missing a '-' here.

>       - items:
>           - description: Error interrupt
>           - description: Receive buffer full interrupt
>           - description: Transmit buffer empty interrupt
>           - description: Transmit End interrupt
>       - items:
>           - description: Error interrupt
>           - description: Receive buffer full interrupt
>           - description: Transmit buffer empty interrupt
>           - description: Break interrupt
>           - description: Data Ready interrupt
>           - description: Transmit End interrupt
>
> That fails for devices with 4 or 6 interrupts, e.g.:
>
>     arch/arm/boot/dts/r7s9210-rza2mevb.dt.yaml: serial@e8007000:
> interrupts: [[0, 265, 4], [0, 266, 4], [0, 267, 4], [0, 265, 4], [0,
> 268, 4], [0, 268, 4]] is valid under each of {'additionalItems':
> False, 'items': [{}, {}, {}, {}, {}, {}], 'maxItems': 6, 'minItems':
> 6, 'type': 'array'}, {'items': {}, 'type': 'array'}
>
> Note that initially I forgot to cater for the 4-interrupt case used in
> arch/arm/boot/dts/r7s72100.dtsi, and "make dtbs_check" did not complain.
>
> > > +
> > > +  interrupt-names:
> > > +    minItems: 1
> > > +    maxItems: 6
> > > +    items:
> > > +      enum:
> > > +        - eri # Error
> > > +        - rxi # Receive buffer full
> > > +        - txi # Transmit buffer empty
> > > +        - bri # Break
> > > +        - dri # Data Ready
> > > +        - tei # Transmit End
> >
> > Based on above, you probably want 'items' to be a list, not a
> > dict(schema).
>
> Like
>
>   interrupt-names:
>     oneOf:
>       - items:
>           - const: eri
>           - const: rxi
>           - const: txi
>           - const: tei
>       - items:
>           - const: eri
>           - const: rxi
>           - const: txi
>           - const: bri
>           - const: dri
>           - const: tei
>
> ?
> Seems to work, but needs the duplication as the 4-interrupt case is not
> just the 4 first entries of the 6-interrupt case (tei is always last).

Yes, that looks right. No name for the single irq case?

>
> > > +
> > > +  clocks:
> > > +    minItems: 1
> > > +    maxItems: 4
> > > +
> > > +  clock-names:
> > > +    minItems: 1
> > > +    maxItems: 4
> > > +    items:
> > > +      enum:
> > > +        - fck # UART functional clock
> > > +        - sck # optional external clock input
> > > +        - brg_int # optional internal clock source for BRG frequency divider
> > > +        - scif_clk # optional external clock source for BRG frequency divider
> >
> > Same issue again...
>
> The order is not fixed for the three optional clocks, as they may or
> may not be wired (for sck and scif_clk), or the BRG may not be present.
> Hence unlike for interrupts, I cannot drop the "enum", IIUIC?

Well, you could list out every possible combination, but the above is
fine I guess.

Rob



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux