| Message ID | 20230118121224.253241-1-michael@walle.cc |
|---|---|
| State | Accepted |
| Commit | 72b8c6d1ebfd40a6ee6be0345ff7abb30c9f9e3d |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/3] pinctrl: don't fall back to pinctrl_select_state_simple() | expand |
On 1/18/23 13:12, Michael Walle wrote: > If CONFIG_PINCTRL_FULL is enabled, never fall back to the simple > implementation. pinctrl_select_state() is called for each device and it > is expected to fail. A fallback to the simple imeplementation doesn't > make much sense. > > To keep the return code consistent, we need to change the -EINVAL (which > was ignored before) to -ENOSYS. > > Signed-off-by: Michael Walle <michael@walle.cc> Reviewed-by: Marek Vasut <marex@denx.de>
On Wed, 18 Jan 2023 at 06:45, Marek Vasut <marex@denx.de> wrote: > > On 1/18/23 13:12, Michael Walle wrote: > > If CONFIG_PINCTRL_FULL is enabled, never fall back to the simple > > implementation. pinctrl_select_state() is called for each device and it > > is expected to fail. A fallback to the simple imeplementation doesn't > > make much sense. > > > > To keep the return code consistent, we need to change the -EINVAL (which > > was ignored before) to -ENOSYS. > > > > Signed-off-by: Michael Walle <michael@walle.cc> > > Reviewed-by: Marek Vasut <marex@denx.de> Reviewed-by: Simon Glass <sjg@chromium.org>
On Wed, Jan 18, 2023 at 01:12:22PM +0100, Michael Walle wrote: > If CONFIG_PINCTRL_FULL is enabled, never fall back to the simple > implementation. pinctrl_select_state() is called for each device and it > is expected to fail. A fallback to the simple imeplementation doesn't > make much sense. > > To keep the return code consistent, we need to change the -EINVAL (which > was ignored before) to -ENOSYS. > > Signed-off-by: Michael Walle <michael@walle.cc> > Reviewed-by: Marek Vasut <marex@denx.de> > Reviewed-by: Simon Glass <sjg@chromium.org> Applied to u-boot/master, thanks!
diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c index ce2d5ddf6d..419ef5f52f 100644 --- a/drivers/pinctrl/pinctrl-uclass.c +++ b/drivers/pinctrl/pinctrl-uclass.c @@ -71,13 +71,13 @@ static int pinctrl_select_state_full(struct udevice *dev, const char *statename) */ state = dectoul(statename, &end); if (*end) - return -EINVAL; + return -ENOSYS; } snprintf(propname, sizeof(propname), "pinctrl-%d", state); list = dev_read_prop(dev, propname, &size); if (!list) - return -EINVAL; + return -ENOSYS; size /= sizeof(*list); for (i = 0; i < size; i++) { @@ -162,11 +162,6 @@ U_BOOT_DRIVER(pinconfig_generic) = { }; #else -static int pinctrl_select_state_full(struct udevice *dev, const char *statename) -{ - return -ENODEV; -} - static int pinconfig_post_bind(struct udevice *dev) { return 0; @@ -317,10 +312,10 @@ int pinctrl_select_state(struct udevice *dev, const char *statename) * Try full-implemented pinctrl first. * If it fails or is not implemented, try simple one. */ - if (pinctrl_select_state_full(dev, statename)) - return pinctrl_select_state_simple(dev); + if (CONFIG_IS_ENABLED(PINCTRL_FULL)) + return pinctrl_select_state_full(dev, statename); - return 0; + return pinctrl_select_state_simple(dev); } int pinctrl_request(struct udevice *dev, int func, int flags)
If CONFIG_PINCTRL_FULL is enabled, never fall back to the simple implementation. pinctrl_select_state() is called for each device and it is expected to fail. A fallback to the simple imeplementation doesn't make much sense. To keep the return code consistent, we need to change the -EINVAL (which was ignored before) to -ENOSYS. Signed-off-by: Michael Walle <michael@walle.cc> --- The real underlying problem is that the pinctrl_select_state_simple() doesn't really work before relocation. This implementation calls uclass_get_device() which will try to probe the pinctrl, but that device might not be ready yet, i.e. in my case, it is an MFD and it needs its parent. For each device in the devicetree, there will be a device_probe(pinctrl) attempt and if that device has some kind of private or platform data, it will eat away the early memory, because there is no free. I'm not really sure how that could be fixed. But still, if there is a full implementation, it shouldn't fall back to the simple one. --- drivers/pinctrl/pinctrl-uclass.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)