On 2/12/21 9:43 AM, Linus Walleij wrote:
On Thu, Feb 4, 2021 at 11:05 PM Marek Vasut <marex@xxxxxxx> wrote:
On 2/4/21 10:10 PM, Doug Anderson wrote:
+ /*
+ * Reset the chip, pull EN line low for t_reset=10ms,
+ * then high for t_en=1ms.
+ */
+ gpiod_set_value(ctx->enable_gpio, 0);
Why not use the "cansleep" version to give some flexibility?
Does that make a difference in non-interrupt context ?
As I understand it:
* If a client calls gpiod_set_value() then the underlying GPIO can
only be one that doesn't sleep.
* If a client calls gpiod_set_value_cansleep() then the underlying
GPIO can be either one that does or doesn't sleep.
* A client is only allowed to call gpiod_set_value_cansleep() if it's
not in interrupt context.
You are definitely not in an interrupt context (right?), so calling
the "cansleep" version has no downsides but allows board designers to
hook up an enable that can sleep.
Linus, can you please confirm this ? I find this hard to believe, since
there are plenty of places in the kernel which use gpiod_set_value()
without the _cansleep, are those problematic then ?
The function looks like so:
void gpiod_set_value(struct gpio_desc *desc, int value)
{
VALIDATE_DESC_VOID(desc);
/* Should be using gpiod_set_value_cansleep() */
WARN_ON(desc->gdev->chip->can_sleep);
gpiod_set_value_nocheck(desc, value);
}
So if the chip has set ->can_sleep (i,e, this GPIO is on something
like an I2C bus) then a warning will appear.
The warning only really appear if you have a driver that was
previously only used on a gpiochip with "fast" (write a register)
GPIOs and somebody start to use the same driver with a
GPIO on e.g. an I2C bus, which will define ->can_sleep.
The simple solution to the warning is to switch to the
_cansleep() variant but really it is a sign to check that
this is not being called in atomic context and just check
that the driver overall will survive sleeping context
here.
In a way _cansleep() is just syntactic sugar.
In this driver, you can use _cansleep() if you like but not
using it mostly works too, if used with SoC-type GPIOs.
Whoever uses the driver with a GPIO on an I2C chip
or similar gets to fix it.
Nice, I'm gonna archive this, this explanation is very clear, thanks.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
