Commit c5acf4a2b3c6 ("pinctrl: Add the concept of peripheral IDs") added some additional change that was not mentioned in the git-log.

That commit added dm_scan_fdt_node() in the pinctrl uclass binding. It should be handled by the simple-bus driver, and it is totally unrelated to pinctrl in the first place.

I guess Simon's motivation was to bind GPIO banks located under the Rockchip pinctrl device. It is true some chips have sub-devices under their pinctrl devices, but it is basically SoC-specific matter.

Keep only pinctrl-generic features in the uclass and move others to each low-level driver.

By the way, this strategy produces code duplication between low-level drivers and the simple-bus driver. Theoretically, it is possible to have multiple compatible strings like this:

compatible = "rockchip,rk3288-pinctrl", "simple-bus";

Actually, some device trees do this and Linux can handle it properly, but for now U-Boot cannot. In the current implementation of U-boot, only the best match ("rockchip,rk3288-pinctrl" in this case) is picked and the others are ignored.

Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---

drivers/pinctrl/pinctrl-uclass.c | 15 +++++++++------ drivers/pinctrl/rockchip/pinctrl_rk3288.c | 8 ++++++++ 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c index 58001ef..b5fdcd1 100644 --- a/drivers/pinctrl/pinctrl-uclass.c +++ b/drivers/pinctrl/pinctrl-uclass.c @@ -11,7 +11,6 @@ #include <dm/device.h> #include <dm/lists.h> #include <dm/pinctrl.h> -#include <dm/root.h> #include <dm/uclass.h>

DECLARE_GLOBAL_DATA_PTR; @@ -160,8 +159,7 @@ static int pinctrl_select_state_full(struct udevice *dev, const char *statename)

static int pinconfig_post_bind(struct udevice *dev) { - /* Scan the bus for devices */ - return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false); + return 0; } #endif

@@ -249,10 +247,15 @@ static int pinctrl_post_bind(struct udevice *dev) }

/* - * The pinctrl driver child nodes should be bound so that peripheral - * devices can easily search in parent devices during later DT-parsing. + * If set_state callback is set, we assume this pinctrl driver is the + * full implementation. In this case, its child nodes should be bound + * so that peripheral devices can easily search in parent devices + * during later DT-parsing. */ - return pinconfig_post_bind(dev); + if (ops->set_state) + return pinconfig_post_bind(dev); + + return 0; }

UCLASS_DRIVER(pinctrl) = { diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3288.c b/drivers/pinctrl/rockchip/pinctrl_rk3288.c index 5205498..c432a00 100644 --- a/drivers/pinctrl/rockchip/pinctrl_rk3288.c +++ b/drivers/pinctrl/rockchip/pinctrl_rk3288.c @@ -17,6 +17,7 @@ #include <asm/arch/periph.h> #include <asm/arch/pmu_rk3288.h> #include <dm/pinctrl.h> +#include <dm/root.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -415,6 +416,12 @@ static struct pinctrl_ops rk3288_pinctrl_ops = { .get_periph_id = rk3288_pinctrl_get_periph_id, };

+static int rk3288_pinctrl_bind(struct udevice *dev) +{ + /* scan child GPIO banks */ + return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false); +} + static int rk3288_pinctrl_probe(struct udevice *dev) { struct rk3288_pinctrl_priv *priv = dev_get_priv(dev); @@ -437,5 +444,6 @@ U_BOOT_DRIVER(pinctrl_rk3288) = { .of_match = rk3288_pinctrl_ids, .priv_auto_alloc_size = sizeof(struct rk3288_pinctrl_priv), .ops = &rk3288_pinctrl_ops, + .bind = rk3288_pinctrl_bind, .probe = rk3288_pinctrl_probe, };