On 15/11/2023 04:25, Luo Jie wrote:
> On platform IPQ5332, the MDIO address of qca8084 can be programed
> when the device tree property "fixup" defined, the clock sequence
> needs to be completed before the PHY probeable.
>
> Signed-off-by: Luo Jie <quic_luoj@xxxxxxxxxxx>
> ---
> .../bindings/net/qcom,ipq4019-mdio.yaml | 138 +++++++++++++++++-
> 1 file changed, 130 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> index 3407e909e8a7..7ff92be14ee1 100644
> --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> @@ -15,11 +15,13 @@ properties:
> - enum:
> - qcom,ipq4019-mdio
> - qcom,ipq5018-mdio
> + - qcom,ipq5332-mdio
>
> - items:
> - enum:
> - qcom,ipq6018-mdio
> - qcom,ipq8074-mdio
> + - qcom,ipq9574-mdio
> - const: qcom,ipq4019-mdio
>
> "#address-cells":
> @@ -30,19 +32,47 @@ 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
You must describe the items and constrain them per each variant.
>
> clocks:
> - items:
> - - description: MDIO clock source frequency fixed to 100MHZ
> + minItems: 1
> + maxItems: 5
> + description:
Doesn't this make all other variants with incorrect constraints?
> + MDIO system clock frequency fixed to 100MHZ, and the GCC uniphy
> + clocks enabled for resetting ethernet PHY.
>
> clock-names:
> - items:
> - - const: gcc_mdio_ahb_clk
> + minItems: 1
> + maxItems: 5
> +
> + phy-reset-gpio:
No, for multiple reasons. It's gpios first of all. Where do you see such
property? Where is the existing definition?
Then it is "reset-gpios" if this is MDIO. Why do you put phy properties
in MDIO?
> + minItems: 1
> + maxItems: 3
> + description:
> + GPIO used to reset the PHY, each GPIO is for resetting the connected
> + ethernet PHY device.
> +
> + phyaddr-fixup:
> + description: Register address for programing MDIO address of PHY devices
You did not test code which you sent.
> +
> + pcsaddr-fixup:
> + description: Register address for programing MDIO address of PCS devices
> +
> + mdio-clk-fixup:
> + description: The initialization clocks to be configured
> +
> + fixup:
> + description: The MDIO address of PHY/PCS device to be programed
Please do not send untested code.
...
> +
> + qca8kphy2: ethernet-phy@3 {
> + reg = <3>;
> + fixup;
> + };
> +
> + qca8kphy3: ethernet-phy@4 {
> + reg = <4>;
> + fixup;
> + };
> +
> + qca8kpcs0: pcsphy0@5 {
> + compatible = "qcom,qca8k_pcs";
> + reg = <5>;
> + fixup;
> + };
Fix indentation.
Best regards,
Krzysztof
