lkml.org 
[lkml]   [2012]   [Nov]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] leds: Add generic support for memory mapped LEDs
From
Date
On Mon, 2012-11-05 at 12:55 +0000, Pawel Moll wrote:
> On Mon, 2012-11-05 at 09:39 +0000, Jon Medhurst (Tixy) wrote:
> > > +static void mmio_led_brightness_set(struct led_classdev *cdev,
> > > + enum led_brightness brightness)
> > > +{
> > > + struct mmio_led *led = container_of(cdev, struct mmio_led, cdev);
> > > + unsigned long uninitialized_var(flags);
> >
> > uninitialized_var seems to be a bit contentious, Linus Torvalds had a
> > recent complaint about it which prompted Ingo to post a patch proposing
> > to removing it: https://patchwork.kernel.org/patch/1655621/ So perhaps
> > best to avoid using it ;-).
> >
> > In this case, you could possibly keep gcc quite with something like:
> >
> > spinlock_t *lock = led->lock;
> >
> > and then use the local variable 'lock' everywhere instead of led->lock.
> > Or just keep it simple an initialise flags to 0 instead.
>
> Yeah, = 0 will do...
>
> > > + if (!pdata)
> > > + return -EINVAL;
> > > +
> > > + if (pdata->reg_size != 8 && pdata->reg_size != 16 &&
> > > + pdata->reg_size != 32)
> > > + return -EFAULT;
> >
> > Is EFAULT appropriate here? Why not EINVAL?
>
> Hm. To distinguish it from !pdata case I guess (and a 13 bit wide
> transaction sounds like a fault to me ;-), but I can be persuaded
> otherwise without much effort...

I was asking as much for my own education about use of error values as
anything else. The comments in errno-base.h are:

#define EINVAL 22 /* Invalid argument */
#define EFAULT 14 /* Bad address */

and from looking in the source tree it seems EFAULT is mostly used to
indicate a bad memory address passed from user-side to the kernel.

It's a trivial point so it's not worth wasting time on a long
discussion.

--
Tixy



\
 
 \ /
  Last update: 2012-11-05 16:01    [W:0.050 / U:2.364 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site