Messages in this thread | | | Date | Mon, 12 Sep 2022 22:58:05 +0530 | Subject | Re: [PATCH 1/4] dt-bindings: net: qcom,ethqos: Convert bindings to yaml | From | Bhupesh Sharma <> |
| |
Hi Krzysztof,
Thanks for your comments.
On 9/8/22 8:08 PM, Krzysztof Kozlowski wrote: > On 07/09/2022 22:49, Bhupesh Sharma wrote: >> Convert Qualcomm ETHQOS Ethernet devicetree binding to YAML. >> >> Cc: Bjorn Andersson <andersson@kernel.org> >> Cc: Rob Herring <robh@kernel.org> >> Cc: Vinod Koul <vkoul@kernel.org> >> Cc: David Miller <davem@davemloft.net> >> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > Thank you for your patch. There is something to discuss/improve. > >> --- >> .../devicetree/bindings/net/qcom,ethqos.txt | 66 --------- >> .../devicetree/bindings/net/qcom,ethqos.yaml | 139 ++++++++++++++++++ > > You need to update maintainers - old path.
Sure, my bad. Will do in v2.
>> 2 files changed, 139 insertions(+), 66 deletions(-) >> delete mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.txt >> create mode 100644 Documentation/devicetree/bindings/net/qcom,ethqos.yaml >> >> diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.txt b/Documentation/devicetree/bindings/net/qcom,ethqos.txt >> deleted file mode 100644 >> index 1f5746849a71..000000000000 >> --- a/Documentation/devicetree/bindings/net/qcom,ethqos.txt >> +++ /dev/null >> @@ -1,66 +0,0 @@ >> -Qualcomm Ethernet ETHQOS device >> - >> -This documents dwmmac based ethernet device which supports Gigabit >> -ethernet for version v2.3.0 onwards. >> - >> -This device has following properties: >> - >> -Required properties: >> - >> -- compatible: Should be one of: >> - "qcom,qcs404-ethqos" >> - "qcom,sm8150-ethqos" >> - >> -- reg: Address and length of the register set for the device >> - >> -- reg-names: Should contain register names "stmmaceth", "rgmii" >> - >> -- clocks: Should contain phandle to clocks >> - >> -- clock-names: Should contain clock names "stmmaceth", "pclk", >> - "ptp_ref", "rgmii" >> - >> -- interrupts: Should contain phandle to interrupts >> - >> -- interrupt-names: Should contain interrupt names "macirq", "eth_lpi" >> - >> -Rest of the properties are defined in stmmac.txt file in same directory >> - >> - >> -Example: >> - >> -ethernet: ethernet@7a80000 { >> - compatible = "qcom,qcs404-ethqos"; >> - reg = <0x07a80000 0x10000>, >> - <0x07a96000 0x100>; >> - reg-names = "stmmaceth", "rgmii"; >> - clock-names = "stmmaceth", "pclk", "ptp_ref", "rgmii"; >> - clocks = <&gcc GCC_ETH_AXI_CLK>, >> - <&gcc GCC_ETH_SLAVE_AHB_CLK>, >> - <&gcc GCC_ETH_PTP_CLK>, >> - <&gcc GCC_ETH_RGMII_CLK>; >> - interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>, >> - <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>; >> - interrupt-names = "macirq", "eth_lpi"; >> - snps,reset-gpio = <&tlmm 60 GPIO_ACTIVE_LOW>; >> - snps,reset-active-low; >> - >> - snps,txpbl = <8>; >> - snps,rxpbl = <2>; >> - snps,aal; >> - snps,tso; >> - >> - phy-handle = <&phy1>; >> - phy-mode = "rgmii"; >> - >> - mdio { >> - #address-cells = <0x1>; >> - #size-cells = <0x0>; >> - compatible = "snps,dwmac-mdio"; >> - phy1: phy@4 { >> - device_type = "ethernet-phy"; >> - reg = <0x4>; >> - }; >> - }; >> - >> -}; >> diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.yaml b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml >> new file mode 100644 >> index 000000000000..f05df9b0d106 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml >> @@ -0,0 +1,139 @@ >> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/net/qcom,ethqos.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm Ethernet ETHQOS device >> + >> +maintainers: >> + - Bhupesh Sharma <bhupesh.sharma@linaro.org> >> + >> +description: >> + This binding describes the dwmmac based Qualcomm ethernet devices which >> + support Gigabit ethernet (version v2.3.0 onwards). >> + >> + So, this file documents platform glue layer for dwmmac stmmac based Qualcomm >> + ethernet devices. >> + >> +allOf: >> + - $ref: "snps,dwmac.yaml#" > > No need for quotes.
Ok.
>> + >> +properties: >> + compatible: >> + enum: >> + - qcom,qcs404-ethqos >> + - qcom,sm8150-ethqos >> + >> + reg: true > > I think both devices use two reg spaces.
On this platform the two reg spaces are 64-bit, whereas for other platforms based on dwmmac, for e.g. stm32 have 32-bit address space.
>> + >> + reg-names: >> + minItems: 1 > > Why allowing only one item?
Ok, let me remove this in v2.
>> + items: >> + - const: stmmaceth >> + - const: rgmii >> + >> + interrupts: true > > This should be specific/fixed. > >> + >> + interrupt-names: true > > This should be specific/fixed.
These are same as in $ref: "snps,dwmac.yaml#", so do we really need to specify them here? I remember on the sdhci-msm YAML patch review, Rob mentioned that we should just set the property to true, in such cases.
Am I missing something here?
>> + >> + clocks: >> + minItems: 1 >> + maxItems: 4 > > Why such flexibility?
Ok, let me just keep 'maxItems: 4' here for now.
>> + >> + clock-names: >> + minItems: 1 >> + items: >> + - const: stmmaceth >> + - const: pclk >> + - const: ptp_ref >> + - const: rgmii >> + >> + iommus: >> + minItems: 1 >> + maxItems: 2 > > Aren't we using only one MMU?
It was just for future compatibility, but I get your point. Let me keep the 'maxItems: 1' here for now.
>> + >> + mdio: true >> + >> + phy-handle: true >> + >> + phy-mode: true >> + >> + snps,reset-gpio: true >> + >> + snps,tso: >> + $ref: /schemas/types.yaml#/definitions/flag >> + description: >> + Enables the TSO feature otherwise it will be managed by MAC HW capability register. >> + >> + power-domains: true >> + >> + resets: true >> + >> + rx-fifo-depth: true >> + >> + tx-fifo-depth: true > > You do not list all these properties, because you use > unevaluatedProperties. Drop all of these "xxx :true".
Same query as above. May be I am missing something here.
>> + >> +required: >> + - compatible >> + - clocks >> + - clock-names
Thanks, Bhupesh
| |