On Fri, 31 Oct 2014 09:53:52 +0200
Siarhei Siamashka <siarhei.siamas...@gmail.com> wrote:
> 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.
Hans, I'm not going to start lecturing you about why the copy/paste
duplicated code is bad. And why diverging the separate instances
of already existing duplicated code is making everything even worse.
Most likely you know it yourself perfectly fine.
You probably think that this patch is good enough for the sunxi-3.4
kernel. And maybe you are even right. However I'm not very happy,
because it is serving as a bad example for the others.
Anyway, this is already taking too much time, so I just pushed your
patch to the stage/sunxi-3.4 branch as-is (without my Acked-by).
With the assumption that you are fully accountable for the quality
of this fix. Please keep in mind that I have not volunteered to be
maintaining the sunxi-3.4 kernel. And there are other people on this
mailing list, who could have approved and pushed your patch much
earlier. But appears that everyone is too busy at the moment.
Moreover, I believe that you can get the commit access yourself
any time. This should not be a problem if you just request for it.
Thanks for your contribution. It is really appreciated.
--
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.
