| 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: Fri, 19 Oct 2012 16:51:20 +0200
From: Jean-Nicolas GRAUX <jean-nicolas.graux@...ricsson.com>
To: Linus WALLEIJ <linus.walleij@...ricsson.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
Stephen Warren <swarren@...dia.com>,
Anmar Oueja <anmar.oueja@...aro.org>,
Linus Walleij <linus.walleij@...aro.org>,
Patrice CHOTARD <patrice.chotard@...com>,
Loic PALLARDY <loic.pallardy@...com>
Subject: Re: [PATCH v2] pinctrl: reserve pins when states are activated
Tested-by: Jean Nicolas Graux <jean-nicolas.graux@...ricsson.com>
Le 10/19/2012 03:05 PM, Linus WALLEIJ a écrit :
> From: Linus Walleij<linus.walleij@...aro.org>
>
> This switches the way that pins are reserved for multiplexing:
>
> We used to do this when the map was parsed, at the creation of
> the settings inside the pinctrl handle, in pinmux_map_to_setting().
>
> However this does not work for us, because we want to use the
> same set of pins with different devices at different times: the
> current code assumes that the pin groups in a pinmux state will
> only be used with one single device, albeit different groups can
> be active at different times. For example if a single I2C driver
> block is used to drive two different busses located on two
> pin groups A and B, then the pins for all possible states of a
> function are reserved when fetching the pinctrl handle: the
> I2C bus can choose either set A or set B by a mux state at
> runtime, but all pins in both group A and B (the superset) are
> effectively reserved for that I2C function and mapped to the
> device. Another device can never get in and use the pins in
> group A, even if the device/function is using group B at the
> moment.
>
> Instead: let use reserve the pins when the state is activated
> and drop them when the state is disabled, i.e. when we move to
> another state. This way different devices/functions can use the
> same pins at different times.
>
> We know that this is an odd way of doing things, but we really
> need to switch e.g. an SD-card slot to become a tracing output
> sink at runtime: we plug in a special "tracing card" then mux
> the pins that used to be an SD slot around to the tracing
> unit and push out tracing data there instead of SD-card
> traffic.
>
> As a side effect pinmux_free_setting() is unused and gets
> deleted.
>
> Cc: Patrice Chotard<patrice.chotard@...com>
> Cc: Jean Nicolas Graux<jean-nicolas.graux@...ricsson.com>
> Cc: Loic Pallardy<loic.pallardy@...com>
> Signed-off-by: Linus Walleij<linus.walleij@...aro.org>
> ---
> ChangeLog v1->v2:
> - The code was already accounting for the case where the setting
> was not active and called pinmux_disable_setting()
> from the core, so skip this and delete the now empty
> pinmux_free_setting() altogether.
> ---
> Documentation/pinctrl.txt | 4 ++-
> drivers/pinctrl/core.c | 3 +-
> drivers/pinctrl/core.h | 2 ++
> drivers/pinctrl/pinmux.c | 70 ++++++++++++++---------------------------------
> drivers/pinctrl/pinmux.h | 5 ----
> 5 files changed, 28 insertions(+), 56 deletions(-)
>
> diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt
> index 3b4ee53..a1cd2f9 100644
> --- a/Documentation/pinctrl.txt
> +++ b/Documentation/pinctrl.txt
> @@ -1193,4 +1193,6 @@ foo_switch()
> ...
> }
>
> -The above has to be done from process context.
> +The above has to be done from process context. The reservation of the pins
> +will be done when the state is activated, so in effect one specific pin
> +can be used by different functions at different times on a running system.
> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
> index 0f1ec9e..bbd930e 100644
> --- a/drivers/pinctrl/core.c
> +++ b/drivers/pinctrl/core.c
> @@ -563,6 +563,8 @@ static int add_setting(struct pinctrl *p, struct pinctrl_map const *map)
> return -EPROBE_DEFER;
> }
>
> + setting->dev_name = map->dev_name;
> +
> switch (map->type) {
> case PIN_MAP_TYPE_MUX_GROUP:
> ret = pinmux_map_to_setting(map, setting);
> @@ -689,7 +691,6 @@ static void pinctrl_put_locked(struct pinctrl *p, bool inlist)
> case PIN_MAP_TYPE_MUX_GROUP:
> if (state == p->state)
> pinmux_disable_setting(setting);
> - pinmux_free_setting(setting);
> break;
> case PIN_MAP_TYPE_CONFIGS_PIN:
> case PIN_MAP_TYPE_CONFIGS_GROUP:
> diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
> index 1f40ff6..12f5694 100644
> --- a/drivers/pinctrl/core.h
> +++ b/drivers/pinctrl/core.h
> @@ -105,12 +105,14 @@ struct pinctrl_setting_configs {
> * @type: the type of setting
> * @pctldev: pin control device handling to be programmed. Not used for
> * PIN_MAP_TYPE_DUMMY_STATE.
> + * @dev_name: the name of the device using this state
> * @data: Data specific to the setting type
> */
> struct pinctrl_setting {
> struct list_head node;
> enum pinctrl_map_type type;
> struct pinctrl_dev *pctldev;
> + const char *dev_name;
> union {
> struct pinctrl_setting_mux mux;
> struct pinctrl_setting_configs configs;
> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> index 9301a7a..0ecdf54 100644
> --- a/drivers/pinctrl/pinmux.c
> +++ b/drivers/pinctrl/pinmux.c
> @@ -314,14 +314,11 @@ int pinmux_map_to_setting(struct pinctrl_map const *map,
> {
> struct pinctrl_dev *pctldev = setting->pctldev;
> const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
> - const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
> char const * const *groups;
> unsigned num_groups;
> int ret;
> const char *group;
> int i;
> - const unsigned *pins;
> - unsigned num_pins;
>
> if (!pmxops) {
> dev_err(pctldev->dev, "does not support mux function\n");
> @@ -376,55 +373,9 @@ int pinmux_map_to_setting(struct pinctrl_map const *map,
> }
> setting->data.mux.group = ret;
>
> - ret = pctlops->get_group_pins(pctldev, setting->data.mux.group, &pins,
> - &num_pins);
> - if (ret) {
> - dev_err(pctldev->dev,
> - "could not get pins for device %s group selector %d\n",
> - pinctrl_dev_get_name(pctldev), setting->data.mux.group);
> - return -ENODEV;
> - }
> -
> - /* Try to allocate all pins in this group, one by one */
> - for (i = 0; i < num_pins; i++) {
> - ret = pin_request(pctldev, pins[i], map->dev_name, NULL);
> - if (ret) {
> - dev_err(pctldev->dev,
> - "could not request pin %d on device %s\n",
> - pins[i], pinctrl_dev_get_name(pctldev));
> - /* On error release all taken pins */
> - i--; /* this pin just failed */
> - for (; i >= 0; i--)
> - pin_free(pctldev, pins[i], NULL);
> - return -ENODEV;
> - }
> - }
> -
> return 0;
> }
>
> -void pinmux_free_setting(struct pinctrl_setting const *setting)
> -{
> - struct pinctrl_dev *pctldev = setting->pctldev;
> - const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
> - const unsigned *pins;
> - unsigned num_pins;
> - int ret;
> - int i;
> -
> - ret = pctlops->get_group_pins(pctldev, setting->data.mux.group,
> - &pins, &num_pins);
> - if (ret) {
> - dev_err(pctldev->dev,
> - "could not get pins for device %s group selector %d\n",
> - pinctrl_dev_get_name(pctldev), setting->data.mux.group);
> - return;
> - }
> -
> - for (i = 0; i < num_pins; i++)
> - pin_free(pctldev, pins[i], NULL);
> -}
> -
> int pinmux_enable_setting(struct pinctrl_setting const *setting)
> {
> struct pinctrl_dev *pctldev = setting->pctldev;
> @@ -446,6 +397,22 @@ int pinmux_enable_setting(struct pinctrl_setting const *setting)
> num_pins = 0;
> }
>
> + /* Try to allocate all pins in this group, one by one */
> + for (i = 0; i < num_pins; i++) {
> + ret = pin_request(pctldev, pins[i], setting->dev_name, NULL);
> + if (ret) {
> + dev_err(pctldev->dev,
> + "could not request pin %d on device %s\n",
> + pins[i], pinctrl_dev_get_name(pctldev));
> + /* On error release all taken pins */
> + i--; /* this pin just failed */
> + for (; i >= 0; i--)
> + pin_free(pctldev, pins[i], NULL);
> + return -ENODEV;
> + }
> + }
> +
> + /* Now that we have acquired the pins, encode the mux setting */
> for (i = 0; i < num_pins; i++) {
> desc = pin_desc_get(pctldev, pins[i]);
> if (desc == NULL) {
> @@ -482,6 +449,7 @@ void pinmux_disable_setting(struct pinctrl_setting const *setting)
> num_pins = 0;
> }
>
> + /* Flag the descs that no setting is active */
> for (i = 0; i < num_pins; i++) {
> desc = pin_desc_get(pctldev, pins[i]);
> if (desc == NULL) {
> @@ -493,6 +461,10 @@ void pinmux_disable_setting(struct pinctrl_setting const *setting)
> desc->mux_setting = NULL;
> }
>
> + /* And release the pins */
> + for (i = 0; i < num_pins; i++)
> + pin_free(pctldev, pins[i], NULL);
> +
> if (ops->disable)
> ops->disable(pctldev, setting->data.mux.func, setting->data.mux.group);
> }
> diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
> index d1a98b1..3c2aafa 100644
> --- a/drivers/pinctrl/pinmux.h
> +++ b/drivers/pinctrl/pinmux.h
> @@ -27,7 +27,6 @@ int pinmux_gpio_direction(struct pinctrl_dev *pctldev,
>
> int pinmux_map_to_setting(struct pinctrl_map const *map,
> struct pinctrl_setting *setting);
> -void pinmux_free_setting(struct pinctrl_setting const *setting);
> int pinmux_enable_setting(struct pinctrl_setting const *setting);
> void pinmux_disable_setting(struct pinctrl_setting const *setting);
>
> @@ -69,10 +68,6 @@ static inline int pinmux_map_to_setting(struct pinctrl_map const *map,
> return 0;
> }
>
> -static inline void pinmux_free_setting(struct pinctrl_setting const *setting)
> -{
> -}
> -
> static inline int pinmux_enable_setting(struct pinctrl_setting const *setting)
> {
> return 0;
--
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