Skip to contentSkip to conversation

staging: r8188eu: make rtw_deinit_intf_priv return void

PATCH In review Pavel Skripkin 4 participants 7 messages Aug 23, 2021 Staging

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 staging: r8188eu: make rtw_deinit_intf_priv return void

rtw_deinit_intf_priv() always return success, so there is no need in return value

---
 drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/r8188eu/os_dep/usb_intf.c +1 -4
index e002070f7fba..37694aa96d13 100644
@@ -129,13 +129,10 @@ static u8 rtw_init_intf_priv(struct dvobj_priv *dvobj)
 	return rst;
 }
 
-static u8 rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
+static void rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
 {
-	u8 rst = _SUCCESS;
-
 	kfree(dvobj->usb_alloc_vendor_req_buf);
 	_rtw_mutex_free(&dvobj->usb_vendor_req_mutex);
-	return rst;
 }
 
 static struct dvobj_priv *usb_dvobj_init(struct usb_interface *usb_intf)
Signature · 1 line
2.32.0
Acked-by on v1 1/1
#1 lore raw

Thus wrote Pavel Skripkin ([email protected]):

Quoted 29 lines

rtw_deinit_intf_priv() always return success, so there is no need in return value

---
 drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/r8188eu/os_dep/usb_intf.c +1 -4
index e002070f7fba..37694aa96d13 100644
@@ -129,13 +129,10 @@ static u8 rtw_init_intf_priv(struct dvobj_priv *dvobj)
 	return rst;
 }
 
-static u8 rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
+static void rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
 {
-	u8 rst = _SUCCESS;
-
 	kfree(dvobj->usb_alloc_vendor_req_buf);
 	_rtw_mutex_free(&dvobj->usb_vendor_req_mutex);
-	return rst;
 }
 
 static struct dvobj_priv *usb_dvobj_init(struct usb_interface *usb_intf)
Signature · 1 line
2.32.0
Acked-by on v1 1/1
#2 lore raw
In reply to Pavel Skripkin
Quoted 18 lines · Pavel Skripkin

rtw_deinit_intf_priv() always return success, so there is no need in return value

---
  drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
  1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/r8188eu/os_dep/usb_intf.c +0 -1
index e002070f7fba..37694aa96d13 100644
@@ -129,13 +129,10 @@ static u8 rtw_init_intf_priv(struct dvobj_priv *dvobj)
  	return rst;
  }
 
-static u8 rtw_deinit_intf_priv(struct dvobj_priv *dvobj)

drivers/staging/r8188eu/os_dep/usb_intf.c static u8 rtw_init_intf_priv(struct dvobj_priv *dvobj) · line 138

Quoted 10 lines
drivers/staging/r8188eu/os_dep/usb_intf.c +1 -3
+static void rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
  {
-	u8 rst = _SUCCESS;
-
  	kfree(dvobj->usb_alloc_vendor_req_buf);
  	_rtw_mutex_free(&dvobj->usb_vendor_req_mutex);
-	return rst;
  }
 
  static struct dvobj_priv *usb_dvobj_init(struct usb_interface *usb_intf)

Looks good to me, thanks.

Acked-by on v1 1/1
#3 lore raw
In reply to Pavel Skripkin
Quoted 29 lines · Pavel Skripkin

rtw_deinit_intf_priv() always return success, so there is no need in return value

---
 drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/r8188eu/os_dep/usb_intf.c +1 -5
index e002070f7fba..37694aa96d13 100644
@@ -129,13 +129,10 @@ static u8 rtw_init_intf_priv(struct dvobj_priv *dvobj)
        return rst;
 }
 
-static u8 rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
+static void rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
 {
-       u8 rst = _SUCCESS;
-
        kfree(dvobj->usb_alloc_vendor_req_buf);
        _rtw_mutex_free(&dvobj->usb_vendor_req_mutex);
-       return rst;
 }
 
 static struct dvobj_priv *usb_dvobj_init(struct usb_interface *usb_intf)
--
Quoted 2 lines

2.32.0

Dear Pavel,

Looks good - going to test your RFC series now btw.

Regards,
Phil

#4 lore raw
Quoted 28 lines · Phillip Potter
Quoted 27 lines · Pavel Skripkin

rtw_deinit_intf_priv() always return success, so there is no need in return value

---
 drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/r8188eu/os_dep/usb_intf.c +1 -4
