Skip to contentSkip to conversation

meson8b-odroidc1: ethernet support

PATCH New Emiliano Ingrassia 4 participants 26 messages Sep 27, 2017 – Dec 18, 2017

Subsystems (8)
lore
Share & tools
lore
#0 lore raw

cover meson8b-odroidc1: ethernet support

This patchset enables ethernet support on Odroid-C1/C1+ boards, improving and extending the ethernet description in the relative device tree.

In particular:
- the mpll2 clock is enabled to avoid timeout during ethernet DMA reset;
- the Meson8b ethernet controller description is fixed and extended;
- the Odroid-C1/C1+ ethernet PHY description is added;
- the dwmac sleep and timeout values used during DMA reset are fixed.

This patchset has been tested on Odroid-C1+ board.

Emiliano Ingrassia (4):
clk: meson8b: keep mpll2 clock enabled
ARM: dts: meson8b: extending ethernet controller description
ARM: dts: meson8b-odroidc1: enabling ethernet support
net: stmmac: fixing DMA reset sleep and timeout values

 arch/arm/boot/dts/meson8b-odroidc1.dts          | 30 +++++++++++++++++
 arch/arm/boot/dts/meson8b.dtsi                  | 43 +++++++++++++++++++++++--
 drivers/clk/meson/meson8b.c                     |  1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c |  2 +-
 4 files changed, 73 insertions(+), 3 deletions(-)
Signature · 1 line
2.14.1
#1 lore raw

PATCH 1/4 clk: meson8b: keep mpll2 clock enabled

The mpll2 clock, enabled by the bootloader, is disabled at boot. Enabling ethernet on Odroid-C1+ board leads to DMA initialization failure caused by a timeout on reset.
Keeping the mpll2 clock enabled solve this issue.

---
 drivers/clk/meson/meson8b.c | 1 +
 1 file changed, 1 insertion(+)
drivers/clk/meson/meson8b.c +1 -0
index 20ab7190d328..5096539e4a63 100644
@@ -347,6 +347,7 @@ static struct meson_clk_mpll meson8b_mpll2 = {
 		.ops = &meson_clk_mpll_ops,
 		.parent_names = (const char *[]){ "fixed_pll" },
 		.num_parents = 1,
+		.flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
 	},
 };
 
Signature · 1 line
2.14.1
maintainer
#2 lore raw

PATCH 1/4 clk: meson8b: keep mpll2 clock enabled

Quoted 4 lines · Emiliano Ingrassia

The mpll2 clock, enabled by the bootloader, is disabled at boot. Enabling ethernet on Odroid-C1+ board leads to DMA initialization failure caused by a timeout on reset.
Keeping the mpll2 clock enabled solve this issue.

Shouldn't the DMA driver emable the clocks it needs itself instead ? BTW, I'm bit surprised an mpll is used to clock a DMA, is it possible we missed something here ?

Quoted 18 lines
---
 drivers/clk/meson/meson8b.c | 1 +
 1 file changed, 1 insertion(+)
drivers/clk/meson/meson8b.c +1 -0
index 20ab7190d328..5096539e4a63 100644
@@ -347,6 +347,7 @@ static struct meson_clk_mpll meson8b_mpll2 = {
 		.ops = &meson_clk_mpll_ops,
 		.parent_names = (const char *[]){ "fixed_pll" },
 		.num_parents = 1,
+		.flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
 	},
 };
 
#3 lore raw

PATCH 1/4 clk: meson8b: keep mpll2 clock enabled

Hi Jerome,

thanks for review!

Quoted 10 lines · Jerome Brunet
Quoted 4 lines · Emiliano Ingrassia

The mpll2 clock, enabled by the bootloader, is disabled at boot. Enabling ethernet on Odroid-C1+ board leads to DMA initialization failure caused by a timeout on reset.
Keeping the mpll2 clock enabled solve this issue.

Shouldn't the DMA driver emable the clocks it needs itself instead ? BTW, I'm bit surprised an mpll is used to clock a DMA, is it possible we missed something here ?

The mpll2 clocks the meson8b ethernet controller which, as one can suppose, internally have a DMA engine. If the controller is enabled, the first operation on hardware is the DMA engine reset which fails if mpll2 is disabled.
Actually the patch not only solves that issue, but also permits the ethernet controller to work correctly.

The correct solution would be to enable the mpll2 clock from dwmac-meson8b driver; I'm working on this.
In the meantime I release this patch to give others the chance to use the ethernet controller on Odroid-C1/C1+.

Regards,

Emiliano

Quoted 19 lines
Quoted 18 lines
---
 drivers/clk/meson/meson8b.c | 1 +
 1 file changed, 1 insertion(+)
drivers/clk/meson/meson8b.c +1 -0
index 20ab7190d328..5096539e4a63 100644
@@ -347,6 +347,7 @@ static struct meson_clk_mpll meson8b_mpll2 = {
 		.ops = &meson_clk_mpll_ops,
 		.parent_names = (const char *[]){ "fixed_pll" },
 		.num_parents = 1,
+		.flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
 	},
 };
 
maintainer
#4 lore raw

PATCH 1/4 clk: meson8b: keep mpll2 clock enabled

Quoted 17 lines · Emiliano Ingrassia

Hi Jerome,

thanks for review!

Quoted 11 lines · Jerome Brunet
Quoted 4 lines · Emiliano Ingrassia

The mpll2 clock, enabled by the bootloader, is disabled at boot. Enabling ethernet on Odroid-C1+ board leads to DMA initialization failure caused by a timeout on reset.
Keeping the mpll2 clock enabled solve this issue.

Shouldn't the DMA driver emable the clocks it needs itself instead ? BTW, I'm bit surprised an mpll is used to clock a DMA, is it possible we missed
something here ?

Quoted 4 lines

The mpll2 clocks the meson8b ethernet controller which, as one can suppose, internally have a DMA engine. If the controller is enabled, the first operation on hardware is the DMA engine reset which fails if mpll2 is disabled.

I understand. I mentioned that's it strange to use mpll2 because:
- On gxbb (and following SoCs), There is 2 clockin sources to the mac: FDIV2 and MPLL2. There is , of course, an mux in the ethernet device to select the clock you need/want.
- I would not be surprised if it was the same thing on meson8(b). In such case, I think it is better to use FDIV2.

Quoted 4 lines

Actually the patch not only solves that issue, but also permits the ethernet controller to work correctly.
The correct solution would be to enable the mpll2 clock from dwmac-meson8b driver;

Indeed.
I would prefer that you fix the ethernet driver rather than adding this to the clock driver.

Quoted 4 lines

I'm working on this.
In the meantime I release this patch to give others the chance to use the ethernet controller on Odroid-C1/C1+.

Ok

Quoted 22 lines

Regards,

Emiliano

Quoted 18 lines
Quoted 18 lines
---
 drivers/clk/meson/meson8b.c | 1 +
 1 file changed, 1 insertion(+)
drivers/clk/meson/meson8b.c +1 -0
index 20ab7190d328..5096539e4a63 100644
@@ -347,6 +347,7 @@ static struct meson_clk_mpll meson8b_mpll2 = {
 		.ops = &meson_clk_mpll_ops,
 		.parent_names = (const char *[]){ "fixed_pll" },
 		.num_parents = 1,
+		.flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
 	},
 };
 
#5 lore raw

PATCH 1/4 clk: meson8b: keep mpll2 clock enabled

Hi Emiliano, Hi Jerome,

Quoted 12 lines · Jerome Brunet
Quoted 11 lines · Emiliano Ingrassia

Hi Jerome,

thanks for review!

Quoted 6 lines · Jerome Brunet
Quoted 4 lines · Emiliano Ingrassia

The mpll2 clock, enabled by the bootloader, is disabled at boot. Enabling ethernet on Odroid-C1+ board leads to DMA initialization failure caused by a timeout on reset.
Keeping the mpll2 clock enabled solve this issue.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

Shouldn't the DMA driver emable the clocks it needs itself instead ? BTW, I'm bit surprised an mpll is used to clock a DMA, is it possible we missed
something here ?

originally reported on IRC:

[  416.293456] meson8b-dwmac c9410000.ethernet eth0: stmmac_hw_setup:
DMA engine initialization failed
[  416.302480] meson8b-dwmac c9410000.ethernet eth0: stmmac_open: Hw

setup failed
this issue is mentioned by the stmmac developers - together with a hint how to fix it: [0]

Quoted 11 lines
Quoted 4 lines

The mpll2 clocks the meson8b ethernet controller which, as one can suppose, internally have a DMA engine. If the controller is enabled, the first operation on hardware is the DMA engine reset which fails if mpll2 is disabled.

I understand. I mentioned that's it strange to use mpll2 because:
- On gxbb (and following SoCs), There is 2 clockin sources to the mac: FDIV2 and MPLL2. There is , of course, an mux in the ethernet device to select the clock you need/want.
- I would not be surprised if it was the same thing on meson8(b). In such case, I think it is better to use FDIV2.

I *think* that clock-source 0 (at least on Meson8b) is *NOT* fclk_div2 Amlogic's Meson8 and Meson8b u-boot GPL kernel sources (see [1] for the Odroid-C1 code) configure HHI_GEN_CLK_CNTL to 0xb803, which translates to:
- divide by 2+1 = 3
- clock enabled
- parent clock is 0xb (according to the S912 datasheet this is fclk_div5 - it may be a different input on Meson8 and Meson8b) (taken from the S912 datasheet released by Khadas)

it also enables HHI_MPLL_CNTL6 bit 27 which is "CKEN[4] MPLL output DIV2 enable"

and finally it uses:
SET_CBUS_REG_MASK(HHI_MPLL_CNTL9, (1638 << 0)

| (0 << 14) | (1 << 15) | (1 << 14)
| (5 << 16)
| (0 << 25) | (0 << 26) | (0 << 30) | (0 << 31));

further up in the S912 datasheet (clock and reset - section 21.1 overview) one finds:
MPLL_CLK_OUT_DIV2_GPIO with comment: to GPIOCLK

I assume that this clock is used as "parent" of the internal mux in dwmac-meson8b

Quoted 10 lines
Quoted 4 lines

Actually the patch not only solves that issue, but also permits the ethernet controller to work correctly.
The correct solution would be to enable the mpll2 clock from dwmac-meson8b driver;

Indeed.
I would prefer that you fix the ethernet driver rather than adding this to the clock driver.

Quoted 6 lines
Quoted 4 lines

I'm working on this.
In the meantime I release this patch to give others the chance to use the ethernet controller on Odroid-C1/C1+.

Ok

Jerome: maybe those MPLL registers mentioned above make sense to you - if you have an *idea* what they are about then please let us know :)

Emiliano: there's also a "clkmsr" command in u-boot which *MIGHT* help you to find out the actual frequency of a clock
however, don't blindly trust that output, I had some cases on GXBB where I think the clkmsr result didn't match my expectations

by the way: on my Meson8m2 board which comes with a RMII PHY I attached the u-boot clkmsr output and a dump of the clock registers

Quoted 3 lines
Quoted 3 lines

Regards,

Emiliano

thank you for your improvements!

Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
---
 drivers/clk/meson/meson8b.c | 1 +
 1 file changed, 1 insertion(+)
drivers/clk/meson/meson8b.c +1 -0
index 20ab7190d328..5096539e4a63 100644
@@ -347,6 +347,7 @@ static struct meson_clk_mpll meson8b_mpll2 = {
          .ops = &meson_clk_mpll_ops,
          .parent_names = (const char *[]){ "fixed_pll" },
          .num_parents = 1,
+         .flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
  },
Quoted 8 lines
Quoted 2 lines
Quoted 2 lines

drivers/clk/meson/meson8b.c static struct meson_clk_mpll meson8b_mpll2 = { · line 353

Quoted 2 lines
drivers/clk/meson/meson8b.c +0 -0
 };
 

_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[0] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/492629.html
[1] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L29
-------------- next part --------------
m8m2_n200_v1#clkmsr

[   0 MHz]  AM_RING_OSC_CLK_OUT0(0)
[   0 MHz]  AM_RING_OSC_CLK_OUT1(1)
[   0 MHz]  SYS_PLL_CLK(2)
[   0 MHz]  DDR_PLL_CLK(3)
[   0 MHz]  MISC_PLL_CLK(4)
[   0 MHz]  AUD_PLL_CLK(5)
[ 216 MHz]  VID_PLL_CLK(6)
[ 142 MHz]  CLK81(7)
[  54 MHz]  CTS_ENCP_CLK(8)
[  54 MHz]  CTS_ENCL_CLK(9)
[   0 MHz]  CTS_ENCT_CLK(10)
[   0 MHz]  CTS_ETH_RMII(11)
[   0 MHz]  VID2_PLL_CLK(12)
[   0 MHz]  CTS_AMCLK(13)
[   0 MHz]  CTS_FEC_CLK_0 (14)
[   0 MHz]  CTS_FEC_CLK_1 (15)
[   0 MHz]  CTS_FEC_CLK_2(16)
[   0 MHz]  CTS_VGHL_PLL_CLK (17)
[  75 MHz]  CTS_LED_PLL_CLK(18)
[  24 MHz]  CTS_HDMI_SYS_CLK(19)
[   0 MHz]  CTS_BTCLK27 (20)
[   0 MHz]  MOD_AUDIN_AMCLK_I  (21)
[   0 MHz]  MOD_ETH_CLK50_I (22)
[   0 MHz]  HDMI_CH3_TMDSCLK(23)
[   0 MHz]  LVDS_FIFO_CLK (24)
[   0 MHz]  USB_CLK_12MHZ (25)
[   0 MHz]  SC_CLK_INT(26)
[   0 MHz]  CTS_ENCI_CL(27)
[   1 MHz]  CTS_SAR_ADC_CLK (28)
[   0 MHz]  CTS_VDAC_CLK0(29)
[   0 MHz]  CTS_DDR_CLK(30)
[   0 MHz]  CTS_A9_CLK(31)
[   0 MHz]  CTS_AUDAC_CLKPI(32)
[   0 MHz]  CTS_SDHC_CLK0(33)
[   0 MHz]  CTS_SDHC_CLK1(34)
[   0 MHz]  CTS_MALI_CLK (35)
[  27 MHz]  CTS_HDMI_TX_PIXEL_CLK(36)
[   0 MHz]  CTS_VDAC_CLK1(37)
[   0 MHz]  CTS_VDIN_MEAS_CLK(38)
[   0 MHz]  CTS_PCM_SCLK(39)
[   0 MHz]  CTS_PCM_MCLK(40)
[  50 MHz]  CTS_ETH_RX_TX (41)
[   0 MHz]  CTS_PWM_D_CLK(42)
[   0 MHz]  CTS_PWM_C_CLK(43)
[   0 MHz]  CTS_PWM_B_CLK(44)
[   0 MHz]  CTS_PWM_A_CLK(45)

