Re: gpiod_set_value(): BUG: sleeping function called from invalid context

[Hans Verkuil <hverkuil@xxxxxxxxx>]

On 05/12/2021 15:49, Petko Manolov wrote:
> On 21-12-05 12:59:46, Hans Verkuil wrote:
>>
> 
> <snip>
> 
>> I'm not sure if the pctldev->mutex can be replaced with rcu, I suspect not.
> 
> Not in this case. :)
> 
>> For something that's supposed to be atomic, there seem to be an awful lot of 
>> mutexes...
> 
> By looking at the generic gpiolib code, it seems that none of these are supposed 
> to be called from atomic context.
> 
>> In the case of the bcm2711 the bcm2835_gpio_direction_output() function calls
>> pinctrl_gpio_direction_output, which in turn (via pinctrl_gpio_direction()
>> and pinmux_gpio_direction()) calls bcm2835_pmx_gpio_set_direction().
> 
> Is the aforementioned code mainlined or is it in some other git tree?  I'd like 
> to take a quick look.

Mainline.

> 
>> It appears to me that bcm2835_gpio_direction_output() can call
>> bcm2835_pmx_gpio_set_direction() directly, thus avoiding all the mutexes.
>> That's exactly what samsung/pinctrl-samsung.c and qcom/pinctrl-msm.c do,
>> from what I can tell.
>>
>> Are pinctrl drivers supposed to call pinctrl_gpio_direction_output() from the 
>> direction_output() op? Isn't that perhaps the bug?
> 
> If direction_output() is supposed to be called from atomic context, then that's 
> where the bug is.  Again, i am not familiar with this particular subsystem, but 
> these are pretty basic kernel concepts people must pay attention to.  You should 
> perhaps focus on Broadcom's code rather than pinctrl's.

That's my suspicion as well, except that the same thing can be seen in several
other non-Broadcom pinctrl drivers as well. Probably copy-and-paste.

Linus, can you confirm that it is indeed the Broadcom pinctrl driver that's wrong?
And thus several others as well, 'git grep pinctrl_gpio_direction_output drivers'
reports a bunch of them.

Regards,

	Hans