linux-kernel - Re: [PATCH 2/4] pinctrl: add helpers for group based drivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141030194839.GB6764@kwain>
Date:	Thu, 30 Oct 2014 20:48:39 +0100
From:	Antoine Tenart <antoine.tenart@...e-electrons.com>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	Antoine Tenart <antoine.tenart@...e-electrons.com>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/4] pinctrl: add helpers for group based drivers

Hi Linus,

On Tue, Oct 28, 2014 at 04:38:27PM +0100, Linus Walleij wrote:
> On Mon, Oct 20, 2014 at 10:04 AM, Antoine Tenart
> <antoine.tenart@...e-electrons.com> wrote:
> 
> > +#ifdef CONFIG_OF
> > +int of_pinctrl_utils_read_function(struct pinctrl_dev *pctldev,
> > +               struct device_node *node, const char **function_name,
> > +               int *ngroups)
> > +{
> > +       int ret;
> > +
> > +       ret = of_property_read_string(node, "function", function_name);
> > +       if (ret) {
> > +               dev_err(pctldev->dev, "missing function property in node %s\n",
> > +                       node->name);
> > +               return -EINVAL;
> > +       }
> > +
> > +       *ngroups = of_property_count_strings(node, "groups");
> > +       if (ngroups <= 0) {
> > +               dev_err(pctldev->dev, "missing groups property in node %s\n",
> > +                       node->name);
> > +               return -EINVAL;
> > +       }
> > +
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(of_pinctrl_utils_read_function);
> > +#endif
> 
> Isn't it nasty to print dev_err() here if you maybe just want
> to check if this node has a "function" element and proceed to
> see if it's a pin config group in case it hasn't?
> 
> Or should we add of_pinctrl_utils_node_is_mux() and
> of_pinctrl_utils_node_is_config() to check this?

We could have an additional pin_name parameter and return an error if both
function_name *and* pin_name are NULL. That could also allow to have nodes with
both a function and a pin, if that make any sense.

Maybe the of_pinctrl_utils_node_is_mux/config() solution is more
appropriate to avoid having a confusing function. Plus we would have a
dedicated of_pinctrl_utils_read_pins() function.

I think I prefer the second solution because it could avoid some
confusion and the logic would stay in the pinctrl core functions. 

I'll cook up a new version if the of_pinctrl_utils_node_is_mux/config()
is okay with you.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/