On Tue, 9 Nov 2021 at 10:34, Andy Shevchenko [off-list ref] wrote:

On Tue, Nov 9, 2021 at 11:21 AM Emil Renner Berthing [off-list ref] wrote:

quoted

On Tue, 9 Nov 2021 at 02:01, Linus Walleij [off-list ref] wrote:

quoted

On Tue, Nov 2, 2021 at 9:08 PM Andy Shevchenko
[off-list ref] wrote:

...

quoted

quoted

quoted

Linus any comments on this code (sorry if I missed your reply)? The
idea behind above is to skip all settings from the same category and
apply only the last one, e.g. if we have "bias set to X", ..., "bias
disable", ..., "bias set to Y", the hardware will see only the last
operation, i.e. "bias set to Y". I think it may not be the best
approach (theoretically?) since the hardware definitely may behave
differently on the other side in case of such series of the
configurations (yes, I have seen some interesting implementations of
the touchpad / touchscreen GPIOs that may be affected).

That sounds weird. I think we need to look at how other drivers
deal with this.

To me it seems more natural that
starfive_padctl_rmw(sfp, group->pins[i], mask, value);
would get called at the end of each iteration of the
for (i = 0; i < num_configs; i++) loop.

That would work, but when the loop is done the end result would be
exactly the same.

It seems we interpret the term "result" differently. The result when
we talking about GPIOs is the series of pin state changes incl.
configuration. This is how it should be recognized when programming
hardware.

quoted

 The only difference is that the above would rapidly
"blink" the different states during the loop until it arrives at the
result. This would certainly be different, but it can never be the
intended behaviour and only a side-effect on how the pinctrl framework
works.

Is it? That's what I'm trying to get an answer to. If you may
guarantee this (the keywords "intended behaviour" and "side effect"),
I wouldn't object.

quoted

The order the different states are blinked depends entirely on
how the pinctrl framework parses the device tree. I still think it
would be more natural to cleanly go to the end result without this
blinking.

Hmm.. but if going through the different states is what you want, then
wouldn't you need the device tree to have an ordered list of the
states rather than just a single node and also a way to tune how long
time the different states are blinked?