| Message ID | 20180116003412.GA10581@ingrassia.epigenesys.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Jan 16, 2018 at 1:34 AM, Emiliano Ingrassia <ingrassia@epigenesys.com> wrote: > Extend ethernet controller description adding pin multiplexing and > setting the needed attributes in ethmac node. > As reported in S805 SoC manual, the MAC clock source is MPLL2 only. > > Signed-off-by: Emiliano Ingrassia <ingrassia@epigenesys.com> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> I had your patch also in my local tree, where I added the following comments to the patch description (this is nothing final though, I just added them so I don't forget about these facts): Until now we have been using the "amlogic,meson6-dwmac" binding with the register offset (0xc1108108) defined in meson.dtsi. During testing (and by reading Hardkernel's u-boot sources for the Odroid-C1) it turns out that the actual register that should be used is at 0xc1108140. This also requires us to switch to the new "amlogic,meson8b-dwmac" binding, because the dwmac-meson8b driver knows how to configure that register. The old register is a no-op, so using the old "amlogic,meson6-dwmac" binding with the old register meant that we relied on the bootloader to set up the Ethernet clocks etc. correctly. > --- > arch/arm/boot/dts/meson8b.dtsi | 35 +++++++++++++++++++++++++++++++++-- > 1 file changed, 33 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi > index 7cd03ed3742e..3c66d9bdc3a8 100644 > --- a/arch/arm/boot/dts/meson8b.dtsi > +++ b/arch/arm/boot/dts/meson8b.dtsi > @@ -185,6 +185,27 @@ > #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"; > + }; > + }; > }; > }; > > @@ -203,8 +224,18 @@ > }; > > ðmac { > - clocks = <&clkc CLKID_ETH>; > - clock-names = "stmmaceth"; > + compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac"; > + > + reg = <0xc9410000 0x10000 > + 0xc1108140 0x4>; > + > + clocks = <&clkc CLKID_ETH>, > + <&clkc CLKID_MPLL2>, > + <&clkc CLKID_MPLL2>; > + clock-names = "stmmaceth", "clkin0", "clkin1"; > + > + resets = <&reset RESET_ETHERNET>; > + reset-names = "stmmaceth"; > }; > > &gpio_intc { > -- > 2.15.1 >
On Tue, 2018-01-16 at 01:34 +0100, Emiliano Ingrassia wrote: > Extend ethernet controller description adding pin multiplexing and > setting the needed attributes in ethmac node. > As reported in S805 SoC manual, the MAC clock source is MPLL2 only. > > Signed-off-by: Emiliano Ingrassia <ingrassia@epigenesys.com> > --- > arch/arm/boot/dts/meson8b.dtsi | 35 +++++++++++++++++++++++++++++++++-- > 1 file changed, 33 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi > index 7cd03ed3742e..3c66d9bdc3a8 100644 > --- a/arch/arm/boot/dts/meson8b.dtsi > +++ b/arch/arm/boot/dts/meson8b.dtsi > @@ -185,6 +185,27 @@ > #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"; > + }; > + }; > }; > }; > > @@ -203,8 +224,18 @@ > }; > > ðmac { > - clocks = <&clkc CLKID_ETH>; > - clock-names = "stmmaceth"; > + compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac"; Does meson8 shared the same IP block ? is yes, then this compatible should probably be changed one level up, along with the regs If not, it means that ethmac node should not be defined in meson.dtsi but in meson8.dtsi and meson8b.dtsi In any case, overloading the node like this really clean, even if it works. > + > + reg = <0xc9410000 0x10000 > + 0xc1108140 0x4>; > + > + clocks = <&clkc CLKID_ETH>, > + <&clkc CLKID_MPLL2>, > + <&clkc CLKID_MPLL2>; > + clock-names = "stmmaceth", "clkin0", "clkin1"; > + > + resets = <&reset RESET_ETHERNET>; > + reset-names = "stmmaceth"; > }; > > &gpio_intc {
Hi Jerome, On Tue, Jan 16, 2018 at 11:17 AM, Jerome Brunet <jbrunet@baylibre.com> wrote: > On Tue, 2018-01-16 at 01:34 +0100, Emiliano Ingrassia wrote: >> Extend ethernet controller description adding pin multiplexing and >> setting the needed attributes in ethmac node. >> As reported in S805 SoC manual, the MAC clock source is MPLL2 only. >> >> Signed-off-by: Emiliano Ingrassia <ingrassia@epigenesys.com> >> --- >> arch/arm/boot/dts/meson8b.dtsi | 35 +++++++++++++++++++++++++++++++++-- >> 1 file changed, 33 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi >> index 7cd03ed3742e..3c66d9bdc3a8 100644 >> --- a/arch/arm/boot/dts/meson8b.dtsi >> +++ b/arch/arm/boot/dts/meson8b.dtsi >> @@ -185,6 +185,27 @@ >> #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"; >> + }; >> + }; >> }; >> }; >> >> @@ -203,8 +224,18 @@ >> }; >> >> ðmac { >> - clocks = <&clkc CLKID_ETH>; >> - clock-names = "stmmaceth"; >> + compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac"; > > Does meson8 shared the same IP block ? is yes, then this compatible should > probably be changed one level up, along with the regs > > If not, it means that ethmac node should not be defined in meson.dtsi but in > meson8.dtsi and meson8b.dtsi it's tricky - I have ordered a Meson8 board (which has not arrived yet), but according to my findings: - Meson6 uses the old binding - Meson8 uses the same binding as Meson6 - Meson8b uses the new binding / same binding as GXBB - Meson8m2 (which is mostly identical to Meson8) uses the same binding as Meson8b / GXBB > In any case, overloading the node like this really clean, even if it works. my plan is to restructure the ethmac node once I have my Meson8 board: - add the the "amlogic,meson6-dwmac" compatible and the 0xc1108108 register to meson6.dtsi and meson8.dtsi - remove the "amlogic,meson6-dwmac" binding from meson.dtsi - figure out whether meson8m2.dtsi (upcoming, on my TODO-list...) should inherit meson8.dtsi or meson.dtsi some more context: Meson8 and Meson8m2 share a lot of functionality: - SMP / CPU hotplug procedure - CPU frequencies (OPP tables) - pinctrl however, there are some differences too: - Meson8m2 uses the WDT layout from Meson8b - Meson8m2 uses the same CPU temperature calibration procedure as Meson8b - (not tested yet) Meson8m2 uses the Ethernet registers from Meson8b in other words: I think it's a good idea to restructure the ethmac node however, I would prefer to do it outside of this series so we don't have to do it twice (once with limited testing, then fixing some bugs in a follow-up series once I could test it on Meson8) >> + >> + reg = <0xc9410000 0x10000 >> + 0xc1108140 0x4>; >> + >> + clocks = <&clkc CLKID_ETH>, >> + <&clkc CLKID_MPLL2>, >> + <&clkc CLKID_MPLL2>; >> + clock-names = "stmmaceth", "clkin0", "clkin1"; >> + >> + resets = <&reset RESET_ETHERNET>; >> + reset-names = "stmmaceth"; >> }; >> >> &gpio_intc { > Regards Martin
On Tue, 2018-01-16 at 11:41 +0100, Martin Blumenstingl wrote: > > > ðmac { > > > - clocks = <&clkc CLKID_ETH>; > > > - clock-names = "stmmaceth"; > > > + compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac"; > > > > Does meson8 shared the same IP block ? is yes, then this compatible should > > probably be changed one level up, along with the regs > > > > If not, it means that ethmac node should not be defined in meson.dtsi but in > > meson8.dtsi and meson8b.dtsi > > it's tricky - I have ordered a Meson8 board (which has not arrived > yet), but according to my findings: > - Meson6 uses the old binding > - Meson8 uses the same binding as Meson6 > - Meson8b uses the new binding / same binding as GXBB > - Meson8m2 (which is mostly identical to Meson8) uses the same binding > as Meson8b / GXBB > > > In any case, overloading the node like this really clean, even if it works. > > my plan is to restructure the ethmac node once I have my Meson8 board: > - add the the "amlogic,meson6-dwmac" compatible and the 0xc1108108 > register to meson6.dtsi and meson8.dtsi > - remove the "amlogic,meson6-dwmac" binding from meson.dtsi > - figure out whether meson8m2.dtsi (upcoming, on my TODO-list...) > should inherit meson8.dtsi or meson.dtsi > > some more context: > Meson8 and Meson8m2 share a lot of functionality: > - SMP / CPU hotplug procedure > - CPU frequencies (OPP tables) > - pinctrl > however, there are some differences too: > - Meson8m2 uses the WDT layout from Meson8b > - Meson8m2 uses the same CPU temperature calibration procedure as Meson8b > - (not tested yet) Meson8m2 uses the Ethernet registers from Meson8b > > in other words: > I think it's a good idea to restructure the ethmac node > however, I would prefer to do it outside of this series so we don't > have to do it twice (once with limited testing, then fixing some bugs > in a follow-up series once I could test it on Meson8) Sounds like a plan. Fine by me. It's up to Kevin now ;)
Hi Jerome, On Tue, Jan 16, 2018 at 11:17:53AM +0100, Jerome Brunet wrote: > On Tue, 2018-01-16 at 01:34 +0100, Emiliano Ingrassia wrote: > > Extend ethernet controller description adding pin multiplexing and > > setting the needed attributes in ethmac node. > > As reported in S805 SoC manual, the MAC clock source is MPLL2 only. > > > > Signed-off-by: Emiliano Ingrassia <ingrassia@epigenesys.com> > > --- > > arch/arm/boot/dts/meson8b.dtsi | 35 +++++++++++++++++++++++++++++++++-- > > 1 file changed, 33 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi > > index 7cd03ed3742e..3c66d9bdc3a8 100644 > > --- a/arch/arm/boot/dts/meson8b.dtsi > > +++ b/arch/arm/boot/dts/meson8b.dtsi > > @@ -185,6 +185,27 @@ > > #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"; > > + }; > > + }; > > }; > > }; > > > > @@ -203,8 +224,18 @@ > > }; > > > > ðmac { > > - clocks = <&clkc CLKID_ETH>; > > - clock-names = "stmmaceth"; > > + compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac"; > > Does meson8 shared the same IP block ? is yes, then this compatible should > probably be changed one level up, along with the regs > I don't have any information about S802 SoC (aka meson8). It seems that no public informations are available. If you have any, please share. > If not, it means that ethmac node should not be defined in meson.dtsi but in > meson8.dtsi and meson8b.dtsi > Sounds logic! As I understand, Martin has a plan about this. > In any case, overloading the node like this really clean, even if it works. > > > + > > + reg = <0xc9410000 0x10000 > > + 0xc1108140 0x4>; > > + > > + clocks = <&clkc CLKID_ETH>, > > + <&clkc CLKID_MPLL2>, > > + <&clkc CLKID_MPLL2>; > > + clock-names = "stmmaceth", "clkin0", "clkin1"; > > + > > + resets = <&reset RESET_ETHERNET>; > > + reset-names = "stmmaceth"; > > }; > > > > &gpio_intc { > > > _______________________________________________ > linux-amlogic mailing list > linux-amlogic@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-amlogic Thanks, Emiliano
Hi Martin, thanks for the feedback! On Tue, Jan 16, 2018 at 11:14:37AM +0100, Martin Blumenstingl wrote: > On Tue, Jan 16, 2018 at 1:34 AM, Emiliano Ingrassia > <ingrassia@epigenesys.com> wrote: > > Extend ethernet controller description adding pin multiplexing and > > setting the needed attributes in ethmac node. > > As reported in S805 SoC manual, the MAC clock source is MPLL2 only. > > > > Signed-off-by: Emiliano Ingrassia <ingrassia@epigenesys.com> > Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > > I had your patch also in my local tree, where I added the following > comments to the patch description (this is nothing final though, I > just added them so I don't forget about these facts): > Until now we have been using the "amlogic,meson6-dwmac" binding with the > register offset (0xc1108108) defined in meson.dtsi. > During testing (and by reading Hardkernel's u-boot sources for the > Odroid-C1) it turns out that the actual register that should be used is > at 0xc1108140. This also requires us to switch to the new > "amlogic,meson8b-dwmac" binding, because the dwmac-meson8b driver knows > how to configure that register. The old register is a no-op, so using > the old "amlogic,meson6-dwmac" binding with the old register meant that > we relied on the bootloader to set up the Ethernet clocks etc. > correctly. > Ok, I could send a second version of the patch integrating these informations in log message. What do you think? > > > --- > > arch/arm/boot/dts/meson8b.dtsi | 35 +++++++++++++++++++++++++++++++++-- > > 1 file changed, 33 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi > > index 7cd03ed3742e..3c66d9bdc3a8 100644 > > --- a/arch/arm/boot/dts/meson8b.dtsi > > +++ b/arch/arm/boot/dts/meson8b.dtsi > > @@ -185,6 +185,27 @@ > > #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"; > > + }; > > + }; > > }; > > }; > > > > @@ -203,8 +224,18 @@ > > }; > > > > ðmac { > > - clocks = <&clkc CLKID_ETH>; > > - clock-names = "stmmaceth"; > > + compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac"; > > + > > + reg = <0xc9410000 0x10000 > > + 0xc1108140 0x4>; > > + > > + clocks = <&clkc CLKID_ETH>, > > + <&clkc CLKID_MPLL2>, > > + <&clkc CLKID_MPLL2>; > > + clock-names = "stmmaceth", "clkin0", "clkin1"; > > + > > + resets = <&reset RESET_ETHERNET>; > > + reset-names = "stmmaceth"; > > }; > > > > &gpio_intc { > > -- > > 2.15.1 > > Regards, Emiliano
Hi Emiliano, On Wed, Jan 17, 2018 at 3:13 PM, Emiliano Ingrassia <ingrassia@epigenesys.com> wrote: > Hi Martin, > > thanks for the feedback! > > On Tue, Jan 16, 2018 at 11:14:37AM +0100, Martin Blumenstingl wrote: >> On Tue, Jan 16, 2018 at 1:34 AM, Emiliano Ingrassia >> <ingrassia@epigenesys.com> wrote: >> > Extend ethernet controller description adding pin multiplexing and >> > setting the needed attributes in ethmac node. >> > As reported in S805 SoC manual, the MAC clock source is MPLL2 only. >> > >> > Signed-off-by: Emiliano Ingrassia <ingrassia@epigenesys.com> >> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >> >> I had your patch also in my local tree, where I added the following >> comments to the patch description (this is nothing final though, I >> just added them so I don't forget about these facts): >> Until now we have been using the "amlogic,meson6-dwmac" binding with the >> register offset (0xc1108108) defined in meson.dtsi. >> During testing (and by reading Hardkernel's u-boot sources for the >> Odroid-C1) it turns out that the actual register that should be used is >> at 0xc1108140. This also requires us to switch to the new >> "amlogic,meson8b-dwmac" binding, because the dwmac-meson8b driver knows >> how to configure that register. The old register is a no-op, so using >> the old "amlogic,meson6-dwmac" binding with the old register meant that >> we relied on the bootloader to set up the Ethernet clocks etc. >> correctly. >> > > Ok, I could send a second version of the patch integrating these > informations in log message. > What do you think? if you send a v2 of this series anyways then it would be great if you could include it (feel free to change it where needed) >> >> > --- >> > arch/arm/boot/dts/meson8b.dtsi | 35 +++++++++++++++++++++++++++++++++-- >> > 1 file changed, 33 insertions(+), 2 deletions(-) >> > >> > diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi >> > index 7cd03ed3742e..3c66d9bdc3a8 100644 >> > --- a/arch/arm/boot/dts/meson8b.dtsi >> > +++ b/arch/arm/boot/dts/meson8b.dtsi >> > @@ -185,6 +185,27 @@ >> > #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"; >> > + }; >> > + }; >> > }; >> > }; >> > >> > @@ -203,8 +224,18 @@ >> > }; >> > >> > ðmac { >> > - clocks = <&clkc CLKID_ETH>; >> > - clock-names = "stmmaceth"; >> > + compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac"; >> > + >> > + reg = <0xc9410000 0x10000 >> > + 0xc1108140 0x4>; >> > + >> > + clocks = <&clkc CLKID_ETH>, >> > + <&clkc CLKID_MPLL2>, >> > + <&clkc CLKID_MPLL2>; >> > + clock-names = "stmmaceth", "clkin0", "clkin1"; >> > + >> > + resets = <&reset RESET_ETHERNET>; >> > + reset-names = "stmmaceth"; >> > }; >> > >> > &gpio_intc { >> > -- >> > 2.15.1 >> > > > Regards, > > Emiliano Regards Martin
diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi index 7cd03ed3742e..3c66d9bdc3a8 100644 --- a/arch/arm/boot/dts/meson8b.dtsi +++ b/arch/arm/boot/dts/meson8b.dtsi @@ -185,6 +185,27 @@ #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"; + }; + }; }; }; @@ -203,8 +224,18 @@ }; ðmac { - clocks = <&clkc CLKID_ETH>; - clock-names = "stmmaceth"; + compatible = "amlogic,meson8b-dwmac", "snps,dwmac-3.70a", "snps,dwmac"; + + reg = <0xc9410000 0x10000 + 0xc1108140 0x4>; + + clocks = <&clkc CLKID_ETH>, + <&clkc CLKID_MPLL2>, + <&clkc CLKID_MPLL2>; + clock-names = "stmmaceth", "clkin0", "clkin1"; + + resets = <&reset RESET_ETHERNET>; + reset-names = "stmmaceth"; }; &gpio_intc {
Extend ethernet controller description adding pin multiplexing and setting the needed attributes in ethmac node. As reported in S805 SoC manual, the MAC clock source is MPLL2 only. Signed-off-by: Emiliano Ingrassia <ingrassia@epigenesys.com> --- arch/arm/boot/dts/meson8b.dtsi | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-)