Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 26 Aug 2022 00:27:51 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     Oleksij Rempel <o.rempel@pengutronix.de>
Cc:     Heiner Kallweit <hkallweit1@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Russell King <linux@armlinux.org.uk>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Jonathan Corbet <corbet@lwn.net>, kernel@pengutronix.de,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
        David Jander <david@protonic.nl>,
        Luka Perkov <luka.perkov@sartura.hr>,
        Robert Marko <robert.marko@sartura.hr>
Subject: Re: [PATCH net-next v2 1/7] dt-bindings: net: pse-dt: add bindings
 for generic PSE controller
Message-ID: <Ywf3Z+1VFy/2+P78@lunn.ch>
References: <20220825130211.3730461-1-o.rempel@pengutronix.de>
 <20220825130211.3730461-2-o.rempel@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220825130211.3730461-2-o.rempel@pengutronix.de>
Precedence: bulk

> +  ieee802.3-pairs:
> +    $ref: /schemas/types.yaml#/definitions/int8-array
> +    description: Array of number of twisted-pairs capable to deliver power.
> +      Since not all circuits are able to support all pair variants, the array of
> +      supported variants should be specified.
> +      Note - single twisted-pair PSE is formally know as PoDL PSE.
> +    items:
> +      enum: [1, 2, 4]

It is not clear to me what you are describing here. It looks like the
number of pairs? That does not seem like a hardware property. The
controller itself should be able to tell you how many pairs it can
feed.

A hardware property would be which pairs of the socket are connected
to a PSE and so can be used to deliver power. But i'm not sure how
that would be useful to know. I suppose a controller capable of
powering 4 pair, but connected to a socket only wired to supply 2, can
then disable 2 pairs?

> +
> +  ieee802.3-pse-type:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    minimum: 1
> +    maximum: 2
> +    description: PSE Type. Describes classification- and class-capabilities.
> +      Not compatible with PoDL PSE Type.
> +      Type 1 - provides a Class 0, 1, 2, or 3 signature during Physical Layer
> +      classification.
> +      Type 2 - provides a Class 4 signature during Physical Layer
> +      classification, understands 2-Event classification, and is capable of
> +      Data Link Layer classification.

Again, the controller should know what class it can support. Why do we
need to specify it?  What could make sense is we want to limit the
controller to a specific type? 

> +  ieee802.3-podl-pse-class:
> +    $ref: /schemas/types.yaml#/definitions/int8-array
> +    items:
> +      enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
> +    description: PoDL PSE Class. Array of supported classes by the
> +      single twisted-pair PoDL PSE.

Why? I could understand we might want to limit the higher classes,
because the board is not designed for them, but the controller on the
board can actually offer them. But if it tries to use them, the board
will melt/blow a fuse.

So i'm wondering if it should actually be something like:

> +  ieee802.3-podl-limit-pse-classes:
> +    $ref: /schemas/types.yaml#/definitions/int8-array
> +    items:
> +      enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
> +    description: PoDL PSE Class. Limit the PoDL PSE to only these classes,
         due to hardware design limitations. If not specified, the PoDL PSE
	 will offer all classes its supports.

Remember, DT describes the hardware, not software configuration of the
hardware.

	Andrew