m8m2_n200_v1#md 0xC1104000 0x100
c1104000: 00000000 00000000 00000000 00000000 ................
c1104010: 00000000 00000000 00000000 00000000 ................
c1104020: 00000000 00000000 00000000 01ea9612 ................
c1104030: 00000000 00000000 00000000 00000000 ................
c1104040: c00206b6 59c88000 ca463823 0286a027 .......Y#8F.'...
c1104050: 00003000 00000000 00000000 00000000 .0..............
c1104060: 00000000 00000000 00000000 00000000 ................
c1104070: 00000000 00000000 00000000 00000000 ................
c1104080: 00000000 00000000 00000000 00000000 ................
c1104090: 00000000 00000000 00000000 00000000 ................
c11040a0: 00000000 00000000 00000000 00000000 ................
c11040b0: 00000000 00000000 00000000 00000000 ................
c11040c0: 00000000 00000000 00000000 00000000 ................
c11040d0: 00000000 00000000 00000000 00000000 ................
c11040e0: 00000000 00000000 00000000 00000384 ................
c11040f0: 00000000 00000000 00000000 00000000 ................
c1104100: ffff00f3 00000000 00000000 00000000 ................
c1104110: 00000000 00000000 00000000 00000000 ................
c1104120: 00000000 00000000 10000101 00000000 ................
c1104130: 00010000 00000000 00000000 00000000 ................
c1104140: ffffffff ffffffff ffffffff 00000000 ................
c1104150: ffffffff 000000ff 00000000 800016a2 ................
c1104160: 00000000 10000103 00000000 00000000 ................
c1104170: 00000000 0000e185 00000004 00080007 ................
c1104180: 00000000 00000000 00000000 00000000 ................
c1104190: 00000004 000000ff 00010853 000000b1 ........S.......
c11041a0: 00000000 00000000 00000000 00000000 ................
c11041b0: 00000000 00000000 00000000 00000700 ................
c11041c0: 00000000 00000000 00000000 00010100 ................
c11041d0: 00000000 00000000 00000000 00000000 ................
c11041e0: 00000000 00000000 00000000 00000000 ................
c11041f0: 00000000 00000000 00000000 00000000 ................
c1104200: 00000000 00000000 00000000 00000000 ................
c1104210: 00000000 00000000 00000000 00000000 ................
c1104220: 00000000 00000000 00000000 00000000 ................
c1104230: 00000000 00000000 00000000 00000000 ................
c1104240: 00000000 00000000 00000000 00000000 ................
c1104250: 00000b00 00000000 00000000 00000501 ................
c1104260: 00000000 00000000 00000000 00000000 ................
c1104270: 00000000 30000000 00000000 00000000 .......0........
c1104280: c00009a9 59c80000 ca45b822 00014007 .......Y".E.. at ..
c1104290: b5500e1a f4454545 00000000 00000000 ..P.EEE.........
c11042a0: 00000000 00000000 00000000 00000000 ................
c11042b0: 00000000 00000000 00000000 00000000 ................
c11042c0: 00000000 00000000 00000000 00000000 ................
c11042d0: 00000000 00000000 00000000 00000000 ................
c11042e0: 00000000 00000000 00000000 00000000 ................
c11042f0: 00000000 00000001 00000000 00000000 ................
c1104300: c1200232 5ac82000 8e452015 0001d40c 2. .. .Z. E.....
c1104310: 00000870 00000000 00000100 00180007 p...............
c1104320: c0080436 59c88000 ca49b022 0023b100 6......Y".I...#.
c1104330: 00016385 00000003 00000000 00000000 .c..............
c1104340: 00000000 00000003 001e0000 00000000 ................
c1104350: 00000000 00000000 00ffffff 00000000 ................
c1104360: 00000000 00000000 00000000 00000000 ................
c1104370: 00000000 007f0000 00000000 02000000 ................
c1104380: 21001001 0430a800 00000000 00000000 ...!..0.........
c1104390: 00000000 00000002 00000000 00000000 ................
c11043a0: 00000000 00000000 00000000 00000000 ................
c11043b0: 00000000 00000000 00000000 00000000 ................
c11043c0: 00000000 00000000 00000000 00000000 ................
c11043d0: 00000000 00000000 00000000 00000000 ................
c11043e0: 00000000 00000000 00000000 00000000 ................
c11043f0: 00000000 00000000 00000000 00000000 ................
m8m2_n200_v1#

#6 lore raw

PATCH 1/4 clk: meson8b: keep mpll2 clock enabled

Hi Martin, Hi Jerome,

Quoted 58 lines · Martin Blumenstingl

Hi Emiliano, Hi Jerome,

Quoted 12 lines · Jerome Brunet
Quoted 11 lines · Emiliano Ingrassia

Hi Jerome,

thanks for review!

Quoted 6 lines · Jerome Brunet
Quoted 4 lines · Emiliano Ingrassia

The mpll2 clock, enabled by the bootloader, is disabled at boot. Enabling ethernet on Odroid-C1+ board leads to DMA initialization failure caused by a timeout on reset.
Keeping the mpll2 clock enabled solve this issue.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

Shouldn't the DMA driver emable the clocks it needs itself instead ? BTW, I'm bit surprised an mpll is used to clock a DMA, is it possible we missed
something here ?

originally reported on IRC:

[  416.293456] meson8b-dwmac c9410000.ethernet eth0: stmmac_hw_setup:
DMA engine initialization failed
[  416.302480] meson8b-dwmac c9410000.ethernet eth0: stmmac_open: Hw

setup failed
this issue is mentioned by the stmmac developers - together with a hint how to fix it: [0]

Quoted 11 lines
Quoted 4 lines

The mpll2 clocks the meson8b ethernet controller which, as one can suppose, internally have a DMA engine. If the controller is enabled, the first operation on hardware is the DMA engine reset which fails if mpll2 is disabled.

I understand. I mentioned that's it strange to use mpll2 because:
- On gxbb (and following SoCs), There is 2 clockin sources to the mac: FDIV2 and MPLL2. There is , of course, an mux in the ethernet device to select the clock you need/want.
- I would not be surprised if it was the same thing on meson8(b). In such case, I think it is better to use FDIV2.

I *think* that clock-source 0 (at least on Meson8b) is *NOT* fclk_div2 Amlogic's Meson8 and Meson8b u-boot GPL kernel sources (see [1] for the Odroid-C1 code) configure HHI_GEN_CLK_CNTL to 0xb803, which translates to:
- divide by 2+1 = 3
- clock enabled
- parent clock is 0xb (according to the S912 datasheet this is fclk_div5 - it may be a different input on Meson8 and Meson8b) (taken from the S912 datasheet released by Khadas)

it also enables HHI_MPLL_CNTL6 bit 27 which is "CKEN[4] MPLL output DIV2 enable"

and finally it uses:
SET_CBUS_REG_MASK(HHI_MPLL_CNTL9, (1638 << 0)

| (0 << 14) | (1 << 15) | (1 << 14)
| (5 << 16)
| (0 << 25) | (0 << 26) | (0 << 30) | (0 << 31));
Quoted 7 lines

further up in the S912 datasheet (clock and reset - section 21.1 overview) one finds:
MPLL_CLK_OUT_DIV2_GPIO with comment: to GPIOCLK

I assume that this clock is used as "parent" of the internal mux in dwmac-meson8b

If you take a look at S805 (Meson8b) SoC manual, pag.122, you can read, in PRG_ETHERNET_ADDR0[9:7] bits description:
"M8Baby internal clock source is mp2_clk_out only."

This seems correct. In fact, if you enable fclk_div2 and keep mpll2 (aka mp2_clk) disabled, the ethernet controller does not work.

So, we can conclude that, for Meson8b, that should be the correct parent clock for ethernet controller.

Quoted 15 lines
Quoted 15 lines
Quoted 4 lines

Actually the patch not only solves that issue, but also permits the ethernet controller to work correctly.
The correct solution would be to enable the mpll2 clock from dwmac-meson8b driver;

Indeed.
I would prefer that you fix the ethernet driver rather than adding this to the clock driver.

Quoted 4 lines

I'm working on this.
In the meantime I release this patch to give others the chance to use the ethernet controller on Odroid-C1/C1+.

Quoted 9 lines
Quoted 1 line

Ok

Jerome: maybe those MPLL registers mentioned above make sense to you - if you have an *idea* what they are about then please let us know :)

Emiliano: there's also a "clkmsr" command in u-boot which *MIGHT* help you to find out the actual frequency of a clock
however, don't blindly trust that output, I had some cases on GXBB where I think the clkmsr result didn't match my expectations

Thanks for the advice! I'll keep it in mind.

Quoted 8 lines

by the way: on my Meson8m2 board which comes with a RMII PHY I attached the u-boot clkmsr output and a dump of the clock registers

Quoted 3 lines
Quoted 3 lines

Regards,

Emiliano

thank you for your improvements!

Thank you for your suggestions!

Regards,

Emiliano

Quoted 142 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
---
 drivers/clk/meson/meson8b.c | 1 +
 1 file changed, 1 insertion(+)
drivers/clk/meson/meson8b.c +1 -0
index 20ab7190d328..5096539e4a63 100644
@@ -347,6 +347,7 @@ static struct meson_clk_mpll meson8b_mpll2 = {
          .ops = &meson_clk_mpll_ops,
          .parent_names = (const char *[]){ "fixed_pll" },
          .num_parents = 1,
+         .flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
  },
Quoted 8 lines
Quoted 2 lines
Quoted 2 lines

drivers/clk/meson/meson8b.c static struct meson_clk_mpll meson8b_mpll2 = { · line 353

Quoted 2 lines
drivers/clk/meson/meson8b.c +0 -0
 };
 

_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[0] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/492629.html
[1] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L29

m8m2_n200_v1#clkmsr

[   0 MHz]  AM_RING_OSC_CLK_OUT0(0)
[   0 MHz]  AM_RING_OSC_CLK_OUT1(1)
[   0 MHz]  SYS_PLL_CLK(2)
[   0 MHz]  DDR_PLL_CLK(3)
[   0 MHz]  MISC_PLL_CLK(4)
[   0 MHz]  AUD_PLL_CLK(5)
[ 216 MHz]  VID_PLL_CLK(6)
[ 142 MHz]  CLK81(7)
[  54 MHz]  CTS_ENCP_CLK(8)
[  54 MHz]  CTS_ENCL_CLK(9)
[   0 MHz]  CTS_ENCT_CLK(10)
[   0 MHz]  CTS_ETH_RMII(11)
[   0 MHz]  VID2_PLL_CLK(12)
[   0 MHz]  CTS_AMCLK(13)
[   0 MHz]  CTS_FEC_CLK_0 (14)
[   0 MHz]  CTS_FEC_CLK_1 (15)
[   0 MHz]  CTS_FEC_CLK_2(16)
[   0 MHz]  CTS_VGHL_PLL_CLK (17)
[  75 MHz]  CTS_LED_PLL_CLK(18)
[  24 MHz]  CTS_HDMI_SYS_CLK(19)
[   0 MHz]  CTS_BTCLK27 (20)
[   0 MHz]  MOD_AUDIN_AMCLK_I  (21)
[   0 MHz]  MOD_ETH_CLK50_I (22)
[   0 MHz]  HDMI_CH3_TMDSCLK(23)
[   0 MHz]  LVDS_FIFO_CLK (24)
[   0 MHz]  USB_CLK_12MHZ (25)
[   0 MHz]  SC_CLK_INT(26)
[   0 MHz]  CTS_ENCI_CL(27)
[   1 MHz]  CTS_SAR_ADC_CLK (28)
[   0 MHz]  CTS_VDAC_CLK0(29)
[   0 MHz]  CTS_DDR_CLK(30)
[   0 MHz]  CTS_A9_CLK(31)
[   0 MHz]  CTS_AUDAC_CLKPI(32)
[   0 MHz]  CTS_SDHC_CLK0(33)
[   0 MHz]  CTS_SDHC_CLK1(34)
[   0 MHz]  CTS_MALI_CLK (35)
[  27 MHz]  CTS_HDMI_TX_PIXEL_CLK(36)
[   0 MHz]  CTS_VDAC_CLK1(37)
[   0 MHz]  CTS_VDIN_MEAS_CLK(38)
[   0 MHz]  CTS_PCM_SCLK(39)
[   0 MHz]  CTS_PCM_MCLK(40)
[  50 MHz]  CTS_ETH_RX_TX (41)
[   0 MHz]  CTS_PWM_D_CLK(42)
[   0 MHz]  CTS_PWM_C_CLK(43)
[   0 MHz]  CTS_PWM_B_CLK(44)
[   0 MHz]  CTS_PWM_A_CLK(45)

m8m2_n200_v1#md 0xC1104000 0x100
c1104000: 00000000 00000000 00000000 00000000 ................
c1104010: 00000000 00000000 00000000 00000000 ................
c1104020: 00000000 00000000 00000000 01ea9612 ................
c1104030: 00000000 00000000 00000000 00000000 ................
c1104040: c00206b6 59c88000 ca463823 0286a027 .......Y#8F.'...
c1104050: 00003000 00000000 00000000 00000000 .0..............
c1104060: 00000000 00000000 00000000 00000000 ................
c1104070: 00000000 00000000 00000000 00000000 ................
c1104080: 00000000 00000000 00000000 00000000 ................
c1104090: 00000000 00000000 00000000 00000000 ................
c11040a0: 00000000 00000000 00000000 00000000 ................
c11040b0: 00000000 00000000 00000000 00000000 ................
c11040c0: 00000000 00000000 00000000 00000000 ................
c11040d0: 00000000 00000000 00000000 00000000 ................
c11040e0: 00000000 00000000 00000000 00000384 ................
c11040f0: 00000000 00000000 00000000 00000000 ................
c1104100: ffff00f3 00000000 00000000 00000000 ................
c1104110: 00000000 00000000 00000000 00000000 ................
c1104120: 00000000 00000000 10000101 00000000 ................
c1104130: 00010000 00000000 00000000 00000000 ................
c1104140: ffffffff ffffffff ffffffff 00000000 ................
c1104150: ffffffff 000000ff 00000000 800016a2 ................
c1104160: 00000000 10000103 00000000 00000000 ................
c1104170: 00000000 0000e185 00000004 00080007 ................
c1104180: 00000000 00000000 00000000 00000000 ................
c1104190: 00000004 000000ff 00010853 000000b1 ........S.......
c11041a0: 00000000 00000000 00000000 00000000 ................
c11041b0: 00000000 00000000 00000000 00000700 ................
c11041c0: 00000000 00000000 00000000 00010100 ................
c11041d0: 00000000 00000000 00000000 00000000 ................
c11041e0: 00000000 00000000 00000000 00000000 ................
c11041f0: 00000000 00000000 00000000 00000000 ................
c1104200: 00000000 00000000 00000000 00000000 ................
c1104210: 00000000 00000000 00000000 00000000 ................
c1104220: 00000000 00000000 00000000 00000000 ................
c1104230: 00000000 00000000 00000000 00000000 ................
c1104240: 00000000 00000000 00000000 00000000 ................
c1104250: 00000b00 00000000 00000000 00000501 ................
c1104260: 00000000 00000000 00000000 00000000 ................
c1104270: 00000000 30000000 00000000 00000000 .......0........
c1104280: c00009a9 59c80000 ca45b822 00014007 .......Y".E.. at ..
c1104290: b5500e1a f4454545 00000000 00000000 ..P.EEE.........
c11042a0: 00000000 00000000 00000000 00000000 ................
c11042b0: 00000000 00000000 00000000 00000000 ................
c11042c0: 00000000 00000000 00000000 00000000 ................
c11042d0: 00000000 00000000 00000000 00000000 ................
c11042e0: 00000000 00000000 00000000 00000000 ................
c11042f0: 00000000 00000001 00000000 00000000 ................
c1104300: c1200232 5ac82000 8e452015 0001d40c 2. .. .Z. E.....
c1104310: 00000870 00000000 00000100 00180007 p...............
c1104320: c0080436 59c88000 ca49b022 0023b100 6......Y".I...#.
c1104330: 00016385 00000003 00000000 00000000 .c..............
c1104340: 00000000 00000003 001e0000 00000000 ................
c1104350: 00000000 00000000 00ffffff 00000000 ................
c1104360: 00000000 00000000 00000000 00000000 ................
c1104370: 00000000 007f0000 00000000 02000000 ................
c1104380: 21001001 0430a800 00000000 00000000 ...!..0.........
c1104390: 00000000 00000002 00000000 00000000 ................
c11043a0: 00000000 00000000 00000000 00000000 ................
c11043b0: 00000000 00000000 00000000 00000000 ................
c11043c0: 00000000 00000000 00000000 00000000 ................
c11043d0: 00000000 00000000 00000000 00000000 ................
c11043e0: 00000000 00000000 00000000 00000000 ................
c11043f0: 00000000 00000000 00000000 00000000 ................
m8m2_n200_v1#

