[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <76e081ba-9d5a-41df-9c1b-d782e5656973@quicinc.com>
Date: Fri, 17 Nov 2023 18:36:18 +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 9/9] dt-bindings: net: ipq4019-mdio: Document ipq5332
platform
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@...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.
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
>
Powered by blists - more mailing lists