[U-Boot] [RFC PATCH 1/7] pinctrl: add pinctrl framework

[Simon Glass]

Hi Masahiro,

On 18 July 2015 at 08:37, Simon Glass <sjg at chromium.org> wrote:
> Hi Masahiro,
>
> On 15 July 2015 at 02:16, Masahiro Yamada <yamada.masahiro at socionext.com> wrote:
>> Now, a simple pinctrl patch is being proposed by Simon.
>> http://patchwork.ozlabs.org/patch/487801/
>>
>> In the design above, as you see, the uclass is just like a wrapper layer
>> to invoke .request and .get_periph_id of low-level drivers.
>> In other words, it is Do-It-Yourself thing, so it is up to you how to identify
>> which peripheral is being handled in your .get_periph_id().
>>
>> And here is one example for how a low-level pinctrl driver could be implemented.
>> http://patchwork.ozlabs.org/patch/487874/
>>
>> As you see in the thread, honestly, I do not like this approach.
>>
>> It is true that you can implement .get_periph_id in your driver
>> better than parsing "interrupts" properties, but I guess
>> many drivers would follow the rockchip implmentation because
>> no helpful infrastructure is provided by the uclass (at least now).
>>
>> Device trees describe hardwares in a way independent of software
>> that they are used with.  So, identical device trees can be (should be)
>> used with U-Boot as well as Linux or whatever.
>>
>> Thus, I want the pinctrl can be controllable by device trees in the same way
>> of Linux, that is, by parsing "pinctrl-names" and "pinctrl-<N>" properties.
>>
>> Of course, it would be possible to do it in my own .get_periph_id,
>> but "pinctrl-names" and "pinctrl-<N>" are too generic to be done in each
>> low-level driver.
>>
>> In this series, I'd like to propose to support it in the uclass, so that
>> we can easily reuse device trees for pinctrl.
>> Please put it on the table for discussion.
>>
>> Let me explain how it works.
>>
>> The basic idea is pretty much like Linux, but it has been much simplified
>> because full-support of the Linux's pinctrl is too much a burden for a boot-loader.
>>
>>  Device Tree
>>  -----------
>>
>> To use pinctrl from each peripheral, add some properties in the device node.
>>
>> "pinctrl-names" is a list of pin states.  The "default" state is mandatory,
>> and it would probably be enough for U-Boot.  But, in order to show how it works,
>> say the example device supports two states: "default" and "sleep".
>> In this case, the properties should be like this.
>>
>>    pinctrl-names = "default", "sleep";
>>
>> And then, add as many "pinctrl-<N>" properties as the number of states.
>>
>>    pinctrl-0 = <phandle to default config node>;
>>    pinctrl-1 = <phandle to sleep config node>;
>>
>> Here, pinctrl-0, pinctrl-1 corresponds to "default", "sleep", respectively.
>>
>> The config nodes are (direct or indirect) children of a pinctrl device.
>>
>> To sum up, the device tree would be like this:
>>
>>    foo {
>>         compatible = "...";
>>         reg = <...>;
>>         pinctrl-names = "default", "sleep";
>>         pinctrl-0 = <&foo_default_pin>;
>>         pinctrl-1 = <&foo_sleep_pin>;
>>         ...
>>    };
>>
>>    pinctrl {
>>          compatible = "...";
>>          reg = <...>;
>>          foo_default_pin: foo_default {
>>               groups = "...";
>>               functions = "...";
>>          };
>>          foo_sleep_pin: foo_sleep {
>>               groups = "...";
>>               functions = "...";
>>          };
>>    };
>>
>>  API
>>  ---
>>
>>
>> To set a device into a particular pin state, call
>> int pinctrl_set_state(struct udevice *dev, const char *state_name).
>>
>> For example, if you want to set the foo device into the sleep state,
>> you can do like this:
>>
>>    struct udevice *foo_dev;
>>
>>    (device_get or whatever)
>>
>>    pinctrl_set_state(foo_dev, "sleep");
>>
>> When each device is probed, pinctrl_init() is invoked,
>> which initializes some pinctrl-specific parameters and set it into "default"
>> pin state.  Because it is automatically done by the core of driver model,
>> when a device is probed, its pins are in the "default" state.
>>
>>  Implementation of low-level driver
>>  ----------------------------------
>>
>> Currently, two methods are supported in the pinctrl operation:
>>   struct pinctrl_ops {
>>         int (*pinmux_set) (struct udevice *dev, const char *group,
>>                            const char *function);
>>         int (*pinconf_set) (struct udevice *dev, const char *group,
>>                             const char *conf_param, unsigned conf_arg);
>>   };
>>
>> They are used to change pin-mux, pin-conf, respectively.
>>
>> If the pin-config node for the target pin-state is like this,
>>          i2c_default_pin: i2c_default {
>>               groups = "i2c-0a";
>>               functions = "i2c-0";
>>          };
>>
>> Your pinmux_set() is called with "i2c-0a" for the group and "i2c-0"
>> for the function.
>>
>> It is totally up to you what you do for each group & function.
>>
>>  Difference from Linux pinctrl (limitation)
>>  -----------------------------------------
>>
>> As you can see in pinctrl drivers in Linux (drivers/pinctrl/*),
>> the drivers usually contain huge tables for pins, groups, functions,...
>> But I do not want to force that for U-Boot.
>>
>> In Linux, "group" represents a group of pins although a group sometimes
>> consists of a signle pin (like GPIO).  Pin-muxing is available only against
>> groups, while pin-conf is supported for both pins and groups.
>>
>> In contrast, in U-Boot, "pins" and "groups" are handled exactly in the same way,
>> so you can use either to specify the target of pin-mux or pin-conf.
>>
>> Pin/group tables are not required for U-boot pinctrl drivers, so
>> we never know which pins belong to which group, function, that is,
>> we can not avoid conflicts on pins.
>>
>> For example, let's say some pins are shared between UART0 and I2C0.
>> In this case, Linux pinctrl does not allow to use them simultaneously,
>> while U-boot pinctrl does not (can not) care about it.
>>
>> If you want to use multiple functions that share the same pins,
>> you must make sure pin-muxing is correctly set with pinctrl_set_state().
>>
>>  TODO
>>  ----
>>
>> [1] Pinctrl drivers are usually used with device trees (or ACPI), but
>>     not all the boards in U-boot support device tree.
>>     I will add board file support (plat data) later.
>>     (we will need some function to register pin settings array)
>>
>> [2] SPL support is not completed.   Tweak Kconfig and Makefile a little.
>>     This should be easy.
>>
>> [3] Currently, properties other than "function" are assumed
>>     as pin-conf parameters.  Perhaps, should we filter out
>>     unsupported properties?  A table for supported properties
>>     such "bias-pull-up", "driver-strength", etc. ?
>>
>> [4] For "function", "groups" should be able to be omitted.
>>
>> Comments are welcome.
>
> (long pause for thought)
>
> I would certainly like to have this in U-Boot. I think for most uses
> it is the right thing to do. However I'm not sure it is sufficient.
>
> For SPL we are limited on code size / space. In that case we often
> need to do a little pinmuxing but we cannot always afford the space of
> this implementation. The device tree for pinmux is pretty large for
> some SoCs quite apart from the extra code. I found this recently with
> Rockchip which has a 32KB limit.
>
> The nice thing about my simple implementation is that it can be used
> from SPL efficiently. For example:
>
> static void soc_spi_pinmux(enum periph_id id, int flags)
> {
>    switch (id) {
>       case PERIPH_ID_SPI0:
>          writel(...)
>          break;
>       case PERIPH_ID_SPI1:
>          writel(...)
>          break;
>    }
> }
>
> static void soc_i2c_pinmux(enum periph_id id, int flags)
> {
>       case PERIPH_ID_I2C0:
>          writel(...)
>          break;
>       case PERIPH_ID_I2C1:
>          writel(...)
>          break;
>    }
> }
>
> static void soc_set_pinmux(enum_periph_id id, int flags)
> {
>    switch (id) {
> #ifdef CONFIG_SPI
>       case PERIPH_ID_SPI0:
>       case PERIPH_ID_SPI0:
>          soc_i2c_pinmux(id, flags);
>          break;
> #endif
> #ifdef CONFIG_SPI
>       case PERIPH_ID_I2C0:
>       case PERIPH_ID_I2C0:
>          soc_i2c_pinmux(id, flags);
>          break;
> #endif
> }
>
> In SPL we can do something like:
>
>    soc_i2c_pinmux(PERIPH_ID_I2C0, 0);
>
> to set up I2C port 0 with default pin settings. This is about as
> efficient as you can get.
>
> With your full implementation we have no such option.
>
> So I think your implementation makes more sense in general, but mine
> is better for SPL.
>
> Can we combine the two? What do you think about allowing the uclass to
> provide both interfaces? You can then use the full pinctl one, but
> implement a simple one also. This could be achieved simply by adding
> two more methods to the uclass.

If you are OK with this then I'll have a try at basing my pinctrl
patches on top of your series. I think it can be made to work.

>
> BTW I've been meaning to discuss how to deal with SPL config as it is
> painful in this area, but I'll start a new thread for that.
>
> Regards,
> Simon

Regards,
Simon