#7 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

In reply to Emiliano Ingrassia

This patch adds ethernet controller pin description and extend its attributes in the relative node.

---
 arch/arm/boot/dts/meson8b.dtsi | 43 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)
arch/arm/boot/dts/meson8b.dtsi +41 -2
index bc278da7df0d..26372b019897 100644
@@ -154,12 +154,51 @@
 			#gpio-cells = <2>;
 			gpio-ranges = <&pinctrl_cbus 0 0 130>;
 		};
+
+		eth_rgmii_pins: eth-rgmii {
+			mux {
+				groups = "eth_tx_clk",
+					 "eth_tx_en",
+					 "eth_txd1_0",
+					 "eth_txd1_1",
+					 "eth_txd0_0",
+					 "eth_txd0_1",
+					 "eth_rx_clk",
+					 "eth_rx_dv",
+					 "eth_rxd1",
+					 "eth_rxd0",
+					 "eth_mdio_en",
+					 "eth_mdc",
+					 "eth_ref_clk",
+					 "eth_txd2",
+					 "eth_txd3";
+				function = "ethernet";
+			};
+		};
 	};
 };
 
 &ethmac {
-	clocks = <&clkc CLKID_ETH>;
-	clock-names = "stmmaceth";
+	compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";
+
+	reg = <0xc9410000 0x10000
+	       0xc1108140 0x8>;
+
+	interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+		     <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>;
+	interrupt-names = "macirq",
+			  "eth_lpi";
+
+	clock-names = "stmmaceth", "clkin0", "clkin1";
+	clocks = <&clkc CLKID_ETH>,
+		 <&clkc CLKID_FCLK_DIV2>,
+		 <&clkc CLKID_MPLL2>;
+
+	resets = <&reset RESET_ETHERNET>;
+	reset-names = "stmmaceth";
+
+	rx-fifo-depth=<4000>;
+	tx-fifo-depth=<2000>;
 };
 
 &hwrng {
Signature · 1 line
2.14.1
#8 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

In reply to Emiliano Ingrassia

This patch adds ethernet controller pin description and extend its attributes in the relative node.

---

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

Please, apply this patch and discard the previous
(450a483abe07f8d903c6cb74091592743975a8eb).

 arch/arm/boot/dts/meson8b.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)
arch/arm/boot/dts/meson8b.dtsi +38 -2
index bc278da7df0d..816bc9188f44 100644
@@ -154,12 +154,48 @@
 			#gpio-cells = <2>;
 			gpio-ranges = <&pinctrl_cbus 0 0 130>;
 		};
+
+		eth_rgmii_pins: eth-rgmii {
+			mux {
+				groups = "eth_tx_clk",
+					 "eth_tx_en",
+					 "eth_txd1_0",
+					 "eth_txd1_1",
+					 "eth_txd0_0",
+					 "eth_txd0_1",
+					 "eth_rx_clk",
+					 "eth_rx_dv",
+					 "eth_rxd1",
+					 "eth_rxd0",
+					 "eth_mdio_en",
+					 "eth_mdc",
+					 "eth_ref_clk",
+					 "eth_txd2",
+					 "eth_txd3";
+				function = "ethernet";
+			};
+		};
 	};
 };
 
 &ethmac {
-	clocks = <&clkc CLKID_ETH>;
-	clock-names = "stmmaceth";
+	compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";
+
+	interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+		     <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>;
+	interrupt-names = "macirq",
+			  "eth_lpi";
+
+	clock-names = "stmmaceth", "clkin0", "clkin1";
+	clocks = <&clkc CLKID_ETH>,
+		 <&clkc CLKID_FCLK_DIV2>,
+		 <&clkc CLKID_MPLL2>;
+
+	resets = <&reset RESET_ETHERNET>;
+	reset-names = "stmmaceth";
+
+	rx-fifo-depth=<4000>;
+	tx-fifo-depth=<2000>;
 };
 
 &hwrng {
Signature · 1 line
2.14.1
#9 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

Quoted 17 lines · Emiliano Ingrassia

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

[..]
&ethmac {

arch/arm/boot/dts/meson8b.dtsi +8 -2
-	clocks = <&clkc CLKID_ETH>;
-	clock-names = "stmmaceth";
+	compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";
+
+	interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+		     <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>;
+	interrupt-names = "macirq",
+			  "eth_lpi";
+
+	clock-names = "stmmaceth", "clkin0", "clkin1";

arch/arm/boot/dts/meson8b.dtsi line 199

Quoted 10 lines
arch/arm/boot/dts/meson8b.dtsi +9 -0
+	clocks = <&clkc CLKID_ETH>,
+		 <&clkc CLKID_FCLK_DIV2>,
+		 <&clkc CLKID_MPLL2>;
+
+	resets = <&reset RESET_ETHERNET>;
+	reset-names = "stmmaceth";
+
+	rx-fifo-depth=<4000>;
+	tx-fifo-depth=<2000>;
 };

Hi Emiliano,

Did you accidentally delete instead of replace the reg values? (or are there default values hidden somewhere else?)

Also, please don't forget to add a v2/v3/etc. next time, makes it easier to follow here and in Patchwork, thanks :-).

And thanks a lot at looking into ethernet support as well. Will try this patchset soon, too! You guys are killing it :D!

Regards, Linus

#10 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

Hi Linus,

thanks for the review!

Quoted 31 lines · Linus L?ssing
Quoted 17 lines · Emiliano Ingrassia

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

[..]
&ethmac {

arch/arm/boot/dts/meson8b.dtsi +8 -2
-	clocks = <&clkc CLKID_ETH>;
-	clock-names = "stmmaceth";
+	compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";
+
+	interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+		     <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>;
+	interrupt-names = "macirq",
+			  "eth_lpi";
+
+	clock-names = "stmmaceth", "clkin0", "clkin1";

arch/arm/boot/dts/meson8b.dtsi line 199

Quoted 10 lines
arch/arm/boot/dts/meson8b.dtsi +9 -0
+	clocks = <&clkc CLKID_ETH>,
+		 <&clkc CLKID_FCLK_DIV2>,
+		 <&clkc CLKID_MPLL2>;
+
+	resets = <&reset RESET_ETHERNET>;
+	reset-names = "stmmaceth";
+
+	rx-fifo-depth=<4000>;
+	tx-fifo-depth=<2000>;
 };

Hi Emiliano,

Quoted 3 lines

Did you accidentally delete instead of replace the reg values? (or are there default values hidden somewhere else?)

I intentionally deleted the reg attribute which is already included in meson.dtsi. That attribute contains two address ranges, the second of which is not reported on the S805 SoC manual and probably taken from Amlogic's 3.10 GPL kernel sources.

Quoted 4 lines

Also, please don't forget to add a v2/v3/etc. next time, makes it easier to follow here and in Patchwork, thanks :-).

Sure, sorry for the incovenience.

Quoted 3 lines

And thanks a lot at looking into ethernet support as well. Will try this patchset soon, too! You guys are killing it :D!

Thanks! :D

Quoted 1 line

Regards, Linus

Regards,

Emiliano

#11 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

In reply to Emiliano Ingrassia

Hi Emiliano,

Quoted 43 lines · Emiliano Ingrassia

This patch adds ethernet controller pin description and extend its attributes in the relative node.

---

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

Please, apply this patch and discard the previous
(450a483abe07f8d903c6cb74091592743975a8eb).

 arch/arm/boot/dts/meson8b.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)
arch/arm/boot/dts/meson8b.dtsi +18 -0
index bc278da7df0d..816bc9188f44 100644
@@ -154,12 +154,48 @@
                        #gpio-cells = <2>;
                        gpio-ranges = <&pinctrl_cbus 0 0 130>;
                };
+
+               eth_rgmii_pins: eth-rgmii {
+                       mux {
+                               groups = "eth_tx_clk",
+                                        "eth_tx_en",
+                                        "eth_txd1_0",
+                                        "eth_txd1_1",
+                                        "eth_txd0_0",
+                                        "eth_txd0_1",
+                                        "eth_rx_clk",
+                                        "eth_rx_dv",
+                                        "eth_rxd1",
+                                        "eth_rxd0",
+                                        "eth_mdio_en",
+                                        "eth_mdc",
+                                        "eth_ref_clk",
+                                        "eth_txd2",
+                                        "eth_txd3";

arch/arm/boot/dts/meson8b.dtsi line 182

Quoted 10 lines
arch/arm/boot/dts/meson8b.dtsi +4 -2
+                               function = "ethernet";
+                       };
+               };
        };
 };
 
 &ethmac {
-       clocks = <&clkc CLKID_ETH>;
-       clock-names = "stmmaceth";
+       compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";

without a reg property this passes 0xc1108108 (as defined in meson.dtsi) to the meson8b-dwmac driver.
are you sure that this shouldn't be 0xc1108140 (like in your initial patch)? 0xc1108108 translates to 0x2050 (calculation formula: (0xc1108108 - cbus base addr 0xc1100000) / 4) which is used in Amlogic's u-boot sources, for example [0]

currently the meson8b-dwmac driver is writing to the old register location which probably does nothing.
if above statement is true then you are relying on the bootloader to set up 0xc1108140 correctly.
the reason why I wrote this meson8b-dwmac driver is because I had a GXBB board with RGMII PHY but u-boot configured the register to RMII mode -> ethernet wasn't working.
you could verify this by zeroing both (0xc1108108 and 0xc1108140) in u-boot or at the start of the meson8b-dwmac driver and see if ethernet still works for you

arch/arm/boot/dts/meson8b.dtsi line 187

Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+                    <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>;
+       interrupt-names = "macirq",
+                         "eth_lpi";

did you receive one of the eth_lpi interrupts? if it works for you then we should try to add this to meson-gx.dtsi as well
I also wonder if we should configure it in meson8b.dtsi or meson.dtsi

arch/arm/boot/dts/meson8b.dtsi line 195

Quoted 8 lines
arch/arm/boot/dts/meson8b.dtsi +8 -0
+
+       clock-names = "stmmaceth", "clkin0", "clkin1";
+       clocks = <&clkc CLKID_ETH>,
+                <&clkc CLKID_FCLK_DIV2>,
+                <&clkc CLKID_MPLL2>;
+
+       resets = <&reset RESET_ETHERNET>;
+       reset-names = "stmmaceth";

I'm not sure if this works:
our reset controller implements a reset pulse (write bit, IP block executes a reset and clears the bit again)
stmmac on the other hand manually asserts and deasserts the reset line (which is not implemented by our reset driver), see [1]

arch/arm/boot/dts/meson8b.dtsi line 198

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       rx-fifo-depth=<4000>;
+       tx-fifo-depth=<2000>;

could you please add spaces around "=" and some info to the commit message why this is necessary and where you got these values from

Quoted 11 lines
arch/arm/boot/dts/meson8b.dtsi +0 -1
 };
 
 &hwrng {
--

2.14.1

_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

looking forward to proper ethernet support on Meson8/Meson8b!

Regards,
Martin

[0] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L29
[1] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#L4123

#12 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

Hi Martin,

thanks for the review!

Quoted 53 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 49 lines · Emiliano Ingrassia

This patch adds ethernet controller pin description and extend its attributes in the relative node.

---

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

Please, apply this patch and discard the previous
(450a483abe07f8d903c6cb74091592743975a8eb).

 arch/arm/boot/dts/meson8b.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)
arch/arm/boot/dts/meson8b.dtsi +21 -0
index bc278da7df0d..816bc9188f44 100644
@@ -154,12 +154,48 @@
                        #gpio-cells = <2>;
                        gpio-ranges = <&pinctrl_cbus 0 0 130>;
                };
+
+               eth_rgmii_pins: eth-rgmii {
+                       mux {
+                               groups = "eth_tx_clk",
+                                        "eth_tx_en",
+                                        "eth_txd1_0",
+                                        "eth_txd1_1",
+                                        "eth_txd0_0",
+                                        "eth_txd0_1",
+                                        "eth_rx_clk",
+                                        "eth_rx_dv",
+                                        "eth_rxd1",
+                                        "eth_rxd0",
+                                        "eth_mdio_en",
+                                        "eth_mdc",
+                                        "eth_ref_clk",
+                                        "eth_txd2",
+                                        "eth_txd3";
+                               function = "ethernet";
+                       };
+               };
        };
 };
 
Quoted 11 lines

arch/arm/boot/dts/meson8b.dtsi line 182

Quoted 4 lines
arch/arm/boot/dts/meson8b.dtsi +1 -2
 &ethmac {
-       clocks = <&clkc CLKID_ETH>;
-       clock-names = "stmmaceth";
+       compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";

without a reg property this passes 0xc1108108 (as defined in meson.dtsi) to the meson8b-dwmac driver.
are you sure that this shouldn't be 0xc1108140 (like in your initial patch)? 0xc1108108 translates to 0x2050 (calculation formula: (0xc1108108 - cbus base addr 0xc1100000) / 4) which is used in Amlogic's u-boot sources, for example [0]

Yes, I know. This was the intention.

Quoted 2 lines

currently the meson8b-dwmac driver is writing to the old register location which probably does nothing.

Actually, changing the second addresses range from 0xc1108108 to 0xc1108140 leads to an unusable ethernet controller.

Quoted 2 lines

if above statement is true then you are relying on the bootloader to set up 0xc1108140 correctly.

OK, sure this is bad! Those addresses are documented in S805 SoC manual and we should set up them correctly.
However, checking the Odroid-C2 device tree I couldn't find the PRG_ETHERNET_ADDR{0,1} addresses, which are documented in S905 SoC manual.
Probably I'm missing something, but don't we have the same situation on that board?

Quoted 3 lines

the reason why I wrote this meson8b-dwmac driver is because I had a GXBB board with RGMII PHY but u-boot configured the register to RMII mode -> ethernet wasn't working.

Meson8b refers to S805 SoC while GXBB should refer to S905 SoC. At first look it seemed to me that dwmac-meson8b driver correctly support dwmac on Meson8b or we should extend the driver to better support it?

Quoted 4 lines

you could verify this by zeroing both (0xc1108108 and 0xc1108140) in u-boot or at the start of the meson8b-dwmac driver and see if ethernet still works for you

I'll check it.

Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 187

Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+                    <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>;
+       interrupt-names = "macirq",
+                         "eth_lpi";

did you receive one of the eth_lpi interrupts? if it works for you then we should try to add this to meson-gx.dtsi as well
I also wonder if we should configure it in meson8b.dtsi or meson.dtsi

Needs more testing.

Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       clock-names = "stmmaceth", "clkin0", "clkin1";
+       clocks = <&clkc CLKID_ETH>,
+                <&clkc CLKID_FCLK_DIV2>,
+                <&clkc CLKID_MPLL2>;
Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 195

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       resets = <&reset RESET_ETHERNET>;
+       reset-names = "stmmaceth";

I'm not sure if this works:
our reset controller implements a reset pulse (write bit, IP block executes a reset and clears the bit again)
stmmac on the other hand manually asserts and deasserts the reset line (which is not implemented by our reset driver), see [1]

OK, I'll check and eventually fix this.

Quoted 6 lines

arch/arm/boot/dts/meson8b.dtsi line 198

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       rx-fifo-depth=<4000>;
+       tx-fifo-depth=<2000>;

could you please add spaces around "=" and some info to the commit message why this is necessary and where you got these values from

Those are optional attributes documented in Documentation/devicetree/bindings/net/stmmac.txt.
The values were taken from S805 SoC manual, ch. 22 (ETHERNET MAC).

Quoted 7 lines
Quoted 7 lines
arch/arm/boot/dts/meson8b.dtsi +0 -1
 };
 
 &hwrng {
--

2.14.1

Quoted 7 lines
Quoted 4 lines

_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

looking forward to proper ethernet support on Meson8/Meson8b!

OK, thanks for your suggestions!

Quoted 4 lines

Regards,
Martin

Regards,

Emiliano

Quoted 3 lines

[0] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L29
[1] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#L4123

#13 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

Hi Emiliano,

Quoted 70 lines · Emiliano Ingrassia

Hi Martin,

thanks for the review!

Quoted 53 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 49 lines · Emiliano Ingrassia

This patch adds ethernet controller pin description and extend its attributes in the relative node.

---

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

Please, apply this patch and discard the previous
(450a483abe07f8d903c6cb74091592743975a8eb).

 arch/arm/boot/dts/meson8b.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)
