summaryrefslogtreecommitdiff
path: root/drivers/pinctrl/pinctrl-tz1090.c
diff options
context:
space:
mode:
authorFan Wu <fwu@marvell.com>2014-06-09 09:37:56 +0800
committerLinus Walleij <linus.walleij@linaro.org>2014-07-11 14:08:26 +0200
commit2243a87d90b42eb38bc281957df3e57c712b5e56 (patch)
tree8083e8a008c65b667de08e2603032e80516f4ecb /drivers/pinctrl/pinctrl-tz1090.c
parent607af165c0470e7297e1a04f528176222849b5a9 (diff)
pinctrl: avoid duplicated calling enable_pinmux_setting for a pin
What the patch does: 1. Call pinmux_disable_setting ahead of pinmux_enable_setting each time pinctrl_select_state is called 2. Remove the HW disable operation in pinmux_disable_setting function. 3. Remove the disable ops in struct pinmux_ops 4. Remove all the disable ops users in current code base. Notes: 1. Great thanks for the suggestion from Linus, Tony Lindgren and Stephen Warren and Everyone that shared comments on this patch. 2. The patch also includes comment fixes from Stephen Warren. The reason why we do this: 1. To avoid duplicated calling of the enable_setting operation without disabling operation inbetween which will let the pin descriptor desc->mux_usecount increase monotonously. 2. The HW pin disable operation is not useful for any of the existing platforms. And this can be used to avoid the HW glitch after using the item #1 modification. In the following case, the issue can be reproduced: 1. There is a driver that need to switch pin state dynamically, e.g. between "sleep" and "default" state 2. The pin setting configuration in a DTS node may be like this: component a { pinctrl-names = "default", "sleep"; pinctrl-0 = <&a_grp_setting &c_grp_setting>; pinctrl-1 = <&b_grp_setting &c_grp_setting>; } The "c_grp_setting" config node is totally identical, maybe like following one: c_grp_setting: c_grp_setting { pinctrl-single,pins = <GPIO48 AF6>; } 3. When switching the pin state in the following official pinctrl sequence: pin = pinctrl_get(); state = pinctrl_lookup_state(wanted_state); pinctrl_select_state(state); pinctrl_put(); Test Result: 1. The switch is completed as expected, that is: the device's pin configuration is changed according to the description in the "wanted_state" group setting 2. The "desc->mux_usecount" of the corresponding pins in "c_group" is increased without being decreased, because the "desc" is for each physical pin while the setting is for each setting node in the DTS. Thus, if the "c_grp_setting" in pinctrl-0 is not disabled ahead of enabling "c_grp_setting" in pinctrl-1, the desc->mux_usecount will keep increasing without any chance to be decreased. According to the comments in the original code, only the setting, in old state but not in new state, will be "disabled" (calling pinmux_disable_setting), which is correct logic but not intact. We still need consider case that the setting is in both old state and new state. We can do this in the following two ways: 1. Avoid to "enable"(calling pinmux_enable_setting) the "same pin setting" repeatedly 2. "Disable"(calling pinmux_disable_setting) the "same pin setting", actually two setting instances, ahead of enabling them. Analysis: 1. The solution #2 is better because it can avoid too much iteration. 2. If we disable all of the settings in the old state and one of the setting(s) exist in the new state, the pins mux function change may happen when some SoC vendors defined the "pinctrl-single,function-off" in their DTS file. old_setting => disabled_setting => new_setting. 3. In the pinmux framework, when a pin state is switched, the setting in the old state should be marked as "disabled". Conclusion: 1. To Remove the HW disabling operation to above the glitch mentioned above. 2. Handle the issue mentioned above by disabling all of the settings in old state and then enable the all of the settings in new state. Signed-off-by: Fan Wu <fwu@marvell.com> Acked-by: Stephen Warren <swarren@nvidia.com> Acked-by: Patrice Chotard <patrice.chotard@st.com> Acked-by: Heiko Stuebner <heiko@sntech.de> Acked-by: Maxime Coquelin <maxime.coquelin@st.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Diffstat (limited to 'drivers/pinctrl/pinctrl-tz1090.c')
-rw-r--r--drivers/pinctrl/pinctrl-tz1090.c58
1 files changed, 0 insertions, 58 deletions
diff --git a/drivers/pinctrl/pinctrl-tz1090.c b/drivers/pinctrl/pinctrl-tz1090.c
index bc9cd7a7602e..24082216842e 100644
--- a/drivers/pinctrl/pinctrl-tz1090.c
+++ b/drivers/pinctrl/pinctrl-tz1090.c
@@ -1479,63 +1479,6 @@ mux_pins:
}
/**
- * tz1090_pinctrl_disable() - Disable a function on a pin group.
- * @pctldev: Pin control data
- * @function: Function index to disable
- * @group: Group index to disable
- *
- * Disable a particular function on a group of pins. The per GPIO pin pseudo pin
- * groups can be used (in which case the pin will be taken out of peripheral
- * mode. Some convenience pin groups can also be used in which case the effect
- * is the same as enabling the function on each individual pin in the group.
- */
-static void tz1090_pinctrl_disable(struct pinctrl_dev *pctldev,
- unsigned int function, unsigned int group)
-{
- struct tz1090_pmx *pmx = pinctrl_dev_get_drvdata(pctldev);
- struct tz1090_pingroup *grp;
- unsigned int pin_num, mux_group, i, npins;
- const unsigned int *pins;
-
- /* group of pins? */
- if (group < ARRAY_SIZE(tz1090_groups)) {
- grp = &tz1090_groups[group];
- npins = grp->npins;
- pins = grp->pins;
- /*
- * All pins in the group must belong to the same mux group,
- * which allows us to just use the mux group of the first pin.
- * By explicitly listing permitted pingroups for each function
- * the pinmux core should ensure this is always the case.
- */
- } else {
- pin_num = group - ARRAY_SIZE(tz1090_groups);
- npins = 1;
- pins = &pin_num;
- }
- mux_group = tz1090_mux_pins[*pins];
-
- /* no mux group, but can still be individually muxed to peripheral */
- if (mux_group >= TZ1090_MUX_GROUP_MAX) {
- if (function == TZ1090_MUX_PERIP)
- goto unmux_pins;
- return;
- }
-
- /* mux group already set to a different function? */
- grp = &tz1090_mux_groups[mux_group];
- dev_dbg(pctldev->dev, "%s: unmuxing %u pin(s) in '%s' from '%s'\n",
- __func__, npins, grp->name, tz1090_functions[function].name);
-
- /* subtract pins from ref count and unmux individually */
- WARN_ON(grp->func_count < npins);
- grp->func_count -= npins;
-unmux_pins:
- for (i = 0; i < npins; ++i)
- tz1090_pinctrl_perip_select(pmx, pins[i], false);
-}
-
-/**
* tz1090_pinctrl_gpio_request_enable() - Put pin in GPIO mode.
* @pctldev: Pin control data
* @range: GPIO range
@@ -1575,7 +1518,6 @@ static struct pinmux_ops tz1090_pinmux_ops = {
.get_function_name = tz1090_pinctrl_get_func_name,
.get_function_groups = tz1090_pinctrl_get_func_groups,
.enable = tz1090_pinctrl_enable,
- .disable = tz1090_pinctrl_disable,
.gpio_request_enable = tz1090_pinctrl_gpio_request_enable,
.gpio_disable_free = tz1090_pinctrl_gpio_disable_free,
};