Message ID | 1421753325.25187.58.camel@merveille.lan |
---|---|
State | Accepted |
Headers | show |
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
--- 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;