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.ten...@free-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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
