[PATCH v5 0/4] Mediatek ethernet patches for mt8188

[PATCH v5 0/4] Mediatek ethernet patches for mt8188

[Jianguo Zhang]

Changes in v5:

v5:
1) Rename the property 'clk_csr' as 'snps,clk-csr' in binding
file as Krzysztof Kozlowski'comment.
2) Add DTS patch 'arm64: dts: mediatek: mt2712e: Update the name of property 'clk_csr''
as Krzysztof Kozlowski'comment.
3) Add driver patch 'net: stmmac: Update the name of property 'clk_csr''
as Krzysztof Kozlowski'comment.

v4:
1) Update the commit message of patch 'dt-bindings: net: snps,dwmac: add clk_csr property'
as Krzysztof Kozlowski'comment.

v3:
1) List the names of SoCs mt8188 and mt8195 in correct order as
AngeloGioacchino Del Regno's comment.
2) Add patch version info as Krzysztof Kozlowski'comment.

v2:
1) Delete patch 'stmmac: dwmac-mediatek: add support for mt8188' as
Krzysztof Kozlowski's comment.
2) Update patch 'dt-bindings: net: mediatek-dwmac: add support for
mt8188' as Krzysztof Kozlowski's comment.
3) Add clk_csr property to fix warning ('clk_csr' was unexpected) when
runnig 'make dtbs_check'.

v1:
1) Add ethernet driver entry for mt8188.
2) Add binding document for ethernet on mt8188.

Jianguo Zhang (4):
  dt-bindings: net: mediatek-dwmac: add support for mt8188
  dt-bindings: net: snps,dwmac: add clk_csr property
  arm64: dts: mediatek: mt2712e: Update the name of property 'clk_csr'
  net: stmmac: Update the name of property 'clk_csr'

 .../devicetree/bindings/net/mediatek-dwmac.yaml        | 10 ++++++++--
 Documentation/devicetree/bindings/net/snps,dwmac.yaml  |  5 +++++
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi              |  2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c  |  2 +-
 4 files changed, 15 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH v5 1/4] dt-bindings: net: mediatek-dwmac: add support for mt8188

[Jianguo Zhang]

Add binding document for the ethernet on mt8188

Signed-off-by: Jianguo Zhang <jianguo.zhang@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/net/mediatek-dwmac.yaml        | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
index 61b2fb9e141b..0fa2132fa4f4 100644
--- a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
@@ -19,6 +19,7 @@ select:
       contains:
         enum:
           - mediatek,mt2712-gmac
+          - mediatek,mt8188-gmac
           - mediatek,mt8195-gmac
   required:
     - compatible
@@ -37,6 +38,11 @@ properties:
           - enum:
               - mediatek,mt8195-gmac
           - const: snps,dwmac-5.10a
+      - items:
+          - enum:
+              - mediatek,mt8188-gmac
+          - const: mediatek,mt8195-gmac
+          - const: snps,dwmac-5.10a
 
   clocks:
     minItems: 5
@@ -74,7 +80,7 @@ properties:
       or will round down. Range 0~31*170.
       For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
       or will round down. Range 0~31*550.
-      For MT8195 RGMII/RMII/MII interface, Allowed value need to be a multiple of 290,
+      For MT8188/MT8195 RGMII/RMII/MII interface, Allowed value need to be a multiple of 290,
       or will round down. Range 0~31*290.
 
   mediatek,rx-delay-ps:
@@ -84,7 +90,7 @@ properties:
       or will round down. Range 0~31*170.
       For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
       or will round down. Range 0~31*550.
-      For MT8195 RGMII/RMII/MII interface, Allowed value need to be a multiple
+      For MT8188/MT8195 RGMII/RMII/MII interface, Allowed value need to be a multiple
       of 290, or will round down. Range 0~31*290.
 
   mediatek,rmii-rxc:
-- 
2.25.1

[PATCH v5 2/4] dt-bindings: net: snps,dwmac: add clk_csr property

[Jianguo Zhang]

The clk_csr property is parsed in driver for generating MDC clock
with correct frequency. A warning('clk_csr' was unexpeted) is reported
when runing 'make_dtbs_check' because the clk_csr property
has been not documented in the binding file.

Signed-off-by: Jianguo Zhang <jianguo.zhang@mediatek.com>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 491597c02edf..4d5a56661322 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -288,6 +288,11 @@ properties:
       is supported. For example, this is used in case of SGMII and
       MAC2MAC connection.
 
