On Thu, Jul 24, 2025 at 11:25 AM Bartosz Golaszewski <b...@bgdev.pl> wrote:
>
> The name of the pin function has no real meaning to pinctrl core and is
> there only for human readability of device properties. Some pins are
> muxed as GPIOs but for "strict" pinmuxers it's impossible to request
> them as GPIOs if they're bound to a devide - even if their function name
> explicitly says "gpio". Add a new field to struct pinfunction that
> allows to pass additional flags to pinctrl core. While we could go with
passing
to the pinctrl
> a boolean "is_gpio" field, a flags field is more future-proof.
>
> If the PINFUNCTION_FLAG_GPIO is set for a given function, the pin muxed
> to it can be requested as GPIO even on strict pin controllers. Add a new
"...the pin, which is muxed to it, ..."
> callback to struct pinmux_ops - function_is_gpio() - that allows pinmux
> core to inspect a function and see if it's a GPIO one. Provide a generic
> implementation of this callback.
...
> - if (ops->strict && desc->mux_usecount)
> + if (ops->function_is_gpio && mux_setting)
Seems mux_setting presence is prior to the GPIO checks, I would swap
the parameters of &&.
> + func_is_gpio = ops->function_is_gpio(pctldev,
> + mux_setting->func);
One line is okay.
> + if (ops->strict && desc->mux_usecount && !func_is_gpio)
> return false;
>
> return !(ops->strict && !!desc->gpio_owner);
I think this whole if/return chain can be made slightly more readable,
but I haven't had something to provide right now. Lemme think about
it,
...
> + if (ops->function_is_gpio && mux_setting)
> + func_is_gpio = ops->function_is_gpio(pctldev,
> +
> mux_setting->func);
> + if ((!gpio_range || ops->strict) && !func_is_gpio &&
> desc->mux_usecount && strcmp(desc->mux_owner, owner)) {
This is very similar to the above check, I think at bare minimum here
can be a helper for both cases.
...
> +/**
> + * pinmux_generic_function_is_gpio() - returns true if given function is a
> GPIO
> + * @pctldev: pin controller device
> + * @selector: function number
Missing Return section. Please run kernel-doc validator against new kernel-docs.
> + */
> +bool pinmux_generic_function_is_gpio(struct pinctrl_dev *pctldev,
> + unsigned int selector)
> +{
> + struct function_desc *function;
> +
> + function = radix_tree_lookup(&pctldev->pin_function_tree,
> + selector);
One line is okay.
> + if (!function)
> + return false;
> +
> + return function->func->flags & PINFUNCTION_FLAG_GPIO;
> +}
...
> struct pinfunction {
> const char *name;
> const char * const *groups;
> size_t ngroups;
> + unsigned long flags;
Not sure we need this. If the function is GPIO, pin control already
knows about this. The pin muxing has gpio request / release callbacks
that change the state. Why do we need an additional flag(s)?
> };
--
With Best Regards,
Andy Shevchenko
