| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20120228093552.GA9987@shlinux2.ap.freescale.net>
Date: Tue, 28 Feb 2012 17:35:53 +0800
From: Dong Aisheng <aisheng.dong@...escale.com>
To: Stephen Warren <swarren@...dia.com>
CC: Linus Walleij <linus.walleij@...ricsson.com>,
Linus Walleij <linus.walleij@...aro.org>,
<B29396@...escale.com>, <s.hauer@...gutronix.de>,
<dongas86@...il.com>, <shawn.guo@...aro.org>,
<thomas.abraham@...aro.org>, <tony@...mide.com>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2 2/2] pinctrl: Assume map table entries can't have a
NULL name field
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 <swarren@...dia.com>
> ---
> 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 <dong.aisheng@...aro.org>
Regards
Dong Aisheng
> 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
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists