[lkml]   [2012]   [Feb]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRE: [PATCH 08/20] pinctrl: Assume map table entries can't have a NULL name field
    Dong Aisheng wrote at Tuesday, February 21, 2012 6:08 AM:
    > On Mon, Feb 20, 2012 at 2:45 PM, Stephen Warren <> wrote:
    > > pinctrl_register_mappings() already requires that every mapping table
    > > entry have a non-NULL name field.
    > >
    > > Logically, this makes sense too; drivers should always request a specific
    > > named state so they know what they're getting. Relying on getting the
    > Hmm? It makes me a little confused.
    > IIRC we allow the driver to request NULL map name in the linaro
    > connect discussion.
    > For those devices they do not want to support multi states, they
    > really don't want to care about the state name.
    > The original way is convenient for such a case.

    With device tree, every state is always going to have /some/ name; the
    binding we discussed didn't include any way to specify "no name" or
    "default name" or "default state", so this will keeps things a little
    more consistent.

    I possibly could be persuaded to allow NULL state names in the mapping
    table. However, if so, I think they should work a little differently to
    the code currently in pinctrl: Right now, a NULL parameter to pinmux_get()
    allows matching against /any/ state name in the table. If we changed this
    to only allow matching against mapping table with a NULL name, that might
    be OK, since it'd be very explicit what it matched. But, as I mentione
    above, migrating such a driver to device tree would require changing this
    so I'm not inclined to like that solution without strong requirements.

    In other words, what we have right now is something like:

    if name:
    match = !strcmp(name, map->name)
    match = true

    If we continue to allow NULL names, I think we should change that to:

    if name:
    match = map->name && !strcmp(name, map->name)
    match = !map->name

    > > diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
    > > index b6e3c35..5e30d91 100644
    > > --- a/drivers/pinctrl/core.c
    > > +++ b/drivers/pinctrl/core.c
    > > @@ -488,8 +488,8 @@ static struct pinctrl *pinctrl_get_locked(struct device *dev, const char *name)
    > >        int i;
    > >        struct pinctrl_map const *map;
    > >
    > > -       /* We must have dev or ID or both */
    > > -       if (!dev && !name)
    > > +       /* We must have a state name */
    > > +       if (WARN_ON(!name))
    > >                return ERR_PTR(-EINVAL);
    > We're already using the pinctrl device name for hog map.
    > So should it be something like:
    > if (WARN_ON(!dev) || WARN_ON(!name))

    That's only true starting with the next patch in this series, which does
    modify the code roughly as you described.


    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2012-02-21 18:41    [W:0.023 / U:4.596 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site