On 2018/4/12 0:19, Florian Fainelli wrote:
On 04/11/2018 12:14 AM, Jia-Ju Bai wrote:

On 2018/4/11 13:30, Phil Reid wrote:
On 11/04/2018 09:51, Jia-Ju Bai wrote:
b53_switch_reset_gpio() is never called in atomic context.

The call chain ending up at b53_switch_reset_gpio() is:
[1] b53_switch_reset_gpio() <- b53_switch_reset() <-
     b53_reset_switch() <- b53_setup()

b53_switch_reset_gpio() is set as ".setup" in struct dsa_switch_ops.
This function is not called in atomic context.

Despite never getting called from atomic context,
calls mdelay() to busily wait.
This is not necessary and can be replaced with msleep() to
avoid busy waiting.

This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
   drivers/net/dsa/b53/b53_common.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c
index 274f367..e070ff6 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -597,10 +597,10 @@ static void b53_switch_reset_gpio(struct
b53_device *dev)
       /* Reset sequence: RESET low(50ms)->high(20ms)
       gpio_set_value(gpio, 0);
-    mdelay(50);
+    msleep(50);
         gpio_set_value(gpio, 1);
-    mdelay(20);
+    msleep(20);
         dev->current_page = 0xff;

Would that also imply gpio_set_value could be gpio_set_value_cansleep?

Yes, I think gpio_set_value_cansleep() is okay here?
Do I need to send a V2 patch to replace gpio_set_value()?
Yes, I would lump these two changes in the same patch since this is
effectively about solving sleeping vs. non sleeping operations.

Okay, I have sent a V2 patch, and you can have a look :)

Best wishes,
Jia-Ju Bai

Reply via email to