arch/arm/boot/dts/meson8b.dtsi +21 -0
index bc278da7df0d..816bc9188f44 100644
@@ -154,12 +154,48 @@
                        #gpio-cells = <2>;
                        gpio-ranges = <&pinctrl_cbus 0 0 130>;
                };
+
+               eth_rgmii_pins: eth-rgmii {
+                       mux {
+                               groups = "eth_tx_clk",
+                                        "eth_tx_en",
+                                        "eth_txd1_0",
+                                        "eth_txd1_1",
+                                        "eth_txd0_0",
+                                        "eth_txd0_1",
+                                        "eth_rx_clk",
+                                        "eth_rx_dv",
+                                        "eth_rxd1",
+                                        "eth_rxd0",
+                                        "eth_mdio_en",
+                                        "eth_mdc",
+                                        "eth_ref_clk",
+                                        "eth_txd2",
+                                        "eth_txd3";
+                               function = "ethernet";
+                       };
+               };
        };
 };
 
Quoted 11 lines

arch/arm/boot/dts/meson8b.dtsi line 182

Quoted 4 lines
arch/arm/boot/dts/meson8b.dtsi +1 -2
 &ethmac {
-       clocks = <&clkc CLKID_ETH>;
-       clock-names = "stmmaceth";
+       compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";

without a reg property this passes 0xc1108108 (as defined in meson.dtsi) to the meson8b-dwmac driver.
are you sure that this shouldn't be 0xc1108140 (like in your initial patch)? 0xc1108108 translates to 0x2050 (calculation formula: (0xc1108108 - cbus base addr 0xc1100000) / 4) which is used in Amlogic's u-boot sources, for example [0]

Quoted 1 line

Yes, I know. This was the intention.

OK, this is interesting

Quoted 2 lines
Quoted 2 lines

currently the meson8b-dwmac driver is writing to the old register location which probably does nothing.

without your change the meson6-dwmac driver is used. when I wrote the meson8b-dwmac driver I documented the following behavior (see [0]):
"This worked for many boards because the bootloader programs the PRG_ETHERNET registers correctly. Additionally the meson6-dwmac driver only sets bit 1 of PRG_ETHERNET_ADDR0 which (according to the datasheet) is only used during reset."

Quoted 2 lines

Actually, changing the second addresses range from 0xc1108108 to 0xc1108140 leads to an unusable ethernet controller.

Odroid-C1's u-boot writes 0x7d21 to 0xc1108140 (= (0x2050 * 4) + cbus base), see [1]
have you tried to verify that writing 0x7d21 (= the value used in the Odroid-C1 u-boot sources) at the end of meson8b_init_prg_eth makes it work for you again? if it does then we are not setting the register values correctly (which may simply be related to the clock setup - either the internal clock in the meson8b-dwmac driver or the "other" ethernet clock)

Quoted 10 lines
Quoted 2 lines

if above statement is true then you are relying on the bootloader to set up 0xc1108140 correctly.

OK, sure this is bad! Those addresses are documented in S805 SoC manual and we should set up them correctly.
However, checking the Odroid-C2 device tree I couldn't find the PRG_ETHERNET_ADDR{0,1} addresses, which are documented in S905 SoC manual.
Probably I'm missing something, but don't we have the same situation on that board?

I'm not sure if I understand you correctly:
- the S805 datasheet mentioned that the addresses of the PRG_ETHERNET_ADDR{0,1} registers are 0x2050 and 0x2051
- we assume that the PRG_ETHERNET_ADDR{0,1} address from the S905 datasheet is wrong, see [4]
- Mike got feedback from Amlogic confirming that the documentation in the S905 datasheet is not fully correct, see [2] and [5]. if they use the same IP block in Meson8b (like, due to the identical register description in the S805 and S905 datasheets) then the S805 datasheet is also wrong

Quoted 8 lines
Quoted 3 lines

the reason why I wrote this meson8b-dwmac driver is because I had a GXBB board with RGMII PHY but u-boot configured the register to RMII mode -> ethernet wasn't working.

Meson8b refers to S805 SoC while GXBB should refer to S905 SoC. At first look it seemed to me that dwmac-meson8b driver correctly support dwmac on Meson8b or we should extend the driver to better support it?

I added compatible strings for both SoCs (see [3]) because the register description in both datasheets is identical. dwmac-meson8b works fine on all known GXBB/GXL/GXM devices
so it's the Meson8b compatibility that has to be improved (if it has to be improved, but for that we have to figure out the clock setup too).

Quoted 6 lines
Quoted 4 lines

you could verify this by zeroing both (0xc1108108 and 0xc1108140) in u-boot or at the start of the meson8b-dwmac driver and see if ethernet still works for you

I'll check it.

thank you!
please also see also my comment above regarding 0x7d21 from Odroid-C1's u-boot sources

Quoted 27 lines
Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 187

Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+                    <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>;
+       interrupt-names = "macirq",
+                         "eth_lpi";

did you receive one of the eth_lpi interrupts? if it works for you then we should try to add this to meson-gx.dtsi as well
I also wonder if we should configure it in meson8b.dtsi or meson.dtsi

Needs more testing.

Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       clock-names = "stmmaceth", "clkin0", "clkin1";
+       clocks = <&clkc CLKID_ETH>,
+                <&clkc CLKID_FCLK_DIV2>,
+                <&clkc CLKID_MPLL2>;
Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 195

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       resets = <&reset RESET_ETHERNET>;
+       reset-names = "stmmaceth";

I'm not sure if this works:
our reset controller implements a reset pulse (write bit, IP block executes a reset and clears the bit again)
stmmac on the other hand manually asserts and deasserts the reset line (which is not implemented by our reset driver), see [1]

Quoted 1 line

OK, I'll check and eventually fix this.

as a small hint: on Meson GX (GXBB and newer) we currently do not configure the reset line
if it's needed on Meson8b then you could implement it in a separate patch

Quoted 10 lines
Quoted 6 lines

arch/arm/boot/dts/meson8b.dtsi line 198

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       rx-fifo-depth=<4000>;
+       tx-fifo-depth=<2000>;

could you please add spaces around "=" and some info to the commit message why this is necessary and where you got these values from

Those are optional attributes documented in Documentation/devicetree/bindings/net/stmmac.txt.
The values were taken from S805 SoC manual, ch. 22 (ETHERNET MAC).

ok, then these are identical on GXBB and newer (according to the datasheets) I wonder if the stmmac driver auto-detects the correct value when these are not set. otherwise we should also configure these on the GX SoCs

Quoted 18 lines
Quoted 7 lines
Quoted 7 lines
arch/arm/boot/dts/meson8b.dtsi +0 -1
 };
 
 &hwrng {
--

2.14.1

Quoted 7 lines
Quoted 4 lines

_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

looking forward to proper ethernet support on Meson8/Meson8b!

OK, thanks for your suggestions!

Quoted 1 line
Quoted 11 lines
Quoted 3 lines

Regards,
Martin

Regards,

Emiliano

Quoted 3 lines

[0] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L29
[1] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#L4123

Regards,
Martin

[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-August/000764.html
[1] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L59
[2] http://lists.infradead.org/pipermail/linux-amlogic/2016-July/000645.html
[3] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c#L330
[4] http://lists.infradead.org/pipermail/linux-amlogic/2016-September/001055.html
[5] http://lists.infradead.org/pipermail/linux-amlogic/2016-July/000646.html

#14 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

Hi Martin,

sorry for my very late response!

Quoted 74 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 70 lines · Emiliano Ingrassia

Hi Martin,

thanks for the review!

Quoted 64 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 43 lines · Emiliano Ingrassia

This patch adds ethernet controller pin description and extend its attributes in the relative node.

---

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

Please, apply this patch and discard the previous
(450a483abe07f8d903c6cb74091592743975a8eb).

 arch/arm/boot/dts/meson8b.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)
arch/arm/boot/dts/meson8b.dtsi +18 -0
index bc278da7df0d..816bc9188f44 100644
@@ -154,12 +154,48 @@
                        #gpio-cells = <2>;
                        gpio-ranges = <&pinctrl_cbus 0 0 130>;
                };
+
+               eth_rgmii_pins: eth-rgmii {
+                       mux {
+                               groups = "eth_tx_clk",
+                                        "eth_tx_en",
+                                        "eth_txd1_0",
+                                        "eth_txd1_1",
+                                        "eth_txd0_0",
+                                        "eth_txd0_1",
+                                        "eth_rx_clk",
+                                        "eth_rx_dv",
+                                        "eth_rxd1",
+                                        "eth_rxd0",
+                                        "eth_mdio_en",
+                                        "eth_mdc",
+                                        "eth_ref_clk",
+                                        "eth_txd2",
+                                        "eth_txd3";

arch/arm/boot/dts/meson8b.dtsi line 182

Quoted 10 lines
arch/arm/boot/dts/meson8b.dtsi +4 -2
+                               function = "ethernet";
+                       };
+               };
        };
 };
 
 &ethmac {
-       clocks = <&clkc CLKID_ETH>;
-       clock-names = "stmmaceth";
+       compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";

without a reg property this passes 0xc1108108 (as defined in meson.dtsi) to the meson8b-dwmac driver.
are you sure that this shouldn't be 0xc1108140 (like in your initial patch)? 0xc1108108 translates to 0x2050 (calculation formula: (0xc1108108 - cbus base addr 0xc1100000) / 4) which is used in Amlogic's u-boot sources, for example [0]

Quoted 3 lines
Quoted 1 line

Yes, I know. This was the intention.

OK, this is interesting

After some research I agree with you: the correct ethernet register address is 0xc1108140.

Quoted 11 lines
Quoted 2 lines
Quoted 2 lines

currently the meson8b-dwmac driver is writing to the old register location which probably does nothing.

without your change the meson6-dwmac driver is used. when I wrote the meson8b-dwmac driver I documented the following behavior (see [0]):
"This worked for many boards because the bootloader programs the PRG_ETHERNET registers correctly. Additionally the meson6-dwmac driver only sets bit 1 of PRG_ETHERNET_ADDR0 which (according to the datasheet) is only used during reset."

Quoted 2 lines

Actually, changing the second addresses range from 0xc1108108 to 0xc1108140 leads to an unusable ethernet controller.

Quoted 8 lines

Odroid-C1's u-boot writes 0x7d21 to 0xc1108140 (= (0x2050 * 4) + cbus base), see [1]
have you tried to verify that writing 0x7d21 (= the value used in the Odroid-C1 u-boot sources) at the end of meson8b_init_prg_eth makes it work for you again? if it does then we are not setting the register values correctly (which may simply be related to the clock setup - either the internal clock in the meson8b-dwmac driver or the "other" ethernet clock)

Actually, writing 0x7d21 at the end of the initialization procedure leads to a working ethernet controller. Consider that I'm using MPLL2 as clock source for both "clkin0" and "clkin1" because, as stated in S805 SoC manual, "M8Baby internal clock source is mp2_clk_out only.".

Investigating dwmac-meson8b.c, a possible error lies in the use of prg_ethernet_addr0 register bits 9-7.
Infact, they are used as field for CLK_M250_DIV value, which it seems to me incorrect. From the SoC manual, those bits should be set as mpll2 clock rate divided by 250*1000*1000.
Then, I guess that the correct divider value for 25 Mhz PHY clock is internally derived.

Best regards,

Emiliano

Quoted 135 lines
Quoted 11 lines
Quoted 3 lines

if above statement is true then you are relying on the bootloader to set up 0xc1108140 correctly.

OK, sure this is bad! Those addresses are documented in S805 SoC manual and we should set up them correctly.
However, checking the Odroid-C2 device tree I couldn't find the PRG_ETHERNET_ADDR{0,1} addresses, which are documented in S905 SoC manual.
Probably I'm missing something, but don't we have the same situation on that board?

I'm not sure if I understand you correctly:
- the S805 datasheet mentioned that the addresses of the PRG_ETHERNET_ADDR{0,1} registers are 0x2050 and 0x2051
- we assume that the PRG_ETHERNET_ADDR{0,1} address from the S905 datasheet is wrong, see [4]
- Mike got feedback from Amlogic confirming that the documentation in the S905 datasheet is not fully correct, see [2] and [5]. if they use the same IP block in Meson8b (like, due to the identical register description in the S805 and S905 datasheets) then the S805 datasheet is also wrong

Quoted 8 lines
Quoted 3 lines

the reason why I wrote this meson8b-dwmac driver is because I had a GXBB board with RGMII PHY but u-boot configured the register to RMII mode -> ethernet wasn't working.

Meson8b refers to S805 SoC while GXBB should refer to S905 SoC. At first look it seemed to me that dwmac-meson8b driver correctly support dwmac on Meson8b or we should extend the driver to better support it?

I added compatible strings for both SoCs (see [3]) because the register description in both datasheets is identical. dwmac-meson8b works fine on all known GXBB/GXL/GXM devices
so it's the Meson8b compatibility that has to be improved (if it has to be improved, but for that we have to figure out the clock setup too).

Quoted 6 lines
Quoted 4 lines

you could verify this by zeroing both (0xc1108108 and 0xc1108140) in u-boot or at the start of the meson8b-dwmac driver and see if ethernet still works for you

I'll check it.

thank you!
please also see also my comment above regarding 0x7d21 from Odroid-C1's u-boot sources

Quoted 27 lines
Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 187

Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+                    <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>;
+       interrupt-names = "macirq",
+                         "eth_lpi";

did you receive one of the eth_lpi interrupts? if it works for you then we should try to add this to meson-gx.dtsi as well
I also wonder if we should configure it in meson8b.dtsi or meson.dtsi

Needs more testing.

Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       clock-names = "stmmaceth", "clkin0", "clkin1";
+       clocks = <&clkc CLKID_ETH>,
+                <&clkc CLKID_FCLK_DIV2>,
+                <&clkc CLKID_MPLL2>;
Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 195

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       resets = <&reset RESET_ETHERNET>;
+       reset-names = "stmmaceth";

I'm not sure if this works:
our reset controller implements a reset pulse (write bit, IP block executes a reset and clears the bit again)
stmmac on the other hand manually asserts and deasserts the reset line (which is not implemented by our reset driver), see [1]

Quoted 1 line

OK, I'll check and eventually fix this.

as a small hint: on Meson GX (GXBB and newer) we currently do not configure the reset line
if it's needed on Meson8b then you could implement it in a separate patch

Quoted 10 lines
Quoted 6 lines

arch/arm/boot/dts/meson8b.dtsi line 198

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       rx-fifo-depth=<4000>;
+       tx-fifo-depth=<2000>;

could you please add spaces around "=" and some info to the commit message why this is necessary and where you got these values from

Those are optional attributes documented in Documentation/devicetree/bindings/net/stmmac.txt.
The values were taken from S805 SoC manual, ch. 22 (ETHERNET MAC).

ok, then these are identical on GXBB and newer (according to the datasheets) I wonder if the stmmac driver auto-detects the correct value when these are not set. otherwise we should also configure these on the GX SoCs

Quoted 18 lines
Quoted 7 lines
Quoted 7 lines
arch/arm/boot/dts/meson8b.dtsi +0 -1
 };
 
 &hwrng {
--

2.14.1

Quoted 7 lines
Quoted 4 lines

_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

looking forward to proper ethernet support on Meson8/Meson8b!

OK, thanks for your suggestions!

Quoted 1 line
Quoted 11 lines
Quoted 3 lines

Regards,
Martin

Regards,

Emiliano

Quoted 3 lines

[0] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L29
[1] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#L4123

Regards,
Martin

[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-August/000764.html
[1] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L59
[2] http://lists.infradead.org/pipermail/linux-amlogic/2016-July/000645.html
[3] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c#L330
[4] http://lists.infradead.org/pipermail/linux-amlogic/2016-September/001055.html
[5] http://lists.infradead.org/pipermail/linux-amlogic/2016-July/000646.html

#15 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

Hi Emiliano,

Quoted 3 lines · Emiliano Ingrassia

Hi Martin,

sorry for my very late response!

no worries, I'm also very late with this mail

Quoted 76 lines · Martin Blumenstingl
Quoted 74 lines

Hi Emiliano,

Quoted 70 lines · Emiliano Ingrassia

Hi Martin,

thanks for the review!

Quoted 64 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 43 lines · Emiliano Ingrassia

This patch adds ethernet controller pin description and extend its attributes in the relative node.

---

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

Please, apply this patch and discard the previous
(450a483abe07f8d903c6cb74091592743975a8eb).

 arch/arm/boot/dts/meson8b.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)
arch/arm/boot/dts/meson8b.dtsi +18 -0
index bc278da7df0d..816bc9188f44 100644
@@ -154,12 +154,48 @@
                        #gpio-cells = <2>;
                        gpio-ranges = <&pinctrl_cbus 0 0 130>;
                };
