Am Donnerstag, den 28.03.2013, 22:43 +0800 schrieb Shawn Guo:
> On Thu, Mar 28, 2013 at 10:58:00AM +0100, Philipp Zabel wrote:
> > In the loops in _get_table_maxdiv(), _get_table_div(), and
> > _get_table_val(), in drivers/clk/clk-divider.c the exit condition
> > is .div == 0, so there needs to be a sentinel with .div = 0 at the end
> > of each clk_div_table. It's also documented in the kerneldoc comment for
> > clk_register_divider_table.
>
> Ah, I do not know that. So clk_enet_ref_table is actually buggy?
Yes. For some reason I thought that for 2 bit dividers, the code would
stop after four entries, but that is not true. The clk_enet_ref_table is
missing the final { .div = 0 } entry.
> > I can write that as "{ }", like this:
> >
> > static struct clk_div_table test_div_table[] = {
> > { .val = 2, .div = 1 },
> > { .val = 1, .div = 2 },
> > { .val = 0, .div = 4 },
> > { }
> > };
> >
> Yes, that's much better for reading. But to be consistent with
> clk_enet_ref_table, please also put a comma after .div.
I'll do that.
> > static struct clk_div_table control3_table[] = {
> > { .val = 0, .div = 1 },
> > { .val = 1, .div = 2 },
> > { .val = 3, .div = 4 },
> > { }
> > };
> >
> Per Reference Manual, we should have one more entry below?
>
> { .val = 2, .div = 1, }
Ok. The clock code will always choose val=0, but in the unlikely case
that some boot code writes val=2 before starting Linux, at least we can
detect it correctly.
> ...
>
> > Because I didn't realize that the names were fixed in the final
> > documentation. I'll rename pll[45]_test_div to pll[45]_post_div.
> > pll5_control3 should be renamed to pll5_video_div, I guess.
>
> Yea, that's much easier for users to map code and document.
regards
Philipp
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