+  snps,clk-csr:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Frequency division factor for MDC clock.
+
   mdio:
     $ref: mdio.yaml#
     unevaluatedProperties: false
-- 
2.25.1

Re: [PATCH v5 2/4] dt-bindings: net: snps,dwmac: add clk_csr property

[Krzysztof Kozlowski]

On 23/09/2022 07:28, Jianguo Zhang wrote:
> The clk_csr property is parsed in driver for generating MDC clock
> with correct frequency. A warning('clk_csr' was unexpeted) is reported
> when runing 'make_dtbs_check' because the clk_csr property
> has been not documented in the binding file.

Your subject is not accurate anymore. Maybe mention that instead of
existing clk_csr, you add a different property.

With commit msg fixes:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v5 2/4] dt-bindings: net: snps,dwmac: add clk_csr property

[Jianguo Zhang]

Dear Krzysztof,

	Thanks for your comment.

On Fri, 2022-09-23 at 20:11 +0200, Krzysztof Kozlowski wrote:
> On 23/09/2022 07:28, Jianguo Zhang wrote:
> > The clk_csr property is parsed in driver for generating MDC clock
> > with correct frequency. A warning('clk_csr' was unexpeted) is
> > reported
> > when runing 'make_dtbs_check' because the clk_csr property
> > has been not documented in the binding file.
> 
> Your subject is not accurate anymore. Maybe mention that instead of
> existing clk_csr, you add a different property.
> 
> With commit msg fixes:
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
We will fix commit message in next version patches.

> Best regards,
> Krzysztof
> 
BRS
Jianguo

[PATCH v5 3/4] arm64: dts: mediatek: mt2712e: Update the name of property 'clk_csr'

[Jianguo Zhang]

Update the name of property 'clk_csr' as 'snps,clk-csr' to align with
the property name in the binding file.

Signed-off-by: Jianguo Zhang <jianguo.zhang@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index 4797537cb368..e6d7453e56e0 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -756,7 +756,7 @@ eth: ethernet@1101c000 {
 		snps,mtl-tx-config = <&mtl_tx_setup>;
 		snps,txpbl = <1>;
 		snps,rxpbl = <1>;
-		clk_csr = <0>;
+		snps,clk-csr = <0>;
 		status = "disabled";
 	};
 
-- 
2.25.1

[PATCH v5 4/4] net: stmmac: Update the name of property 'clk_csr'

[Jianguo Zhang]

Update the name of property 'clk_csr' as 'snps,clk-csr' to align with
the property name in the binding file.

Signed-off-by: Jianguo Zhang <jianguo.zhang@mediatek.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 9f5cac4000da..18f9952d667f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -444,7 +444,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
 	 * or get clk_csr from device tree.
 	 */
 	plat->clk_csr = -1;
-	of_property_read_u32(np, "clk_csr", &plat->clk_csr);
+	of_property_read_u32(np, "snps,clk-csr", &plat->clk_csr);
 
 	/* "snps,phy-addr" is not a standard property. Mark it as deprecated
 	 * and warn of its use. Remove this when phy node support is added.
-- 
2.25.1

Re: [PATCH v5 4/4] net: stmmac: Update the name of property 'clk_csr'

[AngeloGioacchino Del Regno]

Il 23/09/22 07:28, Jianguo Zhang ha scritto:
> Update the name of property 'clk_csr' as 'snps,clk-csr' to align with
> the property name in the binding file.
> 
> Signed-off-by: Jianguo Zhang <jianguo.zhang@mediatek.com>
> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 9f5cac4000da..18f9952d667f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -444,7 +444,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>   	 * or get clk_csr from device tree.
>   	 */
>   	plat->clk_csr = -1;
> -	of_property_read_u32(np, "clk_csr", &plat->clk_csr);
> +	of_property_read_u32(np, "snps,clk-csr", &plat->clk_csr);

This is going to break MT2712e on old devicetrees.

The right way of doing that is to check the return value of of_property_read_u32()
for "snps,clk-csr": if the property is not found, fall back to the old "clk_csr".

Regards,
Angelo

