[PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support
Scott Wood
scottwood at freescale.com
Sat May 1 02:22:54 EST 2010
On Fri, Apr 30, 2010 at 10:08:45AM -0500, Timur Tabi wrote:
> On Fri, Apr 30, 2010 at 5:19 AM, Anatolij Gustschin <agust at denx.de> wrote:
>
> >> How about just doing this?
> >>
> >> .init_early = mpc512x_init_diu,
> >
> > I thought it should be prepared for adding other code here.
> > mpc5121_ads_init_early() is generic and could contain other
> > things as well. I would vote for current version.
>
> Do you have any plans to add any additional code? If not, then I say
> skip the middle-man. If someone ever needs to do more, he can always
> put that function back.
>
> >> I'm pretty sure the compiler will optimize this to:
> >>
> >> temp = (1000000000000UL / pixclock);
> >>
> >> so you may as well do it that way.
> >
> > ??
> > 1000000000000 is _not_ UL, but UUL.
>
> That's what I meant. Actually, I think it's ULL. Regardless, I think
> the compiler will see the "1000000000 ... * 1000" and just combine
> them together. You're not actually outsmarting the compiler.
The compiler will do no such thing. That's a valid transformation when
doing pure math, but not when working with integers.
> >> > + err = 100000000;
> >>
> >> Why do you assign err to this arbitrary value?
> >
> > Dunno. It is Freescale's code and I do not have time to check
> > and understand each bit of it and to explain it.
>
> *sigh* You're not the first person to modify the DIU driver without
> understanding what it all does. I suspect that the original author
> should have done this:
>
> err = -1;
>
> because he wanted it to be the largest possible integer.
-1 is not the largest possible integer. LONG_MAX, perhaps?
-Scott
More information about the Linuxppc-dev
mailing list