On Wed, Dec 8, 2021 at 11:26 AM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: > On 08/12/2021 01:30, Linus Walleij wrote: > > On Tue, Dec 7, 2021 at 11:14 PM Andy Shevchenko > > <andy.shevchenko@xxxxxxxxx> wrote: > >> On Monday, December 6, 2021, Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: > >>> Based on this discussion: > >>> > >>> https://lore.kernel.org/linux-gpio/CACRpkdb3q4-9O3dHS6QDWnZZ5JJjXWXS9KPvwXVaowLMRhcejA@xxxxxxxxxxxxxx/T/#t > >>> > >>> I propose this RFC series. > >> > >> > >> When I first saw your report I was thinking about actually adding a new callback ->set_direction_atomic() > >> and then make pinctrl use it, otherwise like you do, I.e. issue a warning when it’s called in atomic context > > > > The problem is inside of pinctrl core, not in any driver. It takes a > > mutex when going over > > the GPIO ranges. > > > > I suggested maybe just replacing these mutexes with spinlocks, or RCU. > > RCU or spinlock would most likely work as a replacement for pinctrldev_list_mutex, but > not for pctldev->mutex. I didn't see any obvious way of replacing it with something else. You can't get rid of locking even with RCU. AFAIR the locking protects "writer" side and it can well be mutex, doesn't really matter. The question about RCU is what we are actually _doing_ in the call we are talking about. If it's a simple _reader_ of some (shared) array of data which is not being updated during this traversing, then it's a very good fit for it. Otherwise other means should be considered. > I'm open to any suggestions, but for now this was the best I could come up with, given > my limited knowledge of the gpio/pinctrl subsystems. It at least warns you that something > is wrong. > > Personally I think that for combined gpio/pinctrl drivers it doesn't make sense to take > this additional loop through the pinctrl core, regardless of whatever locking mechanism > is used. I actually think that it obfuscates those drivers a bit, but that might just be > me. -- With Best Regards, Andy Shevchenko
