diff mbox series

can: dev: can_set_termination(): Allow gpio sleep

Message ID 20241121150209.125772-1-nb@tipi-net.de (mailing list archive)
State New
Delegated to: Netdev Maintainers
Headers show
Series can: dev: can_set_termination(): Allow gpio sleep | expand

Checks

Context Check Description
netdev/tree_selection success Series ignored based on subject

Commit Message

Nicolai Buchwitz Nov. 21, 2024, 3:02 p.m. UTC
The current implementation of can_set_termination() sets the GPIO in a
context which cannot sleep. This is an issue if the GPIO controller can
sleep (e.g. since the concerning GPIO expander is connected via SPI or
I2C). Thus, if the termination resistor is set (eg. with ip link),
a warning splat will be issued in the kernel log.

Fix this by setting the termination resistor with
gpiod_set_value_cansleep() which instead of gpiod_set_value() allows it to
sleep.

Cc: stable@vger.kernel.org
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/can/dev/dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Marc Kleine-Budde Nov. 21, 2024, 3:12 p.m. UTC | #1
Hello Nicolai,

thanks for your contribution!

On 21.11.2024 16:02:09, Nicolai Buchwitz wrote:
> The current implementation of can_set_termination() sets the GPIO in a
> context which cannot sleep. This is an issue if the GPIO controller can
> sleep (e.g. since the concerning GPIO expander is connected via SPI or
> I2C). Thus, if the termination resistor is set (eg. with ip link),
> a warning splat will be issued in the kernel log.
> 
> Fix this by setting the termination resistor with
> gpiod_set_value_cansleep() which instead of gpiod_set_value() allows it to
> sleep.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>

I've send the same patch a few hours ago:

https://lore.kernel.org/all/20241121-dev-fix-can_set_termination-v1-1-41fa6e29216d@pengutronix.de/

Marc
Lino Sanfilippo Nov. 21, 2024, 3:17 p.m. UTC | #2
Hi,

On 21.11.24 16:12, Marc Kleine-Budde wrote:
> Hello Nicolai,
> 
> thanks for your contribution!
> 
> On 21.11.2024 16:02:09, Nicolai Buchwitz wrote:
>> The current implementation of can_set_termination() sets the GPIO in a
>> context which cannot sleep. This is an issue if the GPIO controller can
>> sleep (e.g. since the concerning GPIO expander is connected via SPI or
>> I2C). Thus, if the termination resistor is set (eg. with ip link),
>> a warning splat will be issued in the kernel log.
>>
>> Fix this by setting the termination resistor with
>> gpiod_set_value_cansleep() which instead of gpiod_set_value() allows it to
>> sleep.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> 
> I've send the same patch a few hours ago:
> 
> https://lore.kernel.org/all/20241121-dev-fix-can_set_termination-v1-1-41fa6e29216d@pengutronix.de/
> 
> Marc
> 

Shouldnt this also go to stable?

Regards,
Lino
Marc Kleine-Budde Nov. 21, 2024, 3:25 p.m. UTC | #3
On 21.11.2024 16:17:53, Lino Sanfilippo wrote:
> > On 21.11.2024 16:02:09, Nicolai Buchwitz wrote:
> >> The current implementation of can_set_termination() sets the GPIO in a
> >> context which cannot sleep. This is an issue if the GPIO controller can
> >> sleep (e.g. since the concerning GPIO expander is connected via SPI or
> >> I2C). Thus, if the termination resistor is set (eg. with ip link),
> >> a warning splat will be issued in the kernel log.
> >>
> >> Fix this by setting the termination resistor with
> >> gpiod_set_value_cansleep() which instead of gpiod_set_value() allows it to
> >> sleep.
> >>
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> > 
> > I've send the same patch a few hours ago:
> > 
> > https://lore.kernel.org/all/20241121-dev-fix-can_set_termination-v1-1-41fa6e29216d@pengutronix.de/
> > 
> Shouldnt this also go to stable?

Until today no-one complained about the problem, but I'll add stable on
Cc while applying the patch.

regards,
Marc
Lino Sanfilippo Nov. 21, 2024, 3:30 p.m. UTC | #4
On 21.11.24 16:25, Marc Kleine-Budde wrote:
> On 21.11.2024 16:17:53, Lino Sanfilippo wrote:
>>> On 21.11.2024 16:02:09, Nicolai Buchwitz wrote:
>>>> The current implementation of can_set_termination() sets the GPIO in a
>>>> context which cannot sleep. This is an issue if the GPIO controller can
>>>> sleep (e.g. since the concerning GPIO expander is connected via SPI or
>>>> I2C). Thus, if the termination resistor is set (eg. with ip link),
>>>> a warning splat will be issued in the kernel log.
>>>>
>>>> Fix this by setting the termination resistor with
>>>> gpiod_set_value_cansleep() which instead of gpiod_set_value() allows it to
>>>> sleep.
>>>>
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
>>>
>>> I've send the same patch a few hours ago:
>>>
>>> https://lore.kernel.org/all/20241121-dev-fix-can_set_termination-v1-1-41fa6e29216d@pengutronix.de/
>>>
>> Shouldnt this also go to stable?
> 
> Until today no-one complained about the problem, but I'll add stable on
> Cc while applying the patch.
> 
> regards,
> Marc
> 

Great, thanks!

BR,
Lino
diff mbox series

Patch

diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 6792c14fd7eb..681643ab3780 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;
 }