Re: [PATCH V2 2/2] pinctrl: Assume map table entries can't have a NULL name field
From: Dong Aisheng <hidden>
Date: 2012-02-28 09:29:51
On Mon, Feb 27, 2012 at 04:55:09PM -0700, Stephen Warren wrote:
pinctrl_register_mappings() already requires that every mapping table entry have a non-NULL name field. Logically, this makes sense too; drivers should always request a specific named state so they know what they're getting. Relying on getting the first mentioned state in the mapping table is error-prone, and a nasty special case to implement, given that a given the mapping table may define multiple states for a device. Remove a small part of the documentation that talked about optionally requesting a specific state; it's mandatory now. Signed-off-by: Stephen Warren <redacted> --- v2: Update mach-u300/core.c and sirfsoc_uart.c not to request NULL state names. Use PINCTRL_STATE_DEFAULT from previous patch. This is now conceptually ack'd by Dong Aisheng, although I didn't actually include the ack above since I've reworked the patch a little since I last posted it (per the v2 changelog above). ---
It looks very good to me. Acked-by: Dong Aisheng <redacted> Regards Dong Aisheng
quoted hunk ↗ jump to hunk
Documentation/pinctrl.txt | 7 +++---- arch/arm/mach-u300/core.c | 8 ++++---- drivers/pinctrl/core.c | 17 +++++------------ drivers/tty/serial/sirfsoc_uart.c | 2 +- 4 files changed, 13 insertions(+), 21 deletions(-)diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt index 6fe3232..558aac5 100644 --- a/Documentation/pinctrl.txt +++ b/Documentation/pinctrl.txt@@ -782,16 +782,19 @@ spi on the second function mapping: static const struct pinctrl_map __initdata mapping[] = { { .dev_name = "foo-spi.0", + .name = PINCTRL_STATE_DEFAULT, .ctrl_dev_name = "pinctrl-foo", .function = "spi0", }, { .dev_name = "foo-i2c.0", + .name = PINCTRL_STATE_DEFAULT, .ctrl_dev_name = "pinctrl-foo", .function = "i2c0", }, { .dev_name = "foo-mmc.0", + .name = PINCTRL_STATE_DEFAULT, .ctrl_dev_name = "pinctrl-foo", .function = "mmc0", },@@ -944,10 +947,6 @@ foo_remove() pinctrl_put(state->p); } -If you want to grab a specific control mapping and not just the first one -found for this device you can specify a specific mapping name, for example in -the above example the second i2c0 setting: pinctrl_get(&device, "spi0-pos-B"); - This get/enable/disable/put sequence can just as well be handled by bus drivers if you don't want each and every driver to handle it and you know the arrangement on your bus.diff --git a/arch/arm/mach-u300/core.c b/arch/arm/mach-u300/core.c index c9050dd..5b37d84 100644 --- a/arch/arm/mach-u300/core.c +++ b/arch/arm/mach-u300/core.c@@ -1573,9 +1573,9 @@ static struct pinctrl_map __initdata u300_pinmux_map[] = { PIN_MAP_SYS_HOG("pinctrl-u300", "emif0"), PIN_MAP_SYS_HOG("pinctrl-u300", "emif1"), /* per-device maps for MMC/SD, SPI and UART */ - PIN_MAP("MMCSD", "pinctrl-u300", "mmc0", "mmci"), - PIN_MAP("SPI", "pinctrl-u300", "spi0", "pl022"), - PIN_MAP("UART0", "pinctrl-u300", "uart0", "uart0"), + PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "mmc0", "mmci"), + PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "spi0", "pl022"), + PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-u300", "uart0", "uart0"), }; struct u300_mux_hog {@@ -1607,7 +1607,7 @@ static int __init u300_pinctrl_fetch(void) struct pinctrl *p; int ret; - p = pinctrl_get(u300_mux_hogs[i].dev, NULL); + p = pinctrl_get(u300_mux_hogs[i].dev, PINCTRL_STATE_DEFAULT); if (IS_ERR(p)) { pr_err("u300: could not get pinmux hog %s\n", u300_mux_hogs[i].name);diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c index f25307b..6af6d8d 100644 --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c@@ -461,8 +461,8 @@ static struct pinctrl *pinctrl_get_locked(struct device *dev, const char *name) int i; struct pinctrl_map const *map; - /* We must have a dev name */ - if (WARN_ON(!dev)) + /* We must have both a dev and state name */ + if (WARN_ON(!dev || !name)) return ERR_PTR(-EINVAL); devname = dev_name(dev);@@ -504,16 +504,9 @@ static struct pinctrl *pinctrl_get_locked(struct device *dev, const char *name) if (strcmp(map->dev_name, devname)) continue; - /* - * If we're looking for a specific named map, this must match, - * else we loop and look for the next. - */ - if (name != NULL) { - if (map->name == NULL) - continue; - if (strcmp(map->name, name)) - continue; - } + /* State name must be the one we're looking for */ + if (strcmp(map->name, name)) + continue; ret = pinmux_apply_muxmap(pctldev, p, dev, devname, map); if (ret) {diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c index c1a871e..3cabb65 100644 --- a/drivers/tty/serial/sirfsoc_uart.c +++ b/drivers/tty/serial/sirfsoc_uart.c@@ -673,7 +673,7 @@ int sirfsoc_uart_probe(struct platform_device *pdev) port->irq = res->start; if (sirfport->hw_flow_ctrl) { - sirfport->p = pinctrl_get(&pdev->dev, NULL); + sirfport->p = pinctrl_get(&pdev->dev, PINCTRL_STATE_DEFAULT); ret = IS_ERR(sirfport->p); if (ret) goto pin_err;-- 1.7.0.4