On Wed Jul 17, 2024 at 5:39 PM BST, Rob Herring wrote: > On Wed, Jul 17, 2024 at 10:09:27AM +0100, Rayyan Ansari wrote: > > Convert the bindings for the Qualcomm EMAC Ethernet Controller from the > > old text format to yaml. > > > > Also move the phy node of the controller to be within an mdio block so > > we can use mdio.yaml. > > > > Signed-off-by: Rayyan Ansari <rayyan.ansari@xxxxxxxxxx> > > --- > > .../devicetree/bindings/net/qcom,emac.yaml | 98 ++++++++++++++++ > > .../devicetree/bindings/net/qcom-emac.txt | 111 ------------------ > > 2 files changed, 98 insertions(+), 111 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/net/qcom,emac.yaml > > delete mode 100644 Documentation/devicetree/bindings/net/qcom-emac.txt > > > > diff --git a/Documentation/devicetree/bindings/net/qcom,emac.yaml b/Documentation/devicetree/bindings/net/qcom,emac.yaml > > new file mode 100644 > > index 000000000000..cef65130578f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/qcom,emac.yaml > > @@ -0,0 +1,98 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +--- > > +$id: http://devicetree.org/schemas/net/qcom,emac.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Qualcomm EMAC Gigabit Ethernet Controller > > + > > +maintainers: > > + - Timur Tabi <timur@xxxxxxxxxx> > > + > > +properties: > > + compatible: > > + oneOf: > > + - const: qcom,fsm9900-emac > > + - enum: > > + - qcom,fsm9900-emac-sgmii > > + - qcom,qdf2432-emac-sgmii > > You just need a single enum for all 3 compatibles. > > > + reg: > > + minItems: 1 > > + maxItems: 2 > > Need to define what each entry is and perhaps constraints on when it 1 > vs. 2 entries. > > > + > > + interrupts: > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + > > +if: > > + properties: > > + compatible: > > + const: qcom,fsm9900-emac > > +then: > > + allOf: > > + - $ref: ethernet-controller.yaml# > > This goes at the top level and the 'if' schema should be under the > 'allOf'. Wouldn't this make ethernet-controller.yaml also be included for emac-sgmii as well - which isn't a controller? > > + properties: > > + clocks: > > + minItems: 7 > > + maxItems: 7 > > + > > + clock-names: > > + items: > > + - const: axi_clk > > + - const: cfg_ahb_clk > > + - const: high_speed_clk > > + - const: mdio_clk > > + - const: tx_clk > > + - const: rx_clk > > + - const: sys_clk > > Define these at the top level and then exclude them in the if schema. Just clocks and clock-names? Why so, if sgmii does not require clocks? > > + > > + internal-phy: > > + maxItems: 1 > > This needs a type ref. > > > + > > + mdio: > > + $ref: mdio.yaml# > > + unevaluatedProperties: false > > + > > + required: > > + - clocks > > + - clock-names > > + - internal-phy > > + - phy-handle > > + - mdio > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + emac0: ethernet@feb20000 { > > Drop unused labels. > > > + compatible = "qcom,fsm9900-emac"; > > + reg = <0xfeb20000 0x10000>, > > + <0xfeb36000 0x1000>; > > + interrupts = <76>; > > + > > + clocks = <&gcc 0>, <&gcc 1>, <&gcc 3>, <&gcc 4>, <&gcc 5>, > > + <&gcc 6>, <&gcc 7>; > > + clock-names = "axi_clk", "cfg_ahb_clk", "high_speed_clk", > > + "mdio_clk", "tx_clk", "rx_clk", "sys_clk"; > > + > > + internal-phy = <&emac_sgmii>; > > + phy-handle = <&phy0>; > > + > > + mdio { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + phy0: ethernet-phy@0 { > > + reg = <0>; > > + }; > > + }; > > + }; > > + > > + emac_sgmii: ethernet@feb38000 { > > This should be a separate entry. (You need '- |' above it.) Should they not be part of one complete example, since the main node requires a handle to the sgmii node with 'internal-phy'? > > + compatible = "qcom,fsm9900-emac-sgmii"; > > + reg = <0xfeb38000 0x1000>; > > + interrupts = <80>; > > + };