Subject: Re: [net-next RFC PATCH v5 4/4] dt-bindings: Document bindings for Marvell Aquantia PHY

From: Christian Marangi <[email-masked]>
Date: Thu, 9 Nov 2023 11:38:28 +0100

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 <[email-masked]>
> > ---
> > 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 <[email-masked]>
> > +
> > +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.

-- 
	Ansuel