Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support

[Scott Wood <scottwood@freescale.com>]

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@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