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

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@chromium.org>
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Suggested-by: Simon Glass <sjg@chromium.org>
---

 drivers/pinctrl/Kconfig          | 12 ++++++++++--
 drivers/pinctrl/pinctrl-uclass.c | 29 +++++++++++++++++++++++++++++
 include/dm/pinctrl.h             |  2 ++
 3 files changed, 41 insertions(+), 2 deletions(-)

Hi Masahiro,

On 10 August 2015 at 10:05, Masahiro Yamada
<yamada.masahiro@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@chromium.org>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Suggested-by: Simon Glass <sjg@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

Hi Simon,


2015-08-12 23:16 GMT+09:00 Simon Glass <sjg@chromium.org>:

>> 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?
>

After my careful consideration, I did not do this.

If we do this,the corresponding members in all drivers must be also
#ifdef'd out,
including full-pinctrl drivers that do not care about memory footprint.

I do not like adding #ifdefs around to fix build errors found with
"make allyesconfig", "make randconfig".

I think it is a general strategy to not #ifdef out struct members.