Hi Masahiro,
On 18 July 2015 at 08:37, Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On 15 July 2015 at 02:16, Masahiro Yamada yamada.masahiro@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