netdev - Re: [PATCH v3 5/5] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <3de98516-9a28-4f58-8951-2a7752621fee@quicinc.com>
Date: Fri, 15 Dec 2023 18:03:33 +0800
From: Jie Luo <quic_luoj@...cinc.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>, <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 12/15/2023 4:39 PM, Krzysztof Kozlowski wrote:
> On 15/12/2023 09:28, Jie Luo wrote:
>>
>>
>> On 12/15/2023 3:27 PM, Krzysztof Kozlowski wrote:
>>> 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
> 
> Why do you add entries to the end of the list? In reversed order?

Thanks for pointing it out, i will move "- qcom,ipq5332-mdio" before
"- qcom,ipq6018-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
>>
>> With this change, the warning is not reported when i run
>> dt_binding_check, looks the new added property needs
>> the type ref to avoid the warning reported.
> 
> Nothing improved in the property name, nor its style, nor in the actual
> contents/values.

This property is for CMN PLL block reference clock configuration,
so i use this property name.

it will be appreciated if you can suggest a suitable name, thanks.

> 
> ...
> 
>>>> +  reset-gpios:
>>>> +    maxItems: 1
>>>> +
>>>> +  reset-assert-us:
>>>> +    maxItems: 1
>>>
>>> This does not look related to ipq5332.
>>
>> The reset gpio properties are needed on ipq5332, since qca8084 phy is
>> connected, which uses the MDIO bus level gpio reset.
> 
> I am talking about this property, not these properties.

ok.

> 
>>
>> Without declaring these gpio properties, the warning will be reported
>> by dt_binding_check.
> 
> How is it even possible to have warnings if there is no such node in
> DTS? We do not care about warnings in your downstream code.
> 
> 
> Best regards,
> Krzysztof
> 

If i do not declare the property "reset-assert-us" and 
"reset-deassert-us", the warning will be reported by "make 
dt_binding_check" since i
add a example in this file.