Date:	Tue, 13 May 2014 13:53:50 +0800
From:	FanWu <fwu@...vell.com>
To:	Stephen Warren <swarren@...dotorg.org>
CC:	"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"swarren@...dia.com" <swarren@...dia.com>,
	Chao Xie <cxie4@...vell.com>, Yilu Mao <ylmao@...vell.com>,
	Ning Jiang <njiang1@...vell.com>,
	Xiaofan Tian <tianxf@...vell.com>,
	Fangsuo Wu <fswu@...vell.com>
Subject: Re: [Pinctrl] A suggestion to avoid duplicated enabling operation
 on a pin's setting

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@...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