Re: [PATCH 9/9] dt-bindings: net: ipq4019-mdio: Document ipq5332 platform

From: Jie Luo
Date: Fri Nov 17 2023 - 05:36:46 EST




On 11/16/2023 7:56 PM, Krzysztof Kozlowski wrote:
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.

Ok, will describe these items one by one in the next patch set.


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?

There are 5 clock items, the first one is the legacy MDIO clock, the
other clocks are new added for ipq5332 platform, will describe it more
clearly in the next patch set.


+ 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?

will remove this property, and update to use the exited PHY GPIO reset.


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.

Hi Krzysztof,
This patch is passed with the following command in my workspace.
i will upgrade and install yamllint to make sure there is no
warning reported anymore.

make dt_bg_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml


+
+ 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.


Ok, will complete the full test before uploading the patch.


...

+
+ 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.

Will Fix it in the next patch set, thanks.


Best regards,
Krzysztof