Re: [PATCH v6 net-next 01/10] dt-bindings: net: dsa: dt bindings for microchip lan937x

[Vladimir Oltean <olteanv@gmail.com>]

On Fri, Oct 29, 2021 at 10:52:47AM +0530, Prasanna Vengateshan wrote:
> Documentation in .yaml format and updates to the MAINTAINERS
> Also 'make dt_binding_check' is passed.
> 
> Introduced rx-internal-delay-ps & tx-internal-delay-ps for RGMII
> internal delay along with min/max values. This is to address the
> Vladimir proposal from the previous revision and mdio details
> are added as suggested by Rob.

This sort of meta-information does not really belong in a commit
message, there is...

> 
> Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
> ---

...this area for it. People of the future won't have any idea what is
the previous revision or who Vladimir is.

>  .../bindings/net/dsa/microchip,lan937x.yaml   | 180 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  2 files changed, 181 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
> new file mode 100644
> index 000000000000..0bc16894c8c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
> @@ -0,0 +1,180 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/microchip,lan937x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LAN937x Ethernet Switch Series Tree Bindings
> +
> +maintainers:
> +  - UNGLinuxDriver@microchip.com
> +
> +allOf:
> +  - $ref: dsa.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,lan9370
> +      - microchip,lan9371
> +      - microchip,lan9372
> +      - microchip,lan9373
> +      - microchip,lan9374
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 50000000
> +
> +  reset-gpios:
> +    description: Optional gpio specifier for a reset line
> +    maxItems: 1
> +
> +  mdio:
> +    $ref: /schemas/net/mdio.yaml#
> +    unevaluatedProperties: false
> +
> +patternProperties:
> +  "^(ethernet-)?ports$":
> +    patternProperties:
> +      "^(ethernet-)?port@[0-7]+$":
> +        allOf:
> +          - if:
> +              properties:
> +                phy-mode:
> +                  contains:
> +                    enum:
> +                      - rgmii
> +                      - rgmii-rxid
> +                      - rgmii-txid
> +                      - rgmii-id
> +            then:
> +              properties:
> +                rx-internal-delay-ps:
> +                  $ref: "#/$defs/internal-delay-ps"
> +                tx-internal-delay-ps:
> +                  $ref: "#/$defs/internal-delay-ps"
> +
> +required:
> +  - compatible
> +  - reg
> +
> +$defs:
> +  internal-delay-ps:
> +    description: Delay is in pico seconds
> +    minimum: 2170
> +    maximum: 4000
> +    default: 0

Why is the default value smaller than the minimum? What kind of minimum is that?
If the hardware does not accept the continuous range of values between
2170 and 4000, just enumerate the valid discrete values. It takes 30
seconds to write a bash script. That's one of the reasons why I added a
$def in nxp,sja1105.rst for this in the first place, to avoid
duplicating that enumeration.

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    //Ethernet switch connected via spi to the host
> +    ethernet {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      fixed-link {
> +        speed = <1000>;
> +        full-duplex;
> +      };
> +    };
> +
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      lan9374: switch@0 {
> +        compatible = "microchip,lan9374";
> +        reg = <0>;
> +
> +        spi-max-frequency = <44000000>;
> +
> +        ethernet-ports {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +          port@0 {
> +            reg = <0>;
> +            label = "lan1";
> +            phy-mode = "internal";
> +            phy-handle = <&t1phy0>;
> +          };
> +          port@1 {
> +            reg = <1>;
> +            label = "lan2";
> +            phy-mode = "internal";
> +            phy-handle = <&t1phy1>;
> +          };
> +          port@2 {
> +            reg = <2>;
> +            label = "lan4";
> +            phy-mode = "internal";
> +            phy-handle = <&t1phy2>;
> +          };
> +          port@3 {
> +            reg = <3>;
> +            label = "lan6";
> +            phy-mode = "internal";
> +            phy-handle = <&t1phy3>;
> +          };
> +          port@4 {
> +            reg = <4>;
> +            phy-mode = "rgmii";
> +            ethernet = <&ethernet>;
> +            fixed-link {
> +              speed = <1000>;
> +              full-duplex;
> +            };
> +          };
> +          port@5 {
> +            reg = <5>;
> +            label = "lan7";
> +            phy-mode = "rgmii";
> +            fixed-link {
> +              speed = <1000>;
> +              full-duplex;
> +            };
> +          };
> +          port@6 {
> +            reg = <6>;
> +            label = "lan5";
> +            phy-mode = "internal";
> +            phy-handle = <&t1phy4>;
> +          };
> +          port@7 {
> +            reg = <7>;
> +            label = "lan3";
> +            phy-mode = "internal";
> +            phy-handle = <&t1phy5>;
> +          };
> +        };
> +
> +        mdio {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          t1phy0: ethernet-phy@0{
> +            reg = <0x0>;
> +          };
> +          t1phy1: ethernet-phy@1{
> +            reg = <0x1>;
> +          };
> +          t1phy2: ethernet-phy@2{
> +            reg = <0x2>;
> +          };
> +          t1phy3: ethernet-phy@3{
> +            reg = <0x3>;
> +          };
> +          t1phy4: ethernet-phy@6{
> +            reg = <0x6>;
> +          };
> +          t1phy5: ethernet-phy@7{
> +            reg = <0x7>;
> +          };
> +        };
> +      };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3b85f039fbf9..9dfd8169dcdf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12312,6 +12312,7 @@ M:	UNGLinuxDriver@microchip.com
>  L:	netdev@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> +F:	Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
>  F:	drivers/net/dsa/microchip/*
>  F:	include/linux/platform_data/microchip-ksz.h
>  F:	net/dsa/tag_ksz.c
> -- 
> 2.27.0
>