2024-03-28 14:12:30

by Christophe Roullier

[permalink] [raw]
Subject: [PATCH v5 1/1] dt-bindings: net: dwmac: Document STM32 property st,ext-phyclk

The Linux kernel dwmac-stm32 driver currently supports three DT
properties used to configure whether PHY clock are generated by
the MAC or supplied to the MAC from the PHY.

Originally there were two properties, st,eth-clk-sel and
st,eth-ref-clk-sel, each used to configure MAC clocking in
different bus mode and for different MAC clock frequency.
Since it is possible to determine the MAC 'eth-ck' clock
frequency from the clock subsystem and PHY bus mode from
the 'phy-mode' property, two disparate DT properties are
no longer required to configure MAC clocking.

Linux kernel commit 1bb694e20839 ("net: ethernet: stmmac: simplify phy modes management for stm32")
introduced a third, unified, property st,ext-phyclk. This property
covers both use cases of st,eth-clk-sel and st,eth-ref-clk-sel DT
properties, as well as a new use case for 25 MHz clock generated
by the MAC.

The third property st,ext-phyclk is so far undocumented,
document it.

Below table summarizes the clock requirement and clock sources for
supported PHY interface modes.
__________________________________________________________________________
|PHY_MODE | Normal | PHY wo crystal| PHY wo crystal |No 125Mhz from PHY|
| | | 25MHz | 50MHz | |

---------------------------------------------------------------------------
| MII | - | eth-ck | n/a | n/a |
| | | st,ext-phyclk | | |

---------------------------------------------------------------------------
| GMII | - | eth-ck | n/a | n/a |
| | | st,ext-phyclk | | |

---------------------------------------------------------------------------
| RGMII | - | eth-ck | n/a | eth-ck |
| | | st,ext-phyclk | | st,eth-clk-sel or|
| | | | | st,ext-phyclk |

---------------------------------------------------------------------------
| RMII | - | eth-ck | eth-ck | n/a |
| | | st,ext-phyclk | st,eth-ref-clk-sel | |
| | | | or st,ext-phyclk | |

---------------------------------------------------------------------------

Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Christophe Roullier <[email protected]>
---
Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
index fc8c96b08d7dc..b35eae80ed6ac 100644
--- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
@@ -82,6 +82,13 @@ properties:
Should be phandle/offset pair. The phandle to the syscon node which
encompases the glue register, and the offset of the control register

+st,ext-phyclk:
+ description:
+ set this property in RMII mode when you have PHY without crystal 50MHz and want to
+ select RCC clock instead of ETH_REF_CLK. OR in RGMII mode when you want to select
+ RCC clock instead of ETH_CLK125.
+ type: boolean
+
st,eth-clk-sel:
description:
set this property in RGMII PHY when you want to select RCC clock instead of ETH_CLK125.
--
2.25.1



2024-03-28 14:19:26

by Marek Vasut

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: net: dwmac: Document STM32 property st,ext-phyclk

On 3/28/24 3:08 PM, Christophe Roullier wrote:

[...]

> | RMII | - | eth-ck | eth-ck | n/a |
> | | | st,ext-phyclk | st,eth-ref-clk-sel | |
> | | | | or st,ext-phyclk | |
>
> ---------------------------------------------------------------------------
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Christophe Roullier <[email protected]>
> ---
> Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> index fc8c96b08d7dc..b35eae80ed6ac 100644
> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> @@ -82,6 +82,13 @@ properties:
> Should be phandle/offset pair. The phandle to the syscon node which
> encompases the glue register, and the offset of the control register
>
> +st,ext-phyclk:

Don't you need two spaces in front of the 'st,' here ?

> + description:
> + set this property in RMII mode when you have PHY without crystal 50MHz and want to
> + select RCC clock instead of ETH_REF_CLK. OR in RGMII mode when you want to select
> + RCC clock instead of ETH_CLK125.
> + type: boolean
> +

With that fixed:

Reviewed-by: Marek Vasut <[email protected]>

2024-03-28 15:09:13

by Christophe Roullier

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: net: dwmac: Document STM32 property st,ext-phyclk


