[PATCH] staging: r8188eu: make rtw_deinit_intf_priv return void

[PATCH] staging: r8188eu: make rtw_deinit_intf_priv return void

[Pavel Skripkin]

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

Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
index e002070f7fba..37694aa96d13 100644
--- a/drivers/staging/r8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
@@ -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)
-- 
2.32.0

Re: [PATCH] staging: r8188eu: make rtw_deinit_intf_priv return void

[Martin Kaiser]

Thus wrote Pavel Skripkin (paskripkin@gmail.com):

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

> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
>  drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)

> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> index e002070f7fba..37694aa96d13 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> @@ -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)
> -- 
> 2.32.0

Acked-by: Martin Kaiser <martin@kaiser.cx>

Re: [PATCH] staging: r8188eu: make rtw_deinit_intf_priv return void

[Michael Straube]

On 8/23/21 8:40 PM, Pavel Skripkin wrote:
> rtw_deinit_intf_priv() always return success, so there is no need in
> return value
> 
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
>   drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> index e002070f7fba..37694aa96d13 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> @@ -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)
> 

Looks good to me, thanks.

Acked-by: Michael Straube <straube.linux@gmail.com>

Re: [PATCH] staging: r8188eu: make rtw_deinit_intf_priv return void

[Phillip Potter]

On Mon, 23 Aug 2021 at 19:41, Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> rtw_deinit_intf_priv() always return success, so there is no need in
> return value
>
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
>  drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> index e002070f7fba..37694aa96d13 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> @@ -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)
> --
> 2.32.0
>

Dear Pavel,

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

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH] staging: r8188eu: make rtw_deinit_intf_priv return void

[Pavel Skripkin]

On 8/24/21 12:20 AM, Phillip Potter wrote:
> On Mon, 23 Aug 2021 at 19:41, Pavel Skripkin <paskripkin@gmail.com> wrote:
>>
>> rtw_deinit_intf_priv() always return success, so there is no need in
>> return value
>>
>> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
>> ---
>>  drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
>> index e002070f7fba..37694aa96d13 100644
>> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
>> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
>> @@ -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)
>> --
>> 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

Re: [PATCH] staging: r8188eu: make rtw_deinit_intf_priv return void

[Phillip Potter]

On Mon, 23 Aug 2021 at 22:29, Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> On 8/24/21 12:20 AM, Phillip Potter wrote:
> > On Mon, 23 Aug 2021 at 19:41, Pavel Skripkin <paskripkin@gmail.com> wrote:
> >>
> >> rtw_deinit_intf_priv() always return success, so there is no need in
> >> return value
> >>
> >> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> >> ---
> >>  drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
> >>  1 file changed, 1 insertion(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> >> index e002070f7fba..37694aa96d13 100644
> >> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> >> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> >> @@ -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)
> >> --
> >> 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

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

Re: [PATCH] staging: r8188eu: make rtw_deinit_intf_priv return void

[Phillip Potter]

On Mon, 23 Aug 2021 at 22:29, Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> On 8/24/21 12:20 AM, Phillip Potter wrote:
> > On Mon, 23 Aug 2021 at 19:41, Pavel Skripkin <paskripkin@gmail.com> wrote:
> >>
> >> rtw_deinit_intf_priv() always return success, so there is no need in
> >> return value
> >>
> >> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> >> ---
> >>  drivers/staging/r8188eu/os_dep/usb_intf.c | 5 +----
> >>  1 file changed, 1 insertion(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> >> index e002070f7fba..37694aa96d13 100644
> >> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> >> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> >> @@ -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)
> >> --
> >> 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

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