Re: [PATCH] at86rf230: Allow slow GPIO pins for "rstn"
Hello. On 21/12/16 19:30, Chris Healy wrote: On Dec 21, 2016 5:11 AM, "Stefan Schmidt"> wrote: Hello. On 19/12/16 00:25, Andrey Smirnov wrote: Driver code never touches "rstn" signal in atomic context, so there's no need to implicitly put such restriction on it by using gpio_set_value to manipulate it. Replace gpio_set_value to gpio_set_value_cansleep to fix that. We need to make sure we are not assuming it can be called in such a context in the future now. But that is something we can worry about if it comes up. As a an example of where such restriction might be inconvenient, consider a hardware design where "rstn" is connected to a pin of I2C/SPI GPIO expander chip. Is this a real life issue you run into? I have a platform with this configuration. The DTS for the platform is in the process of being mainlined right now. Thanks for letting us know. What platform is that? I'm always interested in hearing about devices that use the Linux ieee802154 subsystem. :) regards Stefan Schmidt
Re: [PATCH] at86rf230: Allow slow GPIO pins for "rstn"
Hello. On 21/12/16 19:30, Chris Healy wrote: On Dec 21, 2016 5:11 AM, "Stefan Schmidt" mailto:ste...@osg.samsung.com>> wrote: Hello. On 19/12/16 00:25, Andrey Smirnov wrote: Driver code never touches "rstn" signal in atomic context, so there's no need to implicitly put such restriction on it by using gpio_set_value to manipulate it. Replace gpio_set_value to gpio_set_value_cansleep to fix that. We need to make sure we are not assuming it can be called in such a context in the future now. But that is something we can worry about if it comes up. As a an example of where such restriction might be inconvenient, consider a hardware design where "rstn" is connected to a pin of I2C/SPI GPIO expander chip. Is this a real life issue you run into? I have a platform with this configuration. The DTS for the platform is in the process of being mainlined right now. Thanks for letting us know. What platform is that? I'm always interested in hearing about devices that use the Linux ieee802154 subsystem. :) regards Stefan Schmidt
Re: [PATCH] at86rf230: Allow slow GPIO pins for "rstn"
Hello. On 19/12/16 00:25, Andrey Smirnov wrote: Driver code never touches "rstn" signal in atomic context, so there's no need to implicitly put such restriction on it by using gpio_set_value to manipulate it. Replace gpio_set_value to gpio_set_value_cansleep to fix that. We need to make sure we are not assuming it can be called in such a context in the future now. But that is something we can worry about if it comes up. As a an example of where such restriction might be inconvenient, consider a hardware design where "rstn" is connected to a pin of I2C/SPI GPIO expander chip. Is this a real life issue you run into? Cc: Chris HealySigned-off-by: Andrey Smirnov --- drivers/net/ieee802154/at86rf230.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c index 9f10da6..7700690 100644 --- a/drivers/net/ieee802154/at86rf230.c +++ b/drivers/net/ieee802154/at86rf230.c @@ -1710,9 +1710,9 @@ static int at86rf230_probe(struct spi_device *spi) /* Reset */ if (gpio_is_valid(rstn)) { udelay(1); - gpio_set_value(rstn, 0); + gpio_set_value_cansleep(rstn, 0); udelay(1); - gpio_set_value(rstn, 1); + gpio_set_value_cansleep(rstn, 1); usleep_range(120, 240); } Acked-by: Stefan Schmidt regards Stefan Schmidt
Re: [PATCH] at86rf230: Allow slow GPIO pins for "rstn"
Hello. On 19/12/16 00:25, Andrey Smirnov wrote: Driver code never touches "rstn" signal in atomic context, so there's no need to implicitly put such restriction on it by using gpio_set_value to manipulate it. Replace gpio_set_value to gpio_set_value_cansleep to fix that. We need to make sure we are not assuming it can be called in such a context in the future now. But that is something we can worry about if it comes up. As a an example of where such restriction might be inconvenient, consider a hardware design where "rstn" is connected to a pin of I2C/SPI GPIO expander chip. Is this a real life issue you run into? Cc: Chris Healy Signed-off-by: Andrey Smirnov --- drivers/net/ieee802154/at86rf230.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c index 9f10da6..7700690 100644 --- a/drivers/net/ieee802154/at86rf230.c +++ b/drivers/net/ieee802154/at86rf230.c @@ -1710,9 +1710,9 @@ static int at86rf230_probe(struct spi_device *spi) /* Reset */ if (gpio_is_valid(rstn)) { udelay(1); - gpio_set_value(rstn, 0); + gpio_set_value_cansleep(rstn, 0); udelay(1); - gpio_set_value(rstn, 1); + gpio_set_value_cansleep(rstn, 1); usleep_range(120, 240); } Acked-by: Stefan Schmidt regards Stefan Schmidt
[PATCH] at86rf230: Allow slow GPIO pins for "rstn"
Driver code never touches "rstn" signal in atomic context, so there's no need to implicitly put such restriction on it by using gpio_set_value to manipulate it. Replace gpio_set_value to gpio_set_value_cansleep to fix that. As a an example of where such restriction might be inconvenient, consider a hardware design where "rstn" is connected to a pin of I2C/SPI GPIO expander chip. Cc: Chris HealySigned-off-by: Andrey Smirnov --- drivers/net/ieee802154/at86rf230.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c index 9f10da6..7700690 100644 --- a/drivers/net/ieee802154/at86rf230.c +++ b/drivers/net/ieee802154/at86rf230.c @@ -1710,9 +1710,9 @@ static int at86rf230_probe(struct spi_device *spi) /* Reset */ if (gpio_is_valid(rstn)) { udelay(1); - gpio_set_value(rstn, 0); + gpio_set_value_cansleep(rstn, 0); udelay(1); - gpio_set_value(rstn, 1); + gpio_set_value_cansleep(rstn, 1); usleep_range(120, 240); } -- 2.5.5
[PATCH] at86rf230: Allow slow GPIO pins for "rstn"
Driver code never touches "rstn" signal in atomic context, so there's no need to implicitly put such restriction on it by using gpio_set_value to manipulate it. Replace gpio_set_value to gpio_set_value_cansleep to fix that. As a an example of where such restriction might be inconvenient, consider a hardware design where "rstn" is connected to a pin of I2C/SPI GPIO expander chip. Cc: Chris Healy Signed-off-by: Andrey Smirnov --- drivers/net/ieee802154/at86rf230.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c index 9f10da6..7700690 100644 --- a/drivers/net/ieee802154/at86rf230.c +++ b/drivers/net/ieee802154/at86rf230.c @@ -1710,9 +1710,9 @@ static int at86rf230_probe(struct spi_device *spi) /* Reset */ if (gpio_is_valid(rstn)) { udelay(1); - gpio_set_value(rstn, 0); + gpio_set_value_cansleep(rstn, 0); udelay(1); - gpio_set_value(rstn, 1); + gpio_set_value_cansleep(rstn, 1); usleep_range(120, 240); } -- 2.5.5