[PATCH 0/5] net: ethernet: stmmac: some fixes and optimization

[PATCH 0/5] net: ethernet: stmmac: some fixes and optimization

[Christophe Roullier]

Some improvements (manage syscfg as optional clock, update slew rate of
ETH_MDIO pin, Enable gating of the MAC TX clock during TX low-power mode)
Fix warning build message when W=1

Christophe Roullier (5):
  net: ethernet: stmmac: Add support for syscfg clock
  net: ethernet: stmmac: fix warning when w=1 option is used during
    build
  ARM: dts: stm32: remove syscfg clock on stm32mp157c ethernet
  ARM: dts: stm32: adjust slew rate for Ethernet
  ARM: dts: stm32: Enable gating of the MAC TX clock during TX low-power
    mode on stm32mp157c

 arch/arm/boot/dts/stm32mp157-pinctrl.dtsi     |  9 +++-
 arch/arm/boot/dts/stm32mp157c.dtsi            |  7 ++--
 .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 42 ++++++++++++-------
 3 files changed, 38 insertions(+), 20 deletions(-)

-- 
2.17.1

[PATCH 1/5] net: ethernet: stmmac: Add support for syscfg clock

[Christophe Roullier]

Add optional support for syscfg clock in dwmac-stm32.c
Now Syscfg clock is activated automatically when syscfg
registers are used

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 36 +++++++++++++------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index 4ef041bdf6a1..7e6619868cc1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -152,23 +152,32 @@ static int stm32mp1_clk_prepare(struct stm32_dwmac *dwmac, bool prepare)
 	int ret = 0;
 
 	if (prepare) {
-		ret = clk_prepare_enable(dwmac->syscfg_clk);
-		if (ret)
-			return ret;
-
+		if (dwmac->syscfg_clk) {
+			ret = clk_prepare_enable(dwmac->syscfg_clk);
+			if (ret)
+				return ret;
+		}
 		if (dwmac->clk_eth_ck) {
 			ret = clk_prepare_enable(dwmac->clk_eth_ck);
 			if (ret) {
-				clk_disable_unprepare(dwmac->syscfg_clk);
+				if (dwmac->syscfg_clk)
+					goto unprepare_syscfg;
 				return ret;
 			}
 		}
 	} else {
-		clk_disable_unprepare(dwmac->syscfg_clk);
+		if (dwmac->syscfg_clk)
+			clk_disable_unprepare(dwmac->syscfg_clk);
+
 		if (dwmac->clk_eth_ck)
 			clk_disable_unprepare(dwmac->clk_eth_ck);
 	}
 	return ret;
+
+unprepare_syscfg:
+	clk_disable_unprepare(dwmac->syscfg_clk);
+
+	return ret;
 }
 
 static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
@@ -296,7 +305,7 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct device_node *np = dev->of_node;
-	int err = 0;
+	int err;
 
 	/* Gigabit Ethernet 125MHz clock selection. */
 	dwmac->eth_clk_sel_reg = of_property_read_bool(np, "st,eth-clk-sel");
@@ -320,13 +329,17 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
 		return PTR_ERR(dwmac->clk_ethstp);
 	}
 
-	/*  Clock for sysconfig */
+	/*  Optional Clock for sysconfig */
 	dwmac->syscfg_clk = devm_clk_get(dev, "syscfg-clk");
 	if (IS_ERR(dwmac->syscfg_clk)) {
-		dev_err(dev, "No syscfg clock provided...\n");
-		return PTR_ERR(dwmac->syscfg_clk);
+		err = PTR_ERR(dwmac->syscfg_clk);
+		if (err != -ENOENT)
+			return err;
+		dwmac->syscfg_clk = NULL;
 	}
 
+	err = 0;
+
 	/* Get IRQ information early to have an ability to ask for deferred
 	 * probe if needed before we went too far with resource allocation.
 	 */
@@ -436,7 +449,8 @@ static int stm32mp1_suspend(struct stm32_dwmac *dwmac)
 		return ret;
 
 	clk_disable_unprepare(dwmac->clk_tx);
-	clk_disable_unprepare(dwmac->syscfg_clk);
+	if (dwmac->syscfg_clk)
+		clk_disable_unprepare(dwmac->syscfg_clk);
 	if (dwmac->clk_eth_ck)
 		clk_disable_unprepare(dwmac->clk_eth_ck);
 
-- 
2.17.1

[PATCH 2/5] net: ethernet: stmmac: fix warning when w=1 option is used during build

[Christophe Roullier]

