Re: [PATCH v3 01/16] dt-bindings: net: Add YAML schemas for the generic Ethernet options

[Maxime Ripard <maxime.ripard@bootlin.com>]

[-- Attachment #1: Type: text/plain, Size: 2740 bytes --]

Hi Andrew,

On Wed, Jun 19, 2019 at 04:03:14PM +0200, Andrew Lunn wrote:
> > +  phy-connection-type:
> > +    description:
> > +      Operation mode of the PHY interface
> > +    enum:
> > +      # There is not a standard bus between the MAC and the PHY,
> > +      # something proprietary is being used to embed the PHY in the
> > +      # MAC.
>
> ...
>
> > +
> > +  phy-mode:
> > +    $ref: "#/properties/phy-connection-type"
> > +    deprecated: true
>
> I don't think phy-mode is actually deprecated. ethernet.txt actually says:
>
> "This is now a de-facto standard property;" and no mentions that is
> should not be used. Looking at actual device trees, phy-mode is by far
> more popular than phy-connection-type.

Looking at the phy-connection-type documentation, I was under this
impression, sorry.

I'll drop it then.

> fwnode_get_phy_mode() first looks for phy-mode and only falls back to
> phy-connection-type if it is not present. The same is true for
> of_get_phy_mode().
>
> > +  fixed-link:
> > +    allOf:
> > +      - if:
> > +          type: array
> > +        then:
> > +          minItems: 1
> > +          maxItems: 1
> > +          items:
> > +            items:
> > +              - minimum: 0
> > +                maximum: 31
> > +                description:
> > +                  Emulated PHY ID, choose any but unique to the all
> > +                  specified fixed-links
> > +
> > +              - enum: [0, 1]
> > +                description:
> > +                  Duplex configuration. 0 for half duplex or 1 for
> > +                  full duplex
> > +
> > +              - enum: [10, 100, 1000]
> > +                description:
> > +                  Link speed in Mbits/sec.
> > +
> > +              - enum: [0, 1]
> > +                description:
> > +                  Pause configuration. 0 for no pause, 1 for pause
> > +
> > +              - enum: [0, 1]
> > +                description:
> > +                  Asymmetric pause configuration. 0 for no asymmetric
> > +                  pause, 1 for asymmetric pause
> > +
>
> This array of 5 values format should be marked as deprecated.

Right, I'll add it.

> > +
> > +      - if:
> > +          type: object
> > +        then:
> > +          properties:
> > +            speed:
> > +              allOf:
> > +                - $ref: /schemas/types.yaml#definitions/uint32
> > +                - enum: [10, 100, 1000]
>
> This recently changed, depending on context. If PHYLINK is being used,
> any speed is allowed. If phylib is used, then only these speeds are
> allowed. And we are starting to see some speeds other than listed
> here.

phylink seems to be described in a separate binding document, maybe we
can adjust that later?

Maxime