| 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, 5 Jun 2014 09:29:06 +0200
From: Maxime Coquelin <maxime.coquelin@...com>
To: <fwu@...vell.com>, <linus.walleij@...aro.org>,
<swarren@...dotorg.org>, <tony@...mide.com>,
<plagnioj@...osoft.com>, <kgene.kim@...sung.com>,
<heiko@...ech.de>, <t.figa@...sung.com>,
<thomas.abraham@...aro.org>, <srinivas.kandagatla@...il.com>,
<patrice.chotard@...com>, <thierry.reding@...il.com>,
<baohua@...nel.org>, <viresh.linux@...il.com>,
<matt.porter@...aro.org>, <syin@...adcom.com>,
<jg1.han@...sung.com>, <Rongjun.Ying@....com>, <Rong.Wang@....com>,
<linux@...sktech.co.nz>, <yongjun_wei@...ndmicro.com.cn>,
<laurent.navet@...il.com>
Cc: <linux-kernel@...r.kernel.org>, <swarren@...dia.com>,
<cxie4@...vell.com>, <ylmao@...vell.com>, <njiang1@...vell.com>,
<tianxf@...vell.com>, <fswu@...vell.com>
Subject: Re: [PATCH v5] pinctrl: to avoid duplicated calling enable_pinmux_setting
for a pin
Hi Fan,
On 06/05/2014 08:50 AM, fwu@...vell.com wrote:
> From: Fan Wu <fwu@...vell.com>
>
> What the patch did:
> 1.To call pinmux_disable_setting ahead of pinmux_enable_setting in each time of
> calling pinctrl_select_state
> 2.Remove the HW disable operation in 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.
> 2.The patch also includes comment fixes from Stephen Warren.
>
> The reason why to do this is that:
> 1.To avoid duplicated calling enable_setting operation without disabling
> operation which will let Pin's desc->mux_usecount keep being added.
> 2.The HW pin disable operation is not useful for most of the vendors' platform.
> 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 need to switch Pin state dynamicly, E.g. b/t "sleep" and
> "default" state
> 2)The Pin setting configuration in DTS node may be like the following one:
> 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 totaly same, maybe like following one:
> c_grp_setting: c_grp_setting {
> pinctrl-single,pins = <GPIO48 AF6>;
> MFP_DEFAULT;
> }
> 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 expectation, that is: component's
> Pins configuration are changed according to the description in the
> "wanted_state" group setting
> 2)The "desc->mux_usecount" of corresponding Pins in "c_group" is added 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 be kept added 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 "disable"(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 "enable"(calling pinmux_enable_setting) the Same Pins setting repeatedly.
> 2) "Disable"(calling pinmux_disable_setting) the "Same Pins setting", actually
> two setting instance, 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 setting in the old state and one/ones of the setting(s) is/are
> existed in the new state, the Pin's 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 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@...vell.com>
> ---
> drivers/pinctrl/core.c | 24 +++-----------
> drivers/pinctrl/pinctrl-abx500.c | 15 ---------
> drivers/pinctrl/pinctrl-adi2.c | 30 -----------------
> drivers/pinctrl/pinctrl-at91.c | 21 ------------
> drivers/pinctrl/pinctrl-bcm2835.c | 11 -------
> drivers/pinctrl/pinctrl-exynos5440.c | 8 -----
> drivers/pinctrl/pinctrl-msm.c | 25 --------------
> drivers/pinctrl/pinctrl-nomadik.c | 16 ---------
> drivers/pinctrl/pinctrl-rockchip.c | 18 ----------
> drivers/pinctrl/pinctrl-samsung.c | 8 -----
> drivers/pinctrl/pinctrl-single.c | 56 -------------------------------
> drivers/pinctrl/pinctrl-st.c | 6 ----
> drivers/pinctrl/pinctrl-tb10x.c | 17 ----------
> drivers/pinctrl/pinctrl-tegra.c | 19 -----------
> drivers/pinctrl/pinctrl-tz1090-pdc.c | 28 ----------------
> drivers/pinctrl/pinctrl-tz1090.c | 58 ---------------------------------
> drivers/pinctrl/pinctrl-u300.c | 14 --------
> drivers/pinctrl/pinmux.c | 4 ---
> drivers/pinctrl/sh-pfc/pinctrl.c | 22 -------------
> drivers/pinctrl/sirf/pinctrl-sirf.c | 10 ------
> drivers/pinctrl/spear/pinctrl-spear.c | 7 ----
> drivers/pinctrl/vt8500/pinctrl-wmt.c | 12 -------
> include/linux/pinctrl/pinmux.h | 2 --
> 23 files changed, 5 insertions(+), 426 deletions(-)
>
I would have preferred this patch to be split in multiple ones.
Anyway, if Linus W. agrees for one patch, that's fine for me too.
For the pinctrl-st changes, you can add my:
Acked-by: Maxime Coquelin <maxime.coquelin@...com>
Thanks,
Maxime
--
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