This patch fix the following warning:

warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
  int val, ret;

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index 7e6619868cc1..167a5e99960a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -184,7 +184,7 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
 {
 	struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
 	u32 reg = dwmac->mode_reg;
-	int val, ret;
+	int val;
 
 	switch (plat_dat->interface) {
 	case PHY_INTERFACE_MODE_MII:
@@ -220,8 +220,8 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
 	}
 
 	/* Need to update PMCCLRR (clear register) */
-	ret = regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET,
-			   dwmac->ops->syscfg_eth_mask);
+	regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET,
+		     dwmac->ops->syscfg_eth_mask);
 
 	/* Update PMCSETR (set register) */
 	return regmap_update_bits(dwmac->regmap, reg,
-- 
2.17.1

[PATCH 3/5] ARM: dts: stm32: remove syscfg clock on stm32mp157c ethernet

[Christophe Roullier]

Syscfg is now activated automatically when syscfg registers are used

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 arch/arm/boot/dts/stm32mp157c.dtsi | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index 0c4e6ebc3529..f51d6222a0e8 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -1285,13 +1285,11 @@
 			clock-names = "stmmaceth",
 				      "mac-clk-tx",
 				      "mac-clk-rx",
-				      "ethstp",
-				      "syscfg-clk";
+				      "ethstp";
 			clocks = <&rcc ETHMAC>,
 				 <&rcc ETHTX>,
 				 <&rcc ETHRX>,
-				 <&rcc ETHSTP>,
-				 <&rcc SYSCFG>;
+				 <&rcc ETHSTP>;
 			st,syscon = <&syscfg 0x4>;
 			snps,mixed-burst;
 			snps,pbl = <2>;
-- 
2.17.1

[PATCH 4/5] ARM: dts: stm32: adjust slew rate for Ethernet

[Christophe Roullier]

ETH_MDIO slew-rate should be set to "0" instead of "2"

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
index df6470133574..7667fe758957 100644
--- a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
@@ -239,13 +239,18 @@
 						 <STM32_PINMUX('C', 2, AF11)>, /* ETH_RGMII_TXD2 */
 						 <STM32_PINMUX('E', 2, AF11)>, /* ETH_RGMII_TXD3 */
 						 <STM32_PINMUX('B', 11, AF11)>, /* ETH_RGMII_TX_CTL */
-						 <STM32_PINMUX('A', 2, AF11)>, /* ETH_MDIO */
 						 <STM32_PINMUX('C', 1, AF11)>; /* ETH_MDC */
 					bias-disable;
 					drive-push-pull;
-					slew-rate = <3>;
+					slew-rate = <2>;
 				};
 				pins2 {
+					pinmux = <STM32_PINMUX('A', 2, AF11)>; /* ETH_MDIO */
+					bias-disable;
+					drive-push-pull;
+					slew-rate = <0>;
+				};
+				pins3 {
 					pinmux = <STM32_PINMUX('C', 4, AF11)>, /* ETH_RGMII_RXD0 */
 						 <STM32_PINMUX('C', 5, AF11)>, /* ETH_RGMII_RXD1 */
 						 <STM32_PINMUX('B', 0, AF11)>, /* ETH_RGMII_RXD2 */
-- 
2.17.1

[PATCH 5/5] ARM: dts: stm32: Enable gating of the MAC TX clock during TX low-power mode on stm32mp157c

[Christophe Roullier]

When there is no activity on ethernet phy link, the ETH_GTX_CLK is cut

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 arch/arm/boot/dts/stm32mp157c.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index f51d6222a0e8..d78dfc44a1fb 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -1293,6 +1293,7 @@
 			st,syscon = <&syscfg 0x4>;
 			snps,mixed-burst;
 			snps,pbl = <2>;
+			snps,en-tx-lpi-clockgating;
 			snps,axi-config = <&stmmac_axi_config_0>;
 			snps,tso;
 			status = "disabled";
-- 
2.17.1

Re: [PATCH 0/5] net: ethernet: stmmac: some fixes and optimization

[Jakub Kicinski]

On Fri, 20 Sep 2019 07:38:12 +0200, Christophe Roullier wrote:
> Some improvements (manage syscfg as optional clock, update slew rate of
> ETH_MDIO pin, Enable gating of the MAC TX clock during TX low-power mode)
> Fix warning build message when W=1

There seems to be some new features/cleanups (or improvements as
you say) here. Could you explain the negative impact not applying 
these changes will have? Patches 1 and 3 in particular.

net-next is now closed [1], and will reopen some time after the merge
window is over. For now we are only expecting fixes for the net tree.

Could you (a) provide stronger motivation these changes are fixes; or
(b) separate the fixes from improvements?

Thank you!

[1] https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html

Re: [PATCH 0/5] net: ethernet: stmmac: some fixes and optimization

[Christophe ROULLIER]

Hi Jakub, all,

It is not urgent, no problem to wait next merge window (release 5.5)

For patch 1 and 3, it is improvement/cleanup because now syscfg clock is 
not mandatory (I put code backward compatible).

Regards,

Christophe

On 9/23/19 12:12 AM, Jakub Kicinski wrote:
> On Fri, 20 Sep 2019 07:38:12 +0200, Christophe Roullier wrote:
>> Some improvements (manage syscfg as optional clock, update slew rate of
>> ETH_MDIO pin, Enable gating of the MAC TX clock during TX low-power mode)
>> Fix warning build message when W=1
> There seems to be some new features/cleanups (or improvements as
> you say) here. Could you explain the negative impact not applying
> these changes will have? Patches 1 and 3 in particular.
>
> net-next is now closed [1], and will reopen some time after the merge
> window is over. For now we are only expecting fixes for the net tree.
>
> Could you (a) provide stronger motivation these changes are fixes; or
> (b) separate the fixes from improvements?
>
> Thank you!
>
> [1] https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html

Re: [PATCH 1/5] net: ethernet: stmmac: Add support for syscfg clock

[Alexandre Torgue]

Hi,

On 9/20/19 7:38 AM, Christophe Roullier wrote:
> Add optional support for syscfg clock in dwmac-stm32.c
> Now Syscfg clock is activated automatically when syscfg
> registers are used
> 
> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
> ---
>   .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 36 +++++++++++++------
>   1 file changed, 25 insertions(+), 11 deletions(-)

Acked-by: Alexandre TORGUE <alexandre.torgue@st.com>

> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> index 4ef041bdf6a1..7e6619868cc1 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> @@ -152,23 +152,32 @@ static int stm32mp1_clk_prepare(struct stm32_dwmac *dwmac, bool prepare)
>   	int ret = 0;
>   
>   	if (prepare) {
> -		ret = clk_prepare_enable(dwmac->syscfg_clk);
> -		if (ret)
> -			return ret;
> -
> +		if (dwmac->syscfg_clk) {
> +			ret = clk_prepare_enable(dwmac->syscfg_clk);
> +			if (ret)
> +				return ret;
> +		}
>   		if (dwmac->clk_eth_ck) {
>   			ret = clk_prepare_enable(dwmac->clk_eth_ck);
>   			if (ret) {
> -				clk_disable_unprepare(dwmac->syscfg_clk);
> +				if (dwmac->syscfg_clk)
> +					goto unprepare_syscfg;
>   				return ret;
>   			}
>   		}
>   	} else {
> -		clk_disable_unprepare(dwmac->syscfg_clk);
> +		if (dwmac->syscfg_clk)
> +			clk_disable_unprepare(dwmac->syscfg_clk);
> +
>   		if (dwmac->clk_eth_ck)
>   			clk_disable_unprepare(dwmac->clk_eth_ck);
>   	}
>   	return ret;
> +
> +unprepare_syscfg:
> +	clk_disable_unprepare(dwmac->syscfg_clk);
> +
> +	return ret;
>   }
>   
>   static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
> @@ -296,7 +305,7 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
>   {
>   	struct platform_device *pdev = to_platform_device(dev);
>   	struct device_node *np = dev->of_node;
> -	int err = 0;
> +	int err;
>   
>   	/* Gigabit Ethernet 125MHz clock selection. */
>   	dwmac->eth_clk_sel_reg = of_property_read_bool(np, "st,eth-clk-sel");
> @@ -320,13 +329,17 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
>   		return PTR_ERR(dwmac->clk_ethstp);
>   	}
>   
> -	/*  Clock for sysconfig */
> +	/*  Optional Clock for sysconfig */
>   	dwmac->syscfg_clk = devm_clk_get(dev, "syscfg-clk");
>   	if (IS_ERR(dwmac->syscfg_clk)) {
> -		dev_err(dev, "No syscfg clock provided...\n");
> -		return PTR_ERR(dwmac->syscfg_clk);
> +		err = PTR_ERR(dwmac->syscfg_clk);
> +		if (err != -ENOENT)
> +			return err;
> +		dwmac->syscfg_clk = NULL;
>   	}
>   
> +	err = 0;
> +
>   	/* Get IRQ information early to have an ability to ask for deferred
>   	 * probe if needed before we went too far with resource allocation.
>   	 */
> @@ -436,7 +449,8 @@ static int stm32mp1_suspend(struct stm32_dwmac *dwmac)
>   		return ret;
>   
>   	clk_disable_unprepare(dwmac->clk_tx);
> -	clk_disable_unprepare(dwmac->syscfg_clk);
> +	if (dwmac->syscfg_clk)
> +		clk_disable_unprepare(dwmac->syscfg_clk);
>   	if (dwmac->clk_eth_ck)
>   		clk_disable_unprepare(dwmac->clk_eth_ck);
>   
> 

