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@quicinc.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
clocks:
- items:
- - description: MDIO clock source frequency fixed to 100MHZ
+ minItems: 1
+ maxItems: 5
+ description:
+ 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:
+ 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
+
+ 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
required:
- compatible
@@ -61,6 +91,8 @@ allOf:
- qcom,ipq5018-mdio
- qcom,ipq6018-mdio
- qcom,ipq8074-mdio
+ - qcom,ipq5332-mdio
+ - qcom,ipq9574-mdio
then:
required:
- clocks
@@ -70,6 +102,29 @@ allOf:
clocks: false
clock-names: false
+ - 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
+
unevaluatedProperties: false
examples:
@@ -100,3 +155,70 @@ examples:
reg = <4>;
};
};
+
+ - |
+ #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ mdio@90000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "qcom,ipq5332-mdio";
+ reg = <0x90000 0x64>, <0x7A00610 0x4>, <0x7A10610 0x4>, <0x9B000 0x800>;
+ reg-names = "mdio", "eth_ldo1", "eth_ldo2", "cmn_blk";
+
+ clocks = <&gcc GCC_MDIO_AHB_CLK>,
+ <&gcc GCC_UNIPHY0_AHB_CLK>,
+ <&gcc GCC_UNIPHY0_SYS_CLK>,
+ <&gcc GCC_UNIPHY1_AHB_CLK>,
+ <&gcc GCC_UNIPHY1_SYS_CLK>;
+
+ clock-names = "gcc_mdio_ahb_clk",
+ "gcc_uniphy0_ahb_clk",
+ "gcc_uniphy0_sys_clk",
+ "gcc_uniphy1_ahb_clk",
+ "gcc_uniphy1_sys_clk";
+
+ phy-reset-gpio = <&tlmm 51 GPIO_ACTIVE_LOW>;
+ phyaddr-fixup = <0xC90F018>;
+ pcsaddr-fixup = <0xC90F014>;
+ mdio-clk-fixup;
+
+ qca8kphy0: ethernet-phy@1 {
+ reg = <1>;
+ fixup;
+ };
+
+ qca8kphy1: ethernet-phy@2 {
+ reg = <2>;
+ fixup;
+ };
+
+ qca8kphy2: ethernet-phy@3 {
+ reg = <3>;
+ fixup;
+ };
+
+ qca8kphy3: ethernet-phy@4 {
+ reg = <4>;
+ fixup;
+ };
+
+ qca8kpcs0: pcsphy0@5 {
+ compatible = "qcom,qca8k_pcs";
+ reg = <5>;
+ fixup;
+ };
+
+ qca8kpcs1: pcsphy1@6 {
+ compatible = "qcom,qca8k_pcs";
+ reg = <6>;
+ fixup;
+ };
+
+ qca8kxpcs: xpcsphy@7 {
+ compatible = "qcom,qca8k_pcs";
+ reg = <7>;
+ fixup;
+ };
+ };
--
2.42.0On 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@quicinc.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
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@quicinc.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
>
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. > >> >>> + 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 Best regards, Krzysztof
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. 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. md64 dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml warning: python package 'yamllint' not installed, skipping CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.example.dts DTC_CHK Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.example.dtb > > Best regards, > Krzysztof >
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. > > 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. Best regards, Krzysztof
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
>
> 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
As i said before, the frequency of the clocks does not matter
here. That appears to be the drivers problem. I assume every board
design, with any sort of PHY, needs the same clock configuration?
Andrew
On 11/18/2023 11:36 PM, Andrew Lunn wrote: >> 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 > > As i said before, the frequency of the clocks does not matter > here. That appears to be the drivers problem. I assume every board > design, with any sort of PHY, needs the same clock configuration? > > Andrew Yes, Andrew, no matter what kind of PHY is connected, these clocks are fix clocks, the clock rates are same as mentioned above, which are the SOC clock configurations.
> + phy-reset-gpio:
> + minItems: 1
> + maxItems: 3
> + description:
> + GPIO used to reset the PHY, each GPIO is for resetting the connected
> + ethernet PHY device.
This is a PHY property, so should be in the PHY node. phylib should
then take care of fit.
> +
> + phyaddr-fixup:
> + description: Register address for programing MDIO address of PHY devices
Please give a better description of this and the next two properties.
> +
> + 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
> + 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
The clock frequencies is not relevent here, the driver sets that up.
Andrew
On 11/15/2023 10:35 PM, Andrew Lunn wrote: >> + phy-reset-gpio: >> + minItems: 1 >> + maxItems: 3 >> + description: >> + GPIO used to reset the PHY, each GPIO is for resetting the connected >> + ethernet PHY device. > > This is a PHY property, so should be in the PHY node. phylib should > then take care of fit. will check this and update in the next patch set. > >> + >> + phyaddr-fixup: >> + description: Register address for programing MDIO address of PHY devices > > Please give a better description of this and the next two properties. these fixup flags are for customizing the MDIO address of qca8084 PHY & PCS and doing the initialization configurations to bring up PHY. will describe it more detail in the next patch set. > >> + >> + 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 >> + 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 > > The clock frequencies is not relevent here, the driver sets that up. OK, will remove the clock frequency values in the next patch set. > > Andrew
On Wed, 15 Nov 2023 11:25:15 +0800, 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@quicinc.com> > --- > .../bindings/net/qcom,ipq4019-mdio.yaml | 138 +++++++++++++++++- > 1 file changed, 130 insertions(+), 8 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml: phyaddr-fixup: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml: pcsaddr-fixup: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml: mdio-clk-fixup: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml: fixup: missing type definition Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.example.dtb: /example-1/mdio@90000/pcsphy0@5: failed to match any schema with compatible: ['qcom,qca8k_pcs'] Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.example.dtb: /example-1/mdio@90000/pcsphy1@6: failed to match any schema with compatible: ['qcom,qca8k_pcs'] Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.example.dtb: /example-1/mdio@90000/xpcsphy@7: failed to match any schema with compatible: ['qcom,qca8k_pcs'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231115032515.4249-10-quic_luoj@quicinc.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
© 2016 - 2025 Red Hat, Inc.