Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Jia-Ju Bai]

On 2018/4/11 22:26, David Miller wrote:

From: Jia-Ju Bai 
Date: Wed, 11 Apr 2018 16:20:22 +0800


Okay, I now know why many of my patches were not replied.

Many of your patches are not responded to because you handle patch
feedback poorly sometimes.


Okay, thanks for pointing it out.
I will handle patch feedback much more carefully.


Also, all of your networking submissions have been dropped because
the net-next tree is closed.



Okay, I will choose the proper tree to submit.


Best wishes,
Jia-Ju Bai

Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[David Miller]

From: Jia-Ju Bai 
Date: Wed, 11 Apr 2018 16:20:22 +0800

> Okay, I now know why many of my patches were not replied.

Many of your patches are not responded to because you handle patch
feedback poorly sometimes.

Also, all of your networking submissions have been dropped because
the net-next tree is closed.

Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Jia-Ju Bai]

On 2018/4/11 16:17, Greg KH wrote:

On Wed, Apr 11, 2018 at 04:11:00PM +0800, Jia-Ju Bai wrote:


On 2018/4/11 16:03, Greg KH wrote:

On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:

On 2018/4/11 14:41, Greg KH wrote:

On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:

stir421x_fw_upload() is never called in atomic context.

The call chain ending up at stir421x_fw_upload() is:
[1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()

irda_usb_probe() is set as ".probe" in struct usb_driver.
This function is not called in atomic context.

Despite never getting called from atomic context, stir421x_fw_upload()
calls mdelay() to busily wait.
This is not necessary and can be replaced with usleep_range() 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 
---
drivers/staging/irda/drivers/irda-usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Please, at the very least, work off of Linus's tree.  There is no
drivers/staging/irda/ anymore :)


Okay, sorry.
Could you please recommend me a right tree or its git address?

Have you looked in the MAINTAINERS file?  Worst case, always use
linux-next.

greg k-h

Oh, sorry, I did notice the git tree in the MAINTAINERS file.
I always used linux-stable.

linux-stable is almost never the tree to use as it is almost always
12000 patches behind what is in Linus's tree and about 2 changes
behind linux-next.



Okay, I now know why many of my patches were not replied.
I should choose correct tree in the MAINTAINERS file.
Thanks :)


Best wishes,
Jia-Ju Bai

Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Greg KH]

On Wed, Apr 11, 2018 at 04:11:00PM +0800, Jia-Ju Bai wrote:
> 
> 
> On 2018/4/11 16:03, Greg KH wrote:
> > On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
> > > 
> > > On 2018/4/11 14:41, Greg KH wrote:
> > > > On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> > > > > stir421x_fw_upload() is never called in atomic context.
> > > > > 
> > > > > The call chain ending up at stir421x_fw_upload() is:
> > > > > [1] stir421x_fw_upload() <- stir421x_patch_device() <- 
> > > > > irda_usb_probe()
> > > > > 
> > > > > irda_usb_probe() is set as ".probe" in struct usb_driver.
> > > > > This function is not called in atomic context.
> > > > > 
> > > > > Despite never getting called from atomic context, stir421x_fw_upload()
> > > > > calls mdelay() to busily wait.
> > > > > This is not necessary and can be replaced with usleep_range() 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 
> > > > > ---
> > > > >drivers/staging/irda/drivers/irda-usb.c | 2 +-
> > > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > > Please, at the very least, work off of Linus's tree.  There is no
> > > > drivers/staging/irda/ anymore :)
> > > > 
> > > Okay, sorry.
> > > Could you please recommend me a right tree or its git address?
> > Have you looked in the MAINTAINERS file?  Worst case, always use
> > linux-next.
> > 
> > greg k-h
> 
> Oh, sorry, I did notice the git tree in the MAINTAINERS file.
> I always used linux-stable.

linux-stable is almost never the tree to use as it is almost always
12000 patches behind what is in Linus's tree and about 2 changes
behind linux-next.

thanks,

greg k-h

Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Jia-Ju Bai]

On 2018/4/11 16:03, Greg KH wrote:

On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:


On 2018/4/11 14:41, Greg KH wrote:

On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:

stir421x_fw_upload() is never called in atomic context.

The call chain ending up at stir421x_fw_upload() is:
[1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()

irda_usb_probe() is set as ".probe" in struct usb_driver.
This function is not called in atomic context.

Despite never getting called from atomic context, stir421x_fw_upload()
calls mdelay() to busily wait.
This is not necessary and can be replaced with usleep_range() 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 
---
   drivers/staging/irda/drivers/irda-usb.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

Please, at the very least, work off of Linus's tree.  There is no
drivers/staging/irda/ anymore :)


