On Mon, Jun 27, 2022 at 08:12:38AM +0200, Greg KH wrote: >> On Sun, Jun 26, 2022 at 09:53:33PM -0700, Arun Vijayshankar wrote: >>> qlge_close uses msleep for 1ms, in which case (1 - 20ms) it is preferable >>> to use usleep_range in non-atomic contexts, as documented in >>> Documentation/timers/timers-howto.rst. A range of 1 - 1.05ms is >>> specified here, in case the device takes slightly longer than 1ms to recover from >>> reset. >>> >>> Signed-off-by: Arun Vijayshankar <arunvijayshankar@xxxxxxxxx> >>> --- >>> drivers/staging/qlge/qlge_main.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c >>> index 689a87d58f27..3cc4f1902c80 100644 >>> --- a/drivers/staging/qlge/qlge_main.c >>> +++ b/drivers/staging/qlge/qlge_main.c >>> @@ -3886,7 +3886,7 @@ static int qlge_close(struct net_device *ndev) >>> * (Rarely happens, but possible.) >>> */ >>> while (!test_bit(QL_ADAPTER_UP, &qdev->flags)) >>> - msleep(1); >>> + usleep_range(1000, 1050); >> Have you tested this with a device? Doing these types of changes >> without access to the hardware isn't a good idea. >> >> Also, a loop that has the chance to never end should be fixed up, don't >> you think? >> >> thanks, >> >> greg k-h > I have not tested the change with the hardware. I have checked that the module loads on my PC, but I do not have the actual adapter > to test it out. Would you mind sharing any pointers on testing changes on the actual device if I don't have the specific device? > I tried to find an emulator or virtual device for the QL adapter, but there doesn't seem to be any. Sorry if this is a very basic > question. This is my first patch ever. If there isn't a good way to test the changes with hardware, would you recommend withdrawing it? > > Thanks for pointing out the potential runaway loop. I'm a little embarrased on having missed it. I can add a timeout if the device doesn't > recover within a given time, but I was wondering if there is a better way of doing it? > > Best regards > Arun Vijayshankar
