It is being observed some time the Ethernet interface
will not send / recive any packet after reboot.
Earlier I had submitted Ethernet reset ID patch
but it did not resolve it issue much, Adding new
reset controller of the Ethernet PHY for Amlogic SoC
could help resolve the issue.
Thanks
-Anand
Anand Moon (3):
arm64: dts: amlogic: add missing ethernet reset ID
ARM: dts: meson: Use new reset id for reset controller
net: stmmac: dwmac-meson8b: Add reset controller for ethernet phy
arch/arm/boot/dts/meson8b.dtsi | 2 +-
arch/arm/boot/dts/meson8m2.dtsi | 2 +-
arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 2 ++
.../boot/dts/amlogic/meson-g12-common.dtsi | 2 ++
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 3 +++
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 20 +++++++++++++++++++
6 files changed, 29 insertions(+), 2 deletions(-)
--
2.32.0
Add reset external reset of the ethernet mac controller,
used new reset id for reset controller as it conflict
with the core reset id.
Fixes: f3362f0c1817 ("arm64: dts: amlogic: add missing ethernet reset ID")
Cc: Jerome Brunet <[email protected]>
Cc: Neil Armstrong <[email protected]>
Cc: Martin Blumenstingl <[email protected]>
Signed-off-by: Anand Moon <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 2 ++
arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 2 ++
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 3 +++
3 files changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index 3f5254eeb47b..da3bf9f7c1c6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -280,6 +280,8 @@ ethmac: ethernet@ff3f0000 {
"timing-adjustment";
rx-fifo-depth = <4096>;
tx-fifo-depth = <2048>;
+ resets = <&reset RESET_ETHERNET>;
+ reset-names = "ethreset";
power-domains = <&pwrc PWRC_AXG_ETHERNET_MEM_ID>;
status = "disabled";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 00c6f53290d4..c174ed50705f 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -230,6 +230,8 @@ ethmac: ethernet@ff3f0000 {
"timing-adjustment";
rx-fifo-depth = <4096>;
tx-fifo-depth = <2048>;
+ resets = <&reset RESET_ETHERNET>;
+ reset-names = "ethreset";
status = "disabled";
mdio0: mdio {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 6b457b2c30a4..717fa3134882 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -13,6 +13,7 @@
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/power/meson-gxbb-power.h>
+#include <dt-bindings/reset/amlogic,meson-gxbb-reset.h>
#include <dt-bindings/thermal/thermal.h>
/ {
@@ -582,6 +583,8 @@ ethmac: ethernet@c9410000 {
interrupt-names = "macirq";
rx-fifo-depth = <4096>;
tx-fifo-depth = <2048>;
+ resets = <&reset RESET_ETHERNET>;
+ reset-names = "ethreset";
power-domains = <&pwrc PWRC_GXBB_ETHERNET_MEM_ID>;
status = "disabled";
};
--
2.32.0
Used new reset id for reset controller as it conflict
with the core reset id.
Fixes: b96446541d83 ("ARM: dts: meson8b: extend ethernet controller description")
Cc: Jerome Brunet <[email protected]>
Cc: Neil Armstrong <[email protected]>
Cc: Martin Blumenstingl <[email protected]>
Signed-off-by: Anand Moon <[email protected]>
---
arch/arm/boot/dts/meson8b.dtsi | 2 +-
arch/arm/boot/dts/meson8m2.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index c02b03cbcdf4..cb3a579d09ef 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -511,7 +511,7 @@ ðmac {
tx-fifo-depth = <2048>;
resets = <&reset RESET_ETHERNET>;
- reset-names = "stmmaceth";
+ reset-names = "ethreset";
power-domains = <&pwrc PWRC_MESON8_ETHERNET_MEM_ID>;
};
diff --git a/arch/arm/boot/dts/meson8m2.dtsi b/arch/arm/boot/dts/meson8m2.dtsi
index 6725dd9fd825..cfaf60c4ba5f 100644
--- a/arch/arm/boot/dts/meson8m2.dtsi
+++ b/arch/arm/boot/dts/meson8m2.dtsi
@@ -34,7 +34,7 @@ ðmac {
<&clkc CLKID_FCLK_DIV2>;
clock-names = "stmmaceth", "clkin0", "clkin1", "timing-adjustment";
resets = <&reset RESET_ETHERNET>;
- reset-names = "stmmaceth";
+ reset-names = "ethreset";
};
&pinctrl_aobus {
--
2.32.0
Add reset controller for Ethernet phy reset on every boot for
Amlogic SoC.
Cc: Jerome Brunet <[email protected]>
Cc: Neil Armstrong <[email protected]>
Cc: Martin Blumenstingl <[email protected]>
Signed-off-by: Anand Moon <[email protected]>
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index c7a6588d9398..8b3b5e8c2a8a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -17,6 +17,7 @@
#include <linux/of_net.h>
#include <linux/mfd/syscon.h>
#include <linux/platform_device.h>
+#include <linux/reset.h>
#include <linux/stmmac.h>
#include "stmmac_platform.h"
@@ -95,6 +96,7 @@ struct meson8b_dwmac {
u32 tx_delay_ns;
u32 rx_delay_ps;
struct clk *timing_adj_clk;
+ struct reset_control *eth_reset;
};
struct meson8b_dwmac_clk_configs {
@@ -384,6 +386,17 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TX_AND_PHY_REF_CLK,
PRG_ETH0_TX_AND_PHY_REF_CLK);
+ /* Make sure the Ethernet PHY is properly reseted, as U-Boot may leave
+ * it at deasserted state, and thus it may fail to reset EMAC.
+ *
+ * This assumes the driver has exclusive access to the EPHY reset.
+ */
+ ret = reset_control_reset(dwmac->eth_reset);
+ if (ret) {
+ dev_err(dwmac->dev, "Cannot reset internal PHY\n");
+ return ret;
+ }
+
return 0;
}
@@ -465,6 +478,13 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
goto err_remove_config_dt;
}
+ dwmac->eth_reset = devm_reset_control_get_exclusive(dwmac->dev, "ethreset");
+ if (IS_ERR_OR_NULL(dwmac->eth_reset)) {
+ dev_err(dwmac->dev, "Failed to get Ethernet reset\n");
+ ret = PTR_ERR(dwmac->eth_reset);
+ goto err_remove_config_dt;
+ }
+
ret = meson8b_init_rgmii_delays(dwmac);
if (ret)
goto err_remove_config_dt;
--
2.32.0
> @@ -465,6 +478,13 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
> goto err_remove_config_dt;
> }
>
> + dwmac->eth_reset = devm_reset_control_get_exclusive(dwmac->dev, "ethreset");
> + if (IS_ERR_OR_NULL(dwmac->eth_reset)) {
> + dev_err(dwmac->dev, "Failed to get Ethernet reset\n");
> + ret = PTR_ERR(dwmac->eth_reset);
> + goto err_remove_config_dt;
> + }
> +
Hi Anand
Since this is a new property, you need to handle it not being in the
DT blob. You probably need to use
devm_reset_control_get_optinal_exclusive()
Andrew
Hi Anand,
On Fri, 2021-07-30 at 01:40 +0530, Anand Moon wrote:
> Add reset external reset of the ethernet mac controller,
> used new reset id for reset controller as it conflict
> with the core reset id.
>
> Fixes: f3362f0c1817 ("arm64: dts: amlogic: add missing ethernet reset ID")
>
> Cc: Jerome Brunet <[email protected]>
> Cc: Neil Armstrong <[email protected]>
> Cc: Martin Blumenstingl <[email protected]>
> Signed-off-by: Anand Moon <[email protected]>
> ---
> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 2 ++
> arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 2 ++
> arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 3 +++
> 3 files changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> index 3f5254eeb47b..da3bf9f7c1c6 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> @@ -280,6 +280,8 @@ ethmac: ethernet@ff3f0000 {
> "timing-adjustment";
> rx-fifo-depth = <4096>;
> tx-fifo-depth = <2048>;
> + resets = <&reset RESET_ETHERNET>;
> + reset-names = "ethreset";
This is missing binding documentation. Also, is this reset name taken
from the documentation? Otherwise, it would probably be better to call
it "phy" for a PHY reset.
regards
Philipp
On Fri, 2021-07-30 at 01:40 +0530, Anand Moon wrote:
> Used new reset id for reset controller as it conflict
> with the core reset id.
>
> Fixes: b96446541d83 ("ARM: dts: meson8b: extend ethernet controller description")
>
> Cc: Jerome Brunet <[email protected]>
> Cc: Neil Armstrong <[email protected]>
> Cc: Martin Blumenstingl <[email protected]>
> Signed-off-by: Anand Moon <[email protected]>
> ---
> arch/arm/boot/dts/meson8b.dtsi | 2 +-
> arch/arm/boot/dts/meson8m2.dtsi | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
> index c02b03cbcdf4..cb3a579d09ef 100644
> --- a/arch/arm/boot/dts/meson8b.dtsi
> +++ b/arch/arm/boot/dts/meson8b.dtsi
> @@ -511,7 +511,7 @@ ðmac {
> tx-fifo-depth = <2048>;
>
> resets = <&reset RESET_ETHERNET>;
> - reset-names = "stmmaceth";
> + reset-names = "ethreset";
This looks like an incompatible change. Is the "stmmaceth" reset not
used? It is documented as "MAC reset signal" in [1]. So a PHY reset
should be separate from this.
[1] Documentation/devicetree/bindings/net/snps,dwmac.yaml
regards
Philipp
Hi Philipp,
Thanks for your review comments.
On Fri, 30 Jul 2021 at 15:16, Philipp Zabel <[email protected]> wrote:
>
> On Fri, 2021-07-30 at 01:40 +0530, Anand Moon wrote:
> > Used new reset id for reset controller as it conflict
> > with the core reset id.
> >
> > Fixes: b96446541d83 ("ARM: dts: meson8b: extend ethernet controller description")
> >
> > Cc: Jerome Brunet <[email protected]>
> > Cc: Neil Armstrong <[email protected]>
> > Cc: Martin Blumenstingl <[email protected]>
> > Signed-off-by: Anand Moon <[email protected]>
> > ---
> > arch/arm/boot/dts/meson8b.dtsi | 2 +-
> > arch/arm/boot/dts/meson8m2.dtsi | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
> > index c02b03cbcdf4..cb3a579d09ef 100644
> > --- a/arch/arm/boot/dts/meson8b.dtsi
> > +++ b/arch/arm/boot/dts/meson8b.dtsi
> > @@ -511,7 +511,7 @@ ðmac {
> > tx-fifo-depth = <2048>;
> >
> > resets = <&reset RESET_ETHERNET>;
> > - reset-names = "stmmaceth";
> > + reset-names = "ethreset";
>
> This looks like an incompatible change. Is the "stmmaceth" reset not
> used? It is documented as "MAC reset signal" in [1]. So a PHY reset
> should be separate from this.
>
> [1] Documentation/devicetree/bindings/net/snps,dwmac.yaml
>
From the above device tree binding is been used below.
reset-names:
const: stmmaceth
While testing new reset driver changes I was getting conflict with
reset id, see the below links
hence I opted for a new reset-names = "ethreset".
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac.h?h=v5.14-rc3#n12
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c?h=v5.14-rc3#n598
> regards
> Philipp
Thanks
-Anand
Hi Anand,
On Thu, Jul 29, 2021 at 10:11 PM Anand Moon <[email protected]> wrote:
>
> It is being observed some time the Ethernet interface
> will not send / recive any packet after reboot.
>
> Earlier I had submitted Ethernet reset ID patch
> but it did not resolve it issue much, Adding new
> reset controller of the Ethernet PHY for Amlogic SoC
> could help resolve the issue.
nowhere in this series you are addressing the issue from [0]
Some more comments in the individual patch
Best regards,
Martin
[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/amlogic?id=19f6fe976a61f9afc289b062b7ef67f99b72e7b9
Hi Anand,
On Thu, Jul 29, 2021 at 10:11 PM Anand Moon <[email protected]> wrote:
>
> Add reset controller for Ethernet phy reset on every boot for
> Amlogic SoC.
I think this description does not match what's going on inside the SoC:
- for all SoCs earlier than GXL the PHY is external so the reset for
the PHY is a GPIO
- the reset line you are passing in the .dts belongs to the Ethernet
controller on SoCs earlier than GXL
- I *believe* that the rset line which you're passing in the .dts
belongs to the Ethernet controller AND the built-in MDIO mux on GXL
and newer, see also [0]
- from how the PRG_ETH registers work I doubt that these are connected
to a reset line (as they're managing mostly delays and protocol - so I
don't see what would be reset). This is speculation though.
Best regards,
Martin
[0] https://lore.kernel.org/linux-amlogic/[email protected]/