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

Ardelean, Alexandru
2019-08-06T07:03:24Z

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