On 3/28/24 15:19, Marek Vasut wrote:
> On 3/28/24 3:08 PM, Christophe Roullier wrote:
>
> [...]
>
>> | RMII    |    -   |     eth-ck    | eth-ck        |       n/a        |
>> |         |        | st,ext-phyclk | st,eth-ref-clk-sel
>> |                  |
>> |         |        |               | or st,ext-phyclk
>> |                  |
>>
>> ---------------------------------------------------------------------------
>>
>>
>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>> Signed-off-by: Christophe Roullier <[email protected]>
>> ---
>>   Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> index fc8c96b08d7dc..b35eae80ed6ac 100644
>> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> @@ -82,6 +82,13 @@ properties:
>>         Should be phandle/offset pair. The phandle to the syscon node
>> which
>>         encompases the glue register, and the offset of the control
>> register
>>   +st,ext-phyclk:
>
> Don't you need two spaces in front of the 'st,' here ?
Sorry, that's right.
>
>> +    description:
>> +      set this property in RMII mode when you have PHY without
>> crystal 50MHz and want to
>> +      select RCC clock instead of ETH_REF_CLK. OR in RGMII mode when
>> you want to select
>> +      RCC clock instead of ETH_CLK125.
>> +    type: boolean
>> +
>
> With that fixed:
>
> Reviewed-by: Marek Vasut <[email protected]>

2024-03-28 15:26:08

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: net: dwmac: Document STM32 property st,ext-phyclk


On Thu, 28 Mar 2024 15:08:03 +0100, Christophe Roullier wrote:
> The Linux kernel dwmac-stm32 driver currently supports three DT
> properties used to configure whether PHY clock are generated by
> the MAC or supplied to the MAC from the PHY.
>
> Originally there were two properties, st,eth-clk-sel and
> st,eth-ref-clk-sel, each used to configure MAC clocking in
> different bus mode and for different MAC clock frequency.
> Since it is possible to determine the MAC 'eth-ck' clock
> frequency from the clock subsystem and PHY bus mode from
> the 'phy-mode' property, two disparate DT properties are
> no longer required to configure MAC clocking.
>
> Linux kernel commit 1bb694e20839 ("net: ethernet: stmmac: simplify phy modes management for stm32")
> introduced a third, unified, property st,ext-phyclk. This property
> covers both use cases of st,eth-clk-sel and st,eth-ref-clk-sel DT
> properties, as well as a new use case for 25 MHz clock generated
> by the MAC.
>
> The third property st,ext-phyclk is so far undocumented,
> document it.
>
> Below table summarizes the clock requirement and clock sources for
> supported PHY interface modes.
> __________________________________________________________________________
> |PHY_MODE | Normal | PHY wo crystal| PHY wo crystal |No 125Mhz from PHY|
> | | | 25MHz | 50MHz | |
>
> ---------------------------------------------------------------------------
> | MII | - | eth-ck | n/a | n/a |
> | | | st,ext-phyclk | | |
>
> ---------------------------------------------------------------------------
> | GMII | - | eth-ck | n/a | n/a |
> | | | st,ext-phyclk | | |
>
> ---------------------------------------------------------------------------
> | RGMII | - | eth-ck | n/a | eth-ck |
> | | | st,ext-phyclk | | st,eth-clk-sel or|
> | | | | | st,ext-phyclk |
>
> ---------------------------------------------------------------------------
> | RMII | - | eth-ck | eth-ck | n/a |
> | | | st,ext-phyclk | st,eth-ref-clk-sel | |
> | | | | or st,ext-phyclk | |
>
> ---------------------------------------------------------------------------
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Christophe Roullier <[email protected]>
> ---
> Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>

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:
/Documentation/devicetree/bindings/net/stm32-dwmac.yaml:86:5: [warning] wrong indentation: expected 2 but found 4 (indentation)
/Documentation/devicetree/bindings/net/stm32-dwmac.yaml:92:3: [error] syntax error: expected <block end>, but found '<block mapping start>' (syntax)

dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/net/stm32-dwmac.example.dts'
Documentation/devicetree/bindings/net/stm32-dwmac.yaml:92:3: did not find expected key
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/net/stm32-dwmac.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
/Documentation/devicetree/bindings/net/stm32-dwmac.yaml:92:3: did not find expected key
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.yaml: ignoring, error parsing file
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

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.


2024-03-30 18:40:38

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: net: dwmac: Document STM32 property st,ext-phyclk