+
+               eth_rgmii_pins: eth-rgmii {
+                       mux {
+                               groups = "eth_tx_clk",
+                                        "eth_tx_en",
+                                        "eth_txd1_0",
+                                        "eth_txd1_1",
+                                        "eth_txd0_0",
+                                        "eth_txd0_1",
+                                        "eth_rx_clk",
+                                        "eth_rx_dv",
+                                        "eth_rxd1",
+                                        "eth_rxd0",
+                                        "eth_mdio_en",
+                                        "eth_mdc",
+                                        "eth_ref_clk",
+                                        "eth_txd2",
+                                        "eth_txd3";

arch/arm/boot/dts/meson8b.dtsi line 182

Quoted 10 lines
arch/arm/boot/dts/meson8b.dtsi +4 -2
+                               function = "ethernet";
+                       };
+               };
        };
 };
 
 &ethmac {
-       clocks = <&clkc CLKID_ETH>;
-       clock-names = "stmmaceth";
+       compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";

without a reg property this passes 0xc1108108 (as defined in meson.dtsi) to the meson8b-dwmac driver.
are you sure that this shouldn't be 0xc1108140 (like in your initial patch)? 0xc1108108 translates to 0x2050 (calculation formula: (0xc1108108 - cbus base addr 0xc1100000) / 4) which is used in Amlogic's u-boot sources, for example [0]

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Yes, I know. This was the intention.

OK, this is interesting

After some research I agree with you: the correct ethernet register address is 0xc1108140.

great - thank you for checking!

Quoted 21 lines
Quoted 12 lines
Quoted 3 lines
Quoted 3 lines

currently the meson8b-dwmac driver is writing to the old register location which probably does nothing.

without your change the meson6-dwmac driver is used. when I wrote the meson8b-dwmac driver I documented the following behavior (see [0]):
"This worked for many boards because the bootloader programs the PRG_ETHERNET registers correctly. Additionally the meson6-dwmac driver only sets bit 1 of PRG_ETHERNET_ADDR0 which (according to the datasheet) is only used during reset."

Quoted 2 lines

Actually, changing the second addresses range from 0xc1108108 to 0xc1108140 leads to an unusable ethernet controller.

Quoted 8 lines

Odroid-C1's u-boot writes 0x7d21 to 0xc1108140 (= (0x2050 * 4) + cbus base), see [1]
have you tried to verify that writing 0x7d21 (= the value used in the Odroid-C1 u-boot sources) at the end of meson8b_init_prg_eth makes it work for you again? if it does then we are not setting the register values correctly (which may simply be related to the clock setup - either the internal clock in the meson8b-dwmac driver or the "other" ethernet clock)

Quoted 4 lines

Actually, writing 0x7d21 at the end of the initialization procedure leads to a working ethernet controller. Consider that I'm using MPLL2 as clock source for both "clkin0" and "clkin1" because, as stated in S805 SoC manual, "M8Baby internal clock source is mp2_clk_out only.".

this value is pretty close to the one on GXBB (and newer) SoCs which use 0x1621 what is the MPLL2 clock rate on your board (my Meson8m2 and Meson8b boards both have a RMII PHY)?

Quoted 5 lines

Investigating dwmac-meson8b.c, a possible error lies in the use of prg_ethernet_addr0 register bits 9-7.
Infact, they are used as field for CLK_M250_DIV value, which it seems to me incorrect. From the SoC manual, those bits should be set as mpll2 clock rate divided by 250*1000*1000.

let's do the maths:
this would work fine on GXBB (and later) where PRG_ETH0_CLK_M250_DIV is 0x4 and the parent clock is FCLK_DIV2 which is at 1GHz. 1GHz / 1000 / 1000 / 250 = 0x4
"unfortunately" it also works the other way round: 1GHz / 0x4 = 250MHz

on your Odroid-C1 PRG_ETH0_CLK_M250_DIV is 0x2 - if the same maths still applies then MPLL2 should be at 500MHz:
500MHz / 1000 / 1000 / 250 = 0x2
however, it also works the other way round: 500MHz / 0x2 = 250MHz

Mike was also under the impression that these bits are a divider back when he and Kevin received some clarifications (which unfortunately have not made it into the GXL and GXM datasheets) for the PRG_ETHERNET_ADDR0 registers: [0]

Quoted 2 lines

Then, I guess that the correct divider value for 25 Mhz PHY clock is internally derived.

let's go one-by-one to figure this out and clarify that "divider" first

I also did some homework and found a struct that describes
PRG_ETHERNET_ADDR0: [1]
- the clock bits (mux, divider, phase / delay) are describing the RGMII TX clock (I should probably rename the clocks to rgmii_<current name>)
- in other words: these are not relevant for RMII at all (dwmac-meson8b currently ignores this fact and still tries to do some magic with these bits)
- nice-to-have: the TX delay could be implemented as a clock with .set_phase/.get_phase callbacks (this would also expose the phase in debugfs/clk/...)

Quoted 139 lines

Best regards,

Emiliano

Quoted 135 lines
Quoted 11 lines
Quoted 3 lines

if above statement is true then you are relying on the bootloader to set up 0xc1108140 correctly.

OK, sure this is bad! Those addresses are documented in S805 SoC manual and we should set up them correctly.
However, checking the Odroid-C2 device tree I couldn't find the PRG_ETHERNET_ADDR{0,1} addresses, which are documented in S905 SoC manual.
Probably I'm missing something, but don't we have the same situation on that board?

I'm not sure if I understand you correctly:
- the S805 datasheet mentioned that the addresses of the PRG_ETHERNET_ADDR{0,1} registers are 0x2050 and 0x2051
- we assume that the PRG_ETHERNET_ADDR{0,1} address from the S905 datasheet is wrong, see [4]
- Mike got feedback from Amlogic confirming that the documentation in the S905 datasheet is not fully correct, see [2] and [5]. if they use the same IP block in Meson8b (like, due to the identical register description in the S805 and S905 datasheets) then the S805 datasheet is also wrong

Quoted 8 lines
Quoted 3 lines

the reason why I wrote this meson8b-dwmac driver is because I had a GXBB board with RGMII PHY but u-boot configured the register to RMII mode -> ethernet wasn't working.

Meson8b refers to S805 SoC while GXBB should refer to S905 SoC. At first look it seemed to me that dwmac-meson8b driver correctly support dwmac on Meson8b or we should extend the driver to better support it?

I added compatible strings for both SoCs (see [3]) because the register description in both datasheets is identical. dwmac-meson8b works fine on all known GXBB/GXL/GXM devices
so it's the Meson8b compatibility that has to be improved (if it has to be improved, but for that we have to figure out the clock setup too).

Quoted 6 lines
Quoted 4 lines

you could verify this by zeroing both (0xc1108108 and 0xc1108140) in u-boot or at the start of the meson8b-dwmac driver and see if ethernet still works for you

I'll check it.

thank you!
please also see also my comment above regarding 0x7d21 from Odroid-C1's u-boot sources

Quoted 27 lines
Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 187

Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+                    <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>;
+       interrupt-names = "macirq",
+                         "eth_lpi";

did you receive one of the eth_lpi interrupts? if it works for you then we should try to add this to meson-gx.dtsi as well
I also wonder if we should configure it in meson8b.dtsi or meson.dtsi

Needs more testing.

Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       clock-names = "stmmaceth", "clkin0", "clkin1";
+       clocks = <&clkc CLKID_ETH>,
+                <&clkc CLKID_FCLK_DIV2>,
+                <&clkc CLKID_MPLL2>;
Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 195

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       resets = <&reset RESET_ETHERNET>;
+       reset-names = "stmmaceth";

I'm not sure if this works:
our reset controller implements a reset pulse (write bit, IP block executes a reset and clears the bit again)
stmmac on the other hand manually asserts and deasserts the reset line (which is not implemented by our reset driver), see [1]

Quoted 1 line

OK, I'll check and eventually fix this.

as a small hint: on Meson GX (GXBB and newer) we currently do not configure the reset line
if it's needed on Meson8b then you could implement it in a separate patch

Quoted 10 lines
Quoted 6 lines

arch/arm/boot/dts/meson8b.dtsi line 198

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       rx-fifo-depth=<4000>;
+       tx-fifo-depth=<2000>;

could you please add spaces around "=" and some info to the commit message why this is necessary and where you got these values from

Those are optional attributes documented in Documentation/devicetree/bindings/net/stmmac.txt.
The values were taken from S805 SoC manual, ch. 22 (ETHERNET MAC).

ok, then these are identical on GXBB and newer (according to the datasheets) I wonder if the stmmac driver auto-detects the correct value when these are not set. otherwise we should also configure these on the GX SoCs

Quoted 18 lines
Quoted 7 lines
Quoted 7 lines
arch/arm/boot/dts/meson8b.dtsi +0 -1
 };
 
 &hwrng {
--

2.14.1

Quoted 7 lines
Quoted 4 lines

_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

looking forward to proper ethernet support on Meson8/Meson8b!

OK, thanks for your suggestions!

Quoted 1 line
Quoted 11 lines
Quoted 3 lines

Regards,
Martin

Regards,

Emiliano

Quoted 3 lines

[0] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L29
[1] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#L4123

Regards,
Martin

[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-August/000764.html
[1] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L59
[2] http://lists.infradead.org/pipermail/linux-amlogic/2016-July/000645.html
[3] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c#L330
[4] http://lists.infradead.org/pipermail/linux-amlogic/2016-September/001055.html
[5] http://lists.infradead.org/pipermail/linux-amlogic/2016-July/000646.html

Regards
Martin

[0] https://www.mail-archive.com/netdev at vger.kernel.org/msg119290.html
[1] https://github.com/endlessm/u-boot-meson/blob/master/arch/arm/include/asm/arch-m8/aml_eth_reg.h#L508

#16 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

Quoted 114 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 3 lines · Emiliano Ingrassia

Hi Martin,

sorry for my very late response!

no worries, I'm also very late with this mail

Quoted 76 lines · Martin Blumenstingl
Quoted 74 lines

Hi Emiliano,

Quoted 70 lines · Emiliano Ingrassia

Hi Martin,

thanks for the review!

Quoted 64 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 43 lines · Emiliano Ingrassia

This patch adds ethernet controller pin description and extend its attributes in the relative node.

---

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

Please, apply this patch and discard the previous
(450a483abe07f8d903c6cb74091592743975a8eb).

 arch/arm/boot/dts/meson8b.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)
arch/arm/boot/dts/meson8b.dtsi +18 -0
index bc278da7df0d..816bc9188f44 100644
@@ -154,12 +154,48 @@
                        #gpio-cells = <2>;
                        gpio-ranges = <&pinctrl_cbus 0 0 130>;
                };
+
+               eth_rgmii_pins: eth-rgmii {
+                       mux {
+                               groups = "eth_tx_clk",
+                                        "eth_tx_en",
+                                        "eth_txd1_0",
+                                        "eth_txd1_1",
+                                        "eth_txd0_0",
+                                        "eth_txd0_1",
+                                        "eth_rx_clk",
+                                        "eth_rx_dv",
+                                        "eth_rxd1",
+                                        "eth_rxd0",
+                                        "eth_mdio_en",
+                                        "eth_mdc",
+                                        "eth_ref_clk",
+                                        "eth_txd2",
+                                        "eth_txd3";

arch/arm/boot/dts/meson8b.dtsi line 182

Quoted 10 lines
arch/arm/boot/dts/meson8b.dtsi +4 -2
+                               function = "ethernet";
+                       };
+               };
        };
 };
 
 &ethmac {
-       clocks = <&clkc CLKID_ETH>;
-       clock-names = "stmmaceth";
+       compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";

without a reg property this passes 0xc1108108 (as defined in meson.dtsi) to the meson8b-dwmac driver.
are you sure that this shouldn't be 0xc1108140 (like in your initial patch)? 0xc1108108 translates to 0x2050 (calculation formula: (0xc1108108 - cbus base addr 0xc1100000) / 4) which is used in Amlogic's u-boot sources, for example [0]

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Yes, I know. This was the intention.

OK, this is interesting

After some research I agree with you: the correct ethernet register address is 0xc1108140.

great - thank you for checking!

Quoted 21 lines
Quoted 20 lines
Quoted 3 lines
Quoted 3 lines

currently the meson8b-dwmac driver is writing to the old register location which probably does nothing.

without your change the meson6-dwmac driver is used. when I wrote the meson8b-dwmac driver I documented the following behavior (see [0]):
"This worked for many boards because the bootloader programs the PRG_ETHERNET registers correctly. Additionally the meson6-dwmac driver only sets bit 1 of PRG_ETHERNET_ADDR0 which (according to the datasheet) is only used during reset."

Quoted 2 lines

Actually, changing the second addresses range from 0xc1108108 to 0xc1108140 leads to an unusable ethernet controller.

Odroid-C1's u-boot writes 0x7d21 to 0xc1108140 (= (0x2050 * 4) + cbus base), see [1]
have you tried to verify that writing 0x7d21 (= the value used in the Odroid-C1 u-boot sources) at the end of meson8b_init_prg_eth makes it work for you again? if it does then we are not setting the register values correctly (which may simply be related to the clock setup - either the internal clock in the meson8b-dwmac driver or the "other" ethernet clock)

Quoted 7 lines
Quoted 4 lines

Actually, writing 0x7d21 at the end of the initialization procedure leads to a working ethernet controller. Consider that I'm using MPLL2 as clock source for both "clkin0" and "clkin1" because, as stated in S805 SoC manual, "M8Baby internal clock source is mp2_clk_out only.".

this value is pretty close to the one on GXBB (and newer) SoCs which use 0x1621 what is the MPLL2 clock rate on your board (my Meson8m2 and Meson8b boards both have a RMII PHY)?

based on [0] it might be 510MHz. according to these values (matched with the S912 datasheet):
SDM_IN = 1638
EN_DDS2 = 1
SDM_EN2 = 1
N_IN2 = 5
LP_EN2 = 0
IR_BYIN2 = 0
MODSEL2 = 0
IR_BYPASS2 = 0

according to the clk-mpll.c clock driver:
f(N2_integer, SDM_IN ) = 2.0G/(N2_integer + SDM_IN/16384)

however, the parent clock on Meson8b is fixed_pll at 2.55GHz
so the calculation is:
2550000000/(5 + (1638/16384))
a problem here is that (1638/16384) = 0.1 (rounded up)
assuming that clk-mpll doesn't do fractional digits: 2.55GHz/(5 + 0) = 510MHz assuming that original code did this on purpose and that the MPLL clock hardware rounds values up we get: 2.55GHz/(5 + 0.1) = 500MHz

but like I said: nothing of that was confirmed on actual hardware, so this is just "guessing register bit meanings" episode X ;)

