* Aguirre Rodriguez, Sergio Alberto <saagui...@ti.com> [091021 08:41]:
>
>
> > -----Original Message-----
> > From: linux-omap-ow...@vger.kernel.org
> > [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Roel Kluin
> > Sent: Wednesday, October 21, 2009 10:44 AM
> > To: Aguirre Rodriguez, Sergio Alberto
> > Cc: Imre Deak; linux-fbdev-de...@lists.sourceforge.net;
> > linux-omap@vger.kernel.org; Andrew Morton
> > Subject: Re: [PATCH] omapfb: Wrong test on unsigned?
> >
> > regno is unsigned so the test didn't work. If regno
> > can't be used return an error.
> >
> > Signed-off-by: Roel Kluin <roel.kl...@gmail.com>
> > ---
> > >> Is this correct? please review.
> > >>
> > >> diff --git a/drivers/video/omap/omapfb_main.c
> > >> b/drivers/video/omap/omapfb_main.c
> > >> index 0d0c8c8..cc7dd93 100644
> > >> --- a/drivers/video/omap/omapfb_main.c
> > >> +++ b/drivers/video/omap/omapfb_main.c
> > >> @@ -286,7 +286,7 @@ static int _setcolreg(struct fb_info
> > >> *info, u_int regno, u_int red, u_int green,
> > >> if (r != 0)
> > >> break;
> > >>
> > >> - if (regno < 0) {
> > >> + if ((int)regno < 0) {
> > >
> > > Hmm...
> > >
> > > Isn't regno unsigned integer from the start?
> >
> > yes
> >
> > > 2 things here:
> > >
> > > - regno will never be negative.
> > > - Casting won't make a difference in the meaning., it'll
> > make a negative only when:
> > >
> > > regno > ((2^32) / 2)
> > >
> > > Which doesn't make any sense IMHO.
> >
> > I think it is strange that _setcolreg() accepts a regno that
> > is invalid,
> > ignores it and just returns as if all was OK. If you agree
> > then you may
> > like the patch below.
>
> Yep. Looks nicer to me ;)
>
> Acked-by: Sergio Aguirre <saagui...@ti.com>
Looks good to me too.
Acked-by: Tony Lindgren <t...@atomide.com>
>
> Regards,
> Sergio
> >
> > > Regards,
> > > Sergio
> >
> > Thanks,
> >
> > Roel
> >
> > diff --git a/drivers/video/omap/omapfb_main.c
> > b/drivers/video/omap/omapfb_main.c
> > index 0d0c8c8..4da94d0 100644
> > --- a/drivers/video/omap/omapfb_main.c
> > +++ b/drivers/video/omap/omapfb_main.c
> > @@ -285,12 +285,6 @@ static int _setcolreg(struct fb_info
> > *info, u_int regno, u_int red, u_int green,
> > case OMAPFB_COLOR_RGB444:
> > if (r != 0)
> > break;
> > -
> > - if (regno < 0) {
> > - r = -EINVAL;
> > - break;
> > - }
> > -
> > if (regno < 16) {
> > u16 pal;
> > pal = ((red >> (16 - var->red.length)) <<
> > @@ -299,6 +293,8 @@ static int _setcolreg(struct fb_info
> > *info, u_int regno, u_int red, u_int green,
> > var->green.offset) |
> > (blue >> (16 - var->blue.length));
> > ((u32 *)(info->pseudo_palette))[regno] = pal;
> > + } else {
> > + r = -EINVAL;
> > }
> > break;
> > default:
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-omap" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