>   
>   	/* "snps,phy-addr" is not a standard property. Mark it as deprecated
>   	 * and warn of its use. Remove this when phy node support is added.

Re: [PATCH v5 4/4] net: stmmac: Update the name of property 'clk_csr'

[Krzysztof Kozlowski]

On 23/09/2022 11:10, AngeloGioacchino Del Regno wrote:
> Il 23/09/22 07:28, Jianguo Zhang ha scritto:
>> Update the name of property 'clk_csr' as 'snps,clk-csr' to align with
>> the property name in the binding file.
>>
>> Signed-off-by: Jianguo Zhang <jianguo.zhang@mediatek.com>
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index 9f5cac4000da..18f9952d667f 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -444,7 +444,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>>   	 * or get clk_csr from device tree.
>>   	 */
>>   	plat->clk_csr = -1;
>> -	of_property_read_u32(np, "clk_csr", &plat->clk_csr);
>> +	of_property_read_u32(np, "snps,clk-csr", &plat->clk_csr);
> 
> This is going to break MT2712e on old devicetrees.
> 
> The right way of doing that is to check the return value of of_property_read_u32()
> for "snps,clk-csr": if the property is not found, fall back to the old "clk_csr".

I must admit - I don't care. That's the effect when submitter bypasses
DT bindings review (81311c03ab4d ("net: ethernet: stmmac: add management
of clk_csr property")).

If anyone wants ABI, please document the properties.

If out-of-tree users complain, please upstream your DTS or do not use
undocumented features...

Best regards,
Krzysztof

Re: [PATCH v5 4/4] net: stmmac: Update the name of property 'clk_csr'

[Krzysztof Kozlowski]

On 23/09/2022 20:14, Krzysztof Kozlowski wrote:
>> This is going to break MT2712e on old devicetrees.
>>
>> The right way of doing that is to check the return value of of_property_read_u32()
>> for "snps,clk-csr": if the property is not found, fall back to the old "clk_csr".
> 
> I must admit - I don't care. That's the effect when submitter bypasses
> DT bindings review (81311c03ab4d ("net: ethernet: stmmac: add management
> of clk_csr property")).
> 
> If anyone wants ABI, please document the properties.
> 
> If out-of-tree users complain, please upstream your DTS or do not use
> undocumented features...
> 

OTOH, as Angelo pointed out, handling old and new properties is quite
easy to achieve, so... :)

Best regards,
Krzysztof

Re: [PATCH v5 4/4] net: stmmac: Update the name of property 'clk_csr'

[Jianguo Zhang]

Dear Krzysztof,
	Thanks for your comment.

On Fri, 2022-09-23 at 20:15 +0200, Krzysztof Kozlowski wrote:
> On 23/09/2022 20:14, Krzysztof Kozlowski wrote:
> > > This is going to break MT2712e on old devicetrees.
> > > 
> > > The right way of doing that is to check the return value of
> > > of_property_read_u32()
> > > for "snps,clk-csr": if the property is not found, fall back to
> > > the old "clk_csr".
> > 
> > I must admit - I don't care. That's the effect when submitter
> > bypasses
> > DT bindings review (81311c03ab4d ("net: ethernet: stmmac: add
> > management
> > of clk_csr property")).
> > 
> > If anyone wants ABI, please document the properties.
> > 
> > If out-of-tree users complain, please upstream your DTS or do not
> > use
> > undocumented features...
> > 
> 
> OTOH, as Angelo pointed out, handling old and new properties is quite
> easy to achieve, so... :)
> 
So, the conclusion is as following:

1. add new property 'snps,clk-csr' and document it in binding file.
2. parse new property 'snps,clk-csr' firstly, if failed, fall back to
old property 'clk_csr' in driver.

Is my understanding correct?

> Best regards,
> Krzysztof
> 
BRS
Jianguo

Re: [PATCH v5 4/4] net: stmmac: Update the name of property 'clk_csr'

[Jianguo Zhang]

Dear Krzysztof,
	Thanks for your comment.

On Fri, 2022-09-23 at 20:15 +0200, Krzysztof Kozlowski wrote:
> On 23/09/2022 20:14, Krzysztof Kozlowski wrote:
> > > This is going to break MT2712e on old devicetrees.
> > > 
> > > The right way of doing that is to check the return value of
> > > of_property_read_u32()
> > > for "snps,clk-csr": if the property is not found, fall back to
> > > the old "clk_csr".
> > 
> > I must admit - I don't care. That's the effect when submitter
> > bypasses
> > DT bindings review (81311c03ab4d ("net: ethernet: stmmac: add
> > management
> > of clk_csr property")).
> > 
> > If anyone wants ABI, please document the properties.
> > 
> > If out-of-tree users complain, please upstream your DTS or do not
> > use
> > undocumented features...
> > 
> 
> OTOH, as Angelo pointed out, handling old and new properties is quite
> easy to achieve, so... :)
> 
So, the conclusion is as following:

1. add new property 'snps,clk-csr' and document it in binding file.
2. parse new property 'snps,clk-csr' firstly, if failed, fall back to
old property 'clk_csr' in driver.

Is my understanding correct?

> Best regards,
> Krzysztof
> 
BRS
Jianguo

Re: [PATCH v5 4/4] net: stmmac: Update the name of property 'clk_csr'

[AngeloGioacchino Del Regno]

Il 27/09/22 10:44, Jianguo Zhang ha scritto:
> Dear Krzysztof,
> 	Thanks for your comment.
> 
> On Fri, 2022-09-23 at 20:15 +0200, Krzysztof Kozlowski wrote:
>> On 23/09/2022 20:14, Krzysztof Kozlowski wrote:
>>>> This is going to break MT2712e on old devicetrees.
>>>>
>>>> The right way of doing that is to check the return value of
>>>> of_property_read_u32()
>>>> for "snps,clk-csr": if the property is not found, fall back to
>>>> the old "clk_csr".
>>>
>>> I must admit - I don't care. That's the effect when submitter
>>> bypasses
>>> DT bindings review (81311c03ab4d ("net: ethernet: stmmac: add
>>> management
>>> of clk_csr property")).
>>>
>>> If anyone wants ABI, please document the properties.
>>>
>>> If out-of-tree users complain, please upstream your DTS or do not
>>> use
>>> undocumented features...
>>>
>>
>> OTOH, as Angelo pointed out, handling old and new properties is quite
>> easy to achieve, so... :)
>>
> So, the conclusion is as following:
> 
> 1. add new property 'snps,clk-csr' and document it in binding file.
> 2. parse new property 'snps,clk-csr' firstly, if failed, fall back to
> old property 'clk_csr' in driver.
> 
> Is my understanding correct?

Yes, please.

I think that bindings should also get a 'clk_csr' with deprecated: true,
but that's Krzysztof's call.

Regards,
Angelo

> 
>> Best regards,
>> Krzysztof
>>
> BRS
> Jianguo
> 

Re: [PATCH v5 4/4] net: stmmac: Update the name of property 'clk_csr'

[Krzysztof Kozlowski]

On 27/09/2022 12:44, AngeloGioacchino Del Regno wrote:

>>> OTOH, as Angelo pointed out, handling old and new properties is quite
>>> easy to achieve, so... :)
>>>
>> So, the conclusion is as following:
>>
>> 1. add new property 'snps,clk-csr' and document it in binding file.
>> 2. parse new property 'snps,clk-csr' firstly, if failed, fall back to
>> old property 'clk_csr' in driver.
>>
>> Is my understanding correct?
> 
> Yes, please.
> 
> I think that bindings should also get a 'clk_csr' with deprecated: true,
> but that's Krzysztof's call.

The property was never documented, so I think we can skip it as deprecated.

Best regards,
Krzysztof

Re: [PATCH v5 4/4] net: stmmac: Update the name of property 'clk_csr'

[Jianguo Zhang]

Dear Krzysztof,

	Thanks for your comment.

On Wed, 2022-09-28 at 09:17 +0200, Krzysztof Kozlowski wrote:
> On 27/09/2022 12:44, AngeloGioacchino Del Regno wrote:
> 
> > > > OTOH, as Angelo pointed out, handling old and new properties is
> > > > quite
> > > > easy to achieve, so... :)
> > > > 
> > > 
> > > So, the conclusion is as following:
> > > 
> > > 1. add new property 'snps,clk-csr' and document it in binding
> > > file.
> > > 2. parse new property 'snps,clk-csr' firstly, if failed, fall
> > > back to
> > > old property 'clk_csr' in driver.
> > > 
> > > Is my understanding correct?
> > 
> > Yes, please.
> > 
> > I think that bindings should also get a 'clk_csr' with deprecated:
> > true,
> > but that's Krzysztof's call.
> 
> The property was never documented, so I think we can skip it as
> deprecated.
> 
We will send next version patches according to the conclusion.
> Best regards,
> Krzysztof
> 
BRS
Jianguo