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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 16 Nov 2023 12:56:19 +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 9/9] dt-bindings: net: ipq4019-mdio: Document ipq5332
 platform

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@...cinc.com>
> ---
>  .../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