[OpenWrt-Devel] uboot-lantiq cgu settings for ramboot image

On Tue, 2015-01-20 at 00:39 +0100, Ben Mulvihill wrote:
> On Mon, 2015-01-19 at 19:21 +0100, Ben Mulvihill wrote:
> > On Mon, 2015-01-19 at 16:47 +0100, Daniel Schwierzeck wrote:
> > > 2015-01-19 15:44 GMT+01:00 Ben Mulvihill <ben.mulvihill@gmail.com>:
> > > > On Mon, 2015-01-19 at 11:51 +0000, Conor O'Gorman wrote:
> > > >> On 19/01/15 10:46, Ben Mulvihill wrote:
> > > >> > Hello,
> > > >> >
> > > >> > I am trying to build uboot-lantiq for the BT Home Hub 3A (lantiq
> > > >> > ar9), and am wondering where to initialise the cgu, in the case
> > > >> > of a ramboot image for uart booting. Normally the cgu is initialised
> > > >> > in lowlevel_init, but that code is bypassed in ramboot images. The
> > > >> > result is that the board boots with the wrong cgu settings, which
> > > >> > sends the console haywire. So far I have tried two solutions:
> > > >>
> > > >> Another option is to try and not change anything. The console is already
> > > >> configured and running. The ram does need config.
> > > >>
> > > >> I was used to seeing the ramboot version running at half clock speed, at
> > > >> least on danube, previous to ar9.
> > > >>
> > > >> Conor
> > > >
> > > > Hi Conor,
> > > >
> > > > Thanks for the reply. But with the latest uboot-lantiq, not changing
> > > > anything means that I don't get a usable console. With an older
> > > > version I do at least get a uboot console, but no linux console when
> > > > I boot openwrt. Correcting the cgu settings solves both problems.
> > > >
> > > 
> > > could you try this?
> > > 
> > > diff --git a/arch/mips/cpu/mips32/arx100/cgu.c
> > > b/arch/mips/cpu/mips32/arx100/cgu.c
> > > index 6e71ee7..e0afbda 100644
> > > --- a/arch/mips/cpu/mips32/arx100/cgu.c
> > > +++ b/arch/mips/cpu/mips32/arx100/cgu.c
> > > @@ -95,15 +95,5 @@ unsigned long ltq_get_cpu_clock(void)
> > > 
> > >  unsigned long ltq_get_bus_clock(void)
> > >  {
> > > -       u32 fpi_sel;
> > > -       unsigned long clk;
> > > -
> > > -       fpi_sel = ltq_cgu_sys_readl(1, CGU_SYS_FPI_SEL);
> > > -
> > > -       if (fpi_sel)
> > > -               clk = ltq_get_io_region_clock() / 2;
> > > -       else
> > > -               clk = ltq_get_io_region_clock();
> > > -
> > > -       return clk;
> > > +       return ltq_get_io_region_clock();
> > >  }
> > > 
> > > the UART driver calculates the baudrate from the FPI bus clock, but
> > > FPI_SEL is not available on AR9. FPI bus clock is always the same as
> > > DDR clock, Obviously a copy&paste error from VR9 code ;)
> > > 
> > 
> > No, even with this patch, I still don't get a working console I'm
> > afraid. If I don't set anything explicitly, the board comes up with
> > CGU_SYS set to 0x05, ie CGU_SYS_SYSSEL_PLL0_333_MHZ |
> > CGU_SYS_CPUSEL_EQUAL_DDRCLK | CGU_SYS_DDRSEL_THIRD_SYSCLK.
> > Is this a valid combination without CGU_SYS_PPESEL_250_MHZ ?
> > I don't understand what CGU_SYS_PPESEL_250_MHZ does?
> > The "right setting", as set by the stock uboot, is 0x80.
> 
> P.S. There also seems to be a discrepancy between the uboot and
> linux code. I take it from what you say above that fpi clock, ddr
> clock and io region clock are all the same. Now if the least 
> significant bit of CGU_SYS is set, then according to the uboot
> code - function ltq_get_bus_clock() - their value is one
> third of the system clock. But according to the linux code
> - function ltq_ar9_fpi_hz() in arch/mips/lantiq/xway/clk.c -
> their value in this case is equal to the system clock.
> 
> Or am I getting muddled? It's past my bedtime!
> 
> 

