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

Masahiro Yamada yamada.masahiro at socionext.com
Thu Jul 23 06:06:43 CEST 2015


Hi Simon,
Sorry for being away from this a while.


2015-07-22 23:24 GMT+09:00 Simon Glass <sjg at chromium.org>:
> 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.


I know this RFC is missing some items.
OK, we can have a trial to see what needs to be brushed up.

Yours handles simple peripheral IDs to not have impact on the memory footprint,
while mine handles strings as they are given by device tree nodes.
Maybe we need some conversion callbacks?



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

OK, i will try this.



-- 
Best Regards
Masahiro Yamada


More information about the U-Boot mailing list