| Message ID | 20231023155013.512999-2-romain.gantois@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ipqess: introduce Qualcomm IPQESS driver | expand |
On 23/10/2023 17:50, Romain Gantois wrote: > Add the DT binding for the IPQESS Ethernet switch subsystem, that > integrates a modified QCA8K switch and an EDMA MAC controller. It inherits > from a basic ethernet switch binding and adds three regmaps, a phandle and > reset line for the PSGMII, a phandle to the MDIO bus, a clock, and 32 > interrupts. > > Signed-off-by: Romain Gantois <romain.gantois@bootlin.com> > --- > .../bindings/net/qcom,ipq4019-ess.yaml | 152 ++++++++++++++++++ > 1 file changed, 152 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/qcom,ipq4019-ess.yaml > > diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-ess.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-ess.yaml > new file mode 100644 > index 000000000000..9bb6b010ea6a > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-ess.yaml > @@ -0,0 +1,152 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/qcom,ipq4019-ess.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm IPQ4019 Ethernet switch subsystem driver Bindings should be about hardware. Please drop "driver". "Subsystem" also sounds like Linuxism. > + > +maintainers: > + - Romain Gantois <romain.gantois@bootlin.com> > + > +$ref: ethernet-switch.yaml# > + > +properties: > + compatible: > + const: qca,ipq4019-qca8337n What do you want to express here? ipq4019 is not qca. This is Qualcomm (so qcom) SoC. > + > + reg: > + maxItems: 3 > + description: Base ESS registers, PSGMII registers and EDMA registers You need to describe the items, so: items: - description: foo - description: foo - description: foo > + > + reg-names: > + maxItems: 3 You need to list items instead. > + > + resets: > + maxItems: 2 > + description: Handles to the PSGMII and ESS reset lines You need to list items instead. > + > + reset-names: > + maxItems: 2 You need to list items instead. > + > + clocks: > + maxItems: 1 > + description: Handle to the GCC ESS clock > + > + clock-names: > + maxItems: 1 Drop clock-names, useless for one entry. > + > + psgmii-ethphy: > + maxItems: 1 > + description: Handle to the MDIO bus node corresponding to the PSGMII That's a bit odd property. Where is it defined? > + > + mdio: > + maxItems: 1 > + description: Handle to the IPQ4019 MDIO Controller > + > + interrupts: > + maxItems: 32 > + description: One interrupt per tx and rx queue, the first 16 are rx queues > + and the last 16 are the tx queues > + > +required: > + - compatible > + - reg > + - reg-names > + - resets > + - reset-names > + - clocks > + - clock-names > + - mdio > + - interrupts > + > +unevaluatedProperties: false Best regards, Krzysztof
On Mon, 23 Oct 2023 17:50:08 +0200, Romain Gantois wrote: > Add the DT binding for the IPQESS Ethernet switch subsystem, that > integrates a modified QCA8K switch and an EDMA MAC controller. It inherits > from a basic ethernet switch binding and adds three regmaps, a phandle and > reset line for the PSGMII, a phandle to the MDIO bus, a clock, and 32 > interrupts. > > Signed-off-by: Romain Gantois <romain.gantois@bootlin.com> > --- > .../bindings/net/qcom,ipq4019-ess.yaml | 152 ++++++++++++++++++ > 1 file changed, 152 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/qcom,ipq4019-ess.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ipq4019-ess.yaml: psgmii-ethphy: missing type definition doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231023155013.512999-2-romain.gantois@bootlin.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Hello Krzystof, On Mon, 23 Oct 2023, Krzysztof Kozlowski wrote: [...] > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Qualcomm IPQ4019 Ethernet switch subsystem driver > > Bindings should be about hardware. Please drop "driver". "Subsystem" > also sounds like Linuxism. The "driver" part was a mistake, I will drop it. However, the "subsystem" terminology seems relevant here, as the SoC documentation refers to this IP group as the "Ethernet Switch Subsystem". If it's okay with you, I'll change this to "Qualcomm IPQ4019 Ethernet Switch Subsystem (ESS)" in the v2. > > +properties: > > + compatible: > > + const: qca,ipq4019-qca8337n > > > What do you want to express here? ipq4019 is not qca. This is Qualcomm > (so qcom) SoC. Agreed, I'll change the compatible. Thanks, Romain
Hello Rob, On Mon, 23 Oct 2023, Rob Herring wrote: > pip3 install dtschema --upgrade > > Please check and re-submit after running the above command yourself. Note > that DT_SCHEMA_FILES can be set to your schema file to speed up checking > your schema. However, it must be unset to test all examples with your schema. > > Even after upgrading dtschema to 2023.9, installing yamllint 1.32.0 and running without DT_SCHEMA_FILES, I can't seem to reproduce this error. I've also tried rebasing on v6.5-rc1 which didn't show it either. However, It seems like the error is related to the psgmii-ethphy node which I'm planning on removing anyway. Thanks, Romain
On 24/10/2023 11:54, Romain Gantois wrote: > Hello Rob, > > On Mon, 23 Oct 2023, Rob Herring wrote: > >> pip3 install dtschema --upgrade >> >> Please check and re-submit after running the above command yourself. Note >> that DT_SCHEMA_FILES can be set to your schema file to speed up checking >> your schema. However, it must be unset to test all examples with your schema. >> >> > > Even after upgrading dtschema to 2023.9, installing yamllint 1.32.0 and running > without DT_SCHEMA_FILES, I can't seem to reproduce this error. I've also tried > rebasing on v6.5-rc1 which didn't show it either. However, It seems like v6.5-rc1 is some ancient version, so how can you rebase on top of it? Which commit this is based on? Best regards, Krzysztof
On Tue, 24 Oct 2023, Krzysztof Kozlowski wrote: > On 24/10/2023 11:54, Romain Gantois wrote: > > Hello Rob, > > > > On Mon, 23 Oct 2023, Rob Herring wrote: > > > >> pip3 install dtschema --upgrade > >> > >> Please check and re-submit after running the above command yourself. Note > >> that DT_SCHEMA_FILES can be set to your schema file to speed up checking > >> your schema. However, it must be unset to test all examples with your schema. > >> > >> > > > > Even after upgrading dtschema to 2023.9, installing yamllint 1.32.0 and running > > without DT_SCHEMA_FILES, I can't seem to reproduce this error. I've also tried > > rebasing on v6.5-rc1 which didn't show it either. However, It seems like > > v6.5-rc1 is some ancient version, so how can you rebase on top of it? I just cherry-picked this patch series on v6.5-rc1. I also tried v6.6-rc1. Since Rob mentionned basing his series on rc1 in his last message, I inferred that he compiled the dtb checks on the last kernel rc1, but maybe I misunderstood what he meant. > > Which commit this is based on? This patch series was based on: 6e7ce2d71bb9 net: lan966x: remove useless code in lan966x_xtr_irq_handler which was the latest commit in net-next/main at the time. Essentially, the patch series is meant to be based on net-next. Best Regards, Romain
On 24/10/2023 12:05, Romain Gantois wrote: > On Tue, 24 Oct 2023, Krzysztof Kozlowski wrote: > >> On 24/10/2023 11:54, Romain Gantois wrote: >>> Hello Rob, >>> >>> On Mon, 23 Oct 2023, Rob Herring wrote: >>> >>>> pip3 install dtschema --upgrade >>>> >>>> Please check and re-submit after running the above command yourself. Note >>>> that DT_SCHEMA_FILES can be set to your schema file to speed up checking >>>> your schema. However, it must be unset to test all examples with your schema. >>>> >>>> >>> >>> Even after upgrading dtschema to 2023.9, installing yamllint 1.32.0 and running >>> without DT_SCHEMA_FILES, I can't seem to reproduce this error. I've also tried >>> rebasing on v6.5-rc1 which didn't show it either. However, It seems like >> >> v6.5-rc1 is some ancient version, so how can you rebase on top of it? > I just cherry-picked this patch series on v6.5-rc1. I also tried v6.6-rc1. Since > Rob mentionned basing his series on rc1 in his last message, I inferred that he > compiled the dtb checks on the last kernel rc1, but maybe I misunderstood what > he meant. > >> >> Which commit this is based on? > > This patch series was based on: > > 6e7ce2d71bb9 net: lan966x: remove useless code in lan966x_xtr_irq_handler > > which was the latest commit in net-next/main at the time. Essentially, the patch > series is meant to be based on net-next. > Ah, ok. Rob's bot might be using not-yet-released dtschema from main branch, thus the error. However the error is true: you added a custom field without type. That's why I asked: where is it defined? Best regards, Krzysztof
On Tue, 24 Oct 2023, Krzysztof Kozlowski wrote: > Rob's bot might be using not-yet-released dtschema from main branch, > thus the error. However the error is true: you added a custom field > without type. That's why I asked: where is it defined? > I didn't define it anywhere, that's an oversight on my part. The psgmii_ethphy property is a handle to an MDIO device, which I thought was integrated to the PSGMII bus in the IPQ4019. However, I just learned from Robert Marko that this MDIO device corresponds to a SoC-facing PHY integrated in the external QCA807x IP. Therefore, I'm not convinced that this MDIO device should be handled by the ESS driver. I'm going to have to consider refactoring the psgmii_ethphy handling out of the IPQESS driver, which would make this device tree property unnecessary. Best Regards, Romain
diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-ess.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-ess.yaml new file mode 100644 index 000000000000..9bb6b010ea6a --- /dev/null +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-ess.yaml @@ -0,0 +1,152 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/qcom,ipq4019-ess.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm IPQ4019 Ethernet switch subsystem driver + +maintainers: + - Romain Gantois <romain.gantois@bootlin.com> + +$ref: ethernet-switch.yaml# + +properties: + compatible: + const: qca,ipq4019-qca8337n + + reg: + maxItems: 3 + description: Base ESS registers, PSGMII registers and EDMA registers + + reg-names: + maxItems: 3 + + resets: + maxItems: 2 + description: Handles to the PSGMII and ESS reset lines + + reset-names: + maxItems: 2 + + clocks: + maxItems: 1 + description: Handle to the GCC ESS clock + + clock-names: + maxItems: 1 + + psgmii-ethphy: + maxItems: 1 + description: Handle to the MDIO bus node corresponding to the PSGMII + + mdio: + maxItems: 1 + description: Handle to the IPQ4019 MDIO Controller + + interrupts: + maxItems: 32 + description: One interrupt per tx and rx queue, the first 16 are rx queues + and the last 16 are the tx queues + +required: + - compatible + - reg + - reg-names + - resets + - reset-names + - clocks + - clock-names + - mdio + - interrupts + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/qcom,gcc-ipq4019.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + switch: switch@c000000 { + compatible = "qca,ipq4019-qca8337n"; + reg = <0xc000000 0x80000>, <0x98000 0x800>, <0xc080000 0x80000>; + reg-names = "base", "psgmii_phy", "edma"; + resets = <&gcc ESS_PSGMII_ARES>, <&gcc ESS_RESET>; + reset-names = "psgmii_rst", "ess"; + clocks = <&gcc GCC_ESS_CLK>; + clock-names = "ess"; + mdio = <&mdio>; + interrupts = <GIC_SPI 65 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 66 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 67 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 68 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 69 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 70 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 71 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 72 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 73 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 74 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 76 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 77 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 78 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 79 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 80 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 240 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 241 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 242 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 243 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 244 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 245 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 246 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 247 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 248 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 249 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 250 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 251 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 252 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 253 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 254 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 255 IRQ_TYPE_EDGE_RISING>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + swport1: port@1 { /* MAC1 */ + reg = <1>; + label = "lan1"; + phy-handle = <ðphy0>; + phy-mode = "psgmii"; + }; + + swport2: port@2 { /* MAC2 */ + reg = <2>; + label = "lan2"; + phy-handle = <ðphy1>; + phy-mode = "psgmii"; + }; + + swport3: port@3 { /* MAC3 */ + reg = <3>; + label = "lan3"; + phy-handle = <ðphy2>; + phy-mode = "psgmii"; + }; + + swport4: port@4 { /* MAC4 */ + reg = <4>; + label = "lan4"; + phy-handle = <ðphy3>; + phy-mode = "psgmii"; + }; + + swport5: port@5 { /* MAC5 */ + reg = <5>; + label = "wan"; + phy-handle = <ðphy4>; + phy-mode = "psgmii"; + }; + }; + }; + +...
Add the DT binding for the IPQESS Ethernet switch subsystem, that integrates a modified QCA8K switch and an EDMA MAC controller. It inherits from a basic ethernet switch binding and adds three regmaps, a phandle and reset line for the PSGMII, a phandle to the MDIO bus, a clock, and 32 interrupts. Signed-off-by: Romain Gantois <romain.gantois@bootlin.com> --- .../bindings/net/qcom,ipq4019-ess.yaml | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/qcom,ipq4019-ess.yaml