lkml.org 
[lkml]   [2012]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRE: [PATCH 19/20] pinctrl: API changes to support multiple states per device
Dong Aisheng wrote at Monday, February 27, 2012 8:19 PM:
> On Mon, Feb 27, 2012 at 10:37:16AM -0800, Stephen Warren wrote:
> > Dong Aisheng wrote at Monday, February 27, 2012 2:07 AM:
> > > On Sun, Feb 19, 2012 at 11:45:59PM -0700, Stephen Warren wrote:
> ..........
> >
> > > > +static struct pinctrl *pinctrl_get_locked(struct device *dev)
> > > > +{
> > > > + struct pinctrl *p;
> > > >
> > > > -error:
> > > > - list_for_each_entry(setting, &p->settings, node)
> > > > - pinmux_free_setting(setting);
> > > > + if (WARN_ON(!dev))
> > > > + return ERR_PTR(-EINVAL);
> > > >
> > > > - kfree(p);
> > > > + p = find_pinctrl(dev);
> > > > + if (p == NULL)
> > > > + p = create_pinctrl(dev);
> > > > + if (IS_ERR(p))
> > > > + return p;
> > > > +
> > > > + p->usecount++;
> > >
> > > I still can not understand what's the purpose of p->usecount?
> > > For allowing multi times calling of pinctrl_get for on the same device?
> >
> > pinctrl_get() could be called multiple times for the same device. Rather
> > than create a whole new struct pinctrl each time it's called, we just
> > reference count the object so that each call returns the same one, and
> > it won't be destroyed until all users have called pinctrl_put().
...
> I still can't find in which case the device will have such requirement
> since per my understanding pinctrl is a little different from clock
> (clock can be used by different devices but we do not allow pins to be
> used by difference devices at the same time).

True. It's actually very easy to make pinctrl_get() fail if a struct
pinctrl was already created for the device in question. If we do that,
we can completely remove the usecount field. I'll update my patch to do
that.

> > > > +static inline struct pinctrl * __must_check pinctrl_get_select(
> > > > + struct device *dev, const char *name)
> > > > +{
> > > > + struct pinctrl *p;
> > > > + struct pinctrl_state *s;
> > > > + int ret;
> > > > +
> > > > + p = pinctrl_get(dev);
> > > > + if (IS_ERR(p))
> > > > + return p;
> > > > +
> > > > + s = pinctrl_lookup_state(p, name);
> > > > + if (IS_ERR(s)) {
> > > > + pinctrl_put(p);
> > > > + return ERR_PTR(PTR_ERR(s));
> > > > + }
> > > > +
> > > > + ret = pinctrl_select_state(p, s);
> > > > + if (ret < 0) {
> > > > + pinctrl_put(p);
> > > > + return ERR_PTR(ret);
> > >
> > > s/ERR_PTR(ret)/ret ?
> >
> > The function returns a pointer, whereas ret is an int. ERR_PTR() is used
> > to wrap the int error code into a pointer value so that the function can
> > return either a valid pointer, or an error-code. See include/linux/err.h.
>
> Hmm, below is what i see in your patch:
> +int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *state)
> {
> + int ret;
> +
> mutex_lock(&pinctrl_mutex);
> - pinctrl_disable_locked(p);
> + ret = pinctrl_select_state_locked(p, state);
> mutex_unlock(&pinctrl_mutex);
> +
> + return ret;
> }
>
> It seems pinctrl_select_state does not return a pointer.

pinctrl_select()_state() returns an int error code.

pinctrl_get_select() returns a pointer, or an error code encoded into a
pointer.

ERR_PTR(ret) is used to convert pinctrl_select()'s int error code into
a pointer representation of the error code for pinctrl_get_select() to
return.

--
nvpublic



\
 
 \ /
  Last update: 2012-02-28 18:07    [W:0.108 / U:5.956 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site