On Thu, 16 Oct 2014 10:42:19 +0200
Hans de Goede <hdego...@redhat.com> wrote:

> Hi,
> 
> On 10/15/2014 09:28 PM, Siarhei Siamashka wrote:
> > On Wed, 15 Oct 2014 12:10:45 +0200
> > Hans de Goede <hdego...@redhat.com> wrote:
> > 
> >> Before the u-boot dram cleanup u-boot would always set PLL5 factor m to
> >> 2 (reg value 1) and div p to 1, and get_cmu_clk in the nand code
> >> would calculate the pll5p clk like this:
> >>
> >> clk = 24 * factor_n * factor_k / div_p / factor_m;
> >>
> >> aka:
> >>
> >> clk = 24 * factor_n * factor_k / (div_p * factor_m);
> >>
> >> This is wrong however, factor_m is not used to calculate pll5p, and div_p 
> >> is
> >> not a straight divider, but it divides by 2 ^ div_p. Since with the m == 2 
> >> and
> >> p == 1 settings used before the dram cleanup, this happend to do the right
> >> thing in the form of dividing by 2. But with the new dram code div_p is 0,
> >> and then the old get_cmu_clk code fails with a divide by 0 error.
> >>
> >> This commit fixes this, by changing the clk calculation to the correct 
> >> form of:
> >>
> >> clk = (24 * factor_n * factor_k) >> div_p;
> >>
> >> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> >> ---
> >>  drivers/block/sunxi_nand/nfd/nand_blk.c | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/block/sunxi_nand/nfd/nand_blk.c 
> >> b/drivers/block/sunxi_nand/nfd/nand_blk.c
> >> index a632453..2169301 100644
> >> --- a/drivers/block/sunxi_nand/nfd/nand_blk.c
> >> +++ b/drivers/block/sunxi_nand/nfd/nand_blk.c
> >> @@ -1095,16 +1095,15 @@ __u32 get_cmu_clk(void)
> >>  {
> >>    __u32 reg_val;
> >>    __u32 div_p, factor_n;
> >> -  __u32 factor_k, factor_m;
> >> +  __u32 factor_k;
> >>    __u32 clock;
> >>  
> >>    reg_val  = *(volatile unsigned int *)(0xf1c20000 + 0x20);
> >>    div_p    = (reg_val >> 16) & 0x3;
> >>    factor_n = (reg_val >> 8) & 0x1f;
> >>    factor_k = ((reg_val >> 4) & 0x3) + 1;
> >> -  factor_m = ((reg_val >> 0) & 0x3) + 1;
> >>  
> >> -  clock = 24 * factor_n * factor_k/div_p/factor_m;
> >> +  clock = (24 * factor_n * factor_k) >> div_p;
> >>  
> >>    return clock;
> >>  }
> > 
> > That's a good catch. Also one more copy-paste of exactly the same code
> > appears to be in 'drivers/block/sunxi_nand/nandtest/nand_test.c'
> 
> That code is not compiled / used. Still feel free to submit a patch to fix it 
> if you want
> to.

I see only two acceptable solutions: either the fix should be applied
in both places, or the unused code should be purged from the source
tree. Your current fix is incomplete.

> I don't have commit access to the linux-sunxi kernel sources, can you please 
> push these
> 2 fixes ?

Feel free to resubmit the patch. Or if you wish, I can modify your
patch to apply the changes in both places.

-- 
Best regards,
Siarhei Siamashka

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to