Some of the bitshifting in arch/mips/cpu/mips32/arx100/cgu.c is 1
out. A patch along these lines should fix it:

2015-01-20 12:28 GMT+01:00 Ben Mulvihill <ben.mulvihill@gmail.com>:

>
> Some of the bitshifting in arch/mips/cpu/mips32/arx100/cgu.c is 1
> out. A patch along these lines should fix it:

yes, the code is wrong. I have prepared a patch. Thanks for fixing.

https://github.com/danielschwierzeck/u-boot-lantiq/commit/f38048eb920276d8aa48a4b0bbf653d8fd4ef66b

>
> --- a/arch/mips/cpu/mips32/arx100/cgu.c 2015-01-20 11:57:22.000000000 +0100
> +++ b/arch/mips/cpu/mips32/arx100/cgu.c 2015-01-20 12:00:15.000000000 +0100
> @@ -10,12 +10,17 @@
>  #include <asm/lantiq/clk.h>
>  #include <asm/lantiq/io.h>
>
> -#define CGU_SYS_DDR_SEL                (1 << 0)
> -#define CGU_SYS_CPU_SEL                (1 << 2)
> +#define CGU_SYS_DDR_SHIFT      0
> +#define CGU_SYS_CPU_SHIFT      2
>  #define CGU_SYS_SYS_SHIFT      3
> +#define CGU_SYS_FPI_SHIFT      6
> +#define CGU_SYS_PPE_SHIFT      7
> +
> +#define CGU_SYS_DDR_MASK       (1 << CGU_SYS_DDR_SHIFT)
> +#define CGU_SYS_CPU_MASK       (1 << CGU_SYS_CPU_SHIFT)
>  #define CGU_SYS_SYS_MASK       (0x3 << CGU_SYS_SYS_SHIFT)
> -#define CGU_SYS_FPI_SEL                (1 << 6)
> -#define CGU_SYS_PPE_SEL                (1 << 7)
> +#define CGU_SYS_FPI_MASK       (1 << CGU_SYS_FPI_SHIFT)
> +#define CGU_SYS_PPE_MASK       (1 << CGU_SYS_PPE_SHIFT)
>
>  struct ltq_cgu_regs {
>         u32     rsvd0;
> @@ -68,7 +73,7 @@ unsigned long ltq_get_io_region_clock(vo
>         u32 ddr_sel;
>         unsigned long clk;
>
> -       ddr_sel = ltq_cgu_sys_readl(1, CGU_SYS_DDR_SEL);
> +       ddr_sel = ltq_cgu_sys_readl(CGU_SYS_DDR_MASK, CGU_SYS_DDR_SHIFT);
>
>         if (ddr_sel)
>                 clk = ltq_get_system_clock() / 3;
> @@ -83,7 +88,7 @@ unsigned long ltq_get_cpu_clock(void)
>         u32 cpu_sel;
>         unsigned long clk;
>
> -       cpu_sel = ltq_cgu_sys_readl(1, CGU_SYS_CPU_SEL);
> +       cpu_sel = ltq_cgu_sys_readl(CGU_SYS_CPU_MASK, CGU_SYS_CPU_SHIFT);
>
>         if (cpu_sel)
>                 clk = ltq_get_io_region_clock();
> @@ -98,7 +103,7 @@ unsigned long ltq_get_bus_clock(void)
>         u32 fpi_sel;
>         unsigned long clk;
>
> -       fpi_sel = ltq_cgu_sys_readl(1, CGU_SYS_FPI_SEL);
> +       fpi_sel = ltq_cgu_sys_readl(CGU_SYS_FPI_MASK, CGU_SYS_FPI_SHIFT);
>
>         if (fpi_sel)
>                 clk = ltq_get_io_region_clock() / 2;
>
>

On Tue, 2015-01-20 at 14:05 +0100, Daniel Schwierzeck wrote:
> 2015-01-20 12:28 GMT+01:00 Ben Mulvihill <ben.mulvihill@gmail.com>:
> 
> >
> > Some of the bitshifting in arch/mips/cpu/mips32/arx100/cgu.c is 1
> > out. A patch along these lines should fix it:
> 
> yes, the code is wrong. I have prepared a patch. Thanks for fixing.
> 
Thank you. 

The linux code in arch/mips/lantiq/xway/clk.c is wrong too I think.
Shall I submit a patch to make the functions there return the same 
values as the uboot equivalents?

Also, just as a matter of interest, what does CGU_SYS_PPESEL_250_MHZ 
actually do? What does PPE stand for?

Thanks,

Ben

2015-01-20 15:44 GMT+01:00 Ben Mulvihill <ben.mulvihill@gmail.com>:
> On Tue, 2015-01-20 at 14:05 +0100, Daniel Schwierzeck wrote:
>> 2015-01-20 12:28 GMT+01:00 Ben Mulvihill <ben.mulvihill@gmail.com>:
>>
>> >
>> > Some of the bitshifting in arch/mips/cpu/mips32/arx100/cgu.c is 1
>> > out. A patch along these lines should fix it:
>>
>> yes, the code is wrong. I have prepared a patch. Thanks for fixing.
>>
> Thank you.
>
> The linux code in arch/mips/lantiq/xway/clk.c is wrong too I think.
> Shall I submit a patch to make the functions there return the same
> values as the uboot equivalents?

only ltq_ar9_fpi_hz() should be fixed. If bit 0 is set, than DDR/FPI
clock is 1/3 of SYS clock, otherwise 1/2

>
> Also, just as a matter of interest, what does CGU_SYS_PPESEL_250_MHZ
> actually do? What does PPE stand for?
>

it means Packet Processing Engine. It is a separate on-chip CPU for
network offloading functions which only runs with a propietary
firmware from Lantiq. On AR9 the PPE clock is fixed to 250 MHz. The HW
manual says the according bit in CGU_SYS register should be set but
maybe this could be ignored.

is this patch correct and should be merged or is it a RFC ?

http://patchwork.ozlabs.org/patch/431024/

On Thu, 2015-01-22 at 10:07 +0100, John Crispin wrote:
> is this patch correct and should be merged or is it a RFC ?
> 
> http://patchwork.ozlabs.org/patch/431024/

Daniel found a mistake in the current code. I found another.
This fixes both, so I guess it is a patch rather than an RFC.
I understand it to be correct, but the authoritative answer as
to whether it is indeed correct should really come from
Daniel ;-) In any case Daniel has already committed it to
uboot-lantiq github.

Hope this is some help. Daniel?

Am 22.01.2015 um 22:23 schrieb Ben Mulvihill:
> On Thu, 2015-01-22 at 10:07 +0100, John Crispin wrote:
>> is this patch correct and should be merged or is it a RFC ?
>>
>> http://patchwork.ozlabs.org/patch/431024/
> 
> Daniel found a mistake in the current code. I found another.
> This fixes both, so I guess it is a patch rather than an RFC.
> I understand it to be correct, but the authoritative answer as
> to whether it is indeed correct should really come from
> Daniel ;-) In any case Daniel has already committed it to
> uboot-lantiq github.
> 
> Hope this is some help. Daniel?
> 

I'm going to integrate that patch in the openwrt/14.04 branch. If you
have a working patch for the BT Home Hub; I'd like to integrate it too.
After that we can refresh the patches in the OpenWRT tree.

> 
> I'm going to integrate that patch in the openwrt/14.04 branch. If you
> have a working patch for the BT Home Hub; I'd like to integrate it too.
> After that we can refresh the patches in the OpenWRT tree.
> 
Hi Daniel,

Not yet I'm afraid. I still have some problems with ethernet and the 
nand driver. Both are working on an older version based I think on 
u-boot 2010.03, and I'm hopeful that by comparing the two I'll be
able to work out what's wrong. But it might take a while.

Ben