Re: [Pinctrl] A suggestion to avoid duplicated enabling operation on a pin's setting

From: FanWu
Date: Tue May 13 2014 - 01:54:28 EST


On 05/13/2014 04:20 AM, Stephen Warren wrote:
On 05/07/2014 02:02 AM, FanWu wrote:
...
I think the glitch you mentioned is indeed a problem for the vendors who
define the "pinctrl-single,function-off" which will be activated when
disabling setting.

"pinctrl-single,function-off" is one specific driver's way of
configuring what happens when a mux option is disabled. The issue
applies to all drivers that implement a mux option disable function.

...
I considered my previous option 1 of avoiding duplicated calling
enable_setting. If we want to avoid too much iteration in this solution,
some mark variables need to be introduced, which may make code more
complicated than the previous one.

Probably the best thing is to just remove the concept of "disable a mux
option". It doesn't make sense. All you can do with mux options is to
select some specific mux option. Disabling a mux option isn't something
that's logically possible. On some HW, perhaps you can do something
similar by tri-stating the pin or something, but that should be an
explicit part of the pinctrl state definition rather than an automatic
side-effect.

...
Maybe another topic:
In the original code, the Pin setting will be changed to the
disabled/safe state when Pin state is switched if the old setting is not
existed in new state rather than directly switched to the new Pin
setting. Also a possible glitch?

That's not a glitch, since it's not a temporary state. The mux setting
starts out as configured by the old state, then switches one time to
whatever the disable function sets it to. After that, it won't switch
again, unless there's an explicit SW action to enable a new state.


Dear, Stephen,

Thanks a lot for your reviewing!

I think the solution you mentioned about removing disable_pinmux function may introduce other possible code logic change, like change on enable_pinmux and "pinctrl-single,function-off" usage, which may be complicated thing. I cannot say whether that is a acceptable solution for Pinmux/pinctrl subsystem owner and user.

Thus, I think the patch I filed recently may be more easier way to solve this case.


About the glitch I mentioned before, I want to make myself clear.
If there is a case like the following one:
pinctrl-0 = <&a_grp_settingA>;
pinctrl-1 = <&a_grp_settingB>;
"a_grp_settingA" and "a_grp_settingB" are used to described the same Pin's different mux and function configuration
In my understanding,
When there is a need to switch Pin group state, the current code will disable "a_grp_settingA" first ahead of enabling "a_grp_settingB", right ?

Do you mean the case I mentioned will not be a glitch ?

If I got anything wrong, feel free to correct me.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/