Re: [PATCH 16/16] dt-bindings: net: add bindings for ADIN PHY driver

["Ardelean, Alexandru" <alexandru.Ardelean@analog.com>]

On Mon, 2019-08-05 at 16:11 +0200, Andrew Lunn wrote:
> [External]
> 
> > +  adi,rx-internal-delay:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      RGMII RX Clock Delay used only when PHY operates in RGMII mode (phy-mode
> > +      is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
> > +      default value is 0 (which represents 2 ns)
> > +    enum: [ 0, 1, 2, 6, 7 ]
> 
> We want these numbers to be in ns. So the default value would actually
> be 2. The driver needs to convert the number in DT to a value to poke
> into a PHY register. Please rename the property adi,rx-internal-delay-ns.

ack;
also, good point about ns units and PHY driver to convert it to reg values;

> 
> > +
> > +  adi,tx-internal-delay:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      RGMII TX Clock Delay used only when PHY operates in RGMII mode (phy-mode
> > +      is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
> > +      default value is 0 (which represents 2 ns)
> > +    enum: [ 0, 1, 2, 6, 7 ]
> 
> Same here.
> 
> > +
> > +  adi,fifo-depth:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      When operating in RMII mode, this option configures the FIFO depth.
> > +      See `dt-bindings/net/adin.h`.
> > +    enum: [ 0, 1, 2, 3, 4, 5 ]
> 
> Units? You should probably rename this adi,fifo-depth-bits and list
> the valid values in bits.

units are bits;
will adapt this

> 
> > +
> > +  adi,eee-enabled:
> > +    description: |
> > +      Advertise EEE capabilities on power-up/init (default disabled)
> > +    type: boolean
> 
> It is not the PHY which decides this. The MAC indicates if it is EEE
> capable to phylib. phylib looks into the PHY registers to determine if
> the PHY supports EEE. phylib will then enable EEE
> advertisement. Please remove this, and ensure EEE is disabled by
> default.

ack;
will remove

> 
> 	Andrew