Okay, sorry.
Could you please recommend me a right tree or its git address?

Have you looked in the MAINTAINERS file?  Worst case, always use
linux-next.

greg k-h


Oh, sorry, I did notice the git tree in the MAINTAINERS file.
I always used linux-stable.
Thanks for telling me this :)


Best wishes,
Jia-Ju Bai

Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Greg KH]

On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
> 
> 
> On 2018/4/11 14:41, Greg KH wrote:
> > On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> > > stir421x_fw_upload() is never called in atomic context.
> > > 
> > > The call chain ending up at stir421x_fw_upload() is:
> > > [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
> > > 
> > > irda_usb_probe() is set as ".probe" in struct usb_driver.
> > > This function is not called in atomic context.
> > > 
> > > Despite never getting called from atomic context, stir421x_fw_upload()
> > > calls mdelay() to busily wait.
> > > This is not necessary and can be replaced with usleep_range() 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 
> > > ---
> > >   drivers/staging/irda/drivers/irda-usb.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > Please, at the very least, work off of Linus's tree.  There is no
> > drivers/staging/irda/ anymore :)
> > 
> 
> Okay, sorry.
> Could you please recommend me a right tree or its git address?

Have you looked in the MAINTAINERS file?  Worst case, always use
linux-next.

greg k-h

Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Jia-Ju Bai]

On 2018/4/11 14:41, Greg KH wrote:

On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:

stir421x_fw_upload() is never called in atomic context.

The call chain ending up at stir421x_fw_upload() is:
[1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()

irda_usb_probe() is set as ".probe" in struct usb_driver.
This function is not called in atomic context.

Despite never getting called from atomic context, stir421x_fw_upload()
calls mdelay() to busily wait.
This is not necessary and can be replaced with usleep_range() 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 
---
  drivers/staging/irda/drivers/irda-usb.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Please, at the very least, work off of Linus's tree.  There is no
drivers/staging/irda/ anymore :)



Okay, sorry.
Could you please recommend me a right tree or its git address?


Best wishes,
Jia-Ju Bai

Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Greg KH]

On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> stir421x_fw_upload() is never called in atomic context.
> 
> The call chain ending up at stir421x_fw_upload() is:
> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
> 
> irda_usb_probe() is set as ".probe" in struct usb_driver.
> This function is not called in atomic context.
> 
> Despite never getting called from atomic context, stir421x_fw_upload()
> calls mdelay() to busily wait.
> This is not necessary and can be replaced with usleep_range() 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 
> ---
>  drivers/staging/irda/drivers/irda-usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Please, at the very least, work off of Linus's tree.  There is no
drivers/staging/irda/ anymore :)

thanks,

greg k-h

Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Johan Hovold]

On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> stir421x_fw_upload() is never called in atomic context.
> 
> The call chain ending up at stir421x_fw_upload() is:
> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
> 
> irda_usb_probe() is set as ".probe" in struct usb_driver.
> This function is not called in atomic context.
> 
> Despite never getting called from atomic context, stir421x_fw_upload()
> calls mdelay() to busily wait.
> This is not necessary and can be replaced with usleep_range() 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 

This all looks good (also note the call to usb_control_msg(), which may
sleep, just above the mdelay()).

Reviewed-by: Johan Hovold 

Thanks,
Johan

[PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

[Jia-Ju Bai]

stir421x_fw_upload() is never called in atomic context.

The call chain ending up at stir421x_fw_upload() is:
[1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()

irda_usb_probe() is set as ".probe" in struct usb_driver.
This function is not called in atomic context.

Despite never getting called from atomic context, stir421x_fw_upload()
calls mdelay() to busily wait.
This is not necessary and can be replaced with usleep_range() 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 
---
 drivers/staging/irda/drivers/irda-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/irda/drivers/irda-usb.c 
b/drivers/staging/irda/drivers/irda-usb.c
index 723e49b..c6c8c2c 100644
--- a/drivers/staging/irda/drivers/irda-usb.c
+++ b/drivers/staging/irda/drivers/irda-usb.c
@@ -1050,7 +1050,7 @@ static int stir421x_fw_upload(struct irda_usb_cb *self,
if (ret < 0)
break;
 
-   mdelay(10);
+   usleep_range(1, 11000);
}
 
kfree(patch_block);
-- 
1.9.1