[PATCH v2] drivers/pinctrl: grab default handles from device core

[Linus Walleij]

On Thu, Jan 10, 2013 at 9:42 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:

>> +     /* Allocate a pin state container on-the-fly */
>> +     if (!dev->pins) {
>> +             dpi = devm_kzalloc(dev, sizeof(*dpi), GFP_KERNEL);
>
> This is allocated using a devm_ function. If -EPROBE_DEFER is returned
> below after the assignment to dev->pins or if the driver's own probe()
> returns -EPROBE_DEFER, this allocation will be freed by the driver core.
> This can leave dev->pins pointing to something non-NULL, yet invalid.

OK I (think I) fixed it like this:

For the second deferral, set dev->pins to NULL.

I did some other fixes here to explicitly devm_kfree() the container
if no pinctrl handle was found. No point in keeping it around.

>> +     if (!dpi->p) {
>> +             dpi->p = devm_pinctrl_get(dev);
>
> That won't succeed for a pinctrl device that has a default state in
> order to implement hogs. This will then cause the pin controller device
> to always defer probe and never activate. This will leave HW
> unconfigured and/or prevent other devices from successfully calling
> pinctrl_get().

Fixed by your suggested patch, thanks.

>> +     /*
>> +      * See if somebody else (such as the device core) has already
>> +      * obtained a handle to the pinctrl for this device. In that case,
>> +      * return another pointer to it.
>> +      */
>>       p = find_pinctrl(dev);
>> -     if (p != NULL)
>> -             return ERR_PTR(-EBUSY);
>
> I deliberately returned an error here, because there's no reference
> counting on the struct pinctrl objects. If a driver calls pinctrl_get(),
> with the new code below, it will retrieve the same struct. If it later
> calls pinctrl_put(), the put will immediately free the structure. This
> will invalidate the pointers that reference it in struct device's pins
> field.
>
> This issue will probably trigger on Tegra, since we at least have a
> pinctrl-based I2C mux that calls pinctrl_get().

OK I just introduced a reference counter using <linux/kref.h>.

I've retested on U300 and U8500.

Yours,
Linus Walleij