| Message ID | 20231106165433.2746-4-ansuelsmth@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | None | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Mon, Nov 06, 2023 at 05:54:33PM +0100, Christian Marangi wrote: > Document bindings for Marvell Aquantia PHY. > > The Marvell Aquantia PHY require a firmware to work correctly and there > at least 3 way to load this firmware. > > Describe all the different way and document the binding "firmware-name" > to load the PHY firmware from userspace. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > Changes v5: > - Drop extra entry not related to HW description > Changes v3: > - Make DT description more OS agnostic > - Use custom select to fix dtbs checks > Changes v2: > - Add DT patch > > .../bindings/net/marvell,aquantia.yaml | 123 ++++++++++++++++++ > 1 file changed, 123 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml > > diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml > new file mode 100644 > index 000000000000..7106c5bdf73c > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml > @@ -0,0 +1,123 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/marvell,aquantia.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Marvell Aquantia Ethernet PHY > + > +maintainers: > + - Christian Marangi <ansuelsmth@gmail.com> > + > +description: | > + Marvell Aquantia Ethernet PHY require a firmware to be loaded to actually > + work. > + > + This can be done and is implemented by OEM in 3 different way: > + - Attached SPI flash directly to the PHY with the firmware. The PHY > + will self load the firmware in the presence of this configuration. > + - Dedicated partition on system NAND with firmware in it. NVMEM > + subsystem will be used and the declared NVMEM cell will load > + the firmware to the PHY using the PHY mailbox interface. I'd probably phrase this one as something more like "Read from a dedicated partition on system NAND declared in an NVMEM cell, and loaded to the PHY using its mailbox interface." or something like that - mostly to get rid of the linux specific "NVMEM subsystem" from the description. Otherwise, Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Cheers, Conor. > + - Manually provided firmware loaded from a file in the filesystem. > + > +allOf: > + - $ref: ethernet-phy.yaml# > + > +select: > + properties: > + compatible: > + contains: > + enum: > + - ethernet-phy-id03a1.b445 > + - ethernet-phy-id03a1.b460 > + - ethernet-phy-id03a1.b4a2 > + - ethernet-phy-id03a1.b4d0 > + - ethernet-phy-id03a1.b4e0 > + - ethernet-phy-id03a1.b5c2 > + - ethernet-phy-id03a1.b4b0 > + - ethernet-phy-id03a1.b662 > + - ethernet-phy-id03a1.b712 > + - ethernet-phy-id31c3.1c12 > + required: > + - compatible > + > +properties: > + reg: > + maxItems: 1 > + > + firmware-name: > + description: specify the name of PHY firmware to load > + > + nvmem-cells: > + description: phandle to the firmware nvmem cell > + maxItems: 1 > + > + nvmem-cell-names: > + const: firmware > + > +required: > + - compatible > + - reg > + > +unevaluatedProperties: false > + > +examples: > + - | > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ethernet-phy@0 { > + /* Only needed to make DT lint tools work. Do not copy/paste > + * into real DTS files. > + */ > + compatible = "ethernet-phy-id31c3.1c12", > + "ethernet-phy-ieee802.3-c45"; > + > + reg = <0>; > + firmware-name = "AQR-G4_v5.4.C-AQR_CIG_WF-1945_0x8_ID44776_VER1630.cld"; > + }; > + > + ethernet-phy@1 { > + /* Only needed to make DT lint tools work. Do not copy/paste > + * into real DTS files. > + */ > + compatible = "ethernet-phy-id31c3.1c12", > + "ethernet-phy-ieee802.3-c45"; > + > + reg = <0>; > + nvmem-cells = <&aqr_fw>; > + nvmem-cell-names = "firmware"; > + }; > + }; > + > + flash { > + compatible = "jedec,spi-nor"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + /* ... */ > + > + partition@650000 { > + compatible = "nvmem-cells"; > + label = "0:ethphyfw"; > + reg = <0x650000 0x80000>; > + read-only; > + #address-cells = <1>; > + #size-cells = <1>; > + > + aqr_fw: aqr_fw@0 { > + reg = <0x0 0x5f42a>; > + }; > + }; > + > + /* ... */ > + > + }; > + }; > -- > 2.40.1 >
On Mon, Nov 06, 2023 at 05:54:33PM +0100, Christian Marangi wrote: > Document bindings for Marvell Aquantia PHY. For the subject: dt-bindings: net: Add Marvell Aquantia PHY ('Document bindings' is redundant) > > The Marvell Aquantia PHY require a firmware to work correctly and there > at least 3 way to load this firmware. > > Describe all the different way and document the binding "firmware-name" > to load the PHY firmware from userspace. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > Changes v5: > - Drop extra entry not related to HW description > Changes v3: > - Make DT description more OS agnostic > - Use custom select to fix dtbs checks > Changes v2: > - Add DT patch > > .../bindings/net/marvell,aquantia.yaml | 123 ++++++++++++++++++ > 1 file changed, 123 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml > > diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml > new file mode 100644 > index 000000000000..7106c5bdf73c > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml > @@ -0,0 +1,123 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/marvell,aquantia.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Marvell Aquantia Ethernet PHY > + > +maintainers: > + - Christian Marangi <ansuelsmth@gmail.com> > + > +description: | > + Marvell Aquantia Ethernet PHY require a firmware to be loaded to actually > + work. > + > + This can be done and is implemented by OEM in 3 different way: > + - Attached SPI flash directly to the PHY with the firmware. The PHY > + will self load the firmware in the presence of this configuration. > + - Dedicated partition on system NAND with firmware in it. NVMEM > + subsystem will be used and the declared NVMEM cell will load > + the firmware to the PHY using the PHY mailbox interface. > + - Manually provided firmware loaded from a file in the filesystem. > + > +allOf: > + - $ref: ethernet-phy.yaml# > + > +select: > + properties: > + compatible: > + contains: > + enum: > + - ethernet-phy-id03a1.b445 > + - ethernet-phy-id03a1.b460 > + - ethernet-phy-id03a1.b4a2 > + - ethernet-phy-id03a1.b4d0 > + - ethernet-phy-id03a1.b4e0 > + - ethernet-phy-id03a1.b5c2 > + - ethernet-phy-id03a1.b4b0 > + - ethernet-phy-id03a1.b662 > + - ethernet-phy-id03a1.b712 > + - ethernet-phy-id31c3.1c12 > + required: > + - compatible > + > +properties: > + reg: > + maxItems: 1 > + > + firmware-name: > + description: specify the name of PHY firmware to load > + > + nvmem-cells: > + description: phandle to the firmware nvmem cell > + maxItems: 1 > + > + nvmem-cell-names: > + const: firmware > + > +required: > + - compatible > + - reg > + > +unevaluatedProperties: false > + > +examples: > + - | > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ethernet-phy@0 { > + /* Only needed to make DT lint tools work. Do not copy/paste > + * into real DTS files. > + */ I don't agree with this statement. Pretty sure we've been through this before... If we have a node, we need to define what it is. The way that is done is with compatible. Whether some particular OS implementation (currently) needs compatible or not is irrelevant. It's not about dtschema needing it, that just exposes the issue. These MDIO PHY bindings are all broken because they are never actually applied to anything. Rob
On Tue, Nov 07, 2023 at 01:22:32PM -0600, Rob Herring wrote: > On Mon, Nov 06, 2023 at 05:54:33PM +0100, Christian Marangi wrote: > > Document bindings for Marvell Aquantia PHY. > > For the subject: dt-bindings: net: Add Marvell Aquantia PHY > > ('Document bindings' is redundant) > > > > > The Marvell Aquantia PHY require a firmware to work correctly and there > > at least 3 way to load this firmware. > > > > Describe all the different way and document the binding "firmware-name" > > to load the PHY firmware from userspace. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > Changes v5: > > - Drop extra entry not related to HW description > > Changes v3: > > - Make DT description more OS agnostic > > - Use custom select to fix dtbs checks > > Changes v2: > > - Add DT patch > > > > .../bindings/net/marvell,aquantia.yaml | 123 ++++++++++++++++++ > > 1 file changed, 123 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml > > > > diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml > > new file mode 100644 > > index 000000000000..7106c5bdf73c > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml > > @@ -0,0 +1,123 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/net/marvell,aquantia.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Marvell Aquantia Ethernet PHY > > + > > +maintainers: > > + - Christian Marangi <ansuelsmth@gmail.com> > > + > > +description: | > > + Marvell Aquantia Ethernet PHY require a firmware to be loaded to actually > > + work. > > + > > + This can be done and is implemented by OEM in 3 different way: > > + - Attached SPI flash directly to the PHY with the firmware. The PHY > > + will self load the firmware in the presence of this configuration. > > + - Dedicated partition on system NAND with firmware in it. NVMEM > > + subsystem will be used and the declared NVMEM cell will load > > + the firmware to the PHY using the PHY mailbox interface. > > + - Manually provided firmware loaded from a file in the filesystem. > > + > > +allOf: > > + - $ref: ethernet-phy.yaml# > > + > > +select: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - ethernet-phy-id03a1.b445 > > + - ethernet-phy-id03a1.b460 > > + - ethernet-phy-id03a1.b4a2 > > + - ethernet-phy-id03a1.b4d0 > > + - ethernet-phy-id03a1.b4e0 > > + - ethernet-phy-id03a1.b5c2 > > + - ethernet-phy-id03a1.b4b0 > > + - ethernet-phy-id03a1.b662 > > + - ethernet-phy-id03a1.b712 > > + - ethernet-phy-id31c3.1c12 > > + required: > > + - compatible > > + > > +properties: > > + reg: > > + maxItems: 1 > > + > > + firmware-name: > > + description: specify the name of PHY firmware to load > > + > > + nvmem-cells: > > + description: phandle to the firmware nvmem cell > > + maxItems: 1 > > + > > + nvmem-cell-names: > > + const: firmware > > + > > +required: > > + - compatible > > + - reg > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + mdio { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + ethernet-phy@0 { > > + /* Only needed to make DT lint tools work. Do not copy/paste > > + * into real DTS files. > > + */ > > I don't agree with this statement. Pretty sure we've been through this > before... > > If we have a node, we need to define what it is. The way that is done is > with compatible. Whether some particular OS implementation (currently) > needs compatible or not is irrelevant. It's not about dtschema needing > it, that just exposes the issue. > > These MDIO PHY bindings are all broken because they are never actually > applied to anything. > I will just drop these comments, the additional compatible is redundant as it will be scanned from PHY ID but won't cause any problem. Also the scenario with fw in nvmem cells or in fs is in devices where the PHY is soldered to the device, so describe them in DT is correct.
diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml new file mode 100644 index 000000000000..7106c5bdf73c --- /dev/null +++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml @@ -0,0 +1,123 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/marvell,aquantia.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Marvell Aquantia Ethernet PHY + +maintainers: + - Christian Marangi <ansuelsmth@gmail.com> + +description: | + Marvell Aquantia Ethernet PHY require a firmware to be loaded to actually + work. + + This can be done and is implemented by OEM in 3 different way: + - Attached SPI flash directly to the PHY with the firmware. The PHY + will self load the firmware in the presence of this configuration. + - Dedicated partition on system NAND with firmware in it. NVMEM + subsystem will be used and the declared NVMEM cell will load + the firmware to the PHY using the PHY mailbox interface. + - Manually provided firmware loaded from a file in the filesystem. + +allOf: + - $ref: ethernet-phy.yaml# + +select: + properties: + compatible: + contains: + enum: + - ethernet-phy-id03a1.b445 + - ethernet-phy-id03a1.b460 + - ethernet-phy-id03a1.b4a2 + - ethernet-phy-id03a1.b4d0 + - ethernet-phy-id03a1.b4e0 + - ethernet-phy-id03a1.b5c2 + - ethernet-phy-id03a1.b4b0 + - ethernet-phy-id03a1.b662 + - ethernet-phy-id03a1.b712 + - ethernet-phy-id31c3.1c12 + required: + - compatible + +properties: + reg: + maxItems: 1 + + firmware-name: + description: specify the name of PHY firmware to load + + nvmem-cells: + description: phandle to the firmware nvmem cell + maxItems: 1 + + nvmem-cell-names: + const: firmware + +required: + - compatible + - reg + +unevaluatedProperties: false + +examples: + - | + mdio { + #address-cells = <1>; + #size-cells = <0>; + + ethernet-phy@0 { + /* Only needed to make DT lint tools work. Do not copy/paste + * into real DTS files. + */ + compatible = "ethernet-phy-id31c3.1c12", + "ethernet-phy-ieee802.3-c45"; + + reg = <0>; + firmware-name = "AQR-G4_v5.4.C-AQR_CIG_WF-1945_0x8_ID44776_VER1630.cld"; + }; + + ethernet-phy@1 { + /* Only needed to make DT lint tools work. Do not copy/paste + * into real DTS files. + */ + compatible = "ethernet-phy-id31c3.1c12", + "ethernet-phy-ieee802.3-c45"; + + reg = <0>; + nvmem-cells = <&aqr_fw>; + nvmem-cell-names = "firmware"; + }; + }; + + flash { + compatible = "jedec,spi-nor"; + #address-cells = <1>; + #size-cells = <1>; + + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + /* ... */ + + partition@650000 { + compatible = "nvmem-cells"; + label = "0:ethphyfw"; + reg = <0x650000 0x80000>; + read-only; + #address-cells = <1>; + #size-cells = <1>; + + aqr_fw: aqr_fw@0 { + reg = <0x0 0x5f42a>; + }; + }; + + /* ... */ + + }; + };
Document bindings for Marvell Aquantia PHY. The Marvell Aquantia PHY require a firmware to work correctly and there at least 3 way to load this firmware. Describe all the different way and document the binding "firmware-name" to load the PHY firmware from userspace. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- Changes v5: - Drop extra entry not related to HW description Changes v3: - Make DT description more OS agnostic - Use custom select to fix dtbs checks Changes v2: - Add DT patch .../bindings/net/marvell,aquantia.yaml | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml