Hi Krystof, Thanks for the feedback. > Subject: Re: [PATCH v3 1/6] dt-bindings: can: sja1000: Convert to json- > schema > > On 04/07/2022 09:50, Biju Das wrote: > > Convert the NXP SJA1000 CAN Controller Device Tree binding > > documentation to json-schema. > > > > Update the example to match reality. > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > --- > > v2->v3: > > * Added reg-io-width is a required property for technologic,sja1000 > > * Removed enum type from nxp,tx-output-config and updated the > description > > for combination of TX0 and TX1. > > * Updated the example > > v1->v2: > > * Moved $ref: can-controller.yaml# to top along with if conditional > > to > > avoid multiple mapping issues with the if conditional in the > subsequent > > patch. > > --- > > .../bindings/net/can/nxp,sja1000.yaml | 103 ++++++++++++++++++ > > .../devicetree/bindings/net/can/sja1000.txt | 58 ---------- > > 2 files changed, 103 insertions(+), 58 deletions(-) create mode > > 100644 Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml > > delete mode 100644 > > Documentation/devicetree/bindings/net/can/sja1000.txt > > > > diff --git > > a/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml > > b/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml > > new file mode 100644 > > index 000000000000..d34060226e4e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml > > @@ -0,0 +1,103 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > + > > +title: Memory mapped SJA1000 CAN controller from NXP (formerly > > +Philips) > > + > > +maintainers: > > + - Wolfgang Grandegger <wg@xxxxxxxxxxxxxx> > > + > > +allOf: > > + - $ref: can-controller.yaml# > > + - if: > > The advice of moving it up was not correct. The allOf containing ref and > if:then goes to place like in example-schema, so before > additional/unevaluatedProperties at the bottom. > > Please do not introduce some inconsistent style. There are some examples like[1], where allOf is at the top. [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml?h=next-20220704 Marc, please let us know, if you still prefer allOf at the top. > > > + properties: > > + compatible: > > + contains: > > + const: technologic,sja1000 > > + then: > > + required: > > + - reg-io-width > > + > > +properties: > > + compatible: > > + oneOf: > > + - description: NXP SJA1000 CAN Controller > > + const: nxp,sja1000 > > + - description: Technologic Systems SJA1000 CAN Controller > > + const: technologic,sja1000 > > Entire entry should be just enum. Descriptions do not bring any > information - they copy compatible. OK, I will remove description and will use enum. > > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + reg-io-width: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: I/O register width (in bytes) implemented by this > device > > + default: 1 > > + enum: [ 1, 2, 4 ] > > + > > + nxp,external-clock-frequency: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + default: 16000000 > > + description: | > > + Frequency of the external oscillator clock in Hz. > > + The internal clock frequency used by the SJA1000 is half of that > value. > > + > > + nxp,tx-output-mode: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [ 0, 1, 2, 3 ] > > + default: 1 > > + description: | > > + operation mode of the TX output control logic. Valid values are: > > + <0x0> : bi-phase output mode > > + <0x1> : normal output mode (default) > > + <0x2> : test output mode > > + <0x3> : clock output mode > > Use decimal values, just like in enum. OK. > > > + > > + nxp,tx-output-config: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + default: 0x02 > > + description: | > > + TX output pin configuration. Valid values are any one of the > below > > + or combination of TX0 and TX1: > > + <0x01> : TX0 invert > > + <0x02> : TX0 pull-down (default) > > + <0x04> : TX0 pull-up > > + <0x06> : TX0 push-pull > > + <0x08> : TX1 invert > > + <0x10> : TX1 pull-down > > + <0x20> : TX1 pull-up > > + <0x30> : TX1 push-pull > > + > > + nxp,clock-out-frequency: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + clock frequency in Hz on the CLKOUT pin. > > + If not specified or if the specified value is 0, the CLKOUT pin > > + will be disabled. > > + > > + nxp,no-comparator-bypass: > > + type: boolean > > + description: Allows to disable the CAN input comparator. > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + can@1a000 { > > + compatible = "technologic,sja1000"; > > Unusual indentation. Use 4 spaces for the DTS example. OK, Will do. Cheers, Biju > > > + reg = <0x1a000 0x100>; > > + interrupts = <1>; > > + reg-io-width = <2>; > > + nxp,tx-output-config = <0x06>; > > + nxp,external-clock-frequency = <24000000>; > > + }; > > diff --git a/Documentation/devicetree/bindings/net/can/sja1000.txt > > b/Documentation/devicetree/bindings/net/can/sja1000.txt > > deleted file mode 100644 > > index ac3160eca96a..000000000000 > > --- a/Documentation/devicetree/bindings/net/can/sja1000.txt > > +++ /dev/null > > @@ -1,58 +0,0 @@ > > -Memory mapped SJA1000 CAN controller from NXP (formerly Philips) > > - > > -Required properties: > > - > > -- compatible : should be one of "nxp,sja1000", "technologic,sja1000". > > - > > -- reg : should specify the chip select, address offset and size > required > > - to map the registers of the SJA1000. The size is usually 0x80. > > - > > -- interrupts: property with a value describing the interrupt source > > - (number and sensitivity) required for the SJA1000. > > - > > -Optional properties: > > - > > -- reg-io-width : Specify the size (in bytes) of the IO accesses that > > - should be performed on the device. Valid value is 1, 2 or 4. > > - This property is ignored for technologic version. > > - Default to 1 (8 bits). > > - > > -- nxp,external-clock-frequency : Frequency of the external oscillator > > - clock in Hz. Note that the internal clock frequency used by the > > - SJA1000 is half of that value. If not specified, a default value > > - of 16000000 (16 MHz) is used. > > - > > -- nxp,tx-output-mode : operation mode of the TX output control logic: > > - <0x0> : bi-phase output mode > > - <0x1> : normal output mode (default) > > - <0x2> : test output mode > > - <0x3> : clock output mode > > - > > -- nxp,tx-output-config : TX output pin configuration: > > - <0x01> : TX0 invert > > - <0x02> : TX0 pull-down (default) > > - <0x04> : TX0 pull-up > > - <0x06> : TX0 push-pull > > - <0x08> : TX1 invert > > - <0x10> : TX1 pull-down > > - <0x20> : TX1 pull-up > > - <0x30> : TX1 push-pull > > - > > -- nxp,clock-out-frequency : clock frequency in Hz on the CLKOUT pin. > > - If not specified or if the specified value is 0, the CLKOUT pin > > - will be disabled. > > - > > -- nxp,no-comparator-bypass : Allows to disable the CAN input > comparator. > > - > > -For further information, please have a look to the SJA1000 data sheet. > > - > > -Examples: > > - > > -can@3,100 { > > - compatible = "nxp,sja1000"; > > - reg = <3 0x100 0x80>; > > - interrupts = <2 0>; > > - interrupt-parent = <&mpic>; > > - nxp,external-clock-frequency = <16000000>; > > -}; > > -
