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@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. > > 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@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 > > 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 -- Ansuel
