lkml.org 
[lkml]   [2013]   [Jan]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2] drivers/pinctrl: grab default handles from device core
From
On Thu, Jan 10, 2013 at 9:42 PM, Stephen Warren <swarren@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


\
 
 \ /
  Last update: 2013-01-18 16:41    [W:0.135 / U:0.324 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site