* [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; 6+ 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] 6+ 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; 6+ 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] 6+ 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 ` Matthew Hagan
2 siblings, 2 replies; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ messages in thread