| 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
| ||
|
Date: Thu, 1 Mar 2012 16:46:18 +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 5/6] pinctrl: Enhance mapping table to support pin
config operations
On Tue, Feb 28, 2012 at 01:27:31PM -0700, Stephen Warren wrote:
> The pinctrl mapping table can now contain entries to:
> * Set the mux function of a pin group
> * Apply a set of pin config options to a pin or a group
>
> This allows pinctrl_select_state() to apply pin configs settings as well
> as mux settings.
>
> Signed-off-by: Stephen Warren <swarren@...dia.com>
> ---
....
> +Finally, some devices expect the mapping table to contain certain specific
> +named states. When running on hardware that doesn't need any pin controller
> +configuration, the mapping table must still contain those named states, in
> +order to explicitly indicate that the states were provided and intended to
> +be empty. Table entry macro PIN_MAP_DUMMY_STATE serves the purpose of defining
> +a named state without causing any pin controller to be programmed:
> +
> +static struct pinctrl_map __initdata mapping[] = {
> + PIN_MAP_DUMMY_STATE("foo-i2c.0", PINCTRL_STATE_DEFAULT),
> };
Is this used for shared devices between different platforms which may need pinmux
setting while another not?
>
> -PIN_MAP_SYS_HOG("pinctrl-foo", "power_func")
> +PIN_MAP_MUX_GROUP_HOG("pinctrl-foo", NULL /* group */, "power_func")
Missed state name or should be PIN_MAP_MUX_GROUP_HOG_DEFAULT?
>
> This gives the exact same result as the above construction.
>
> diff --git a/arch/arm/mach-u300/core.c b/arch/arm/mach-u300/core.c
> index 18973ca..c965bb5 100644
> --- a/arch/arm/mach-u300/core.c
> +++ b/arch/arm/mach-u300/core.c
> @@ -1569,13 +1569,13 @@ static struct platform_device dma_device = {
> /* Pinmux settings */
> static struct pinctrl_map __initdata u300_pinmux_map[] = {
> /* anonymous maps for chip power and EMIFs */
> - PIN_MAP_SYS_HOG("pinctrl-u300", "power"),
> - PIN_MAP_SYS_HOG("pinctrl-u300", "emif0"),
> - PIN_MAP_SYS_HOG("pinctrl-u300", "emif1"),
> + PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "power"),
> + PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif0"),
> + PIN_MAP_MUX_GROUP_HOG_DEFAULT("pinctrl-u300", NULL, "emif1"),
> /* per-device maps for MMC/SD, SPI and UART */
> - 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"),
> + PIN_MAP_MUX_GROUP_DEFAULT("mmci", "pinctrl-u300", NULL, "mmc0"),
> + PIN_MAP_MUX_GROUP_DEFAULT("pl022", "pinctrl-u300", NULL, "spi0"),
> + PIN_MAP_MUX_GROUP_DEFAULT("uart0", "pinctrl-u300", NULL, "uart0"),
Although not big issue, but i'm a bit more intended to the original way that
for the NULL group name using:
PIN_MAP_MUX_DEFAULT("mmci", "pinctrl-u300", "mmc0"),
Or using PIN_MAP_MUX_GROUP_DEFAULT, but just totally remove the optional NULL
group name support to keep consistence and avoid confusing.
> diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
> index 05d25c8..b572153 100644
> --- a/include/linux/pinctrl/machine.h
> +++ b/include/linux/pinctrl/machine.h
> @@ -14,6 +14,41 @@
>
> #include "pinctrl.h"
>
> +enum pinctrl_map_type {
> + PIN_MAP_TYPE_INVALID,
> + PIN_MAP_TYPE_DUMMY_STATE,
> + PIN_MAP_TYPE_MUX_GROUP,
> + PIN_MAP_TYPE_CONFIGS_PIN,
> + PIN_MAP_TYPE_CONFIGS_GROUP,
Basically it causes a bit confusing to me that we have per pin config
but we do not have per pin mux.
However, i still have not got a better idea for this issue.
> +#define PIN_MAP_MUX_CONFIGS_GROUP(dev, state, pinctrl, grp, cfgs) \
Actually this macro is only for pin config setting,
maybe PIN_MAP_CONFIGS_GROUP is better, right?
> + { \
> + .dev_name = dev, \
> + .name = state, \
> + .type = PIN_MAP_TYPE_CONFIGS_GROUP, \
> + .ctrl_dev_name = pinctrl, \
We have three entries above duplicated with MUX macro.
> + .data.configs = { \
> + .group_or_pin = grp, \
> + .configs = cfgs, \
> + .num_configs = ARRAY_SIZE(cfgs), \
> + }, \
> + }
It seems you separate the pin mux setting and pin config setting in
different maps.
Can we merge them into one map?
That will save a lot of map entries and improve searching performance.
Another question is that:
the current IMX pinmux setting for non-dt is like:
#define MX51_I2C_PAD_CTRL (PAD_CTL_SRE_FAST | PAD_CTL_ODE | \
PAD_CTL_DSE_HIGH | PAD_CTL_PUS_100K_UP | \
#define MX51_PAD_KEY_COL4__I2C2_SCL \
IOMUX_PAD(0x65c, 0x26c, 0x13, 0x9b8, 1, MX51_I2C_PAD_CTRL)
This macro includes both mux and config setting.
It's very easy and conveniently to use.
So i'd really like to see a similar using after migrate to pinctrl subsystem
that using a macro to set both.
One possible working method i thought is extended macro based on your code:
#define PIN_MAP_MUX_CONFIG_GROUP(dev, state, pinctrl, grp, func, cfgs) \
PIN_MAP_MUX_GROUP(dev, state, pinctrl, grp, func), \
PIN_MAP_CONFIG_GROUP(dev, state, pinctrl, grp, cfgs)
It works for group.
However, we can not also do this for per PIN config since PIN_MAP_MUX_*
is only for group.
Regards
Dong Aisheng
--
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