On 28/03/2024 15:08, Christophe Roullier wrote:
> The Linux kernel dwmac-stm32 driver currently supports three DT
> properties used to configure whether PHY clock are generated by
> the MAC or supplied to the MAC from the PHY.
>
> Originally there were two properties, st,eth-clk-sel and
> st,eth-ref-clk-sel, each used to configure MAC clocking in
> different bus mode and for different MAC clock frequency.
> Since it is possible to determine the MAC 'eth-ck' clock
> frequency from the clock subsystem and PHY bus mode from
> the 'phy-mode' property, two disparate DT properties are
> no longer required to configure MAC clocking.
>
> Linux kernel commit 1bb694e20839 ("net: ethernet: stmmac: simplify phy modes management for stm32")
> introduced a third, unified, property st,ext-phyclk. This property
> covers both use cases of st,eth-clk-sel and st,eth-ref-clk-sel DT
> properties, as well as a new use case for 25 MHz clock generated
> by the MAC.
>
> The third property st,ext-phyclk is so far undocumented,
> document it.
>
> Below table summarizes the clock requirement and clock sources for
> supported PHY interface modes.
> __________________________________________________________________________
> |PHY_MODE | Normal | PHY wo crystal| PHY wo crystal |No 125Mhz from PHY|
> | | | 25MHz | 50MHz | |
>
> ---------------------------------------------------------------------------
> | MII | - | eth-ck | n/a | n/a |
> | | | st,ext-phyclk | | |
>
> ---------------------------------------------------------------------------
> | GMII | - | eth-ck | n/a | n/a |
> | | | st,ext-phyclk | | |
>
> ---------------------------------------------------------------------------
> | RGMII | - | eth-ck | n/a | eth-ck |
> | | | st,ext-phyclk | | st,eth-clk-sel or|
> | | | | | st,ext-phyclk |
>
> ---------------------------------------------------------------------------
> | RMII | - | eth-ck | eth-ck | n/a |
> | | | st,ext-phyclk | st,eth-ref-clk-sel | |
> | | | | or st,ext-phyclk | |
>
> ---------------------------------------------------------------------------
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Christophe Roullier <[email protected]>

Can you please start testing patches *before* sending them?

Best regards,
Krzysztof


2024-04-02 06:25:58

by Christophe Roullier

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: net: dwmac: Document STM32 property st,ext-phyclk


On 3/30/24 19:40, Krzysztof Kozlowski wrote:
> On 28/03/2024 15:08, Christophe Roullier wrote:
>> The Linux kernel dwmac-stm32 driver currently supports three DT
>> properties used to configure whether PHY clock are generated by
>> the MAC or supplied to the MAC from the PHY.
>>
>> Originally there were two properties, st,eth-clk-sel and
>> st,eth-ref-clk-sel, each used to configure MAC clocking in
>> different bus mode and for different MAC clock frequency.
>> Since it is possible to determine the MAC 'eth-ck' clock
>> frequency from the clock subsystem and PHY bus mode from
>> the 'phy-mode' property, two disparate DT properties are
>> no longer required to configure MAC clocking.
>>
>> Linux kernel commit 1bb694e20839 ("net: ethernet: stmmac: simplify phy modes management for stm32")
>> introduced a third, unified, property st,ext-phyclk. This property
>> covers both use cases of st,eth-clk-sel and st,eth-ref-clk-sel DT
>> properties, as well as a new use case for 25 MHz clock generated
>> by the MAC.
>>
>> The third property st,ext-phyclk is so far undocumented,
>> document it.
>>
>> Below table summarizes the clock requirement and clock sources for
>> supported PHY interface modes.
>> __________________________________________________________________________
>> |PHY_MODE | Normal | PHY wo crystal| PHY wo crystal |No 125Mhz from PHY|
>> | | | 25MHz | 50MHz | |
>>
>> ---------------------------------------------------------------------------
>> | MII | - | eth-ck | n/a | n/a |
>> | | | st,ext-phyclk | | |
>>
>> ---------------------------------------------------------------------------
>> | GMII | - | eth-ck | n/a | n/a |
>> | | | st,ext-phyclk | | |
>>
>> ---------------------------------------------------------------------------
>> | RGMII | - | eth-ck | n/a | eth-ck |
>> | | | st,ext-phyclk | | st,eth-clk-sel or|
>> | | | | | st,ext-phyclk |
>>
>> ---------------------------------------------------------------------------
>> | RMII | - | eth-ck | eth-ck | n/a |
>> | | | st,ext-phyclk | st,eth-ref-clk-sel | |
>> | | | | or st,ext-phyclk | |
>>
>> ---------------------------------------------------------------------------
>>
>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>> Signed-off-by: Christophe Roullier <[email protected]>
> Can you please start testing patches *before* sending them?
Yes sorry, when I removed patch with phy-supply property (1/2), I had
conflict merge and I did not pay attention that my commit was modified :-(
>
> Best regards,
> Krzysztof
>