Quoted 178 lines
Quoted 6 lines

Investigating dwmac-meson8b.c, a possible error lies in the use of prg_ethernet_addr0 register bits 9-7.
Infact, they are used as field for CLK_M250_DIV value, which it seems to me incorrect. From the SoC manual, those bits should be set as mpll2 clock rate divided by 250*1000*1000.

let's do the maths:
this would work fine on GXBB (and later) where PRG_ETH0_CLK_M250_DIV is 0x4 and the parent clock is FCLK_DIV2 which is at 1GHz. 1GHz / 1000 / 1000 / 250 = 0x4
"unfortunately" it also works the other way round: 1GHz / 0x4 = 250MHz

on your Odroid-C1 PRG_ETH0_CLK_M250_DIV is 0x2 - if the same maths still applies then MPLL2 should be at 500MHz:
500MHz / 1000 / 1000 / 250 = 0x2
however, it also works the other way round: 500MHz / 0x2 = 250MHz

Mike was also under the impression that these bits are a divider back when he and Kevin received some clarifications (which unfortunately have not made it into the GXL and GXM datasheets) for the PRG_ETHERNET_ADDR0 registers: [0]

Quoted 2 lines

Then, I guess that the correct divider value for 25 Mhz PHY clock is internally derived.

let's go one-by-one to figure this out and clarify that "divider" first

I also did some homework and found a struct that describes
PRG_ETHERNET_ADDR0: [1]
- the clock bits (mux, divider, phase / delay) are describing the RGMII TX clock (I should probably rename the clocks to rgmii_<current name>)
- in other words: these are not relevant for RMII at all (dwmac-meson8b currently ignores this fact and still tries to do some magic with these bits)
- nice-to-have: the TX delay could be implemented as a clock with .set_phase/.get_phase callbacks (this would also expose the phase in debugfs/clk/...)

Quoted 139 lines

Best regards,

Emiliano

Quoted 135 lines
Quoted 11 lines
Quoted 3 lines

if above statement is true then you are relying on the bootloader to set up 0xc1108140 correctly.

OK, sure this is bad! Those addresses are documented in S805 SoC manual and we should set up them correctly.
However, checking the Odroid-C2 device tree I couldn't find the PRG_ETHERNET_ADDR{0,1} addresses, which are documented in S905 SoC manual.
Probably I'm missing something, but don't we have the same situation on that board?

I'm not sure if I understand you correctly:
- the S805 datasheet mentioned that the addresses of the PRG_ETHERNET_ADDR{0,1} registers are 0x2050 and 0x2051
- we assume that the PRG_ETHERNET_ADDR{0,1} address from the S905 datasheet is wrong, see [4]
- Mike got feedback from Amlogic confirming that the documentation in the S905 datasheet is not fully correct, see [2] and [5]. if they use the same IP block in Meson8b (like, due to the identical register description in the S805 and S905 datasheets) then the S805 datasheet is also wrong

Quoted 8 lines
Quoted 3 lines

the reason why I wrote this meson8b-dwmac driver is because I had a GXBB board with RGMII PHY but u-boot configured the register to RMII mode -> ethernet wasn't working.

Meson8b refers to S805 SoC while GXBB should refer to S905 SoC. At first look it seemed to me that dwmac-meson8b driver correctly support dwmac on Meson8b or we should extend the driver to better support it?

I added compatible strings for both SoCs (see [3]) because the register description in both datasheets is identical. dwmac-meson8b works fine on all known GXBB/GXL/GXM devices
so it's the Meson8b compatibility that has to be improved (if it has to be improved, but for that we have to figure out the clock setup too).

Quoted 6 lines
Quoted 4 lines

you could verify this by zeroing both (0xc1108108 and 0xc1108140) in u-boot or at the start of the meson8b-dwmac driver and see if ethernet still works for you

I'll check it.

thank you!
please also see also my comment above regarding 0x7d21 from Odroid-C1's u-boot sources

Quoted 27 lines
Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 187

Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+                    <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>;
+       interrupt-names = "macirq",
+                         "eth_lpi";

did you receive one of the eth_lpi interrupts? if it works for you then we should try to add this to meson-gx.dtsi as well
I also wonder if we should configure it in meson8b.dtsi or meson.dtsi

Needs more testing.

Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       clock-names = "stmmaceth", "clkin0", "clkin1";
+       clocks = <&clkc CLKID_ETH>,
+                <&clkc CLKID_FCLK_DIV2>,
+                <&clkc CLKID_MPLL2>;
Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 195

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       resets = <&reset RESET_ETHERNET>;
+       reset-names = "stmmaceth";

I'm not sure if this works:
our reset controller implements a reset pulse (write bit, IP block executes a reset and clears the bit again)
stmmac on the other hand manually asserts and deasserts the reset line (which is not implemented by our reset driver), see [1]

Quoted 1 line

OK, I'll check and eventually fix this.

as a small hint: on Meson GX (GXBB and newer) we currently do not configure the reset line
if it's needed on Meson8b then you could implement it in a separate patch

Quoted 10 lines
Quoted 6 lines

arch/arm/boot/dts/meson8b.dtsi line 198

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       rx-fifo-depth=<4000>;
+       tx-fifo-depth=<2000>;

could you please add spaces around "=" and some info to the commit message why this is necessary and where you got these values from

Those are optional attributes documented in Documentation/devicetree/bindings/net/stmmac.txt.
The values were taken from S805 SoC manual, ch. 22 (ETHERNET MAC).

ok, then these are identical on GXBB and newer (according to the datasheets) I wonder if the stmmac driver auto-detects the correct value when these are not set. otherwise we should also configure these on the GX SoCs

Quoted 18 lines
Quoted 7 lines
Quoted 7 lines
arch/arm/boot/dts/meson8b.dtsi +0 -1
 };
 
 &hwrng {
--

2.14.1

Quoted 7 lines
Quoted 4 lines

_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

looking forward to proper ethernet support on Meson8/Meson8b!

OK, thanks for your suggestions!

Quoted 1 line
Quoted 11 lines
Quoted 3 lines

Regards,
Martin

Regards,

Emiliano

Quoted 3 lines

[0] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L29
[1] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#L4123

Regards,
Martin

[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-August/000764.html
[1] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L59
[2] http://lists.infradead.org/pipermail/linux-amlogic/2016-July/000645.html
[3] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c#L330
[4] http://lists.infradead.org/pipermail/linux-amlogic/2016-September/001055.html
[5] http://lists.infradead.org/pipermail/linux-amlogic/2016-July/000646.html

Quoted 5 lines

Regards
Martin

[0] https://www.mail-archive.com/netdev at vger.kernel.org/msg119290.html
[1] https://github.com/endlessm/u-boot-meson/blob/master/arch/arm/include/asm/arch-m8/aml_eth_reg.h#L508

Regards
Martin

[0] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L61

#17 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

In reply to Martin Blumenstingl

Hi Martin,

thank you for the response.

Quoted 133 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 3 lines · Emiliano Ingrassia

Hi Martin,

sorry for my very late response!

no worries, I'm also very late with this mail

Quoted 76 lines · Martin Blumenstingl
Quoted 74 lines

Hi Emiliano,

Quoted 70 lines · Emiliano Ingrassia

Hi Martin,

thanks for the review!

Quoted 64 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 43 lines · Emiliano Ingrassia

This patch adds ethernet controller pin description and extend its attributes in the relative node.

---

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

Please, apply this patch and discard the previous
(450a483abe07f8d903c6cb74091592743975a8eb).

 arch/arm/boot/dts/meson8b.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)
arch/arm/boot/dts/meson8b.dtsi +18 -0
index bc278da7df0d..816bc9188f44 100644
@@ -154,12 +154,48 @@
                        #gpio-cells = <2>;
                        gpio-ranges = <&pinctrl_cbus 0 0 130>;
                };
+
+               eth_rgmii_pins: eth-rgmii {
+                       mux {
+                               groups = "eth_tx_clk",
+                                        "eth_tx_en",
+                                        "eth_txd1_0",
+                                        "eth_txd1_1",
+                                        "eth_txd0_0",
+                                        "eth_txd0_1",
+                                        "eth_rx_clk",
+                                        "eth_rx_dv",
+                                        "eth_rxd1",
+                                        "eth_rxd0",
+                                        "eth_mdio_en",
+                                        "eth_mdc",
+                                        "eth_ref_clk",
+                                        "eth_txd2",
+                                        "eth_txd3";

arch/arm/boot/dts/meson8b.dtsi line 182

Quoted 10 lines
arch/arm/boot/dts/meson8b.dtsi +4 -2
+                               function = "ethernet";
+                       };
+               };
        };
 };
 
 &ethmac {
-       clocks = <&clkc CLKID_ETH>;
-       clock-names = "stmmaceth";
+       compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";

without a reg property this passes 0xc1108108 (as defined in meson.dtsi) to the meson8b-dwmac driver.
are you sure that this shouldn't be 0xc1108140 (like in your initial patch)? 0xc1108108 translates to 0x2050 (calculation formula: (0xc1108108 - cbus base addr 0xc1100000) / 4) which is used in Amlogic's u-boot sources, for example [0]

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Yes, I know. This was the intention.

OK, this is interesting

After some research I agree with you: the correct ethernet register address is 0xc1108140.

great - thank you for checking!

Quoted 21 lines
Quoted 12 lines
Quoted 3 lines
Quoted 3 lines

currently the meson8b-dwmac driver is writing to the old register location which probably does nothing.

without your change the meson6-dwmac driver is used. when I wrote the meson8b-dwmac driver I documented the following behavior (see [0]):
"This worked for many boards because the bootloader programs the PRG_ETHERNET registers correctly. Additionally the meson6-dwmac driver only sets bit 1 of PRG_ETHERNET_ADDR0 which (according to the datasheet) is only used during reset."

Quoted 2 lines

Actually, changing the second addresses range from 0xc1108108 to 0xc1108140 leads to an unusable ethernet controller.

Quoted 8 lines

Odroid-C1's u-boot writes 0x7d21 to 0xc1108140 (= (0x2050 * 4) + cbus base), see [1]
have you tried to verify that writing 0x7d21 (= the value used in the Odroid-C1 u-boot sources) at the end of meson8b_init_prg_eth makes it work for you again? if it does then we are not setting the register values correctly (which may simply be related to the clock setup - either the internal clock in the meson8b-dwmac driver or the "other" ethernet clock)

Quoted 4 lines

Actually, writing 0x7d21 at the end of the initialization procedure leads to a working ethernet controller. Consider that I'm using MPLL2 as clock source for both "clkin0" and "clkin1" because, as stated in S805 SoC manual, "M8Baby internal clock source is mp2_clk_out only.".

this value is pretty close to the one on GXBB (and newer) SoCs which use 0x1621 what is the MPLL2 clock rate on your board (my Meson8m2 and Meson8b boards both have a RMII PHY)?

Quoted 5 lines

Investigating dwmac-meson8b.c, a possible error lies in the use of prg_ethernet_addr0 register bits 9-7.
Infact, they are used as field for CLK_M250_DIV value, which it seems to me incorrect. From the SoC manual, those bits should be set as mpll2 clock rate divided by 250*1000*1000.

let's do the maths:
this would work fine on GXBB (and later) where PRG_ETH0_CLK_M250_DIV is 0x4 and the parent clock is FCLK_DIV2 which is at 1GHz. 1GHz / 1000 / 1000 / 250 = 0x4
"unfortunately" it also works the other way round: 1GHz / 0x4 = 250MHz

Quoted 5 lines

on your Odroid-C1 PRG_ETH0_CLK_M250_DIV is 0x2 - if the same maths still applies then MPLL2 should be at 500MHz:
500MHz / 1000 / 1000 / 250 = 0x2
however, it also works the other way round: 500MHz / 0x2 = 250MHz

Yes, of course!
The problem is that a value of 0x5, instead of 0x2, is written in those bits. Actually I'm studying the call chain which starts from "clk_set_rate()" on m25_clk_div. It seems that one of the function called sees a parent clock slightly greater than 100 MHz (for m25_clk_div) and choose 0x5 as divisor (because of round up).

By the way, I confirm you that MPLL2 is at 500 MHz on Odroid-C1+.

Quoted 164 lines

Mike was also under the impression that these bits are a divider back when he and Kevin received some clarifications (which unfortunately have not made it into the GXL and GXM datasheets) for the PRG_ETHERNET_ADDR0 registers: [0]

Quoted 2 lines

Then, I guess that the correct divider value for 25 Mhz PHY clock is internally derived.

let's go one-by-one to figure this out and clarify that "divider" first

I also did some homework and found a struct that describes
PRG_ETHERNET_ADDR0: [1]
- the clock bits (mux, divider, phase / delay) are describing the RGMII TX clock (I should probably rename the clocks to rgmii_<current name>)
- in other words: these are not relevant for RMII at all (dwmac-meson8b currently ignores this fact and still tries to do some magic with these bits)
- nice-to-have: the TX delay could be implemented as a clock with .set_phase/.get_phase callbacks (this would also expose the phase in debugfs/clk/...)

Quoted 139 lines

Best regards,

Emiliano

Quoted 135 lines
Quoted 11 lines
Quoted 3 lines

if above statement is true then you are relying on the bootloader to set up 0xc1108140 correctly.

OK, sure this is bad! Those addresses are documented in S805 SoC manual and we should set up them correctly.
However, checking the Odroid-C2 device tree I couldn't find the PRG_ETHERNET_ADDR{0,1} addresses, which are documented in S905 SoC manual.
Probably I'm missing something, but don't we have the same situation on that board?

I'm not sure if I understand you correctly:
- the S805 datasheet mentioned that the addresses of the PRG_ETHERNET_ADDR{0,1} registers are 0x2050 and 0x2051
- we assume that the PRG_ETHERNET_ADDR{0,1} address from the S905 datasheet is wrong, see [4]
- Mike got feedback from Amlogic confirming that the documentation in the S905 datasheet is not fully correct, see [2] and [5]. if they use the same IP block in Meson8b (like, due to the identical register description in the S805 and S905 datasheets) then the S805 datasheet is also wrong

Quoted 8 lines
Quoted 3 lines

the reason why I wrote this meson8b-dwmac driver is because I had a GXBB board with RGMII PHY but u-boot configured the register to RMII mode -> ethernet wasn't working.

Meson8b refers to S805 SoC while GXBB should refer to S905 SoC. At first look it seemed to me that dwmac-meson8b driver correctly support dwmac on Meson8b or we should extend the driver to better support it?

I added compatible strings for both SoCs (see [3]) because the register description in both datasheets is identical. dwmac-meson8b works fine on all known GXBB/GXL/GXM devices
so it's the Meson8b compatibility that has to be improved (if it has to be improved, but for that we have to figure out the clock setup too).

Quoted 6 lines
Quoted 4 lines

you could verify this by zeroing both (0xc1108108 and 0xc1108140) in u-boot or at the start of the meson8b-dwmac driver and see if ethernet still works for you

I'll check it.

thank you!
please also see also my comment above regarding 0x7d21 from Odroid-C1's u-boot sources

Quoted 27 lines
Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 187

Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>,
+                    <GIC_SPI 14 IRQ_TYPE_EDGE_RISING>;
+       interrupt-names = "macirq",
+                         "eth_lpi";

did you receive one of the eth_lpi interrupts? if it works for you then we should try to add this to meson-gx.dtsi as well
I also wonder if we should configure it in meson8b.dtsi or meson.dtsi

Needs more testing.

Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/meson8b.dtsi +5 -0
+
+       clock-names = "stmmaceth", "clkin0", "clkin1";
+       clocks = <&clkc CLKID_ETH>,
+                <&clkc CLKID_FCLK_DIV2>,
+                <&clkc CLKID_MPLL2>;
Quoted 9 lines

arch/arm/boot/dts/meson8b.dtsi line 195

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       resets = <&reset RESET_ETHERNET>;
+       reset-names = "stmmaceth";

I'm not sure if this works:
our reset controller implements a reset pulse (write bit, IP block executes a reset and clears the bit again)
stmmac on the other hand manually asserts and deasserts the reset line (which is not implemented by our reset driver), see [1]

Quoted 1 line

OK, I'll check and eventually fix this.

as a small hint: on Meson GX (GXBB and newer) we currently do not configure the reset line
if it's needed on Meson8b then you could implement it in a separate patch

Quoted 10 lines
Quoted 6 lines

arch/arm/boot/dts/meson8b.dtsi line 198

Quoted 3 lines
arch/arm/boot/dts/meson8b.dtsi +3 -0
+
+       rx-fifo-depth=<4000>;
+       tx-fifo-depth=<2000>;

could you please add spaces around "=" and some info to the commit message why this is necessary and where you got these values from

Those are optional attributes documented in Documentation/devicetree/bindings/net/stmmac.txt.
The values were taken from S805 SoC manual, ch. 22 (ETHERNET MAC).

ok, then these are identical on GXBB and newer (according to the datasheets) I wonder if the stmmac driver auto-detects the correct value when these are not set. otherwise we should also configure these on the GX SoCs

Quoted 18 lines
Quoted 7 lines
Quoted 7 lines
arch/arm/boot/dts/meson8b.dtsi +0 -1
 };
 
 &hwrng {
--

2.14.1

Quoted 7 lines
Quoted 4 lines

_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

looking forward to proper ethernet support on Meson8/Meson8b!

OK, thanks for your suggestions!

Quoted 1 line
Quoted 11 lines
Quoted 3 lines

Regards,
Martin

Regards,

Emiliano

Quoted 3 lines

[0] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L29
[1] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#L4123

Regards,
Martin

[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-August/000764.html
[1] https://github.com/hardkernel/u-boot/blob/odroidc-v2011.03/board/hardkernel/odroidc/odroidc-eth.c#L59
[2] http://lists.infradead.org/pipermail/linux-amlogic/2016-July/000645.html
[3] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c#L330
[4] http://lists.infradead.org/pipermail/linux-amlogic/2016-September/001055.html
[5] http://lists.infradead.org/pipermail/linux-amlogic/2016-July/000646.html

Regards
Martin

Quoted 7 lines

[0] https://www.mail-archive.com/netdev at vger.kernel.org/msg119290.html
[1] https://github.com/endlessm/u-boot-meson/blob/master/arch/arm/include/asm/arch-m8/aml_eth_reg.h#L508

_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Regards,

Emiliano

#18 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

Hi Emiliano,

sorry for the late reply

Quoted 138 lines · Emiliano Ingrassia

Hi Martin,

thank you for the response.

Quoted 133 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 3 lines · Emiliano Ingrassia

Hi Martin,

sorry for my very late response!

no worries, I'm also very late with this mail

Quoted 76 lines · Martin Blumenstingl
Quoted 74 lines

Hi Emiliano,

Quoted 70 lines · Emiliano Ingrassia

Hi Martin,

thanks for the review!

Quoted 64 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 43 lines · Emiliano Ingrassia

This patch adds ethernet controller pin description and extend its attributes in the relative node.

---

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

Please, apply this patch and discard the previous
(450a483abe07f8d903c6cb74091592743975a8eb).

 arch/arm/boot/dts/meson8b.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)
arch/arm/boot/dts/meson8b.dtsi +18 -0
index bc278da7df0d..816bc9188f44 100644
@@ -154,12 +154,48 @@
                        #gpio-cells = <2>;
                        gpio-ranges = <&pinctrl_cbus 0 0 130>;
                };
