Re: [PATCH v3 12/16] pinctrl: starfive: Add pinctrl driver for StarFive SoCs
From: Andy Shevchenko <hidden>
Date: 2021-11-09 09:34:09
Also in:
linux-clk, linux-devicetree, linux-gpio, linux-riscv, lkml
On Tue, Nov 9, 2021 at 11:21 AM Emil Renner Berthing [off-list ref] wrote:
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
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.
-- With Best Regards, Andy Shevchenko