| Message ID | 20220624200628.77047-3-alexandru.tachici@analog.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | net: ethernet: adi: Add ADIN1110 support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
On 24/06/2022 22:06, alexandru.tachici@analog.com wrote: > From: Alexandru Tachici <alexandru.tachici@analog.com> > > Add bindings for the ADIN1110/2111 MAC-PHY/SWITCH. > > Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com> > --- > .../devicetree/bindings/net/adi,adin1110.yaml | 127 ++++++++++++++++++ > 1 file changed, 127 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/adi,adin1110.yaml > > diff --git a/Documentation/devicetree/bindings/net/adi,adin1110.yaml b/Documentation/devicetree/bindings/net/adi,adin1110.yaml > new file mode 100644 > index 000000000000..0ac18dd62e5a > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/adi,adin1110.yaml > @@ -0,0 +1,127 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/net/adi,adin1110.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" No quotes in both of above. > + > +title: ADI ADIN1110 MAC-PHY > + > +allOf: allOf goes after description. > + - $ref: ethernet-controller.yaml# > + - $ref: spi-controller.yaml# From the description it looks it is SPI device, not a controller. > + > +maintainers: > + - Alexandru Tachici <alexandru.tachici@analog.com> > + > +description: | > + The ADIN1110 is a low power single port 10BASE-T1L MAC- > + PHY designed for industrial Ethernet applications. It integrates > + an Ethernet PHY core with a MAC and all the associated analog > + circuitry, input and output clock buffering. > + > + The ADIN2111 is a low power, low complexity, two-Ethernet ports > + switch with integrated 10BASE-T1L PHYs and one serial peripheral > + interface (SPI) port. The device is designed for industrial Ethernet > + applications using low power constrained nodes and is compliant > + with the IEEE 802.3cg-2019 Ethernet standard for long reach > + 10 Mbps single pair Ethernet (SPE). > + > + The device has a 4-wire SPI interface for communication > + between the MAC and host processor. > + > +properties: > + compatible: > + enum: > + - adi,adin1110 > + - adi,adin2111 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + reg: > + maxItems: 1 > + > + adi,spi-crc: > + description: | > + Enable CRC8 checks on SPI read/writes. > + type: boolean > + > + interrupts: > + maxItems: 1 > + > +patternProperties: > + "^phy@[0-1]$": [01] is shorter "phy" child node is deprecated, so phy@0 and phy@1, I think, as well. Look how other ethernet controllers are doing it. > + description: | > + ADIN1100 PHY that is present on the same chip as the MAC. > + type: object > + > + properties: > + reg: maxItems:1 > + items: > + maximum: 1 > + > + allOf: > + - if: > + properties: > + compatible: I am not sure this works correctly... You reference here parent properties but then change them to some other compatible? Did you actually test that this works as it should? > + contains: > + const: adi,adin1110 > + then: > + properties: > + compatible: > + const: ethernet-phy-id0283.bc91 > + else: > + properties: > + compatible: > + const: ethernet-phy-id0283.bca1 > + > + required: > + - compatible > + - reg > + > +required: > + - compatible > + - reg > + - interrupts > + - phy@0 > + > +unevaluatedProperties: false > + > +examples: > + - | > + spi0 { 1. spi, not spi0 2. Wrong indentation. Use 4 spaces for DTS example. > + #address-cells = <1>; > + #size-cells = <0>; > + status = "okay"; This is not needed. > + > + ethernet@0 { > + compatible = "adi,adin2111"; > + reg = <0>; > + spi-max-frequency = <24500000>; > + > + adi,spi-crc; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + interrupt-parent = <&gpio>; > + interrupts = <25 2>; "2" looks like interrupt flag, so use it. > + > + mac-address = [ ca 2f b7 10 23 63 ]; This should be rather some 00 11 22 type of MAC, or you expect to encode same MAC in several devices? > + > + phy@0 { > + #phy-cells = <0>; > + compatible = "ethernet-phy-id0283.bca1"; > + reg = <0>; > + }; > + > + phy@1 { > + #phy-cells = <0>; > + compatible = "ethernet-phy-id0283.bca1"; > + reg = <1>; > + }; > + }; > + }; Best regards, Krzysztof
> +patternProperties: > + "^phy@[0-1]$": > + description: | > + ADIN1100 PHY that is present on the same chip as the MAC. > + type: object > + > + properties: > + reg: > + items: > + maximum: 1 > + > + allOf: > + - if: > + properties: > + compatible: > + contains: > + const: adi,adin1110 > + then: > + properties: > + compatible: > + const: ethernet-phy-id0283.bc91 > + else: > + properties: > + compatible: > + const: ethernet-phy-id0283.bca1 > + > + required: > + - compatible > + - reg Why is any of this needed? You register an MDIO bus and then use the PHY at address 0 or 1. phylib should find the PHY and read its ID register to load the driver. So i don't think there is anything useful here. Andrew
On Fri, 24 Jun 2022 23:06:28 +0300, alexandru.tachici@analog.com wrote: > From: Alexandru Tachici <alexandru.tachici@analog.com> > > Add bindings for the ADIN1110/2111 MAC-PHY/SWITCH. > > Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com> > --- > .../devicetree/bindings/net/adi,adin1110.yaml | 127 ++++++++++++++++++ > 1 file changed, 127 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/adi,adin1110.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: ./Documentation/devicetree/bindings/net/adi,adin1110.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/net/spi-controller.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/adi,adin1110.example.dtb: ethernet@0: False schema does not allow {'compatible': ['adi,adin2111'], 'reg': [[0]], 'spi-max-frequency': [[24500000]], 'adi,spi-crc': True, '#address-cells': [[1]], '#size-cells': [[0]], 'interrupts': [[25, 2]], 'mac-address': [[202, 47, 183, 16, 35, 99]], 'phy@0': {'#phy-cells': [[0]], 'compatible': ['ethernet-phy-id0283.bca1'], 'reg': [[0]]}, 'phy@1': {'#phy-cells': [[0]], 'compatible': ['ethernet-phy-id0283.bca1'], 'reg': [[1]]}, '$nodename': ['ethernet@0']} From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/adi,adin1110.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
diff --git a/Documentation/devicetree/bindings/net/adi,adin1110.yaml b/Documentation/devicetree/bindings/net/adi,adin1110.yaml new file mode 100644 index 000000000000..0ac18dd62e5a --- /dev/null +++ b/Documentation/devicetree/bindings/net/adi,adin1110.yaml @@ -0,0 +1,127 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/net/adi,adin1110.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: ADI ADIN1110 MAC-PHY + +allOf: + - $ref: ethernet-controller.yaml# + - $ref: spi-controller.yaml# + +maintainers: + - Alexandru Tachici <alexandru.tachici@analog.com> + +description: | + The ADIN1110 is a low power single port 10BASE-T1L MAC- + PHY designed for industrial Ethernet applications. It integrates + an Ethernet PHY core with a MAC and all the associated analog + circuitry, input and output clock buffering. + + The ADIN2111 is a low power, low complexity, two-Ethernet ports + switch with integrated 10BASE-T1L PHYs and one serial peripheral + interface (SPI) port. The device is designed for industrial Ethernet + applications using low power constrained nodes and is compliant + with the IEEE 802.3cg-2019 Ethernet standard for long reach + 10 Mbps single pair Ethernet (SPE). + + The device has a 4-wire SPI interface for communication + between the MAC and host processor. + +properties: + compatible: + enum: + - adi,adin1110 + - adi,adin2111 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + reg: + maxItems: 1 + + adi,spi-crc: + description: | + Enable CRC8 checks on SPI read/writes. + type: boolean + + interrupts: + maxItems: 1 + +patternProperties: + "^phy@[0-1]$": + description: | + ADIN1100 PHY that is present on the same chip as the MAC. + type: object + + properties: + reg: + items: + maximum: 1 + + allOf: + - if: + properties: + compatible: + contains: + const: adi,adin1110 + then: + properties: + compatible: + const: ethernet-phy-id0283.bc91 + else: + properties: + compatible: + const: ethernet-phy-id0283.bca1 + + required: + - compatible + - reg + +required: + - compatible + - reg + - interrupts + - phy@0 + +unevaluatedProperties: false + +examples: + - | + spi0 { + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + ethernet@0 { + compatible = "adi,adin2111"; + reg = <0>; + spi-max-frequency = <24500000>; + + adi,spi-crc; + + #address-cells = <1>; + #size-cells = <0>; + + interrupt-parent = <&gpio>; + interrupts = <25 2>; + + mac-address = [ ca 2f b7 10 23 63 ]; + + phy@0 { + #phy-cells = <0>; + compatible = "ethernet-phy-id0283.bca1"; + reg = <0>; + }; + + phy@1 { + #phy-cells = <0>; + compatible = "ethernet-phy-id0283.bca1"; + reg = <1>; + }; + }; + };