+
+               eth_rgmii_pins: eth-rgmii {
+                       mux {
+                               groups = "eth_tx_clk",
+                                        "eth_tx_en",
+                                        "eth_txd1_0",
+                                        "eth_txd1_1",
+                                        "eth_txd0_0",
+                                        "eth_txd0_1",
+                                        "eth_rx_clk",
+                                        "eth_rx_dv",
+                                        "eth_rxd1",
+                                        "eth_rxd0",
+                                        "eth_mdio_en",
+                                        "eth_mdc",
+                                        "eth_ref_clk",
+                                        "eth_txd2",
+                                        "eth_txd3";

arch/arm/boot/dts/meson8b.dtsi line 182

Quoted 10 lines
arch/arm/boot/dts/meson8b.dtsi +4 -2
+                               function = "ethernet";
+                       };
+               };
        };
 };
 
 &ethmac {
-       clocks = <&clkc CLKID_ETH>;
-       clock-names = "stmmaceth";
+       compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";

without a reg property this passes 0xc1108108 (as defined in meson.dtsi) to the meson8b-dwmac driver.
are you sure that this shouldn't be 0xc1108140 (like in your initial patch)? 0xc1108108 translates to 0x2050 (calculation formula: (0xc1108108 - cbus base addr 0xc1100000) / 4) which is used in Amlogic's u-boot sources, for example [0]

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Yes, I know. This was the intention.

OK, this is interesting

After some research I agree with you: the correct ethernet register address is 0xc1108140.

great - thank you for checking!

Quoted 21 lines
Quoted 12 lines
Quoted 3 lines
Quoted 3 lines

currently the meson8b-dwmac driver is writing to the old register location which probably does nothing.

without your change the meson6-dwmac driver is used. when I wrote the meson8b-dwmac driver I documented the following behavior (see [0]):
"This worked for many boards because the bootloader programs the PRG_ETHERNET registers correctly. Additionally the meson6-dwmac driver only sets bit 1 of PRG_ETHERNET_ADDR0 which (according to the datasheet) is only used during reset."

Quoted 2 lines

Actually, changing the second addresses range from 0xc1108108 to 0xc1108140 leads to an unusable ethernet controller.

Quoted 8 lines

Odroid-C1's u-boot writes 0x7d21 to 0xc1108140 (= (0x2050 * 4) + cbus base), see [1]
have you tried to verify that writing 0x7d21 (= the value used in the Odroid-C1 u-boot sources) at the end of meson8b_init_prg_eth makes it work for you again? if it does then we are not setting the register values correctly (which may simply be related to the clock setup - either the internal clock in the meson8b-dwmac driver or the "other" ethernet clock)

Quoted 4 lines

Actually, writing 0x7d21 at the end of the initialization procedure leads to a working ethernet controller. Consider that I'm using MPLL2 as clock source for both "clkin0" and "clkin1" because, as stated in S805 SoC manual, "M8Baby internal clock source is mp2_clk_out only.".

this value is pretty close to the one on GXBB (and newer) SoCs which use 0x1621 what is the MPLL2 clock rate on your board (my Meson8m2 and Meson8b boards both have a RMII PHY)?

Quoted 5 lines

Investigating dwmac-meson8b.c, a possible error lies in the use of prg_ethernet_addr0 register bits 9-7.
Infact, they are used as field for CLK_M250_DIV value, which it seems to me incorrect. From the SoC manual, those bits should be set as mpll2 clock rate divided by 250*1000*1000.

let's do the maths:
this would work fine on GXBB (and later) where PRG_ETH0_CLK_M250_DIV is 0x4 and the parent clock is FCLK_DIV2 which is at 1GHz. 1GHz / 1000 / 1000 / 250 = 0x4
"unfortunately" it also works the other way round: 1GHz / 0x4 = 250MHz

Quoted 8 lines
Quoted 5 lines

on your Odroid-C1 PRG_ETH0_CLK_M250_DIV is 0x2 - if the same maths still applies then MPLL2 should be at 500MHz:
500MHz / 1000 / 1000 / 250 = 0x2
however, it also works the other way round: 500MHz / 0x2 = 250MHz

Yes, of course!
The problem is that a value of 0x5, instead of 0x2, is written in those bits.

so you mean that "dwmac-meson8b.c" writes a value of 0x5 instead of 0x2 (0x2 is the value set by the vendor driver as far as I know)?

Quoted 4 lines

Actually I'm studying the call chain which starts from "clk_set_rate()" on m25_clk_div. It seems that one of the function called sees a parent clock slightly greater than 100 MHz (for m25_clk_div) and choose 0x5 as divisor (because of round up).

this may be related to my mpll2 number game in my last email.

can you check which clock rate the clk-mpll driver sees for mpll2 in cat /sys/kernel/debug/clk/clk_summary?
sdm = 1638
n2 = 5
parent_rate = 2550000000 (2.55GHz)
resulting clock rate = 500002393Hz (= 500-odd MHz)

you could try the attached patch which enables rounding on the dividers within the dwmac-meson8b driver and see if this solves the problem?

Quoted 1 line

By the way, I confirm you that MPLL2 is at 500 MHz on Odroid-C1+.

great - thanks!

Regards
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: net-stmmac-dwmac-meson8b-fix-setting-the-PHY-clock-on-meson8b.patch
Type: text/x-patch
Size: 3203 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20171217/d3df0dae/attachment.bin>

#19 lore raw

PATCH 2/4 ARM: dts: meson8b: extending ethernet controller description

Hi Martin,

Quoted 4 lines · Martin Blumenstingl

Hi Emiliano,

sorry for the late reply

thanks for the reply, don't worry ;)

Quoted 155 lines
Quoted 138 lines · Emiliano Ingrassia

Hi Martin,

thank you for the response.

Quoted 133 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 3 lines · Emiliano Ingrassia

Hi Martin,

sorry for my very late response!

no worries, I'm also very late with this mail

Quoted 76 lines · Martin Blumenstingl
Quoted 74 lines

Hi Emiliano,

Quoted 70 lines · Emiliano Ingrassia

Hi Martin,

thanks for the review!

Quoted 64 lines · Martin Blumenstingl

Hi Emiliano,

Quoted 43 lines · Emiliano Ingrassia

This patch adds ethernet controller pin description and extend its attributes in the relative node.

---

This patch corrects the meson8b-dwmac reg attributes updated by the previous 2/4 patch (450a483abe07f8d903c6cb74091592743975a8eb).
The second addresses range, taken from S805 (aka Meson8b) SoC manual, was not correct.

Please, apply this patch and discard the previous
(450a483abe07f8d903c6cb74091592743975a8eb).

 arch/arm/boot/dts/meson8b.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)
arch/arm/boot/dts/meson8b.dtsi +18 -0
index bc278da7df0d..816bc9188f44 100644
@@ -154,12 +154,48 @@
                        #gpio-cells = <2>;
                        gpio-ranges = <&pinctrl_cbus 0 0 130>;
                };
