2021-11-04 08:57:23

by Jiasheng Jiang

[permalink] [raw]
Subject: [PATCH] NFC: nfcmrvl: add unanchor after anchor

In the error path, the anchored urb is unanchored.
But in the successful path, the anchored urb is not.
Therefore, it might be better to add unanchor().

Fixes: f26e30c ("NFC: nfcmrvl: Initial commit for Marvell NFC driver")
Signed-off-by: Jiasheng Jiang <[email protected]>
---
drivers/nfc/nfcmrvl/usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/nfcmrvl/usb.c b/drivers/nfc/nfcmrvl/usb.c
index bcd563c..f8ae517 100644
--- a/drivers/nfc/nfcmrvl/usb.c
+++ b/drivers/nfc/nfcmrvl/usb.c
@@ -146,9 +146,9 @@ nfcmrvl_submit_bulk_urb(struct nfcmrvl_usb_drv_data *drv_data, gfp_t mem_flags)
if (err != -EPERM && err != -ENODEV)
nfc_err(&drv_data->udev->dev,
"urb %p submission failed (%d)\n", urb, -err);
- usb_unanchor_urb(urb);
}

+ usb_unanchor_urb(urb);
usb_free_urb(urb);

return err;
--
2.7.4


2021-11-04 11:31:49

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] NFC: nfcmrvl: add unanchor after anchor

On 04/11/2021 09:55, Jiasheng Jiang wrote:
> In the error path, the anchored urb is unanchored.
> But in the successful path, the anchored urb is not.
> Therefore, it might be better to add unanchor().
>
> Fixes: f26e30c ("NFC: nfcmrvl: Initial commit for Marvell NFC driver")
> Signed-off-by: Jiasheng Jiang <[email protected]>
> ---
> drivers/nfc/nfcmrvl/usb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/nfc/nfcmrvl/usb.c b/drivers/nfc/nfcmrvl/usb.c
> index bcd563c..f8ae517 100644
> --- a/drivers/nfc/nfcmrvl/usb.c
> +++ b/drivers/nfc/nfcmrvl/usb.c
> @@ -146,9 +146,9 @@ nfcmrvl_submit_bulk_urb(struct nfcmrvl_usb_drv_data *drv_data, gfp_t mem_flags)
> if (err != -EPERM && err != -ENODEV)
> nfc_err(&drv_data->udev->dev,
> "urb %p submission failed (%d)\n", urb, -err);
> - usb_unanchor_urb(urb);
> }
>
> + usb_unanchor_urb(urb);
> usb_free_urb(urb);
>
> return err;
>

Why only this place in the driver? And why only this driver - some
others miss it as well (e.g. btusb which was probably the template for
this one). Are you sure it is a correct patch? Did you test it?

Best regards,
Krzysztof

2021-11-06 10:10:09

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH] NFC: nfcmrvl: add unanchor after anchor

On Thu, 4 Nov 2021 08:55:41 +0000 Jiasheng Jiang wrote:
> In the error path, the anchored urb is unanchored.
> But in the successful path, the anchored urb is not.
> Therefore, it might be better to add unanchor().
>
> Fixes: f26e30c ("NFC: nfcmrvl: Initial commit for Marvell NFC driver")

Apart from answering Krzysztof's comment please also mend the Fixes tag.
The hash should be at least 12 digits long.