netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET
@ 2021-06-05 17:35 Matthew Hagan
  2021-06-05 17:35 ` [PATCH 2/3] ARM: dts: qcom: add ahb reset to ipq806x-gmac Matthew Hagan
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Matthew Hagan @ 2021-06-05 17:35 UTC (permalink / raw)
  Cc: Matthew Hagan, David S. Miller, Jakub Kicinski, Rob Herring,
	Andy Gross, Bjorn Andersson, Giuseppe Cavallaro,
	Alexandre Torgue, Jose Abreu, Maxime Coquelin, Philipp Zabel,
	Voon Weifeng, Ong Boon Leong, Wong Vee Khee, Tan Tee Min, Wong,
	Vee Khee, Fugang Duan, netdev, devicetree, linux-kernel,
	linux-arm-msm, linux-stm32, linux-arm-kernel

We are currently assuming that GMAC_AHB_RESET will already be deasserted
by the bootloader. However if this has not been done, probing of the GMAC
will fail. To remedy this we must ensure GMAC_AHB_RESET has been deasserted
prior to probing.

Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 7 +++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +++++++
 include/linux/stmmac.h                                | 1 +
 3 files changed, 15 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6d41dd6f9f7a..1e28058b65a8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6840,6 +6840,13 @@ int stmmac_dvr_probe(struct device *device,
 			reset_control_reset(priv->plat->stmmac_rst);
 	}
 
+	if (priv->plat->stmmac_ahb_rst) {
+		ret = reset_control_deassert(priv->plat->stmmac_ahb_rst);
+		if (ret == -ENOTSUPP)
+			dev_err(priv->device,
+				"unable to bring out of ahb reset\n");
+	}
+
 	/* Init MAC and get the capabilities */
 	ret = stmmac_hw_init(priv);
 	if (ret)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 97a1fedcc9ac..d8ae58bdbbe3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -600,6 +600,13 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
 		goto error_hw_init;
 	}
 
+	plat->stmmac_ahb_rst = devm_reset_control_get_optional_shared(
+							&pdev->dev, "ahb");
+	if (IS_ERR(plat->stmmac_ahb_rst)) {
+		ret = plat->stmmac_ahb_rst;
+		goto error_hw_init;
+	}
+
 	return plat;
 
 error_hw_init:
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index e55a4807e3ea..9b6a64f3e3dc 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -239,6 +239,7 @@ struct plat_stmmacenet_data {
 	unsigned int mult_fact_100ns;
 	s32 ptp_max_adj;
 	struct reset_control *stmmac_rst;
+	struct reset_control *stmmac_ahb_rst;
 	struct stmmac_axi *axi;
 	int has_gmac4;
 	bool has_sun8i;
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] ARM: dts: qcom: add ahb reset to ipq806x-gmac
  2021-06-05 17:35 [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET Matthew Hagan
@ 2021-06-05 17:35 ` Matthew Hagan
  2021-06-05 17:35 ` [PATCH 3/3] dt-bindings: net: stmmac: add ahb reset to example Matthew Hagan
  2021-06-06  3:24 ` [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET Bjorn Andersson
  2 siblings, 0 replies; 11+ messages in thread
From: Matthew Hagan @ 2021-06-05 17:35 UTC (permalink / raw)
  Cc: Matthew Hagan, David S. Miller, Jakub Kicinski, Rob Herring,
	Andy Gross, Bjorn Andersson, Giuseppe Cavallaro,
	Alexandre Torgue, Jose Abreu, Maxime Coquelin, Philipp Zabel,
	Voon Weifeng, Ong Boon Leong, Wong Vee Khee, Tan Tee Min, Wong,
	Vee Khee, netdev, devicetree, linux-kernel, linux-arm-msm,
	linux-stm32, linux-arm-kernel

Add GMAC_AHB_RESET to the resets property of each gmac node.

Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
---
 arch/arm/boot/dts/qcom-ipq8064.dtsi | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index 98995ead4413..1dbceaf3454b 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -643,8 +643,9 @@ gmac0: ethernet@37000000 {
 			clocks = <&gcc GMAC_CORE1_CLK>;
 			clock-names = "stmmaceth";
 
-			resets = <&gcc GMAC_CORE1_RESET>;
-			reset-names = "stmmaceth";
+			resets = <&gcc GMAC_CORE1_RESET>,
+				 <&gcc GMAC_AHB_RESET>;
+			reset-names = "stmmaceth", "ahb";
 
 			status = "disabled";
 		};
@@ -666,8 +667,9 @@ gmac1: ethernet@37200000 {
 			clocks = <&gcc GMAC_CORE2_CLK>;
 			clock-names = "stmmaceth";
 
-			resets = <&gcc GMAC_CORE2_RESET>;
-			reset-names = "stmmaceth";
+			resets = <&gcc GMAC_CORE2_RESET>,
+				 <&gcc GMAC_AHB_RESET>;
+			reset-names = "stmmaceth", "ahb";
 
 			status = "disabled";
 		};
@@ -689,8 +691,9 @@ gmac2: ethernet@37400000 {
 			clocks = <&gcc GMAC_CORE3_CLK>;
 			clock-names = "stmmaceth";
 
-			resets = <&gcc GMAC_CORE3_RESET>;
-			reset-names = "stmmaceth";
+			resets = <&gcc GMAC_CORE3_RESET>,
+				 <&gcc GMAC_AHB_RESET>;
+			reset-names = "stmmaceth", "ahb";
 
 			status = "disabled";
 		};
@@ -712,8 +715,9 @@ gmac3: ethernet@37600000 {
 			clocks = <&gcc GMAC_CORE4_CLK>;
 			clock-names = "stmmaceth";
 
-			resets = <&gcc GMAC_CORE4_RESET>;
-			reset-names = "stmmaceth";
+			resets = <&gcc GMAC_CORE4_RESET>,
+				 <&gcc GMAC_AHB_RESET>;
+			reset-names = "stmmaceth", "ahb";
 
 			status = "disabled";
 		};
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] dt-bindings: net: stmmac: add ahb reset to example
  2021-06-05 17:35 [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET Matthew Hagan
  2021-06-05 17:35 ` [PATCH 2/3] ARM: dts: qcom: add ahb reset to ipq806x-gmac Matthew Hagan
@ 2021-06-05 17:35 ` Matthew Hagan
  2021-06-06  3:24 ` [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET Bjorn Andersson
  2 siblings, 0 replies; 11+ messages in thread
From: Matthew Hagan @ 2021-06-05 17:35 UTC (permalink / raw)
  Cc: Matthew Hagan, David S. Miller, Jakub Kicinski, Rob Herring,
	Andy Gross, Bjorn Andersson, Giuseppe Cavallaro,
	Alexandre Torgue, Jose Abreu, Maxime Coquelin, Philipp Zabel,
	Ong Boon Leong, Voon Weifeng, Wong Vee Khee, Tan Tee Min, Wong,
	Vee Khee, netdev, devicetree, linux-kernel, linux-arm-msm,
	linux-stm32, linux-arm-kernel

Add ahb reset to the reset properties within the example gmac node.

Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
---
 Documentation/devicetree/bindings/net/ipq806x-dwmac.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ipq806x-dwmac.txt b/Documentation/devicetree/bindings/net/ipq806x-dwmac.txt
index 6d7ab4e524d4..ef5fd9f0b156 100644
--- a/Documentation/devicetree/bindings/net/ipq806x-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/ipq806x-dwmac.txt
@@ -30,6 +30,7 @@ Example:
 		clocks = <&gcc GMAC_CORE1_CLK>;
 		clock-names = "stmmaceth";
 
-		resets = <&gcc GMAC_CORE1_RESET>;
-		reset-names = "stmmaceth";
+		resets = <&gcc GMAC_CORE1_RESET>,
+			 <&gcc GMAC_AHB_RESET>;
+		reset-names = "stmmaceth", "ahb";
 	};
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET
  2021-06-05 17:35 [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET Matthew Hagan
  2021-06-05 17:35 ` [PATCH 2/3] ARM: dts: qcom: add ahb reset to ipq806x-gmac Matthew Hagan
  2021-06-05 17:35 ` [PATCH 3/3] dt-bindings: net: stmmac: add ahb reset to example Matthew Hagan
@ 2021-06-06  3:24 ` Bjorn Andersson
  2021-06-06  9:36   ` Matthew Hagan
  2021-06-06 14:38   ` [PATCH 1/3] " Matthew Hagan
  2 siblings, 2 replies; 11+ messages in thread
From: Bjorn Andersson @ 2021-06-06  3:24 UTC (permalink / raw)
  To: Matthew Hagan
  Cc: David S. Miller, Jakub Kicinski, Rob Herring, Andy Gross,
	Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	Maxime Coquelin, Philipp Zabel, Voon Weifeng, Ong Boon Leong,
	Wong Vee Khee, Tan Tee Min, Wong, Vee Khee, Fugang Duan, netdev,
	devicetree, linux-kernel, linux-arm-msm, linux-stm32,
	linux-arm-kernel

On Sat 05 Jun 12:35 CDT 2021, Matthew Hagan wrote:

> We are currently assuming that GMAC_AHB_RESET will already be deasserted
> by the bootloader. However if this has not been done, probing of the GMAC
> will fail. To remedy this we must ensure GMAC_AHB_RESET has been deasserted
> prior to probing.
> 

Sounds good, just some small style comments below.

> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 7 +++++++
>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +++++++
>  include/linux/stmmac.h                                | 1 +
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6d41dd6f9f7a..1e28058b65a8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6840,6 +6840,13 @@ int stmmac_dvr_probe(struct device *device,
>  			reset_control_reset(priv->plat->stmmac_rst);
>  	}
>  
> +	if (priv->plat->stmmac_ahb_rst) {

You don't need this conditional, stmmac_ahb_rst will be NULL if not
specified and you can reset_control_deassert(NULL) without any problems.

> +		ret = reset_control_deassert(priv->plat->stmmac_ahb_rst);
> +		if (ret == -ENOTSUPP)
> +			dev_err(priv->device,
> +				"unable to bring out of ahb reset\n");

No need to wrap this line.

> +	}
> +
>  	/* Init MAC and get the capabilities */
>  	ret = stmmac_hw_init(priv);
>  	if (ret)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 97a1fedcc9ac..d8ae58bdbbe3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -600,6 +600,13 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>  		goto error_hw_init;
>  	}
>  
> +	plat->stmmac_ahb_rst = devm_reset_control_get_optional_shared(
> +							&pdev->dev, "ahb");
> +	if (IS_ERR(plat->stmmac_ahb_rst)) {
> +		ret = plat->stmmac_ahb_rst;

You need a PTR_ERR() around the plat->stmmac_ahb_rst.

Regards,
Bjorn

> +		goto error_hw_init;
> +	}
> +
>  	return plat;
>  
>  error_hw_init:
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index e55a4807e3ea..9b6a64f3e3dc 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -239,6 +239,7 @@ struct plat_stmmacenet_data {
>  	unsigned int mult_fact_100ns;
>  	s32 ptp_max_adj;
>  	struct reset_control *stmmac_rst;
> +	struct reset_control *stmmac_ahb_rst;
>  	struct stmmac_axi *axi;
>  	int has_gmac4;
>  	bool has_sun8i;
> -- 
> 2.26.3
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET
  2021-06-06  3:24 ` [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET Bjorn Andersson
@ 2021-06-06  9:36   ` Matthew Hagan
  2021-06-06 10:30     ` [PATCH v2] " Matthew Hagan
  2021-06-06 14:38   ` [PATCH 1/3] " Matthew Hagan
  1 sibling, 1 reply; 11+ messages in thread
From: Matthew Hagan @ 2021-06-06  9:36 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: David S. Miller, Jakub Kicinski, Rob Herring, Andy Gross,
	Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	Maxime Coquelin, Philipp Zabel, Voon Weifeng, Ong Boon Leong,
	Wong Vee Khee, Tan Tee Min, Wong, Vee Khee, Fugang Duan, netdev,
	devicetree, linux-kernel, linux-arm-msm, linux-stm32,
	linux-arm-kernel

On 06/06/2021 04:24, Bjorn Andersson wrote:

> On Sat 05 Jun 12:35 CDT 2021, Matthew Hagan wrote:
>
>> We are currently assuming that GMAC_AHB_RESET will already be deasserted
>> by the bootloader. However if this has not been done, probing of the GMAC
>> will fail. To remedy this we must ensure GMAC_AHB_RESET has been deasserted
>> prior to probing.
>>
> Sounds good, just some small style comments below.
>
>> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 7 +++++++
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +++++++
>>  include/linux/stmmac.h                                | 1 +
>>  3 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 6d41dd6f9f7a..1e28058b65a8 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -6840,6 +6840,13 @@ int stmmac_dvr_probe(struct device *device,
>>  			reset_control_reset(priv->plat->stmmac_rst);
>>  	}
>>  
>> +	if (priv->plat->stmmac_ahb_rst) {
> You don't need this conditional, stmmac_ahb_rst will be NULL if not
> specified and you can reset_control_deassert(NULL) without any problems.
>
>> +		ret = reset_control_deassert(priv->plat->stmmac_ahb_rst);
>> +		if (ret == -ENOTSUPP)
>> +			dev_err(priv->device,
>> +				"unable to bring out of ahb reset\n");
> No need to wrap this line.
>
>> +	}
>> +
>>  	/* Init MAC and get the capabilities */
>>  	ret = stmmac_hw_init(priv);
>>  	if (ret)
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index 97a1fedcc9ac..d8ae58bdbbe3 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -600,6 +600,13 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>>  		goto error_hw_init;
>>  	}
>>  
>> +	plat->stmmac_ahb_rst = devm_reset_control_get_optional_shared(
>> +							&pdev->dev, "ahb");
>> +	if (IS_ERR(plat->stmmac_ahb_rst)) {
>> +		ret = plat->stmmac_ahb_rst;
> You need a PTR_ERR() around the plat->stmmac_ahb_rst.
>
> Regards,
> Bjorn
>
>> +		goto error_hw_init;
>> +	}
>> +
>>  	return plat;
>>  
>>  error_hw_init:
>> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
>> index e55a4807e3ea..9b6a64f3e3dc 100644
>> --- a/include/linux/stmmac.h
>> +++ b/include/linux/stmmac.h
>> @@ -239,6 +239,7 @@ struct plat_stmmacenet_data {
>>  	unsigned int mult_fact_100ns;
>>  	s32 ptp_max_adj;
>>  	struct reset_control *stmmac_rst;
>> +	struct reset_control *stmmac_ahb_rst;
>>  	struct stmmac_axi *axi;
>>  	int has_gmac4;
>>  	bool has_sun8i;
>> -- 
>> 2.26.3
>>
>>
Thanks for the review. Will submit a v2 shortly.

Matthew


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] net: stmmac: explicitly deassert GMAC_AHB_RESET
  2021-06-06  9:36   ` Matthew Hagan
@ 2021-06-06 10:30     ` Matthew Hagan
  2021-06-07  9:45       ` Philipp Zabel
  2021-06-08 18:59       ` [PATCH v3] " Matthew Hagan
  0 siblings, 2 replies; 11+ messages in thread
