[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231127223523.GB4023452-robh@kernel.org>
Date: Mon, 27 Nov 2023 16:35:23 -0600
From: Rob Herring <robh@...nel.org>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, Andy Gross <agross@...nel.org>, Bjorn Andersson <andersson@...nel.org>, Konrad Dybcio <konrad.dybcio@...aro.org>, Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>, Matthias Brugger <matthias.bgg@...il.com>, AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Robert Marko <robert.marko@...tura.hr>, netdev@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [net-next PATCH RFC v3 6/8] dt-bindings: net: Document Qcom
QCA807x PHY package
On Sun, Nov 26, 2023 at 02:53:44AM +0100, Christian Marangi wrote:
> Document Qcom QCA807x PHY package.
>
> Qualcomm QCA807X Ethernet PHY is PHY package of 2 or 5
> IEEE 802.3 clause 22 compliant 10BASE-Te, 100BASE-TX and
> 1000BASE-T PHY-s.
>
> Document the required property to make the PHY package correctly
> configure and work.
>
> Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
> ---
> .../devicetree/bindings/net/qcom,qca807x.yaml | 136 ++++++++++++++++++
> 1 file changed, 136 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/qcom,qca807x.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/qcom,qca807x.yaml b/Documentation/devicetree/bindings/net/qcom,qca807x.yaml
> new file mode 100644
> index 000000000000..136ba2128b73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/qcom,qca807x.yaml
> @@ -0,0 +1,136 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/qcom,qca807x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm QCA807X Ethernet PHY
> +
> +maintainers:
> + - Christian Marangi <ansuelsmth@...il.com>
> + - Robert Marko <robert.marko@...tura.hr>
> +
> +description: |
> + Qualcomm QCA807X Ethernet PHY is PHY package of 2 or 5
> + IEEE 802.3 clause 22 compliant 10BASE-Te, 100BASE-TX and
> + 1000BASE-T PHY-s.
> +
> + They feature 2 SerDes, one for PSGMII or QSGMII connection with
> + MAC, while second one is SGMII for connection to MAC or fiber.
> +
> + Both models have a combo port that supports 1000BASE-X and
> + 100BASE-FX fiber.
> +
> + Each PHY inside of QCA807x series has 4 digitally controlled
> + output only pins that natively drive LED-s for up to 2 attached
> + LEDs. Some vendor also use these 4 output for GPIO usage without
> + attaching LEDs.
> +
> + Note that output pins can be set to drive LEDs OR GPIO, mixed
> + definition are not accepted.
This so far is about the only thing that convinces me of having child
nodes.
> +
> + PHY package can be configured in 3 mode following this table:
> +
> + First Serdes mode Second Serdes mode
> + Option 1 PSGMII for copper Disabled
> + ports 0-4
> + Option 2 PSGMII for copper 1000BASE-X / 100BASE-FX
> + ports 0-4
> + Option 3 QSGMII for copper SGMII for
> + ports 0-3 copper port 4
> +
> +$ref: ethernet-phy-package.yaml#
> +
Did you test this binding? No, because there is nothing to select it.
Just put a wrong value for qcom,package-mode and see.
ethernet-phy-package.yaml will be applied, but since it allows any extra
properties not much is tested.
> +properties:
> + qcom,package-mode:
> + enum:
> + - qsgmii
> + - psgmii
description needed.
I don't understand how the 3 modes works with only 2 modes defined here.
> +
> + qcom,tx-driver-strength:
> + description: set the TX Amplifier value in mv.
millivolts or...
> + If not defined, 600mw is set by default.
milliwatts?
Whatever it is, use standard unit suffix.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [140, 160, 180, 200, 220,
> + 240, 260, 280, 300, 320,
> + 400, 500, 600]
> +
> +patternProperties:
> + ^ethernet-phy(@[a-f0-9]+)?$:
I'm confused. The addresses are MDIO, but you have just 0-4. Might be
correct or might not?
In any case, the unit-address should be restricted to '@1?[0-9a-f]$'.
Though that belongs in mdio.yaml and here '^ethernet-phy@' is sufficient
(once there's a reference to mdio.yaml).
> + $ref: ethernet-phy.yaml#
> +
> + properties:
> + gpio-controller:
> + description: set the output lines as GPIO instead of LEDs
> + type: boolean
> +
> + '#gpio-cells':
> + description: number of GPIO cells for the PHY
> + const: 2
> +
> + dependencies:
> + gpio-controller: ['#gpio-cells']
No need for this, the common gpio schema does this.
> +
> + if:
> + required:
> + - gpio-controller
> + then:
> + properties:
> + leds: false
> +
> + unevaluatedProperties: false
Move under $ref.
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/leds/common.h>
> +
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethernet-phy-package@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "ethernet-phy-package";
> + reg = <0>;
> +
> + qcom,package-mode = "qsgmii";
Nothing in here tells me this is a QCA807X other than this property
(sort of). You need a specific compatible.
> +
> + ethernet-phy@0 {
> + reg = <0>;
> +
> + leds {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led@0 {
> + reg = <0>;
> + color = <LED_COLOR_ID_GREEN>;
> + function = LED_FUNCTION_LAN;
> + default-state = "keep";
> + };
> + };
> + };
> +
> + ethernet-phy@1 {
> + reg = <1>;
> + };
> +
> + ethernet-phy@2 {
> + reg = <2>;
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + ethernet-phy@3 {
> + reg = <3>;
> + };
> +
> + ethernet-phy@4 {
> + reg = <4>;
> + };
> + };
> + };
> --
> 2.40.1
>
Powered by blists - more mailing lists