[U-Boot] [RFC v3 PATCH 3/4] pinctrl: add simple pinctrl implementation

[Simon Glass]

Hi Masahiro,

On 10 August 2015 at 10:05, Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
> The full pinctrl implementation added by the previous commit can
> support the same DT bindings as Linux, but it typically needs about
> 1.5 KB footprint to include the uclass support (CONFIG_PINCTRL +
> CONFIG_PINCTRL_GENERIC + CONFIG_PINMUX on ARM architecture).
>
> We are often limited on code size for SPL.  Besides, we still have
> many boards that do not support device tree configuration.  The full
> pinctrl, which requires OF_CONTROL, does not make sense for those
> boards.
>
> So, we need much more ad-hoc, smaller implementation, providing one
> operation, set_state_simple.  This callback takes two arguments,
> a pinctrl device and a peripheral device on which the operation
> should be done.  The uclass provides no mechanism to identify the
> target peripheral device, so set_state_simple must do it on its own.
> Probably, base addresses, interrupt numbers, etc. would be used for
> that purpose, but it is totally up to the implementation of each
> low-level driver.
>
> There are some more limitations worth mentioning.  The pinctrl
> device should generally be specified by a phandle in DTS, but this
> uclass itself does not parse device tree at all, i.e. there is no
> way to know the pinctrl device that takes care of the peripheral
> devices.  This simple uclass assumes the first pinctrl device is the
> correct one.  This is practically no problem because almost all
> systems have only one pinctrl device.  Another limitation is that
> it can not handle multiple states.  This simplification should be
> also OK since most of systems only uses "default" state during the
> boot stage.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com>
> Suggested-by: Simon Glass <sjg at chromium.org>
> ---
>
>  drivers/pinctrl/Kconfig          | 12 ++++++++++--
>  drivers/pinctrl/pinctrl-uclass.c | 29 +++++++++++++++++++++++++++++
>  include/dm/pinctrl.h             |  2 ++
>  3 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 3f4f4b3..eca83fe 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -7,9 +7,13 @@ menu "Pin controllers"
>  config PINCTRL
>         bool "Support pin controllers"
>
> +config PINCTRL_SIMPLE
> +       bool "Support simple pin controllers"
> +       depends on PINCTRL
> +
>  config PINCTRL_GENERIC
>         bool "Support generic pin controllers"
> -       depends on PINCTRL
> +       depends on PINCTRL && !PINCTRL_SIMPLE
>
>  config PINMUX
>         bool "Support pin multiplexing controllers"
> @@ -23,9 +27,13 @@ config SPL_PINCTRL
>         bool "Support pin controlloers in SPL"
>         depends on SPL
>
> +config SPL_PINCTRL_SIMPLE
> +       bool "Support simple pin controlloers in SPL"
> +       depends on SPL_PINCTRL
> +
>  config SPL_PINCTRL_GENERIC
>         bool "Support generic pin controllers in SPL"
> -       depends on SPL_PINCTRL
> +       depends on SPL_PINCTRL && !SPL_PINCTRL_SIMPLE
>
>  config SPL_PINMUX
>         bool "Support pin multiplexing controllers in SPL"
> diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c
> index ab3c146..853d779 100644
> --- a/drivers/pinctrl/pinctrl-uclass.c
> +++ b/drivers/pinctrl/pinctrl-uclass.c
> @@ -15,6 +15,34 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +#if CONFIG_IS_ENABLED(PINCTRL_SIMPLE)
> +
> +static int pinctrl_select_state(struct udevice *dev, const char *ignored)
> +{
> +       struct udevice *pctldev;
> +       struct pinconf_ops *ops;
> +       int ret;
> +
> +       ret = uclass_get_device(UCLASS_PINCTRL, 0, &pctldev);
> +       if (ret)
> +               return ret;
> +
> +       ops = pctldev->driver->ops;
> +       if (!ops || !ops->set_state_simple) {
> +               dev_err(dev, "set_state_simple op missing\n");
> +               return -EINVAL;
> +       }
> +
> +       return ops->set_state_simple(pctldev, dev);
> +}
> +
> +UCLASS_DRIVER(pinctrl) = {
> +       .id = UCLASS_PINCTRL,
> +       .name = "pinctrl",
> +};
> +
> +#else
> +
>  static int pinctrl_config_one(struct udevice *config)
>  {
>         struct udevice *pctldev;
> @@ -149,3 +177,4 @@ U_BOOT_DRIVER(pinconfig_generic) = {
>         .name = "pinconfig",
>         .id = UCLASS_PINCONFIG,
>  };
> +#endif
> diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h
> index 42008da..b2ba0b4 100644
> --- a/include/dm/pinctrl.h
> +++ b/include/dm/pinctrl.h
> @@ -85,6 +85,8 @@ struct pinctrl_ops {
>         int (*pinconf_group_set)(struct udevice *dev, unsigned group_selector,
>                                  unsigned param, unsigned argument);
>         int (*set_state)(struct udevice *dev, struct udevice *config);
> +       /* for pinctrl-simple */
> +       int (*set_state_simple)(struct udevice *dev, struct udevice *periph);

So should the other members be #idef'd out? Also, comments on this function?

>  };
>
>  /**
> --
> 1.9.1
>

Regards,
Simon