[PATCH can] can: dev: can_set_termination(): allow sleeping GPIOs

Marc Kleine-Budde posted 1 patch 1 year, 2 months ago
drivers/net/can/dev/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH can] can: dev: can_set_termination(): allow sleeping GPIOs
Posted by Marc Kleine-Budde 1 year, 2 months ago
In commit 6e86a1543c37 ("can: dev: provide optional GPIO based
termination support") GPIO based termination support was added.

For no particular reason that patch uses gpiod_set_value() to set the
GPIO. This leads to the following warning, if the systems uses a
sleeping GPIO, i.e. behind an I2C port expander:

| WARNING: CPU: 0 PID: 379 at /drivers/gpio/gpiolib.c:3496 gpiod_set_value+0x50/0x6c
| CPU: 0 UID: 0 PID: 379 Comm: ip Not tainted 6.11.0-20241016-1 #1 823affae360cc91126e4d316d7a614a8bf86236c

Replace gpiod_set_value() by gpiod_set_value_cansleep() to allow
sleeping GPIO.

Reported-by: Leonard Göhrs <l.goehrs@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/dev/dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 6792c14fd7eb009d551ac22bab1f0ee2cd0f0398..681643ab37804e9904cc4a899d44c55cefab7b6e 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -468,7 +468,7 @@ static int can_set_termination(struct net_device *ndev, u16 term)
 	else
 		set = 0;
 
-	gpiod_set_value(priv->termination_gpio, set);
+	gpiod_set_value_cansleep(priv->termination_gpio, set);
 
 	return 0;
 }

---
base-commit: 66418447d27b7f4c027587582a133dd0bc0a663b
change-id: 20241121-dev-fix-can_set_termination-f1a8412b22c5

Best regards,
-- 
Marc Kleine-Budde <mkl@pengutronix.de>


Re: [PATCH can] can: dev: can_set_termination(): allow sleeping GPIOs
Posted by Leonard Göhrs 1 year, 2 months ago

On 21.11.24 13:12, Marc Kleine-Budde wrote:
> In commit 6e86a1543c37 ("can: dev: provide optional GPIO based
> termination support") GPIO based termination support was added.
> 
> For no particular reason that patch uses gpiod_set_value() to set the
> GPIO. This leads to the following warning, if the systems uses a
> sleeping GPIO, i.e. behind an I2C port expander:
> 
> | WARNING: CPU: 0 PID: 379 at /drivers/gpio/gpiolib.c:3496 gpiod_set_value+0x50/0x6c
> | CPU: 0 UID: 0 PID: 379 Comm: ip Not tainted 6.11.0-20241016-1 #1 823affae360cc91126e4d316d7a614a8bf86236c
> 
> Replace gpiod_set_value() by gpiod_set_value_cansleep() to allow
> sleeping GPIO.
> 
> Reported-by: Leonard Göhrs <l.goehrs@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

I can't say if using `_cansleep` here is allowed or if it
would break any assumptions of the calling code.

But what I *can* say is that the patch gets rid of the
warning message I have seen before when toggling the
termination resistor. Thanks!

Tested-by: Leonard Göhrs <l.goehrs@pengutronix.de>

> ---
>   drivers/net/can/dev/dev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
> index 6792c14fd7eb009d551ac22bab1f0ee2cd0f0398..681643ab37804e9904cc4a899d44c55cefab7b6e 100644
> --- a/drivers/net/can/dev/dev.c
> +++ b/drivers/net/can/dev/dev.c
> @@ -468,7 +468,7 @@ static int can_set_termination(struct net_device *ndev, u16 term)
>   	else
>   		set = 0;
>   
> -	gpiod_set_value(priv->termination_gpio, set);
> +	gpiod_set_value_cansleep(priv->termination_gpio, set);
>   
>   	return 0;
>   }
> 
> ---
> base-commit: 66418447d27b7f4c027587582a133dd0bc0a663b
> change-id: 20241121-dev-fix-can_set_termination-f1a8412b22c5
> 
> Best regards,