index e002070f7fba..37694aa96d13 100644
@@ -129,13 +129,10 @@ static u8 rtw_init_intf_priv(struct dvobj_priv *dvobj)
        return rst;
 }
 
-static u8 rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
+static void rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
 {
-       u8 rst = _SUCCESS;
-
        kfree(dvobj->usb_alloc_vendor_req_buf);
        _rtw_mutex_free(&dvobj->usb_vendor_req_mutex);
-       return rst;
 }
 
Quoted 9 lines
Quoted 4 lines
drivers/staging/r8188eu/os_dep/usb_intf.c +0 -1
 static struct dvobj_priv *usb_dvobj_init(struct usb_interface *usb_intf)
--

2.32.0

Dear Pavel,

Looks good - going to test your RFC series now btw.

Thank you, Phillip!

Testing this RFC is very important. If it's all ok with it, I am going to add proper error handling all across the driver code, based on read() errors :)

Btw, we also can add error handling for write() operations, but I think it's not _very_ important, since driver won't misbehave in case of write failures

With regards,
Pavel Skripkin

#5 lore raw
Quoted 47 lines · Pavel Skripkin
Quoted 28 lines · Phillip Potter
Quoted 27 lines · Pavel Skripkin

rtw_deinit_intf_priv() always return success, so there is no need in return value

---
 drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/r8188eu/os_dep/usb_intf.c +1 -4
index e002070f7fba..37694aa96d13 100644
@@ -129,13 +129,10 @@ static u8 rtw_init_intf_priv(struct dvobj_priv *dvobj)
        return rst;
 }
 
-static u8 rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
+static void rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
 {
-       u8 rst = _SUCCESS;
-
        kfree(dvobj->usb_alloc_vendor_req_buf);
        _rtw_mutex_free(&dvobj->usb_vendor_req_mutex);
-       return rst;
 }
 
Quoted 9 lines
Quoted 4 lines
drivers/staging/r8188eu/os_dep/usb_intf.c +0 -1
 static struct dvobj_priv *usb_dvobj_init(struct usb_interface *usb_intf)
--

2.32.0

Dear Pavel,

Looks good - going to test your RFC series now btw.

Thank you, Phillip!

Testing this RFC is very important. If it's all ok with it, I am going to add proper error handling all across the driver code, based on read() errors :)

Quoted 8 lines

Btw, we also can add error handling for write() operations, but I think it's not _very_ important, since driver won't misbehave in case of write failures

With regards,
Pavel Skripkin

Dear Pavel,

Happy to help :-)

Sorry to report, but your RFC series generates an OOPS on boot for me, in usb_read32. Just doing a stack trace decode to figure out where failure is and I will report back.

Regards,
Phil

#6 lore raw
In reply to Pavel Skripkin
Quoted 47 lines · Pavel Skripkin
Quoted 28 lines · Phillip Potter
Quoted 27 lines · Pavel Skripkin

rtw_deinit_intf_priv() always return success, so there is no need in return value

---
 drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/r8188eu/os_dep/usb_intf.c +1 -4
index e002070f7fba..37694aa96d13 100644
@@ -129,13 +129,10 @@ static u8 rtw_init_intf_priv(struct dvobj_priv *dvobj)
        return rst;
 }
 
-static u8 rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
+static void rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
 {
-       u8 rst = _SUCCESS;
-
        kfree(dvobj->usb_alloc_vendor_req_buf);
        _rtw_mutex_free(&dvobj->usb_vendor_req_mutex);
-       return rst;
 }
 
Quoted 9 lines
Quoted 4 lines
drivers/staging/r8188eu/os_dep/usb_intf.c +0 -1
 static struct dvobj_priv *usb_dvobj_init(struct usb_interface *usb_intf)
--

2.32.0

Dear Pavel,

Looks good - going to test your RFC series now btw.

Thank you, Phillip!

Testing this RFC is very important. If it's all ok with it, I am going to add proper error handling all across the driver code, based on read() errors :)

Quoted 8 lines

Btw, we also can add error handling for write() operations, but I think it's not _very_ important, since driver won't misbehave in case of write failures

With regards,
Pavel Skripkin

Sorry, it also occurred to me after sending it would be better to have mentioned this as a reply to that patch series rather than to this e-mail. Apologies if I've confused anyone. Anyhow, I shall report my findings on that e-mail thread.

Regards,
Phil