[PATCH 2/4] ARM: dts: meson8b: extending ethernet controller description
Martin Blumenstingl
martin.blumenstingl at googlemail.com
Sat Sep 30 07:09:48 PDT 2017
Hi Emiliano,
On Fri, Sep 29, 2017 at 9:10 PM, Emiliano Ingrassia
<ingrassia at epigenesys.com> wrote:
> Hi Martin,
>
> thanks for the review!
>
> On Thu, Sep 28, 2017 at 11:41:48PM +0200, Martin Blumenstingl wrote:
>> Hi Emiliano,
>>
>> On Wed, Sep 27, 2017 at 11:39 PM, Emiliano Ingrassia
>> <ingrassia at epigenesys.com> wrote:
>> > This patch adds ethernet controller pin description and extend its
>> > attributes in the relative node.
>> >
>> > Signed-off-by: Emiliano Ingrassia <ingrassia at epigenesys.com>
>> > ---
>> >
>> > 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(-)
>> >
>> > diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
>> > index bc278da7df0d..816bc9188f44 100644
>> > --- a/arch/arm/boot/dts/meson8b.dtsi
>> > +++ b/arch/arm/boot/dts/meson8b.dtsi
>> > @@ -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";
>> > + };
>> > + };
>> > };
>> > };
>> >
>> > ðmac {
>> > - 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.
OK, this is interesting
>> 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."
> 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)
>> 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
>> 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).
>> 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
>> > +
>> > + 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.
>
>> > +
>> > + 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]
>>
>
> 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
>> > +
>> > + 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
>> > };
>> >
>> > &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!
>>
>
> OK, thanks for your suggestions!
>
>>
>> Regards,
>> Martin
>>
>
> Regards,
>
> Emiliano
>
>>
>> [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
More information about the linux-amlogic
mailing list