Re: [PATCH 2/5] net: ethernet: stmmac: fix warning when w=1 option is used during build

[Alexandre Torgue]

Hi,

On 9/20/19 7:38 AM, Christophe Roullier wrote:
> This patch fix the following warning:
> 
> warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>    int val, ret;
> 
> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
> ---

Acked-by: Alexandre TORGUE <alexandre.torgue@st.com>

>   drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> index 7e6619868cc1..167a5e99960a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> @@ -184,7 +184,7 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>   {
>   	struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
>   	u32 reg = dwmac->mode_reg;
> -	int val, ret;
> +	int val;
>   
>   	switch (plat_dat->interface) {
>   	case PHY_INTERFACE_MODE_MII:
> @@ -220,8 +220,8 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>   	}
>   
>   	/* Need to update PMCCLRR (clear register) */
> -	ret = regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET,
> -			   dwmac->ops->syscfg_eth_mask);
> +	regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET,
> +		     dwmac->ops->syscfg_eth_mask);
>   
>   	/* Update PMCSETR (set register) */
>   	return regmap_update_bits(dwmac->regmap, reg,
> 

Re: [PATCH 0/5] net: ethernet: stmmac: some fixes and optimization

[Alexandre Torgue]

Hi Christophe

On 9/20/19 7:38 AM, Christophe Roullier wrote:
> Some improvements (manage syscfg as optional clock, update slew rate of
> ETH_MDIO pin, Enable gating of the MAC TX clock during TX low-power mode)
> Fix warning build message when W=1
> 
> Christophe Roullier (5):
>    net: ethernet: stmmac: Add support for syscfg clock
>    net: ethernet: stmmac: fix warning when w=1 option is used during
>      build
>    ARM: dts: stm32: remove syscfg clock on stm32mp157c ethernet
>    ARM: dts: stm32: adjust slew rate for Ethernet
>    ARM: dts: stm32: Enable gating of the MAC TX clock during TX low-power
>      mode on stm32mp157c
> 

DT patches will be applied on stm32-next after dwmac-stm32 patches merge 
in net-next.

>   arch/arm/boot/dts/stm32mp157-pinctrl.dtsi     |  9 +++-
>   arch/arm/boot/dts/stm32mp157c.dtsi            |  7 ++--
>   .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 42 ++++++++++++-------
>   3 files changed, 38 insertions(+), 20 deletions(-)
> 

Re: [PATCH 0/5] net: ethernet: stmmac: some fixes and optimization

[Christophe ROULLIER]

Hi all,

Just a "gentleman ping" about this series

Regards,

Christophe.

On 9/23/19 9:46 AM, Christophe ROULLIER wrote:
> Hi Jakub, all,
>
> It is not urgent, no problem to wait next merge window (release 5.5)
>
> For patch 1 and 3, it is improvement/cleanup because now syscfg clock 
> is not mandatory (I put code backward compatible).
>
> Regards,
>
> Christophe
>
> On 9/23/19 12:12 AM, Jakub Kicinski wrote:
>> On Fri, 20 Sep 2019 07:38:12 +0200, Christophe Roullier wrote:
>>> Some improvements (manage syscfg as optional clock, update slew rate of
>>> ETH_MDIO pin, Enable gating of the MAC TX clock during TX low-power 
>>> mode)
>>> Fix warning build message when W=1
>> There seems to be some new features/cleanups (or improvements as
>> you say) here. Could you explain the negative impact not applying
>> these changes will have? Patches 1 and 3 in particular.
>>
>> net-next is now closed [1], and will reopen some time after the merge
>> window is over. For now we are only expecting fixes for the net tree.
>>
>> Could you (a) provide stronger motivation these changes are fixes; or
>> (b) separate the fixes from improvements?
>>
>> Thank you!
>>
>> [1] https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html