[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1e4c55c8-8ed6-4f2e-8328-8a173f09b62f@linaro.org>
Date: Fri, 15 Dec 2023 08:27:05 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Luo Jie <quic_luoj@...cinc.com>, agross@...nel.org, andersson@...nel.org,
konrad.dybcio@...aro.org, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org, andrew@...n.ch,
hkallweit1@...il.com, linux@...linux.org.uk, robert.marko@...tura.hr
Cc: linux-arm-msm@...r.kernel.org, netdev@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
quic_srichara@...cinc.com
Subject: Re: [PATCH v3 5/5] dt-bindings: net: ipq4019-mdio: Document ipq5332
platform
On 14/12/2023 10:03, Luo Jie wrote:
> Update the yaml file for the new DTS properties.
>
> 1. cmn-reference-clock for the CMN PLL source clock select.
> 2. clock-frequency for MDIO clock frequency config.
> 3. add uniphy AHB & SYS GCC clocks.
> 4. add reset-gpios for MDIO bus level reset.
>
> Signed-off-by: Luo Jie <quic_luoj@...cinc.com>
> ---
> .../bindings/net/qcom,ipq4019-mdio.yaml | 143 +++++++++++++++++-
> 1 file changed, 139 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> index 3407e909e8a7..79f8513739e7 100644
> --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> @@ -20,6 +20,8 @@ properties:
> - enum:
> - qcom,ipq6018-mdio
> - qcom,ipq8074-mdio
> + - qcom,ipq9574-mdio
> + - qcom,ipq5332-mdio
> - const: qcom,ipq4019-mdio
>
> "#address-cells":
> @@ -30,19 +32,77 @@ properties:
>
> reg:
> minItems: 1
> - maxItems: 2
> + maxItems: 5
> description:
> - the first Address and length of the register set for the MDIO controller.
> - the second Address and length of the register for ethernet LDO, this second
> - address range is only required by the platform IPQ50xx.
> + the first Address and length of the register set for the MDIO controller,
> + the optional second, third and fourth address and length of the register
> + for ethernet LDO, these three address range are required by the platform
> + IPQ50xx/IPQ5332, the last address and length is for the CMN clock to
> + select the reference clock.
> +
> + reg-names:
> + minItems: 1
> + maxItems: 5
>
> clocks:
> + minItems: 1
> items:
> - description: MDIO clock source frequency fixed to 100MHZ
> + - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ
> + - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ
> + - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ
> + - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ
>
> clock-names:
> + minItems: 1
> items:
> - const: gcc_mdio_ahb_clk
> + - const: uniphy0_ahb
> + - const: uniphy1_ahb
> + - const: uniphy0_sys
> + - const: uniphy1_sys
> +
> + cmn-reference-clock:
> + $ref: /schemas/types.yaml#/definitions/uint32
Nothing improved here
> + oneOf:
> + - items:
So it is enum or not?
> + - enum:
> + - 0 # CMN PLL reference internal 48MHZ
> + - 1 # CMN PLL reference external 25MHZ
> + - 2 # CMN PLL reference external 31250KHZ
> + - 3 # CMN PLL reference external 40MHZ
> + - 4 # CMN PLL reference external 48MHZ
> + - 5 # CMN PLL reference external 50MHZ
> + - 6 # CMN PLL reference internal 96MHZ
> +
> + clock-frequency:
> + oneOf:
> + - items:
Same questions
> + - enum:
> + - 12500000
> + - 6250000
> + - 3125000
> + - 1562500
> + - 781250
> + - 390625
> + description:
> + The MDIO bus clock that must be output by the MDIO bus hardware,
> + only the listed frequencies above can be supported, other frequency
> + will cause malfunction. If absent, the default hardware value 0xff
> + is used, which means the default MDIO clock frequency 390625HZ, The
> + MDIO clock frequency is MDIO_SYS_CLK/(MDIO_CLK_DIV + 1), the SoC
> + MDIO_SYS_CLK is fixed to 100MHZ, the MDIO_CLK_DIV is from MDIO control
> + register, there is higher clock frequency requirement on the normal
> + working case where the MDIO slave devices support high clock frequency.
> +
> + reset-gpios:
> + maxItems: 1
> +
> + reset-assert-us:
> + maxItems: 1
This does not look related to ipq5332.
> +
> + reset-deassert-us:
> + maxItems: 1
Neither this.
>
> required:
> - compatible
> @@ -61,6 +121,8 @@ allOf:
> - qcom,ipq5018-mdio
> - qcom,ipq6018-mdio
> - qcom,ipq8074-mdio
> + - qcom,ipq5332-mdio
> + - qcom,ipq9574-mdio
> then:
> required:
> - clocks
> @@ -70,6 +132,20 @@ allOf:
> clocks: false
> clock-names: false
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,ipq5332-mdio
> + then:
> + properties:
> + clocks:
> + minItems: 5
> + maxItems: 5
> + reg-names:
> + minItems: 4
> +
> unevaluatedProperties: false
>
> examples:
> @@ -100,3 +176,62 @@ examples:
> reg = <4>;
> };
> };
> +
> + - |
> + #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> + #include <dt-bindings/gpio/gpio.h>
> +
> + mdio@...00 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "qcom,ipq5332-mdio",
> + "qcom,ipq4019-mdio";
> + cmn-reference-clock = <0>;
> + clock-frequency = <6250000>;
> +
> + reset-gpios = <&tlmm 51 GPIO_ACTIVE_LOW>;
> + reset-assert-us = <100000>;
> + reset-deassert-us = <100000>;
> +
> + reg = <0x90000 0x64>,
> + <0x9B000 0x800>,
> + <0x7A00610 0x4>,
> + <0x7A10610 0x4>;
Lowercase hex, wrong order of properties. Align it with coding style.
Best regards,
Krzysztof
Powered by blists - more mailing lists