Re: [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b

[Martin Blumenstingl]

Hi Emiliano,

On Thu, Dec 28, 2017 at 6:51 PM, Emiliano Ingrassia
 wrote:
> Hi Martin,
>
> thank you for the quick response!
>
> On Thu, Dec 28, 2017 at 05:58:34PM +0100, Martin Blumenstingl wrote:
>> Hi Emiliano,
>>
>> thank you for testing this!
>>
>> On Thu, Dec 28, 2017 at 5:16 PM, Emiliano Ingrassia
>>  wrote:
>> > Hi Martin, Hi Dave,
>> >
>> > On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
>> >> Hi Dave,
>> >>
>> >> please do not apply this series until it got a Tested-by from Emiliano.
>> >>
>> >>
>> >> Hi Emiliano,
>> >>
>> >> you reported [0] that you couldn't get dwmac-meson8b to work on your
>> >> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
>> >> I think I was able to find a fix: it consists of two patches (which you
>> >> find in this series)
>> >>
>> >> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
>> >> only partially test this (I could only check if the clocks were
>> >> calculated correctly when using a dummy 52394Hz input clock instead
>> >> of MPLL2).
>> >>
>> >> Could you please give this series a try and let me know about the
>> >> results?
>> >> You obviously still need your two "ARM: dts: meson8b" patches which
>> >> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
>> >> - enable Ethernet on the Odroid-C1
>> >>
>> >> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
>> >> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
>> >> fine (so let's hope that this also fixes your Meson8b issue :)).
>> >>
>> >>
>> >> changes since v1 at [1]:
>> >> - changed the subject of the cover-letter to indicate that this is all
>> >>   about the RGMII clock
>> >> - added PATCH #1 which ensures that we don't unnecessarily change the
>> >>   parent clocks in RMII mode (and also makes the code easier to
>> >>   understand)
>> >> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
>> >>   is about the RGMII clock
>> >> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
>> >> - replaced PATCH #3 (formerly PATCH #2) with one that sets
>> >>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
>> >>   on Meson8b correctly
>> >>
>> >
>> > Really thank you for your help and effort. I tried your patch but
>> > unfortunately it didn't solve the problem.
>> this is probably my fault: I forgot to mention that it requires a fix
>> for the 32-bit SoCs in the clock driver ("clk: meson: mpll: use 64-bit
>> maths in params_from_rate", see [0]) to work properly
>>
>
> Ok, with that patch applied I got:
>
> xtal   112400  0 0
>  sys_pll   00  12  0 0
>   cpu_clk  00  12  0 0
>  vid_pll   00   73200  0 0
>  fixed_pll 22  255000  0 0
>   mpll211   124999851  0 0
>c941.ethernet#m250_sel  11   124999851  0 0
> c941.ethernet#m250_div 11   124999851  0 0
>  c941.ethernet#m25_div 112471  0 0
in theory this looks good...!

> which is equal to your result. However, the ethernet is still not working.
OK, I'll send an updated version later (or tomorrow, depending on how
much time I have left today) which adds a fixed divider and converts
bit 10 to a gate
on GXBB and newer bit 10 is always true since the m250_div is only
3-bit wide (= max divider of 7, 0 is invalid according to the
datasheet). with a 1000MHz input clock (fclk_div2) m250_div will
divide this by 4 and m25_div divided this by 10.

> The prg0 register is set to 0x70A1.
>
> A problem that I see with this solution is that MPLL2 is set to ~125 MHz.
> The S805 SoC manual reports that bits 9-7 should contain a value x such
> that: MPLL2 = 250 MHz * x (with x >= 1).
> In our case, bits 9-7 are set to 1 which is incorrect.
> I think that MPLL2 should be 250 MHz at least.
when looking at the GXBB clock tree we need a fixed divide by 10.
this also means that the mpll2 clock will probably be set to ~250MHz,
the m250_div to 1 and with the fixed divider "10" we get close to our
desired 25MHz

>> >
>> > Here is the new clk_summary:
>> >
>> > xtal112400  0 0
>> >  sys_pll00  12  0 0
>> >   cpu_clk   00  12  0 0
>> >  vid_pll00   73200  0 0
>> >  fixed_pll  22  255000  0 0
>> >   mpll2 11   10625  0 0
>> >c941.ethernet#m250_sel   11   10625 

Re: [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b

[Emiliano Ingrassia]

Hi Martin,

thank you for the quick response!

On Thu, Dec 28, 2017 at 05:58:34PM +0100, Martin Blumenstingl wrote:
> Hi Emiliano,
> 
> thank you for testing this!
> 
> On Thu, Dec 28, 2017 at 5:16 PM, Emiliano Ingrassia
>  wrote:
> > Hi Martin, Hi Dave,
> >
> > On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
> >> Hi Dave,
> >>
> >> please do not apply this series until it got a Tested-by from Emiliano.
> >>
> >>
> >> Hi Emiliano,
> >>
> >> you reported [0] that you couldn't get dwmac-meson8b to work on your
> >> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
> >> I think I was able to find a fix: it consists of two patches (which you
> >> find in this series)
> >>
> >> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
> >> only partially test this (I could only check if the clocks were
> >> calculated correctly when using a dummy 52394Hz input clock instead
> >> of MPLL2).
> >>
> >> Could you please give this series a try and let me know about the
> >> results?
> >> You obviously still need your two "ARM: dts: meson8b" patches which
> >> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
> >> - enable Ethernet on the Odroid-C1
> >>
> >> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
> >> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
> >> fine (so let's hope that this also fixes your Meson8b issue :)).
> >>
> >>
> >> changes since v1 at [1]:
> >> - changed the subject of the cover-letter to indicate that this is all
> >>   about the RGMII clock
> >> - added PATCH #1 which ensures that we don't unnecessarily change the
> >>   parent clocks in RMII mode (and also makes the code easier to
> >>   understand)
> >> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
> >>   is about the RGMII clock
> >> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
> >> - replaced PATCH #3 (formerly PATCH #2) with one that sets
> >>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
> >>   on Meson8b correctly
> >>
> >
> > Really thank you for your help and effort. I tried your patch but
> > unfortunately it didn't solve the problem.
> this is probably my fault: I forgot to mention that it requires a fix
> for the 32-bit SoCs in the clock driver ("clk: meson: mpll: use 64-bit
> maths in params_from_rate", see [0]) to work properly
>

Ok, with that patch applied I got:

xtal   112400  0 0
 sys_pll   00  12  0 0
  cpu_clk  00  12  0 0
 vid_pll   00   73200  0 0
 fixed_pll 22  255000  0 0
  mpll211   124999851  0 0
   c941.ethernet#m250_sel  11   124999851  0 0
c941.ethernet#m250_div 11   124999851  0 0
 c941.ethernet#m25_div 112471  0 0

which is equal to your result. However, the ethernet is still not working.
The prg0 register is set to 0x70A1.

A problem that I see with this solution is that MPLL2 is set to ~125 MHz.
The S805 SoC manual reports that bits 9-7 should contain a value x such
that: MPLL2 = 250 MHz * x (with x >= 1).
In our case, bits 9-7 are set to 1 which is incorrect.
I think that MPLL2 should be 250 MHz at least.

> >
> > Here is the new clk_summary:
> >
> > xtal112400  0 0
> >  sys_pll00  12  0 0
> >   cpu_clk   00  12  0 0
> >  vid_pll00   73200  0 0
> >  fixed_pll  22  255000  0 0
> >   mpll2 11   10625  0 0
> >c941.ethernet#m250_sel   11   10625  0 0
> > c941.ethernet#m250_div  11   10625  0 0
> >  c941.ethernet#m25_div  112125  0 0
> >
> > which leads to a value of 0x70a1 in the prg0 ethernet register.
> > As you can see, something is changed but the RGMII clock is not at 25 MHz.
> > In particular, the bit 10 of prg0, which enables the "generation of 25 MHz
> > clock for PHY" (see S805 manual), is 0.
> assuming that the description in the datasheet is correct
> after Kevin and Mike got updated information from Amlogic about the
> PRG_ETHERNET0 register documenation (see [1]) we thought that bit 10
> means "0 = divide by 5, 1 = divide by 10" (see [2]). I didn't question
> this so far, but I'll test this on a newer SoC later (by forcing
> m250_div to 125MHz, then m25_div will have register value 0 = divide
> by 5)
> 
> 

Re: [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b

[Martin Blumenstingl]

Hi Emiliano,

thank you for testing this!

On Thu, Dec 28, 2017 at 5:16 PM, Emiliano Ingrassia
 wrote:
> Hi Martin, Hi Dave,
>
> On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
>> Hi Dave,
>>
>> please do not apply this series until it got a Tested-by from Emiliano.
>>
>>
>> Hi Emiliano,
>>
>> you reported [0] that you couldn't get dwmac-meson8b to work on your
>> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
>> I think I was able to find a fix: it consists of two patches (which you
>> find in this series)
>>
>> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
>> only partially test this (I could only check if the clocks were
>> calculated correctly when using a dummy 52394Hz input clock instead
>> of MPLL2).
>>
>> Could you please give this series a try and let me know about the
>> results?
>> You obviously still need your two "ARM: dts: meson8b" patches which
>> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
>> - enable Ethernet on the Odroid-C1
>>
>> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
>> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
>> fine (so let's hope that this also fixes your Meson8b issue :)).
>>
>>
>> changes since v1 at [1]:
>> - changed the subject of the cover-letter to indicate that this is all
>>   about the RGMII clock
>> - added PATCH #1 which ensures that we don't unnecessarily change the
>>   parent clocks in RMII mode (and also makes the code easier to
>>   understand)
>> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
>>   is about the RGMII clock
>> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
>> - replaced PATCH #3 (formerly PATCH #2) with one that sets
>>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
>>   on Meson8b correctly
>>
>
> Really thank you for your help and effort. I tried your patch but
> unfortunately it didn't solve the problem.
this is probably my fault: I forgot to mention that it requires a fix
for the 32-bit SoCs in the clock driver ("clk: meson: mpll: use 64-bit
maths in params_from_rate", see [0]) to work properly

>
> Here is the new clk_summary:
>
> xtal112400  0 0
>  sys_pll00  12  0 0
>   cpu_clk   00  12  0 0
>  vid_pll00   73200  0 0
>  fixed_pll  22  255000  0 0
>   mpll2 11   10625  0 0
>c941.ethernet#m250_sel   11   10625  0 0
> c941.ethernet#m250_div  11   10625  0 0
>  c941.ethernet#m25_div  112125  0 0
>
> which leads to a value of 0x70a1 in the prg0 ethernet register.
> As you can see, something is changed but the RGMII clock is not at 25 MHz.
> In particular, the bit 10 of prg0, which enables the "generation of 25 MHz
> clock for PHY" (see S805 manual), is 0.
assuming that the description in the datasheet is correct
after Kevin and Mike got updated information from Amlogic about the
PRG_ETHERNET0 register documenation (see [1]) we thought that bit 10
means "0 = divide by 5, 1 = divide by 10" (see [2]). I didn't question
this so far, but I'll test this on a newer SoC later (by forcing
m250_div to 125MHz, then m25_div will have register value 0 = divide
by 5)

if the description from the documentation is correct then we need to
replace m25_div with a fixed-factor clock (mult = 1, div = 5) and make
it a m25_en gate clock instead
the resulting clock path would look like this: mpll2 > m250_sel >
m250_div > fixed_factor > m25_en

> Please, if you have other suggestions let me know.
could you please re-test this with the patch from [0] applied? no
other changes should be needed!

> Best regards,
>
> Emiliano
>
>>
>> [0] 
>> http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
>> [1] 
>> http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html
>>
>>
>> Martin Blumenstingl (3):
>>   net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
>>   net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
>>   net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
>>
>>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c| 55 
>> +++---
>>  1 file changed, 27 insertions(+), 28 deletions(-)
>>
>> --
>> 2.15.1
>>

Regards
Martin


[0] https://patchwork.kernel.org/patch/10131677/
[1] https://www.mail-archive.com/netdev@vger.kernel.org/msg119290.html
[2] https://www.mail-archive.com/netdev@vger.kernel.org/msg119293.html

Re: [RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b

[Emiliano Ingrassia]

Hi Martin, Hi Dave,

On Sun, Dec 24, 2017 at 12:40:57AM +0100, Martin Blumenstingl wrote:
> Hi Dave,
> 
> please do not apply this series until it got a Tested-by from Emiliano.
> 
> 
> Hi Emiliano,
> 
> you reported [0] that you couldn't get dwmac-meson8b to work on your
> Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
> I think I was able to find a fix: it consists of two patches (which you
> find in this series)
> 
> Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
> only partially test this (I could only check if the clocks were
> calculated correctly when using a dummy 52394Hz input clock instead
> of MPLL2).
> 
> Could you please give this series a try and let me know about the
> results?
> You obviously still need your two "ARM: dts: meson8b" patches which
> - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
> - enable Ethernet on the Odroid-C1
> 
> I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
> and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
> fine (so let's hope that this also fixes your Meson8b issue :)).
> 
> 
> changes since v1 at [1]:
> - changed the subject of the cover-letter to indicate that this is all
>   about the RGMII clock
> - added PATCH #1 which ensures that we don't unnecessarily change the
>   parent clocks in RMII mode (and also makes the code easier to
>   understand)
> - changed subject of PATCH #2 (formerly PATCH #1) to state that this
>   is about the RGMII clock
> - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
> - replaced PATCH #3 (formerly PATCH #2) with one that sets
>   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
>   on Meson8b correctly
>

Really thank you for your help and effort. I tried your patch but
unfortunately it didn't solve the problem.

Here is the new clk_summary:

xtal112400  0 0
 sys_pll00  12  0 0
  cpu_clk   00  12  0 0
 vid_pll00   73200  0 0
 fixed_pll  22  255000  0 0
  mpll2 11   10625  0 0
   c941.ethernet#m250_sel   11   10625  0 0
c941.ethernet#m250_div  11   10625  0 0
 c941.ethernet#m25_div  112125  0 0

which leads to a value of 0x70a1 in the prg0 ethernet register.
As you can see, something is changed but the RGMII clock is not at 25 MHz.
In particular, the bit 10 of prg0, which enables the "generation of 25 MHz
clock for PHY" (see S805 manual), is 0.

Please, if you have other suggestions let me know.

Best regards,

Emiliano

> 
> [0] 
> http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
> [1] 
> http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html
> 
> 
> Martin Blumenstingl (3):
>   net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
>   net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
>   net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
> 
>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c| 55 
> +++---
>  1 file changed, 27 insertions(+), 28 deletions(-)
> 
> -- 
> 2.15.1
> 

[RFT net-next v2 0/3] dwmac-meson8b: RGMII clock fixes for Meson8b

[Martin Blumenstingl]

Hi Dave,

please do not apply this series until it got a Tested-by from Emiliano.


Hi Emiliano,

you reported [0] that you couldn't get dwmac-meson8b to work on your
Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
I think I was able to find a fix: it consists of two patches (which you
find in this series)

Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
only partially test this (I could only check if the clocks were
calculated correctly when using a dummy 52394Hz input clock instead
of MPLL2).

Could you please give this series a try and let me know about the
results?
You obviously still need your two "ARM: dts: meson8b" patches which
- add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
- enable Ethernet on the Odroid-C1

I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
fine (so let's hope that this also fixes your Meson8b issue :)).


changes since v1 at [1]:
- changed the subject of the cover-letter to indicate that this is all
  about the RGMII clock
- added PATCH #1 which ensures that we don't unnecessarily change the
  parent clocks in RMII mode (and also makes the code easier to
  understand)
- changed subject of PATCH #2 (formerly PATCH #1) to state that this
  is about the RGMII clock
- added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
- replaced PATCH #3 (formerly PATCH #2) with one that sets
  CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
  on Meson8b correctly


[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html


Martin Blumenstingl (3):
  net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
  net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
  net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock

 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c| 55 +++---
 1 file changed, 27 insertions(+), 28 deletions(-)

-- 
2.15.1