From: Matthew Hagan @ 2021-06-06 10:30 UTC (permalink / raw)
  Cc: bjorn.andersson, Matthew Hagan, Giuseppe Cavallaro,
	Alexandre Torgue, Jose Abreu, David S. Miller, Jakub Kicinski,
	Maxime Coquelin, Philipp Zabel, Voon Weifeng, Ong Boon Leong,
	Wong Vee Khee, Tan Tee Min, Wong, Vee Khee,
	Mohammad Athari Bin Ismail, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel

We are currently assuming that GMAC_AHB_RESET will already be deasserted
by the bootloader. However if this has not been done, probing of the GMAC
will fail. To remedy this we must ensure GMAC_AHB_RESET has been deasserted
prior to probing.

v2 changes:
 - remove NULL condition check for stmmac_ahb_rst in stmmac_main.c
 - unwrap dev_err() message in stmmac_main.c
 - add PTR_ERR() around plat->stmmac_ahb_rst in stmmac_platform.c

Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +++++++
 include/linux/stmmac.h                                | 1 +
 3 files changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6d41dd6f9f7a..0d4cb423cbbd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6840,6 +6840,10 @@ int stmmac_dvr_probe(struct device *device,
 			reset_control_reset(priv->plat->stmmac_rst);
 	}
 
