| Message ID | 20240201151747.7524-7-ansuelsmth@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | net: phy: Introduce PHY Package concept | expand |
On Fri, Feb 02, 2024 at 04:12:53PM +0100, Christian Marangi wrote: > On Fri, Feb 02, 2024 at 08:45:52AM +0100, Krzysztof Kozlowski wrote: > > On 01/02/2024 16:17, 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@gmail.com> > > > --- > > > .../devicetree/bindings/net/qcom,qca807x.yaml | 142 ++++++++++++++++++ > > > > Your bindings header must be squashed here. Headers are not separate > > thing from the bindings. > > > > > 1 file changed, 142 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..1c3692897b02 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/net/qcom,qca807x.yaml > > > @@ -0,0 +1,142 @@ > > > +# 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 > > > > What is "X"? Wildcards are usually not expected. > > > > It's to identify the Ethrnet PHY family. Looks wrong to declare qca8072 > or qca8074 since they would refer to a more generic Family of devices. Declare them all or provide some justification such as the exact model is discoverable (and better be sure power on is the same in order to do discovery). > What would be the correct way? We have many other case on net with > schema called qca8k that refer to the family of Ethernet Switch but in > it refer to qca8327 qca8337 qca8334... > > > > + > > > +maintainers: > > > + - Christian Marangi <ansuelsmth@gmail.com> > > > + - Robert Marko <robert.marko@sartura.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. > > > + > > > + 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# > > > + > > > +properties: > > > + compatible: > > > + const: qcom,qca807x-package > > > + > > > + qcom,package-mode: > > > > Where is definition of this property with type and description? > > > > > + enum: > > > + - qsgmii > > > + - psgmii > > > + > > > + qcom,tx-driver-strength: > > > > Use proper unit suffix. > > > > https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml > > > > > + description: set the TX Amplifier value in mv. > > > + If not defined, 600mw is set by default. > > > + $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]+)?$: > > > + $ref: ethernet-phy.yaml# > > > + > > > + properties: > > > + gpio-controller: > > > + description: set the output lines as GPIO instead of LEDs > > > + type: boolean You only need 'gpio-controller: true'. The core already defines the type. > > > + > > > + '#gpio-cells': > > > + description: number of GPIO cells for the PHY Not a useful description. Normally, you'd describe what's in the cells, but GPIO is standardized so no need (unless you are deviating from the norm). > > > + const: 2 > > > + > > > + dependencies: > > > + gpio-controller: ['#gpio-cells'] > > > > Why do you need it? None of gpio-controllers do it, I think. > > > > Well gpio-controller property is optional and having that declared and > #gpio-cells skipped will result in an error on probe. I think it should > be the opposite with other schema having to declare this. If you think everything else is wrong, then please fix them. :) > > In usual way for gpio-controller both property are defined as required > but you can see some (to-be-converted) txt files whith comments where > they say: > > ./mux/adi,adgs1408.txt:10:- gpio-controller : if present, #gpio-cells is required. > > Should I instead add this condition in the if right below? The core schema enforces this dependency, so you don't need to. Rob
On Thu, Feb 01, 2024 at 04:17:32PM +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@gmail.com> > --- > .../devicetree/bindings/net/qcom,qca807x.yaml | 142 ++++++++++++++++++ > 1 file changed, 142 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..1c3692897b02 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/qcom,qca807x.yaml > @@ -0,0 +1,142 @@ > +# 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@gmail.com> > + - Robert Marko <robert.marko@sartura.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. > + > + 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# > + > +properties: > + compatible: > + const: qcom,qca807x-package > + > + qcom,package-mode: > + enum: > + - qsgmii > + - psgmii > + > + qcom,tx-driver-strength: > + description: set the TX Amplifier value in mv. > + If not defined, 600mw is set by default. > + $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 don't get how an address is optional. Rob
On Fri, Feb 02, 2024 at 02:39:15PM -0600, Rob Herring wrote: > On Fri, Feb 02, 2024 at 04:12:53PM +0100, Christian Marangi wrote: > > On Fri, Feb 02, 2024 at 08:45:52AM +0100, Krzysztof Kozlowski wrote: > > > On 01/02/2024 16:17, 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@gmail.com> > > > > --- > > > > .../devicetree/bindings/net/qcom,qca807x.yaml | 142 ++++++++++++++++++ > > > > > > Your bindings header must be squashed here. Headers are not separate > > > thing from the bindings. > > > > > > > 1 file changed, 142 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..1c3692897b02 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/net/qcom,qca807x.yaml > > > > @@ -0,0 +1,142 @@ > > > > +# 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 > > > > > > What is "X"? Wildcards are usually not expected. > > > > > > > It's to identify the Ethrnet PHY family. Looks wrong to declare qca8072 > > or qca8074 since they would refer to a more generic Family of devices. > > Declare them all or provide some justification such as the exact model > is discoverable (and better be sure power on is the same in order to do > discovery). > > > What would be the correct way? We have many other case on net with > > schema called qca8k that refer to the family of Ethernet Switch but in > > it refer to qca8327 qca8337 qca8334... > > > > > > + > > > > +maintainers: > > > > + - Christian Marangi <ansuelsmth@gmail.com> > > > > + - Robert Marko <robert.marko@sartura.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. > > > > + > > > > + 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# > > > > + > > > > +properties: > > > > + compatible: > > > > + const: qcom,qca807x-package > > > > + > > > > + qcom,package-mode: > > > > > > Where is definition of this property with type and description? > > > > > > > + enum: > > > > + - qsgmii > > > > + - psgmii > > > > + > > > > + qcom,tx-driver-strength: > > > > > > Use proper unit suffix. > > > > > > https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml > > > > > > > + description: set the TX Amplifier value in mv. > > > > + If not defined, 600mw is set by default. > > > > + $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]+)?$: > > > > + $ref: ethernet-phy.yaml# > > > > + > > > > + properties: > > > > + gpio-controller: > > > > + description: set the output lines as GPIO instead of LEDs > > > > + type: boolean > > You only need 'gpio-controller: true'. The core already defines the > type. > > > > > + > > > > + '#gpio-cells': > > > > + description: number of GPIO cells for the PHY > > Not a useful description. Normally, you'd describe what's in the cells, > but GPIO is standardized so no need (unless you are deviating from the > norm). > > > > > + const: 2 > > > > + > > > > + dependencies: > > > > + gpio-controller: ['#gpio-cells'] > > > > > > Why do you need it? None of gpio-controllers do it, I think. > > > > > > > Well gpio-controller property is optional and having that declared and > > #gpio-cells skipped will result in an error on probe. I think it should > > be the opposite with other schema having to declare this. > > If you think everything else is wrong, then please fix them. :) > > > > > In usual way for gpio-controller both property are defined as required > > but you can see some (to-be-converted) txt files whith comments where > > they say: > > > > ./mux/adi,adgs1408.txt:10:- gpio-controller : if present, #gpio-cells is required. > > > > Should I instead add this condition in the if right below? > > The core schema enforces this dependency, so you don't need to. > Oh! No idea the dependency was already enforced, guess I don't have to fix everything ahahahha
> > +patternProperties: > > + ^ethernet-phy(@[a-f0-9]+)?$: > > I don't get how an address is optional. Its pretty unusual, but for example: arch/arm/boot/dts/nxp/imx/imx6q-novena.dts &fec { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_enet_novena>; phy-mode = "rgmii"; phy-handle = <ðphy>; phy-reset-gpios = <&gpio3 23 GPIO_ACTIVE_LOW>; status = "okay"; mdio { #address-cells = <1>; #size-cells = <0>; ethphy: ethernet-phy { compatible = "ethernet-phy-ieee802.3-c22"; rxc-skew-ps = <3000>; rxdv-skew-ps = <0>; txc-skew-ps = <3000>; txen-skew-ps = <0>; rxd0-skew-ps = <0>; rxd1-skew-ps = <0>; rxd2-skew-ps = <0>; rxd3-skew-ps = <0>; txd0-skew-ps = <3000>; txd1-skew-ps = <3000>; txd2-skew-ps = <3000>; txd3-skew-ps = <3000>; }; }; }; There is no reg property, because its optional. If there is no reg, there is no address. When phylib finds a DT blob like this, it enumerates the bus, and then assigns the nodes to the devices it finds in the order it finds them. Its old behaviour, from before the times of yaml validation, and current best practices, etc. But because it works, it still used in new bindings. Andrew
diff --git a/Documentation/devicetree/bindings/net/qcom,qca807x.yaml b/Documentation/devicetree/bindings/net/qcom,qca807x.yaml new file mode 100644 index 000000000000..1c3692897b02 --- /dev/null +++ b/Documentation/devicetree/bindings/net/qcom,qca807x.yaml @@ -0,0 +1,142 @@ +# 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@gmail.com> + - Robert Marko <robert.marko@sartura.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. + + 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# + +properties: + compatible: + const: qcom,qca807x-package + + qcom,package-mode: + enum: + - qsgmii + - psgmii + + qcom,tx-driver-strength: + description: set the TX Amplifier value in mv. + If not defined, 600mw is set by default. + $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]+)?$: + $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'] + + if: + required: + - gpio-controller + then: + properties: + leds: false + + unevaluatedProperties: false + +required: + - compatible + +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 = "qcom,qca807x-package"; + reg = <0>; + + qcom,package-mode = "qsgmii"; + + 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>; + }; + }; + };
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@gmail.com> --- .../devicetree/bindings/net/qcom,qca807x.yaml | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/qcom,qca807x.yaml