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

From: Jie Luo
Date: Sat Nov 18 2023 - 03:07:36 EST




On 11/17/2023 8:43 PM, Krzysztof Kozlowski wrote:
On 17/11/2023 12:20, Jie Luo wrote:


On 11/17/2023 6:40 PM, Krzysztof Kozlowski wrote:
On 17/11/2023 11:36, Jie Luo wrote:
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.

OTHER variants. Not this one.

The change here is for the clock number added for the ipq5332 platform,
the other platforms still use only legacy MDIO clock.

Then your patch is wrong as I said. You now affect other variants. I
don't quite get your responses. Style of them suggests that you
disagree, but you are not providing any relevant argument.

The clock arguments are provided in the later part as below. i will also
provide more detail clock names for the new added clocks for the ipq5332
platform in description.

- if:
properties:
compatible:
contains:
enum:
- qcom,ipq5332-mdio
then:
properties:
clocks:
items:
- description: MDIO clock source frequency fixed to 100MHZ
- description: UNIPHY0 AHB clock source frequency fixed to 100MHZ
- description: UNIPHY0 SYS clock source frequency fixed to 24MHZ
- description: UNIPHY1 AHB clock source frequency fixed to 100MHZ
- description: UNIPHY1 SYS clock source frequency fixed to 24MHZ
clock-names:
items:
- const: gcc_mdio_ahb_clk
- const: gcc_uniphy0_ahb_clk
- const: gcc_uniphy0_sys_clk
- const: gcc_uniphy1_ahb_clk
- const: gcc_uniphy1_sys_clk


so i add minItems and maxItems, i will check other .yaml files for the
reference.




+ 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

No clue what's this, but no, I do not believe you tested it at all. It's
not about yamllint. It's was not tested. Look at errors reported on
mailing list.

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


Hi Krzysztof,
Here is the output when i run the dt check with this patch applied in my
workspace, md64 is the alias for compiling the arm64 platform.

We still do not know your base and dtschema version.

The code base is the commit id:
5ba73bec5e7b0494da7fdca3e003d8b97fa932cd
<Add linux-next specific files for 20231114>

The dtschema version is as below.
dt-doc-validate --version
2023.9




Best regards,
Krzysztof