| Message ID | 20240623170933.63864-1-aford173@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dt-bindings: net: davinci_emac: Convert to yaml version from txt | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 111 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 23/06/2024 19:09, Adam Ford wrote: > The davinci_emac is used by several devices which are still maintained, > but to make some improvements, it's necessary to convert from txt to yaml. > > Signed-off-by: Adam Ford <aford173@gmail.com> > > diff --git a/Documentation/devicetree/bindings/net/davinci_emac.txt b/Documentation/devicetree/bindings/net/davinci_emac.txt > deleted file mode 100644 > index 5e3579e72e2d..000000000000 > --- a/Documentation/devicetree/bindings/net/davinci_emac.txt > +++ /dev/null > @@ -1,44 +0,0 @@ > -* Texas Instruments Davinci EMAC > - > -This file provides information, what the device node > -for the davinci_emac interface contains. > - > -Required properties: > -- compatible: "ti,davinci-dm6467-emac", "ti,am3517-emac" or > - "ti,dm816-emac" > -- reg: Offset and length of the register set for the device > -- ti,davinci-ctrl-reg-offset: offset to control register > -- ti,davinci-ctrl-mod-reg-offset: offset to control module register > -- ti,davinci-ctrl-ram-offset: offset to control module ram > -- ti,davinci-ctrl-ram-size: size of control module ram > -- interrupts: interrupt mapping for the davinci emac interrupts sources: > - 4 sources: <Receive Threshold Interrupt > - Receive Interrupt > - Transmit Interrupt > - Miscellaneous Interrupt> > - > -Optional properties: > -- phy-handle: See ethernet.txt file in the same directory. > - If absent, davinci_emac driver defaults to 100/FULL. > -- ti,davinci-rmii-en: 1 byte, 1 means use RMII > -- ti,davinci-no-bd-ram: boolean, does EMAC have BD RAM? > - > -The MAC address will be determined using the optional properties > -defined in ethernet.txt. > - > -Example (enbw_cmc board): > - eth0: emac@1e20000 { > - compatible = "ti,davinci-dm6467-emac"; > - reg = <0x220000 0x4000>; > - ti,davinci-ctrl-reg-offset = <0x3000>; > - ti,davinci-ctrl-mod-reg-offset = <0x2000>; > - ti,davinci-ctrl-ram-offset = <0>; > - ti,davinci-ctrl-ram-size = <0x2000>; > - local-mac-address = [ 00 00 00 00 00 00 ]; > - interrupts = <33 > - 34 > - 35 > - 36 > - >; > - interrupt-parent = <&intc>; > - }; > diff --git a/Documentation/devicetree/bindings/net/davinci_emac.yaml b/Documentation/devicetree/bindings/net/davinci_emac.yaml > new file mode 100644 > index 000000000000..4c2640aef8a1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/davinci_emac.yaml Filename matching compatible format. Missing vendor prefix. Underscores are not used in names or compatibles. > @@ -0,0 +1,111 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/davinci_emac.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Texas Instruments Davici EMAC > + > +maintainers: > + - Adam Ford <aford@gmail.com> > + > +description: > + Ethernet based on the Programmable Real-Time Unit and Industrial > + Communication Subsystem. > + > +allOf: > + - $ref: ethernet-controller.yaml# > + > +properties: > + compatible: > + items: That's just enum, no need for items here. > + - enum: > + - ti,davinci-dm6467-emac # da850 > + - ti,dm816-emac > + - ti,am3517-emac > + > + reg: > + maxItems: 1 > + > + interrupts: > + minItems: 4 You need to list and describe the items. > + > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: ick > + > + power-domains: > + maxItems: 1 > + > + resets: > + maxItems: 1 > + > + local-mac-address: true Drop > + mac-address: true Drop You miss top-level $ref to appropriate schema. > + > + syscon: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: a phandle to the global system controller on > + to enable/disable interrupts Drop entire property. There was no such property in old binding and nothing explains why it was added. > + > + ti,davinci-ctrl-reg-offset: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Offset to control register > + > + ti,davinci-ctrl-mod-reg-offset: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Offset to control module register > + > + ti,davinci-ctrl-ram-offset: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Offset to control module ram > + > + ti,davinci-ctrl-ram-size: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Size of control module ram > + > + ti,davinci-rmii-en: > + $ref: /schemas/types.yaml#/definitions/uint8 > + description: > + RMII enable means use RMII > + > + ti,davinci-no-bd-ram: > + type: boolean > + description: > + Enable if EMAC have BD RAM > + > +additionalProperties: false Look at example-schema. This goes after required, although anyway should be unevaluatedProperties after adding proper $ref. > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - ti,davinci-ctrl-reg-offset > + - ti,davinci-ctrl-mod-reg-offset > + - ti,davinci-ctrl-ram-offset > + - ti,davinci-ctrl-ram-size > + > +examples: > + - | > + eth0: ethernet@220000 { Drop label. > + compatible = "ti,davinci-dm6467-emac"; > + reg = <0x220000 0x4000>; > + ti,davinci-ctrl-reg-offset = <0x3000>; > + ti,davinci-ctrl-mod-reg-offset = <0x2000>; > + ti,davinci-ctrl-ram-offset = <0>; > + ti,davinci-ctrl-ram-size = <0x2000>; > + local-mac-address = [ 00 00 00 00 00 00 ]; > + interrupts = <33>, <34>, <35>,<36>; > + clocks = <&psc1 5>; > + power-domains = <&psc1 5>; > + status = "disabled"; Drop. It cannot be disabled, otherwise what would be the point of this example? Best regards, Krzysztof
On Mon, Jun 24, 2024 at 12:07 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 23/06/2024 19:09, Adam Ford wrote: > > The davinci_emac is used by several devices which are still maintained, > > but to make some improvements, it's necessary to convert from txt to yaml. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > diff --git a/Documentation/devicetree/bindings/net/davinci_emac.txt b/Documentation/devicetree/bindings/net/davinci_emac.txt > > deleted file mode 100644 > > index 5e3579e72e2d..000000000000 > > --- a/Documentation/devicetree/bindings/net/davinci_emac.txt > > +++ /dev/null > > @@ -1,44 +0,0 @@ > > -* Texas Instruments Davinci EMAC > > - > > -This file provides information, what the device node > > -for the davinci_emac interface contains. > > - > > -Required properties: > > -- compatible: "ti,davinci-dm6467-emac", "ti,am3517-emac" or > > - "ti,dm816-emac" > > -- reg: Offset and length of the register set for the device > > -- ti,davinci-ctrl-reg-offset: offset to control register > > -- ti,davinci-ctrl-mod-reg-offset: offset to control module register > > -- ti,davinci-ctrl-ram-offset: offset to control module ram > > -- ti,davinci-ctrl-ram-size: size of control module ram > > -- interrupts: interrupt mapping for the davinci emac interrupts sources: > > - 4 sources: <Receive Threshold Interrupt > > - Receive Interrupt > > - Transmit Interrupt > > - Miscellaneous Interrupt> > > - > > -Optional properties: > > -- phy-handle: See ethernet.txt file in the same directory. > > - If absent, davinci_emac driver defaults to 100/FULL. > > -- ti,davinci-rmii-en: 1 byte, 1 means use RMII > > -- ti,davinci-no-bd-ram: boolean, does EMAC have BD RAM? > > - > > -The MAC address will be determined using the optional properties > > -defined in ethernet.txt. > > - > > -Example (enbw_cmc board): > > - eth0: emac@1e20000 { > > - compatible = "ti,davinci-dm6467-emac"; > > - reg = <0x220000 0x4000>; > > - ti,davinci-ctrl-reg-offset = <0x3000>; > > - ti,davinci-ctrl-mod-reg-offset = <0x2000>; > > - ti,davinci-ctrl-ram-offset = <0>; > > - ti,davinci-ctrl-ram-size = <0x2000>; > > - local-mac-address = [ 00 00 00 00 00 00 ]; > > - interrupts = <33 > > - 34 > > - 35 > > - 36 > > - >; > > - interrupt-parent = <&intc>; > > - }; > > diff --git a/Documentation/devicetree/bindings/net/davinci_emac.yaml b/Documentation/devicetree/bindings/net/davinci_emac.yaml > > new file mode 100644 > > index 000000000000..4c2640aef8a1 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/davinci_emac.yaml > > Filename matching compatible format. Missing vendor prefix. Underscores > are not used in names or compatibles. Thank you for the review. Would a proper name be ti,davinci-emac.yaml? > > > > @@ -0,0 +1,111 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/net/davinci_emac.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Texas Instruments Davici EMAC > > + > > +maintainers: > > + - Adam Ford <aford@gmail.com> > > + > > +description: > > + Ethernet based on the Programmable Real-Time Unit and Industrial > > + Communication Subsystem. > > + > > +allOf: > > + - $ref: ethernet-controller.yaml# > > + > > +properties: > > + compatible: > > + items: > > > That's just enum, no need for items here. > > > + - enum: > > + - ti,davinci-dm6467-emac # da850 > > + - ti,dm816-emac > > + - ti,am3517-emac > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + minItems: 4 > > You need to list and describe the items. > > > + > > + clocks: > > + maxItems: 1 > > + > > + clock-names: > > + items: > > + - const: ick > > + > > + power-domains: > > + maxItems: 1 > > + > > + resets: > > + maxItems: 1 > > + > > + local-mac-address: true > > Drop > > > + mac-address: true > > Drop > > You miss top-level $ref to appropriate schema. > > > + > > + syscon: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: a phandle to the global system controller on > > + to enable/disable interrupts > > Drop entire property. There was no such property in old binding and > nothing explains why it was added. The am3517.dtsi emac node has a syscon, so I didn't want to break it. I'll take a look to see what the syscon node on the am3517 does. I struggle with if statements in yaml, but if it's necessary for the am3517, can we keep it if I elaborate on it in the commit message? > > > + > > + ti,davinci-ctrl-reg-offset: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + Offset to control register > > + > > + ti,davinci-ctrl-mod-reg-offset: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + Offset to control module register > > + > > + ti,davinci-ctrl-ram-offset: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + Offset to control module ram > > + > > + ti,davinci-ctrl-ram-size: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + Size of control module ram > > + > > + ti,davinci-rmii-en: > > + $ref: /schemas/types.yaml#/definitions/uint8 > > + description: > > + RMII enable means use RMII > > + > > + ti,davinci-no-bd-ram: > > + type: boolean > > + description: > > + Enable if EMAC have BD RAM > > + > > +additionalProperties: false > > Look at example-schema. This goes after required, although anyway should > be unevaluatedProperties after adding proper $ref. > > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - clocks > > + - ti,davinci-ctrl-reg-offset > > + - ti,davinci-ctrl-mod-reg-offset > > + - ti,davinci-ctrl-ram-offset > > + - ti,davinci-ctrl-ram-size > > + > > +examples: > > + - | > > + eth0: ethernet@220000 { > > Drop label. > > > + compatible = "ti,davinci-dm6467-emac"; > > + reg = <0x220000 0x4000>; > > + ti,davinci-ctrl-reg-offset = <0x3000>; > > + ti,davinci-ctrl-mod-reg-offset = <0x2000>; > > + ti,davinci-ctrl-ram-offset = <0>; > > + ti,davinci-ctrl-ram-size = <0x2000>; > > + local-mac-address = [ 00 00 00 00 00 00 ]; > > + interrupts = <33>, <34>, <35>,<36>; > > + clocks = <&psc1 5>; > > + power-domains = <&psc1 5>; > > + status = "disabled"; > > Drop. It cannot be disabled, otherwise what would be the point of this > example? Sorry, I copy-pasted this from the da850.dtsi node. I'll remove the label and the status line. > > > Best regards, > Krzysztof >
On 24/06/2024 13:59, Adam Ford wrote: >>> diff --git a/Documentation/devicetree/bindings/net/davinci_emac.yaml b/Documentation/devicetree/bindings/net/davinci_emac.yaml >>> new file mode 100644 >>> index 000000000000..4c2640aef8a1 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/davinci_emac.yaml >> >> Filename matching compatible format. Missing vendor prefix. Underscores >> are not used in names or compatibles. > > Thank you for the review. > > Would a proper name be ti,davinci-emac.yaml? Yes, it's fine. > >> >> >>> @@ -0,0 +1,111 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/net/davinci_emac.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Texas Instruments Davici EMAC >>> + >>> +maintainers: >>> + - Adam Ford <aford@gmail.com> >>> + >>> +description: >>> + Ethernet based on the Programmable Real-Time Unit and Industrial >>> + Communication Subsystem. >>> + >>> +allOf: >>> + - $ref: ethernet-controller.yaml# >>> + >>> +properties: >>> + compatible: >>> + items: >> >> >> That's just enum, no need for items here. >> >>> + - enum: >>> + - ti,davinci-dm6467-emac # da850 >>> + - ti,dm816-emac >>> + - ti,am3517-emac >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + minItems: 4 >> >> You need to list and describe the items. >> >>> + >>> + clocks: >>> + maxItems: 1 >>> + >>> + clock-names: >>> + items: >>> + - const: ick >>> + >>> + power-domains: >>> + maxItems: 1 >>> + >>> + resets: >>> + maxItems: 1 >>> + >>> + local-mac-address: true >> >> Drop >> >>> + mac-address: true >> >> Drop >> >> You miss top-level $ref to appropriate schema. >> >>> + >>> + syscon: >>> + $ref: /schemas/types.yaml#/definitions/phandle >>> + description: a phandle to the global system controller on >>> + to enable/disable interrupts >> >> Drop entire property. There was no such property in old binding and >> nothing explains why it was added. > > The am3517.dtsi emac node has a syscon, so I didn't want to break it. > I'll take a look to see what the syscon node on the am3517 does. I > struggle with if statements in yaml, but if it's necessary for the > am3517, can we keep it if I elaborate on it in the commit message? Explain in commit msg changes to the binding done during conversion. With a rationale why these are needed (e.g. existing DTS and Linux drivers use them). Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/net/davinci_emac.txt b/Documentation/devicetree/bindings/net/davinci_emac.txt deleted file mode 100644 index 5e3579e72e2d..000000000000 --- a/Documentation/devicetree/bindings/net/davinci_emac.txt +++ /dev/null @@ -1,44 +0,0 @@ -* Texas Instruments Davinci EMAC - -This file provides information, what the device node -for the davinci_emac interface contains. - -Required properties: -- compatible: "ti,davinci-dm6467-emac", "ti,am3517-emac" or - "ti,dm816-emac" -- reg: Offset and length of the register set for the device -- ti,davinci-ctrl-reg-offset: offset to control register -- ti,davinci-ctrl-mod-reg-offset: offset to control module register -- ti,davinci-ctrl-ram-offset: offset to control module ram -- ti,davinci-ctrl-ram-size: size of control module ram -- interrupts: interrupt mapping for the davinci emac interrupts sources: - 4 sources: <Receive Threshold Interrupt - Receive Interrupt - Transmit Interrupt - Miscellaneous Interrupt> - -Optional properties: -- phy-handle: See ethernet.txt file in the same directory. - If absent, davinci_emac driver defaults to 100/FULL. -- ti,davinci-rmii-en: 1 byte, 1 means use RMII -- ti,davinci-no-bd-ram: boolean, does EMAC have BD RAM? - -The MAC address will be determined using the optional properties -defined in ethernet.txt. - -Example (enbw_cmc board): - eth0: emac@1e20000 { - compatible = "ti,davinci-dm6467-emac"; - reg = <0x220000 0x4000>; - ti,davinci-ctrl-reg-offset = <0x3000>; - ti,davinci-ctrl-mod-reg-offset = <0x2000>; - ti,davinci-ctrl-ram-offset = <0>; - ti,davinci-ctrl-ram-size = <0x2000>; - local-mac-address = [ 00 00 00 00 00 00 ]; - interrupts = <33 - 34 - 35 - 36 - >; - interrupt-parent = <&intc>; - }; diff --git a/Documentation/devicetree/bindings/net/davinci_emac.yaml b/Documentation/devicetree/bindings/net/davinci_emac.yaml new file mode 100644 index 000000000000..4c2640aef8a1 --- /dev/null +++ b/Documentation/devicetree/bindings/net/davinci_emac.yaml @@ -0,0 +1,111 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/davinci_emac.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Texas Instruments Davici EMAC + +maintainers: + - Adam Ford <aford@gmail.com> + +description: + Ethernet based on the Programmable Real-Time Unit and Industrial + Communication Subsystem. + +allOf: + - $ref: ethernet-controller.yaml# + +properties: + compatible: + items: + - enum: + - ti,davinci-dm6467-emac # da850 + - ti,dm816-emac + - ti,am3517-emac + + reg: + maxItems: 1 + + interrupts: + minItems: 4 + + clocks: + maxItems: 1 + + clock-names: + items: + - const: ick + + power-domains: + maxItems: 1 + + resets: + maxItems: 1 + + local-mac-address: true + mac-address: true + + syscon: + $ref: /schemas/types.yaml#/definitions/phandle + description: a phandle to the global system controller on + to enable/disable interrupts + + ti,davinci-ctrl-reg-offset: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Offset to control register + + ti,davinci-ctrl-mod-reg-offset: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Offset to control module register + + ti,davinci-ctrl-ram-offset: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Offset to control module ram + + ti,davinci-ctrl-ram-size: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Size of control module ram + + ti,davinci-rmii-en: + $ref: /schemas/types.yaml#/definitions/uint8 + description: + RMII enable means use RMII + + ti,davinci-no-bd-ram: + type: boolean + description: + Enable if EMAC have BD RAM + +additionalProperties: false + +required: + - compatible + - reg + - interrupts + - clocks + - ti,davinci-ctrl-reg-offset + - ti,davinci-ctrl-mod-reg-offset + - ti,davinci-ctrl-ram-offset + - ti,davinci-ctrl-ram-size + +examples: + - | + eth0: ethernet@220000 { + compatible = "ti,davinci-dm6467-emac"; + reg = <0x220000 0x4000>; + ti,davinci-ctrl-reg-offset = <0x3000>; + ti,davinci-ctrl-mod-reg-offset = <0x2000>; + ti,davinci-ctrl-ram-offset = <0>; + ti,davinci-ctrl-ram-size = <0x2000>; + local-mac-address = [ 00 00 00 00 00 00 ]; + interrupts = <33>, <34>, <35>,<36>; + clocks = <&psc1 5>; + power-domains = <&psc1 5>; + status = "disabled"; + }; +
The davinci_emac is used by several devices which are still maintained, but to make some improvements, it's necessary to convert from txt to yaml. Signed-off-by: Adam Ford <aford173@gmail.com>