Subject: Re: [PATCH v2 net-next 5/6] dt-bindings: net: add generic ethernet-switch-port binding

From: Rob Herring <[email-masked]>
Date: Fri, 4 Nov 2022 13:50:28 -0500

On Thu, Nov 03, 2022 at 09:52:03PM -0700, Colin Foster wrote:
> The dsa-port.yaml binding had several references that can be common to all
> ethernet ports, not just dsa-specific ones. Break out the generic bindings
> to ethernet-switch-port.yaml they can be used by non-dsa drivers.
> 
> Signed-off-by: Colin Foster <[email-masked]>
> Suggested-by: Vladimir Oltean <[email-masked]>
> ---
> 
> v1 -> v2
>   * Remove accidental addition of
>     "$ref: /schemas/net/ethernet-switch-port.yaml" which should be kept
>     out of dsa-port so that it doesn't get referenced multiple times
>     through both ethernet-switch and dsa-port.
> 
> ---
>  .../devicetree/bindings/net/dsa/dsa-port.yaml | 27 +-----------
>  .../bindings/net/ethernet-switch-port.yaml    | 44 +++++++++++++++++++
>  .../bindings/net/ethernet-switch.yaml         |  4 +-
>  MAINTAINERS                                   |  1 +
>  4 files changed, 49 insertions(+), 27 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/ethernet-switch-port.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> index 10ad7e71097b..d97fb87cccb0 100644
> --- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/net/dsa/dsa-port.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Ethernet Switch port Device Tree Bindings
> +title: DSA Switch port Device Tree Bindings
>  
>  maintainers:
>    - Andrew Lunn <[email-masked]>
> @@ -14,13 +14,7 @@ maintainers:
>  description:
>    Ethernet switch port Description
>  
> -allOf:
> -  - $ref: /schemas/net/ethernet-controller.yaml#
> -
>  properties:
> -  reg:
> -    description: Port number
> -
>    label:
>      description:
>        Describes the label associated with this port, which will become
> @@ -57,25 +51,6 @@ properties:
>        - rtl8_4t
>        - seville
>  
> -  phy-handle: true
> -
> -  phy-mode: true
> -
> -  fixed-link: true
> -
> -  mac-address: true
> -
> -  sfp: true
> -
> -  managed: true
> -
> -  rx-internal-delay-ps: true
> -
> -  tx-internal-delay-ps: true
> -
> -required:
> -  - reg
> -
>  # CPU and DSA ports must have phylink-compatible link descriptions
>  if:
>    oneOf:
> diff --git a/Documentation/devicetree/bindings/net/ethernet-switch-port.yaml b/Documentation/devicetree/bindings/net/ethernet-switch-port.yaml
> new file mode 100644
> index 000000000000..cb1e5e12bf0a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ethernet-switch-port.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/ethernet-switch-port.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ethernet Switch port Device Tree Bindings
> +
> +maintainers:
> +  - Andrew Lunn <[email-masked]>
> +  - Florian Fainelli <[email-masked]>
> +  - Vivien Didelot <[email-masked]>
> +
> +description:
> +  Ethernet switch port Description
> +
> +$ref: ethernet-controller.yaml#
> +
> +properties:
> +  reg:
> +    description: Port number
> +
> +  phy-handle: true
> +
> +  phy-mode: true
> +
> +  fixed-link: true
> +
> +  mac-address: true
> +
> +  sfp: true
> +
> +  managed: true
> +
> +  rx-internal-delay-ps: true
> +
> +  tx-internal-delay-ps: true

I know this is just copied, but these have no effect on validation. I 
assume what they are meant to be is these are the subset of 
ethernet-controller.yaml which are allowed, but that would only work 
with 'additionalProperties: false'. That wouldn't work because we also 
want to users to extend this with custom properties. What's needed here 
is a list of properties not allowed:

disallowed-prop: false

Or we can just allow anything from ethernet-controller.yaml and drop 
this list.

> +
> +required:
> +  - reg
> +
> +additionalProperties: true
> +
> +...