+	ret = reset_control_deassert(priv->plat->stmmac_ahb_rst);
+	if (ret == -ENOTSUPP)
+		dev_err(priv->device, "unable to bring out of ahb reset\n");
+
 	/* Init MAC and get the capabilities */
 	ret = stmmac_hw_init(priv);
 	if (ret)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 97a1fedcc9ac..a178181f6a24 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -600,6 +600,13 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
 		goto error_hw_init;
 	}
 
+	plat->stmmac_ahb_rst = devm_reset_control_get_optional_shared(
+							&pdev->dev, "ahb");
+	if (IS_ERR(plat->stmmac_ahb_rst)) {
+		ret = PTR_ERR(plat->stmmac_ahb_rst);
+		goto error_hw_init;
+	}
+
 	return plat;
 
 error_hw_init:
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index e55a4807e3ea..9b6a64f3e3dc 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -239,6 +239,7 @@ struct plat_stmmacenet_data {
 	unsigned int mult_fact_100ns;
 	s32 ptp_max_adj;
 	struct reset_control *stmmac_rst;
+	struct reset_control *stmmac_ahb_rst;
 	struct stmmac_axi *axi;
 	int has_gmac4;
 	bool has_sun8i;
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET
  2021-06-06  3:24 ` [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET Bjorn Andersson
  2021-06-06  9:36   ` Matthew Hagan
@ 2021-06-06 14:38   ` Matthew Hagan
  1 sibling, 0 replies; 11+ messages in thread
From: Matthew Hagan @ 2021-06-06 14:38 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: David S. Miller, Jakub Kicinski, Rob Herring, Andy Gross,
	Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	Maxime Coquelin, Philipp Zabel, Voon Weifeng, Ong Boon Leong,
	Wong Vee Khee, Tan Tee Min, Wong, Vee Khee, Fugang Duan, netdev,
	devicetree, linux-kernel, linux-arm-msm, linux-stm32,
	linux-arm-kernel

On 06/06/2021 04:24, Bjorn Andersson wrote:

>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index 97a1fedcc9ac..d8ae58bdbbe3 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -600,6 +600,13 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>>  		goto error_hw_init;
>>  	}
>>  
>> +	plat->stmmac_ahb_rst = devm_reset_control_get_optional_shared(
>> +							&pdev->dev, "ahb");
>> +	if (IS_ERR(plat->stmmac_ahb_rst)) {
>> +		ret = plat->stmmac_ahb_rst;
> You need a PTR_ERR() around the plat->stmmac_ahb_rst.

This is giving a warning. Shouldn't v1 be kept as it is here? Please refer
to "net: stmmac: platform: use optional clk/reset get APIs" [1] which
modified error handling for plat->stmmac_rst. PTR_ERR() would then be
called by the parent function on the returned value of ret.

[1]: https://lore.kernel.org/netdev/20201112092606.5173aa6f@xhacker.debian/

Thanks,
Matthew


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] net: stmmac: explicitly deassert GMAC_AHB_RESET
  2021-06-06 10:30     ` [PATCH v2] " Matthew Hagan
@ 2021-06-07  9:45       ` Philipp Zabel
  2021-06-08 18:57         ` Matthew Hagan
  2021-06-08 18:59       ` [PATCH v3] " Matthew Hagan
  1 sibling, 1 reply; 11+ messages in thread
From: Philipp Zabel @ 2021-06-07  9:45 UTC (permalink / raw)
  To: Matthew Hagan
  Cc: bjorn.andersson, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, David S. Miller, Jakub Kicinski, Maxime Coquelin,
	Voon Weifeng, Ong Boon Leong, Wong Vee Khee, Tan Tee Min, Wong,
	Vee Khee, Mohammad Athari Bin Ismail, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel

On Sun, 2021-06-06 at 11:30 +0100, Matthew Hagan wrote:
> We are currently assuming that GMAC_AHB_RESET will already be deasserted
> by the bootloader. However if this has not been done, probing of the GMAC
> will fail. To remedy this we must ensure GMAC_AHB_RESET has been deasserted
> prior to probing.
> 
> v2 changes:
>  - remove NULL condition check for stmmac_ahb_rst in stmmac_main.c
>  - unwrap dev_err() message in stmmac_main.c
>  - add PTR_ERR() around plat->stmmac_ahb_rst in stmmac_platform.c
> 
> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +++++++
>  include/linux/stmmac.h                                | 1 +
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6d41dd6f9f7a..0d4cb423cbbd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6840,6 +6840,10 @@ int stmmac_dvr_probe(struct device *device,
>  			reset_control_reset(priv->plat->stmmac_rst);
>  	}
>  
> +	ret = reset_control_deassert(priv->plat->stmmac_ahb_rst);
> +	if (ret == -ENOTSUPP)
> +		dev_err(priv->device, "unable to bring out of ahb reset\n");
> +

I would make this

	if (ret)
		dev_err(priv->device, "unable to bring out of ahb reset: %pe\n", ERR_PTR(ret));

Also consider asserting the reset again in the remove path. Or is there
a reason not to?

With that addressed,

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] net: stmmac: explicitly deassert GMAC_AHB_RESET
  2021-06-07  9:45       ` Philipp Zabel
@ 2021-06-08 18:57         ` Matthew Hagan
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew Hagan @ 2021-06-08 18:57 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: bjorn.andersson, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, David S. Miller, Jakub Kicinski, Maxime Coquelin,
	Voon Weifeng, Ong Boon Leong, Wong Vee Khee, Tan Tee Min, Wong,
	Vee Khee, Mohammad Athari Bin Ismail, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel

On 07/06/2021 10:45, Philipp Zabel wrote:

> On Sun, 2021-06-06 at 11:30 +0100, Matthew Hagan wrote:
>> We are currently assuming that GMAC_AHB_RESET will already be deasserted
>> by the bootloader. However if this has not been done, probing of the GMAC
>> will fail. To remedy this we must ensure GMAC_AHB_RESET has been deasserted
>> prior to probing.
>>
>> v2 changes:
>>  - remove NULL condition check for stmmac_ahb_rst in stmmac_main.c
>>  - unwrap dev_err() message in stmmac_main.c
>>  - add PTR_ERR() around plat->stmmac_ahb_rst in stmmac_platform.c
>>
>> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +++++++
>>  include/linux/stmmac.h                                | 1 +
>>  3 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 6d41dd6f9f7a..0d4cb423cbbd 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -6840,6 +6840,10 @@ int stmmac_dvr_probe(struct device *device,
>>  			reset_control_reset(priv->plat->stmmac_rst);
>>  	}
>>  
>> +	ret = reset_control_deassert(priv->plat->stmmac_ahb_rst);
>> +	if (ret == -ENOTSUPP)
>> +		dev_err(priv->device, "unable to bring out of ahb reset\n");
>> +
> I would make this
>
> 	if (ret)
> 		dev_err(priv->device, "unable to bring out of ahb reset: %pe\n", ERR_PTR(ret));

Done.

>
> Also consider asserting the reset again in the remove path. Or is there
> a reason not to?

Don't see any issue doing this. As this is a shared reset, the assert will only occur
when the final GMAC is removed, due to the tracking of deassert_count.

> With that addressed,
>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>
> regards
> Philipp

Thanks,

Matthew


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3] net: stmmac: explicitly deassert GMAC_AHB_RESET
  2021-06-06 10:30     ` [PATCH v2] " Matthew Hagan
  2021-06-07  9:45       ` Philipp Zabel
@ 2021-06-08 18:59       ` Matthew Hagan
  2021-06-08 23:50         ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 11+ messages in thread
From: Matthew Hagan @ 2021-06-08 18:59 UTC (permalink / raw)
  Cc: Bjorn Andersson, Philipp Zabel, Matthew Hagan,
	Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Coquelin, Russell King,
	Voon Weifeng, Ong Boon Leong, Wong Vee Khee, Tan Tee Min, Wong,
	Vee Khee, Michael Sit Wei Hong, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel

We are currently assuming that GMAC_AHB_RESET will already be deasserted
by the bootloader. However if this has not been done, probing of the GMAC
will fail. To remedy this we must ensure GMAC_AHB_RESET has been deasserted
prior to probing.

v2 changes:
 - remove NULL condition check for stmmac_ahb_rst in stmmac_main.c
 - unwrap dev_err() message in stmmac_main.c
 - add PTR_ERR() around plat->stmmac_ahb_rst in stmmac_platform.c

v3 changes:
 - add error pointer to dev_err() output
 - add reset_control_assert(stmmac_ahb_rst) in stmmac_dvr_remove
 - revert PTR_ERR() around plat->stmmac_ahb_rst since this is performed
   on the returned value of ret by the calling function

Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 6 ++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +++++++
 include/linux/stmmac.h                                | 1 +
 3 files changed, 14 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6d41dd6f9f7a..78dafde70671 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6840,6 +6840,11 @@ int stmmac_dvr_probe(struct device *device,
 			reset_control_reset(priv->plat->stmmac_rst);
 	}
 
+	ret = reset_control_deassert(priv->plat->stmmac_ahb_rst);
+	if (ret == -ENOTSUPP)
+		dev_err(priv->device, "unable to bring out of ahb reset: %pe\n",
+			ERR_PTR(ret));
+
 	/* Init MAC and get the capabilities */
 	ret = stmmac_hw_init(priv);
 	if (ret)
@@ -7072,6 +7077,7 @@ int stmmac_dvr_remove(struct device *dev)
 	phylink_destroy(priv->phylink);
 	if (priv->plat->stmmac_rst)
 		reset_control_assert(priv->plat->stmmac_rst);
+	reset_control_assert(priv->plat->stmmac_ahb_rst);
 	pm_runtime_put(dev);
 	pm_runtime_disable(dev);
 	if (priv->hw->pcs != STMMAC_PCS_TBI &&
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 97a1fedcc9ac..d8ae58bdbbe3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -600,6 +600,13 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
 		goto error_hw_init;
 	}
 
+	plat->stmmac_ahb_rst = devm_reset_control_get_optional_shared(
+							&pdev->dev, "ahb");
+	if (IS_ERR(plat->stmmac_ahb_rst)) {
+		ret = plat->stmmac_ahb_rst;
+		goto error_hw_init;
+	}
+
 	return plat;
 
 error_hw_init:
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index e55a4807e3ea..9b6a64f3e3dc 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -239,6 +239,7 @@ struct plat_stmmacenet_data {
 	unsigned int mult_fact_100ns;
 	s32 ptp_max_adj;
 	struct reset_control *stmmac_rst;
+	struct reset_control *stmmac_ahb_rst;
 	struct stmmac_axi *axi;
 	int has_gmac4;
 	bool has_sun8i;
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] net: stmmac: explicitly deassert GMAC_AHB_RESET
  2021-06-08 18:59       ` [PATCH v3] " Matthew Hagan
@ 2021-06-08 23:50         ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-06-08 23:50 UTC (permalink / raw)
  To: Matthew Hagan
  Cc: bjorn.andersson, p.zabel, peppe.cavallaro, alexandre.torgue,
	joabreu, davem, kuba, mcoquelin.stm32, linux, weifeng.voon,
	boon.leong.ong, vee.khee.wong, tee.min.tan, vee.khee.wong,
	michael.wei.hong.sit, netdev, linux-stm32, linux-arm-kernel,
	linux-kernel

Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Tue,  8 Jun 2021 19:59:06 +0100 you wrote:
> We are currently assuming that GMAC_AHB_RESET will already be deasserted
> by the bootloader. However if this has not been done, probing of the GMAC
> will fail. To remedy this we must ensure GMAC_AHB_RESET has been deasserted
> prior to probing.
> 
> v2 changes:
>  - remove NULL condition check for stmmac_ahb_rst in stmmac_main.c
>  - unwrap dev_err() message in stmmac_main.c
>  - add PTR_ERR() around plat->stmmac_ahb_rst in stmmac_platform.c
> 
> [...]

Here is the summary with links:
  - [v3] net: stmmac: explicitly deassert GMAC_AHB_RESET
    https://git.kernel.org/netdev/net-next/c/e67f325e9cd6

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-06-08 23:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-05 17:35 [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET Matthew Hagan
2021-06-05 17:35 ` [PATCH 2/3] ARM: dts: qcom: add ahb reset to ipq806x-gmac Matthew Hagan
2021-06-05 17:35 ` [PATCH 3/3] dt-bindings: net: stmmac: add ahb reset to example Matthew Hagan
2021-06-06  3:24 ` [PATCH 1/3] net: stmmac: explicitly deassert GMAC_AHB_RESET Bjorn Andersson
2021-06-06  9:36   ` Matthew Hagan
2021-06-06 10:30     ` [PATCH v2] " Matthew Hagan
2021-06-07  9:45       ` Philipp Zabel
2021-06-08 18:57         ` Matthew Hagan
2021-06-08 18:59       ` [PATCH v3] " Matthew Hagan
2021-06-08 23:50         ` patchwork-bot+netdevbpf
2021-06-06 14:38   ` [PATCH 1/3] " Matthew Hagan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).