On Tue, 9 Nov 2021 at 10:34, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > On Tue, Nov 9, 2021 at 11:21 AM Emil Renner Berthing <kernel@xxxxxxxx> wrote: > > On Tue, 9 Nov 2021 at 02:01, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > > On Tue, Nov 2, 2021 at 9:08 PM Andy Shevchenko > > > <andy.shevchenko@xxxxxxxxx> wrote: > > ... > > > > > 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. > > > 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. > > > 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?
