Messages in this thread | | | Date | Mon, 21 Jan 2013 20:13:59 +0100 | Subject | Re: [PATCH v3] drivers/pinctrl: grab default handles from device core | From | Linus Walleij <> |
| |
On Fri, Jan 18, 2013 at 9:13 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Fri, Jan 18, 2013 at 04:08:48PM +0100, Linus Walleij wrote: >> - Abstain from using IS_ERR_OR_NULL() in the driver core, >> Russell recently sent a patch to remove it. Handle the >> NULL case explicitly even though it's a bogus case. > > Hmmmmmmmm... I assume you're talking about this: > >> + p = create_pinctrl(dev); >> + if (p && !IS_ERR(p)) >> + kref_init(&p->users); >> + return p; > > which is... interesting. In mainline currently, the version I see there > never returns NULL - it either returns an err pointer or a real pointer.
Actually it was this thing:
+ dpi->p = devm_pinctrl_get(dev); + if (!dpi->p) { + /* Shouldn't happen, but handle this anyway */ + dev_warn(dev, "devm_pinctrl_get() returned NULL\n"); + devm_kfree(dev, dpi); + dev->pins = NULL; + return 0; + }
I will delete both NULL checks and spin a v4.
> Next thing though is... > > create_pinctrl adds the struct pinctrl onto the global list, and then you > intialize it here, which is an interesting sequence. It's safe at the > moment because everything here is operating under a mutex, but it's a > little obscure. > > Is there a reason not to initialize p->users inside create_pinctrl() ?
Not really. I'll fix it...
Yours, Linus Walleij
| |