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@xxxxxxxxx> > > --- > > .../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. 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@xxxxxxxxx> > > + - Robert Marko <robert.marko@xxxxxxxxxx> > > + > > +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 > > + > > + '#gpio-cells': > > + description: number of GPIO cells for the PHY > > + 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. 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? > > + > > + if: > > + required: > > + - gpio-controller > > + then: > > + properties: > > + leds: false > > + > > + unevaluatedProperties: false > > + > > +required: > > + - compatible > > + > > +unevaluatedProperties: false > > > Best regards, > Krzysztof > -- Ansuel
