| Message ID | 20220725165312.59471-4-alexandru.tachici@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: adi: Add ADIN1110 support | expand |
On 25/07/2022 18:53, 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> > --- Thank you for your patch. There is something to discuss/improve. > .../devicetree/bindings/net/adi,adin1110.yaml | 81 +++++++++++++++++++ > 1 file changed, 81 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..cc83f08c0a55 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/adi,adin1110.yaml > @@ -0,0 +1,81 @@ > +# 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 > + > +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. > + > +allOf: > + - $ref: ethernet-controller.yaml# > + > +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 > + You had phy nodes here, but they were not replaced with the phy-handle. No ethernet-ports or mdios with phy? Best regards, Krzysztof
On Mon, 25 Jul 2022 19:53:12 +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 | 81 +++++++++++++++++++ > 1 file changed, 81 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.example.dts:26.17-27: Warning (reg_format): /example-0/spi/ethernet@0:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) Documentation/devicetree/bindings/net/adi,adin1110.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/net/adi,adin1110.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/net/adi,adin1110.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/net/adi,adin1110.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/net/adi,adin1110.example.dts:23.13-39.11: Warning (spi_bus_bridge): /example-0/spi: incorrect #address-cells for SPI bus Documentation/devicetree/bindings/net/adi,adin1110.example.dts:23.13-39.11: Warning (spi_bus_bridge): /example-0/spi: incorrect #size-cells for SPI bus Documentation/devicetree/bindings/net/adi,adin1110.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/net/adi,adin1110.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'spi_bus_bridge' Documentation/devicetree/bindings/net/adi,adin1110.example.dts:24.24-38.15: Warning (avoid_default_addr_size): /example-0/spi/ethernet@0: Relying on default #address-cells value Documentation/devicetree/bindings/net/adi,adin1110.example.dts:24.24-38.15: Warning (avoid_default_addr_size): /example-0/spi/ethernet@0: Relying on default #size-cells value Documentation/devicetree/bindings/net/adi,adin1110.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size' /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/adi,adin1110.example.dtb: ethernet@0: Unevaluated properties are not allowed ('spi-max-frequency' was unexpected) 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.
> You had phy nodes here, but they were not replaced with the phy-handle. > No ethernet-ports or mdios with phy? Since this is integrated silicon, much of that is not required. There is a fixed relationship between the MAC and the PHY, so phy-handle is pointless. There is no need to describe the MDIO bus, because nothing can change. phy-mode is pointless, since it can only be internal. ethernet-ports might be useful, if you want to use two different MAC addresses. However, with Ethernet switches, you generally use the same MAC address on all ports. So i don't see a need for any of these properties. Andrew
On 25/07/2022 21:52, Andrew Lunn wrote: >> You had phy nodes here, but they were not replaced with the phy-handle. >> No ethernet-ports or mdios with phy? > > Since this is integrated silicon, much of that is not required. There > is a fixed relationship between the MAC and the PHY, so phy-handle is > pointless. There is no need to describe the MDIO bus, because nothing > can change. phy-mode is pointless, since it can only be internal. > > ethernet-ports might be useful, if you want to use two different MAC > addresses. However, with Ethernet switches, you generally use the same > MAC address on all ports. > > So i don't see a need for any of these properties. Thanks for clarification, appreciated! Best regards, Krzysztof
On 25/07/2022 18:53, 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 | 81 +++++++++++++++++++ > 1 file changed, 81 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..cc83f08c0a55 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/adi,adin1110.yaml > @@ -0,0 +1,81 @@ > +# 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 > + > +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. > + > +allOf: > + - $ref: ethernet-controller.yaml# > + > +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 > + > +required: > + - compatible > + - reg > + - interrupts > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + > + spi { > + ethernet@0 { > + compatible = "adi,adin2111"; > + reg = <0>; > + spi-max-frequency = <24500000>; Rob's bot complained about this. As a SPI slave device, this should also reference spi-peripheral-props.yaml Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/net/adi,adin1110.yaml b/Documentation/devicetree/bindings/net/adi,adin1110.yaml new file mode 100644 index 000000000000..cc83f08c0a55 --- /dev/null +++ b/Documentation/devicetree/bindings/net/adi,adin1110.yaml @@ -0,0 +1,81 @@ +# 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 + +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. + +allOf: + - $ref: ethernet-controller.yaml# + +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 + +required: + - compatible + - reg + - interrupts + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + spi { + 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 IRQ_TYPE_LEVEL_LOW>; + + local-mac-address = [ 00 11 22 33 44 55 ]; + }; + };