+
+               eth_rgmii_pins: eth-rgmii {
+                       mux {
+                               groups = "eth_tx_clk",
+                                        "eth_tx_en",
+                                        "eth_txd1_0",
+                                        "eth_txd1_1",
+                                        "eth_txd0_0",
+                                        "eth_txd0_1",
+                                        "eth_rx_clk",
+                                        "eth_rx_dv",
+                                        "eth_rxd1",
+                                        "eth_rxd0",
+                                        "eth_mdio_en",
+                                        "eth_mdc",
+                                        "eth_ref_clk",
+                                        "eth_txd2",
+                                        "eth_txd3";

arch/arm/boot/dts/meson8b.dtsi line 182

Quoted 10 lines
arch/arm/boot/dts/meson8b.dtsi +4 -2
+                               function = "ethernet";
+                       };
+               };
        };
 };
 
 &ethmac {
-       clocks = <&clkc CLKID_ETH>;
-       clock-names = "stmmaceth";
+       compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac";

without a reg property this passes 0xc1108108 (as defined in meson.dtsi) to the meson8b-dwmac driver.
are you sure that this shouldn't be 0xc1108140 (like in your initial patch)? 0xc1108108 translates to 0x2050 (calculation formula: (0xc1108108 - cbus base addr 0xc1100000) / 4) which is used in Amlogic's u-boot sources, for example [0]

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Yes, I know. This was the intention.

OK, this is interesting

After some research I agree with you: the correct ethernet register address is 0xc1108140.

great - thank you for checking!

Quoted 21 lines
Quoted 12 lines
Quoted 3 lines
Quoted 3 lines

currently the meson8b-dwmac driver is writing to the old register location which probably does nothing.

without your change the meson6-dwmac driver is used. when I wrote the meson8b-dwmac driver I documented the following behavior (see [0]):
"This worked for many boards because the bootloader programs the PRG_ETHERNET registers correctly. Additionally the meson6-dwmac driver only sets bit 1 of PRG_ETHERNET_ADDR0 which (according to the datasheet) is only used during reset."

Quoted 2 lines

Actually, changing the second addresses range from 0xc1108108 to 0xc1108140 leads to an unusable ethernet controller.

Quoted 8 lines

Odroid-C1's u-boot writes 0x7d21 to 0xc1108140 (= (0x2050 * 4) + cbus base), see [1]
have you tried to verify that writing 0x7d21 (= the value used in the Odroid-C1 u-boot sources) at the end of meson8b_init_prg_eth makes it work for you again? if it does then we are not setting the register values correctly (which may simply be related to the clock setup - either the internal clock in the meson8b-dwmac driver or the "other" ethernet clock)

Quoted 4 lines

Actually, writing 0x7d21 at the end of the initialization procedure leads to a working ethernet controller. Consider that I'm using MPLL2 as clock source for both "clkin0" and "clkin1" because, as stated in S805 SoC manual, "M8Baby internal clock source is mp2_clk_out only.".

this value is pretty close to the one on GXBB (and newer) SoCs which use 0x1621 what is the MPLL2 clock rate on your board (my Meson8m2 and Meson8b boards both have a RMII PHY)?

Quoted 5 lines

Investigating dwmac-meson8b.c, a possible error lies in the use of prg_ethernet_addr0 register bits 9-7.
Infact, they are used as field for CLK_M250_DIV value, which it seems to me incorrect. From the SoC manual, those bits should be set as mpll2 clock rate divided by 250*1000*1000.

let's do the maths:
this would work fine on GXBB (and later) where PRG_ETH0_CLK_M250_DIV is 0x4 and the parent clock is FCLK_DIV2 which is at 1GHz. 1GHz / 1000 / 1000 / 250 = 0x4
"unfortunately" it also works the other way round: 1GHz / 0x4 = 250MHz

Quoted 8 lines
Quoted 5 lines

on your Odroid-C1 PRG_ETH0_CLK_M250_DIV is 0x2 - if the same maths still applies then MPLL2 should be at 500MHz:
500MHz / 1000 / 1000 / 250 = 0x2
however, it also works the other way round: 500MHz / 0x2 = 250MHz

Yes, of course!
The problem is that a value of 0x5, instead of 0x2, is written in those bits.

so you mean that "dwmac-meson8b.c" writes a value of 0x5 instead of 0x2 (0x2 is the value set by the vendor driver as far as I know)?

Quoted 4 lines

Actually I'm studying the call chain which starts from "clk_set_rate()" on m25_clk_div. It seems that one of the function called sees a parent clock slightly greater than 100 MHz (for m25_clk_div) and choose 0x5 as divisor (because of round up).

Quoted 9 lines

this may be related to my mpll2 number game in my last email.

can you check which clock rate the clk-mpll driver sees for mpll2 in cat /sys/kernel/debug/clk/clk_summary?
sdm = 1638
n2 = 5
parent_rate = 2550000000 (2.55GHz)
resulting clock rate = 500002393Hz (= 500-odd MHz)

The clk_summary reports:

clock enable_cnt prepare_cnt rate
-------------------------------------------------------------------

xtal                               1            1          24000000
 sys_pll                           0            0        1200000000
  cpu_clk                          0            0        1200000000
 vid_pll                           0            0         732000000
 fixed_pll                         2            2        2550000000
   mpll2                           1            1         500002394
    c9410000.ethernet#m250_sel     1            1         500002394
     c9410000.ethernet#m250_div    1            1         100000479
      c9410000.ethernet#m25_div    1            1          20000096

Accuracy and phase are all zero.

Quoted 4 lines

you could try the attached patch which enables rounding on the dividers within the dwmac-meson8b driver and see if this solves the problem?

I tried your patch with no luck. The prg_ethernet0 register is set to 0x72a1 instead of 0x7d21.
So, bit 9-7 are 0x5 instead of 0x2.
I'll keep on investigating and let you know.
Any advice will be appreciated :D

Quoted 6 lines
Quoted 1 line

By the way, I confirm you that MPLL2 is at 500 MHz on Odroid-C1+.

great - thanks!

Regards
Martin

Thanks for the patch, best regards,

Emiliano

Quoted 68 lines

From 548f99cdf30c57ae6539fa0074082ba45e3ede57 Mon Sep 17 00:00:00 2001
From: Martin Blumenstingl <[email protected]>
Date: Sat, 25 Mar 2017 19:08:36 +0100
Subject: [PATCH] net: stmmac: dwmac-meson8b: fix setting the PHY clock on
Meson8b

Meson8b only supports MPLL2 as clock input. The rate of the MPLL2 clock set by Odroid-C1's u-boot is close to 500MHz. The exact rate is 500002393Hz, which is calculated in drivers/clk/meson/clk-mpll.c using the following formula:
DIV_ROUND_UP_ULL((u64)parent_rate * SDM_DEN, (SDM_DEN * n2) + sdm) Odroid-C1's u-boot configures MPLL2 with the following values:
- SDM_DEN = 16384
- SDM = 1638
- N2 = 5

The 250MHz and 25MHz clocks inside dwmac-meson8b driver are derived from the MPLL2 clock. Due to MPLL2 running slightly faster than 500MHz the common clock framework chooses dividers which are too big to generate the 250MHz and 25MHz clocks. Emiliano Ingrassia observed that the divider for the 250MHz clock was set to 0x5 which results in a clock rate of close to 100MHz instead of 250MHz. The divider for the 25MHz clock is set to 0x0 (which means "divide by 5") so the resulting RGMII clock is running at 20MHz (plus a few additional Hz). The RTL8211F PHY on Odroid-C1 however fails to operate with a 20MHz RGMII clock.

Round the divider's clock rates to prevent this issue on Meson8b. This means we'll now end up with a clock rate of 25000119Hz (= 25MHz plus 119Hz).
This has no effect on the Meson GX SoCs since there fclk_div2 is used as input clock, which has a rate of 1000MHz (and thus is divisible cleanly to 250MHz and 25MHz).

Fixes 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c +5 -2
index 4404650b32c5..c71966332387 100644
@@ -144,7 +144,9 @@ static int meson8b_init_clk(struct meson8b_dwmac *dwmac)
 	dwmac->m250_div.shift = PRG_ETH0_CLK_M250_DIV_SHIFT;
 	dwmac->m250_div.width = PRG_ETH0_CLK_M250_DIV_WIDTH;
 	dwmac->m250_div.hw.init = &init;
-	dwmac->m250_div.flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO;
+	dwmac->m250_div.flags = CLK_DIVIDER_ONE_BASED |
+				CLK_DIVIDER_ALLOW_ZERO |
+				CLK_DIVIDER_ROUND_CLOSEST;
 
 	dwmac->m250_div_clk = devm_clk_register(dev, &dwmac->m250_div.hw);
 	if (WARN_ON(IS_ERR(dwmac->m250_div_clk)))
@@ -164,7 +166,8 @@ static int meson8b_init_clk(struct meson8b_dwmac *dwmac)
 	dwmac->m25_div.width = PRG_ETH0_CLK_M25_DIV_WIDTH;
 	dwmac->m25_div.table = clk_25m_div_table;
 	dwmac->m25_div.hw.init = &init;
-	dwmac->m25_div.flags = CLK_DIVIDER_ALLOW_ZERO;
+	dwmac->m25_div.flags = CLK_DIVIDER_ALLOW_ZERO |
+				CLK_DIVIDER_ROUND_CLOSEST;
 
 	dwmac->m25_div_clk = devm_clk_register(dev, &dwmac->m25_div.hw);
 	if (WARN_ON(IS_ERR(dwmac->m25_div_clk)))
Signature · 1 line
2.15.1
#20 lore raw

PATCH 3/4 ARM: dts: meson8b-odroidc1: enabling ethernet support

In reply to Emiliano Ingrassia

This patch adds ethernet PHY description and enables ethernet support on Odroid-C1/C1+ boards.

---
 arch/arm/boot/dts/meson8b-odroidc1.dts | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
arch/arm/boot/dts/meson8b-odroidc1.dts +30 -0
index 9ff6ca4e20d0..6ac357ee3de6 100644
@@ -54,6 +54,7 @@
 
 	aliases {
 		serial0 = &uart_AO;
+		ethernet = &ethmac;
 	};
 
 	memory {
@@ -77,6 +78,35 @@
 	pinctrl-names = "default";
 };
 
+&ethmac {
+	status = "okay";
+
+	snps,reset-gpio = <&gpio GPIOH_4 GPIO_ACTIVE_HIGH>;
+	snps,reset-active-low;
+	snps,reset-delays-us = <0 10000 30000>;
+
+	pinctrl-0 = <&eth_rgmii_pins>;
+	pinctrl-names = "default";
+
+	phy-mode = "rgmii";
+	phy-handle = <&eth_phy>;
+	amlogic,tx-delay-ns = <2>;
+
+	mdio {
+		compatible = "snps,dwmac-mdio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		eth_phy: ethernet-phy at 0 {
+			/* ethernet PHY RTL8211F */
+			compatible = "ethernet-phy-id001c.c916",
+				     "ethernet-phy-ieee802.3-c22";
+			reg = <0>;
+			max-speed = <1000>;
+		};
+	};
+};
+
 &gpio_ao {
 	/*
 	 * WARNING: The USB Hub on the Odroid-C1/C1+ needs a reset signal
Signature · 1 line
2.14.1
#21 lore raw

PATCH 4/4 net: stmmac: fixing DMA reset sleep and timeout values

In reply to Emiliano Ingrassia

This patch fixes the sleep and timeout values used during DMA reset, which were inverted in a previous patch.

Fixes 8a70aeca80c2 ("net: stmmac: Use readl_poll_timeout")
---
 drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c +1 -1
index 67af0bdd7f10..7516ca210855 100644
@@ -34,7 +34,7 @@ int dwmac_dma_reset(void __iomem *ioaddr)
 
 	err = readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
 				 !(value & DMA_BUS_MODE_SFT_RESET),
-				 100000, 10000);
+				 10000, 100000);
 	if (err)
 		return -EBUSY;
 
Signature · 1 line
2.14.1
#22 lore raw

cover meson8b-odroidc1: ethernet support

In reply to Emiliano Ingrassia

Hi Emiliano,

Quoted 11 lines · Emiliano Ingrassia

This patchset enables ethernet support on Odroid-C1/C1+ boards, improving and extending the ethernet description in the relative device tree.

In particular:
- the mpll2 clock is enabled to avoid timeout during ethernet DMA reset;
- the Meson8b ethernet controller description is fixed and extended;
- the Odroid-C1/C1+ ethernet PHY description is added;
- the dwmac sleep and timeout values used during DMA reset are fixed.

This patchset has been tested on Odroid-C1+ board.

I tried this patchset on an Odroid C1+ with a v4.14-rc3 kernel plus these currently pending patches from this mailing list [0].

I'm observing the following issue:

1) The C1+ successfully gets an IPv6 address from stateless autoconfiguration
2) IPv6 pinging from the C1+ to another host X works

        C1+ -> X => OK
3) IPv6 pinging from host X to the C1+ does not work:
        X -> C1+ => not OK

4) While ping'ing from X->C1+ after some seconds / a minute a parallel C1+->X ping stops working too. After stopping the X->C1+ ping the C1+->X ping quickly recovers within about one or two seconds.

I tried adding the "eee-broken-1000t;" parameter to meson8b-odroidc1.dts similar to meson-gxbb-odroidc2.dts. Did not seem to help though.

Still, this behaviour very much feels like a powersaving issue. And/or an incomplete setup of the random MAC address on the C1+, for instance a missing notification of the MAC address for the ethernet chip, leaving it unable to wake up / create interrupts for incoming frames with the destination of the C1+.

The C1+ and host X are both connected via cable to the same switch of a router.

Can you reproduce this issue on your side, Emiliano?

Regards, Linus

[0]:

e008445 ARM: multi_v7_defconfig: Add Amlogic Meson MMC support a315380 ARM: dts: meson8b: Add MMC nodes
90a73b6 mmc: meson-mx-sdio: Add a driver for the Amlogic Meson8 and Meson8b SoCs 2f0b808 dt-bindings: mmc: Document the Amlogic Meson8 and Meson8b SDIO bindings 31247f2 net: stmmac: fixing DMA reset sleep and timeout values 1f43db3 ARM: dts: meson8b-odroidc1: enabling ethernet support 8ecc9d3 ARM: dts: meson8b: extending ethernet controller description be4a137 clk: meson8b: keep mpll2 clock enabled
54b818c ARM: dts: meson8b: add reserved memory zone to fix silent freezes 113819d ARM: dts: meson: enabling the USB Host controller on Odroid-C1/C1+ board 7240c6d ARM: dts: meson: fixing USB support on Meson8b
a9153ba ARM: dts: meson8b: add support for booting the secondary CPU cores 8529b23 ARM: dts: meson8: add support for booting the secondary CPU cores fd053c9 ARM: meson: Add SMP bringup code for Meson8 and Meson8b d6b74cd ARM: smp_scu: allow the platform code to read the SCU CPU status 822a0d8 ARM: smp_scu: add a helper for powering on a specific CPU 7da8338 dt-bindings: Amlogic: Add Meson8 and Meson8b SMP related documentation

#23 lore raw

cover meson8b-odroidc1: ethernet support

Hi Linus,

thanks for the review!

Quoted 41 lines · Linus L?ssing

Hi Emiliano,

Quoted 11 lines · Emiliano Ingrassia

This patchset enables ethernet support on Odroid-C1/C1+ boards, improving and extending the ethernet description in the relative device tree.

In particular:
- the mpll2 clock is enabled to avoid timeout during ethernet DMA reset;
- the Meson8b ethernet controller description is fixed and extended;
- the Odroid-C1/C1+ ethernet PHY description is added;
- the dwmac sleep and timeout values used during DMA reset are fixed.

This patchset has been tested on Odroid-C1+ board.

I tried this patchset on an Odroid C1+ with a v4.14-rc3 kernel plus these currently pending patches from this mailing list [0].

I'm observing the following issue:

1) The C1+ successfully gets an IPv6 address from stateless autoconfiguration
2) IPv6 pinging from the C1+ to another host X works

        C1+ -> X => OK
3) IPv6 pinging from host X to the C1+ does not work:
        X -> C1+ => not OK

4) While ping'ing from X->C1+ after some seconds / a minute a parallel C1+->X ping stops working too. After stopping the X->C1+ ping the C1+->X ping quickly recovers within about one or two seconds.

I tried adding the "eee-broken-1000t;" parameter to meson8b-odroidc1.dts similar to meson-gxbb-odroidc2.dts. Did not seem to help though.

Still, this behaviour very much feels like a powersaving issue. And/or an incomplete setup of the random MAC address on the C1+, for instance a missing notification of the MAC address for the ethernet chip, leaving it unable to wake up / create interrupts for incoming frames with the destination of the C1+.

Quoted 5 lines

The C1+ and host X are both connected via cable to the same switch of a router.

Can you reproduce this issue on your side, Emiliano?

Ok. Actually I'm working on the dwmac-meson8b glue layer to obtain a functional ethernet MAC. Infact, as stated by Martin in [0], the correct PRG_ETHERNET_ADDR0 register address should be 0xc1108140.

I'll try your tests as soon as I find the correct solution.

Quoted 2 lines

Regards, Linus

Regards,

Emiliano

[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-September/004810.html

Quoted 19 lines

[0]:

e008445 ARM: multi_v7_defconfig: Add Amlogic Meson MMC support a315380 ARM: dts: meson8b: Add MMC nodes
90a73b6 mmc: meson-mx-sdio: Add a driver for the Amlogic Meson8 and Meson8b SoCs 2f0b808 dt-bindings: mmc: Document the Amlogic Meson8 and Meson8b SDIO bindings 31247f2 net: stmmac: fixing DMA reset sleep and timeout values 1f43db3 ARM: dts: meson8b-odroidc1: enabling ethernet support 8ecc9d3 ARM: dts: meson8b: extending ethernet controller description be4a137 clk: meson8b: keep mpll2 clock enabled
54b818c ARM: dts: meson8b: add reserved memory zone to fix silent freezes 113819d ARM: dts: meson: enabling the USB Host controller on Odroid-C1/C1+ board 7240c6d ARM: dts: meson: fixing USB support on Meson8b
a9153ba ARM: dts: meson8b: add support for booting the secondary CPU cores 8529b23 ARM: dts: meson8: add support for booting the secondary CPU cores fd053c9 ARM: meson: Add SMP bringup code for Meson8 and Meson8b d6b74cd ARM: smp_scu: allow the platform code to read the SCU CPU status 822a0d8 ARM: smp_scu: add a helper for powering on a specific CPU 7da8338 dt-bindings: Amlogic: Add Meson8 and Meson8b SMP related documentation

#24 lore raw

cover meson8b-odroidc1: ethernet support

In reply to Linus Lüssing
Quoted 6 lines · Linus L?ssing

I tried this patchset on an Odroid C1+ with a v4.14-rc3 kernel plus these currently pending patches from this mailing list [0].

I'm observing the following issue:

[...]

I have some good and some bad news. The good news: I tried with a 4.14.0 kernel and did not observe the issue anymore. The bad
news: I retried the same v4.14-rc3 uImage I used back then and could not reproduce it either :-(.

I'm unaware of having changed anything. Still using the same ethernet cable and same, dedicated USB network adapter as a counterpart. A couple of weeks ago I was even able to reproduce the issue with 4.14-rc6, but that image too does not seem to reproduce the issue anymore.

I'm blaming the cheap USB network adapter or ethernet cable or some other connectivity issue outside of the Odroid C1+ for now. Sorry :/.

Regards, Linus

#25 lore raw

cover meson8b-odroidc1: ethernet support

Hi Linus,

Quoted 19 lines · Linus L?ssing
Quoted 6 lines · Linus L?ssing

I tried this patchset on an Odroid C1+ with a v4.14-rc3 kernel plus these currently pending patches from this mailing list [0].

I'm observing the following issue:

[...]

I have some good and some bad news. The good news: I tried with a 4.14.0 kernel and did not observe the issue anymore. The bad
news: I retried the same v4.14-rc3 uImage I used back then and could not reproduce it either :-(.

I'm unaware of having changed anything. Still using the same ethernet cable and same, dedicated USB network adapter as a counterpart. A couple of weeks ago I was even able to reproduce the issue with 4.14-rc6, but that image too does not seem to reproduce the issue anymore.

Quoted 4 lines

I'm blaming the cheap USB network adapter or ethernet cable or some other connectivity issue outside of the Odroid C1+ for now. Sorry :/.

It's ok, don't worry! Thanks for the feedback.

Quoted 2 lines

Regards, Linus

Regards,

Emiliano

Quoted 4 lines

_______________________________________________
linux-amlogic mailing list
linux-amlogic at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic