[PATCH 0/2] rtl8723bs: clean up error handling

[PATCH 0/2] rtl8723bs: clean up error handling

[Omer El Idrissi]

This series cleans up error handling for function sdio_init()
and sdio_dvobj_init() by using conventional errno instead of
vendor-defined _SUCCESS/_FAIL macros when returning

Omer El Idrissi (2):
  staging: rtl8723bs: use direct returns in sdio_dvobj_init()
  staging: rtl8723bs: cleanup return in sdio_init()

 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 21 +++++++-------------
 1 file changed, 7 insertions(+), 14 deletions(-)

-- 
2.51.0

[PATCH 1/2] staging: rtl8723bs: use direct returns in sdio_dvobj_init()

[Omer El Idrissi]

Make sdio_dvobj_init() use direct returns

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index d664e254912c..358eac0837cf 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -155,13 +155,12 @@ static void sdio_deinit(struct dvobj_priv *dvobj)
 }
 static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 {
-	int status = _FAIL;
 	struct dvobj_priv *dvobj = NULL;
 	struct sdio_data *psdio;
 
 	dvobj = devobj_init();
 	if (!dvobj)
-		goto exit;
+		return NULL;
 
 	sdio_set_drvdata(func, dvobj);
 
@@ -172,18 +171,14 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 		goto free_dvobj;
 
 	rtw_reset_continual_io_error(dvobj);
-	status = _SUCCESS;
 
-free_dvobj:
-	if (status != _SUCCESS && dvobj) {
-		sdio_set_drvdata(func, NULL);
+	return dvobj;
 
-		devobj_deinit(dvobj);
+free_dvobj:
+	sdio_set_drvdata(func, NULL);
+	devobj_deinit(dvobj);
 
-		dvobj = NULL;
-	}
-exit:
-	return dvobj;
+	return NULL;
 }
 
 static void sdio_dvobj_deinit(struct sdio_func *func)
-- 
2.51.0

[PATCH 2/2] staging: rtl8723bs: cleanup return in sdio_init()

[Omer El Idrissi]

Make sdio_init() return errno from sdio_enable_func or
sdio_set_block_size instead of _SUCCESS/_FAIL vendor-defined
macros.

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 358eac0837cf..01b5d8a70072 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -131,9 +131,7 @@ static u32 sdio_init(struct dvobj_priv *dvobj)
 release:
 	sdio_release_host(func);
 
-	if (err)
-		return _FAIL;
-	return _SUCCESS;
+	return err;
 }
 
 static void sdio_deinit(struct dvobj_priv *dvobj)
-- 
2.51.0

Re: [PATCH 2/2] staging: rtl8723bs: cleanup return in sdio_init()

[Dan Carpenter]

On Tue, Mar 24, 2026 at 11:04:53PM +0100, Omer El Idrissi wrote:
> Make sdio_init() return errno from sdio_enable_func or
> sdio_set_block_size instead of _SUCCESS/_FAIL vendor-defined
> macros.
> 
> Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
> ---

You're going to need to start labeling your patches as v2 etc.
https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/

>  drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> index 358eac0837cf..01b5d8a70072 100644
> --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> @@ -131,9 +131,7 @@ static u32 sdio_init(struct dvobj_priv *dvobj)
>  release:
>  	sdio_release_host(func);
>  
> -	if (err)
> -		return _FAIL;
> -	return _SUCCESS;
> +	return err;

This patch isn't wrong, per se, but I'd really like for you to update
the callers as well like how you did in the earlier patch.  Right now,
they're still testing for _SUCCESS.

Make sdio_dvobj_init() propagate the error code back instead of -1.

Change rtw_resume_process_normal() to:

	ret = sdio_init();
	if (ret)
		goto whatever_and_return_NULL;

It will make the commit message a little bit more complicated.

    Make sdio_init() propagate standard kernel error codes instead
    of returning _SUCCESS/_FAIL.  There are two callers for this
    function.  sdio_dvobj_init() already returns negative values
    but the caller doesn't check for errors so changing this doesn't
    affect anything.  rtw_resume_process_normal() returns NULL on
    error so leave that as-is.

Sorry, for being a bit nit-picky on this but you might end up potentially
sending a lot of these patches.

regards,
dan carpenter

Re: [PATCH 2/2] staging: rtl8723bs: cleanup return in sdio_init()

[Omer]

I will make a v2 patch for this series.
My question is do I go back to returning error pointers for
sdio_dvobj_init? I did that in my original commit so that
the proper errno would be propagated back to the rtw_drv_init probe
function in sdio_intf.c.
sdio_dvobj_init function can fail due to both allocation issues and
sdio_init failure. So i thought it would be more
descriptive to propagate an error pointer instead of NULL for both cases.

Best,
Omer El Idrissi

On Wed, Mar 25, 2026 at 8:36 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Tue, Mar 24, 2026 at 11:04:53PM +0100, Omer El Idrissi wrote:
> > Make sdio_init() return errno from sdio_enable_func or
> > sdio_set_block_size instead of _SUCCESS/_FAIL vendor-defined
> > macros.
> >
> > Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
> > ---
>
> You're going to need to start labeling your patches as v2 etc.
> https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/
>
> >  drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > index 358eac0837cf..01b5d8a70072 100644
> > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > @@ -131,9 +131,7 @@ static u32 sdio_init(struct dvobj_priv *dvobj)
> >  release:
> >       sdio_release_host(func);
> >
> > -     if (err)
> > -             return _FAIL;
> > -     return _SUCCESS;
> > +     return err;
>
> This patch isn't wrong, per se, but I'd really like for you to update
> the callers as well like how you did in the earlier patch.  Right now,
> they're still testing for _SUCCESS.
>
> Make sdio_dvobj_init() propagate the error code back instead of -1.
>
> Change rtw_resume_process_normal() to:
>
>         ret = sdio_init();
>         if (ret)
>                 goto whatever_and_return_NULL;
>
> It will make the commit message a little bit more complicated.
>
>     Make sdio_init() propagate standard kernel error codes instead
>     of returning _SUCCESS/_FAIL.  There are two callers for this
>     function.  sdio_dvobj_init() already returns negative values
>     but the caller doesn't check for errors so changing this doesn't
>     affect anything.  rtw_resume_process_normal() returns NULL on
>     error so leave that as-is.
>
> Sorry, for being a bit nit-picky on this but you might end up potentially
> sending a lot of these patches.
>
> regards,
> dan carpenter

[PATCH v2 0/3] clean up error handling

[Omer El Idrissi]

Make sdio_init() propagate standard kernel error codes instead
of returning _SUCCESS/_FAIL.  There are two callers for this
function.  rtw_resume_process_normal() already returns negative values
but the caller doesn't check for errors so changing this doesn't
affect anything.  sdio_dvobj_init() returns NULL on
error so leave that as-is.

Omer El Idrissi (3):
  staging: rtl8723bs: use direct returns in sdio_dvobj_init()
  staging: rtl8723bs: cleanup return in sdio_init()
  staging: rtl8723bs: clean up callees for sdio_init function

 drivers/staging/rtl8723bs/os_dep/os_intfs.c  |  8 ++++----
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 21 +++++++-------------
 2 files changed, 11 insertions(+), 18 deletions(-)

-- 
2.51.0

[PATCH v2 1/3] staging: rtl8723bs: use direct returns in sdio_dvobj_init()

[Omer El Idrissi]

Make sdio_dvobj_init() use direct returns

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index d664e254912c..358eac0837cf 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -155,13 +155,12 @@ static void sdio_deinit(struct dvobj_priv *dvobj)
 }
 static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 {
-	int status = _FAIL;
 	struct dvobj_priv *dvobj = NULL;
 	struct sdio_data *psdio;
 
 	dvobj = devobj_init();
 	if (!dvobj)
-		goto exit;
+		return NULL;
 
 	sdio_set_drvdata(func, dvobj);
 
@@ -172,18 +171,14 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 		goto free_dvobj;
 
 	rtw_reset_continual_io_error(dvobj);
-	status = _SUCCESS;
 
-free_dvobj:
-	if (status != _SUCCESS && dvobj) {
-		sdio_set_drvdata(func, NULL);
+	return dvobj;
 
-		devobj_deinit(dvobj);
+free_dvobj:
+	sdio_set_drvdata(func, NULL);
+	devobj_deinit(dvobj);
 
-		dvobj = NULL;
-	}
-exit:
-	return dvobj;
+	return NULL;
 }
 
 static void sdio_dvobj_deinit(struct sdio_func *func)
-- 
2.51.0

[PATCH v2 2/3] staging: rtl8723bs: cleanup return in sdio_init()

[Omer El Idrissi]

Make sdio_init() return errno from sdio_enable_func or
sdio_set_block_size instead of _SUCCESS/_FAIL vendor-defined
macros.

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 358eac0837cf..01b5d8a70072 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -131,9 +131,7 @@ static u32 sdio_init(struct dvobj_priv *dvobj)
 release:
 	sdio_release_host(func);
 
-	if (err)
-		return _FAIL;
-	return _SUCCESS;
+	return err;
 }
 
 static void sdio_deinit(struct dvobj_priv *dvobj)
-- 
2.51.0

[PATCH v2 3/3] staging: rtl8723bs: clean up callees for sdio_init function

[Omer El Idrissi]

Let rtw_resume_process_normal return errno returned by sdio_init
instead of -1. sdio_dvobj_init returns NULL on error so leave that
as is.

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/os_intfs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
index 7ba689f2dfc8..e943dcea1a21 100644
--- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
+++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
@@ -1135,10 +1135,10 @@ static int rtw_resume_process_normal(struct adapter *padapter)
 	pwrpriv = adapter_to_pwrctl(padapter);
 	pmlmepriv = &padapter->mlmepriv;
 	/*  interface init */
-	/* if (sdio_init(adapter_to_dvobj(padapter)) != _SUCCESS) */
-	if ((padapter->intf_init) && (padapter->intf_init(adapter_to_dvobj(padapter)) != _SUCCESS)) {
-		ret = -1;
-		goto exit;
+	if (padapter->intf_init) {
+		ret = padapter->intf_init(adapter_to_dvobj(padapter));
+		if (ret)
+			goto exit;
 	}
 	rtw_hal_disable_interrupt(padapter);
 	/* if (sdio_alloc_irq(adapter_to_dvobj(padapter)) != _SUCCESS) */
-- 
2.51.0

Re: [PATCH 2/2] staging: rtl8723bs: cleanup return in sdio_init()

[Dan Carpenter]

On Wed, Mar 25, 2026 at 07:51:27PM +0100, Omer wrote:
> I will make a v2 patch for this series.
> My question is do I go back to returning error pointers for
> sdio_dvobj_init? I did that in my original commit so that
> the proper errno would be propagated back to the rtw_drv_init probe
> function in sdio_intf.c.

No, just keep it as return NULL.  But the check should be:

	ret = sdio_init(dvobj);
	if (ret) {

Because that's the standard way to do error handling.  It's slightly
more readable than the alternatives like:

	if (sdio_init(dvobj) < 0) {

> sdio_dvobj_init function can fail due to both allocation issues and
> sdio_init failure. So i thought it would be more
> descriptive to propagate an error pointer instead of NULL for both cases.

It's more descriptive but returning NULL is fine.  Every change
you make adds places where people can disagree with you.  We
want to change from _SUCCESS/_FAIL to standard error codes.  Fine
everyone agrees with that.  But then changing to error pointers?
Some people might agree and some might not.  And then it's like,
okay we have 99 functions which return NULL and 1 function which
returns error pointers.  And we get into a whole discussion about
it.  Forget it.  Just return NULL.

regards,
dan carpenter

[PATCH v3 0/4] clean up error handling

[Omer El Idrissi]

This series lets sdio_init() propagate standard kernel error codes 
instead of returning _SUCCESS/_FAIL.  There are two callers for this
function.  rtw_resume_process_normal() already returns negative values 
but the caller doesn't check for errors so changing this doesn't
affect anything.  sdio_dvobj_init() returns NULL on
error so leave that as-is.

Signed-off-by: Omer El Idrissi <omer.e.idriss@gmail.com>
---
v3:
- Add one commit that lets sdio_dvobj_init() use more readable error
  handling
- Add v3 and v2 tags

v2:
- Add one commit that lets rtw_resume_process_normal cleanly check
  errors and return errno instead of using vendor-defined slop

v1:
- Use direct returns in sdio_dvobj_init()
- Cleanup return in sdio_init()

Omer El Idrissi (4):
  staging: rtl8723bs: use direct returns in sdio_dvobj_init()
  staging: rtl8723bs: cleanup return in sdio_init()
  staging: rtl8723bs: clean up callees for sdio_init function
  staging: rtl8723bs: Choose slightly more readable error check

 drivers/staging/rtl8723bs/os_dep/os_intfs.c  |  8 +++----
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 25 ++++++++------------
 2 files changed, 14 insertions(+), 19 deletions(-)

-- 
2.51.0

[PATCH v3 1/4] staging: rtl8723bs: use direct returns in sdio_dvobj_init()

[Omer El Idrissi]

Make sdio_dvobj_init() use direct returns

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index d664e254912c..358eac0837cf 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -155,13 +155,12 @@ static void sdio_deinit(struct dvobj_priv *dvobj)
 }
 static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 {
-	int status = _FAIL;
 	struct dvobj_priv *dvobj = NULL;
 	struct sdio_data *psdio;
 
 	dvobj = devobj_init();
 	if (!dvobj)
-		goto exit;
+		return NULL;
 
 	sdio_set_drvdata(func, dvobj);
 
@@ -172,18 +171,14 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 		goto free_dvobj;
 
 	rtw_reset_continual_io_error(dvobj);
-	status = _SUCCESS;
 
-free_dvobj:
-	if (status != _SUCCESS && dvobj) {
-		sdio_set_drvdata(func, NULL);
+	return dvobj;
 
-		devobj_deinit(dvobj);
+free_dvobj:
+	sdio_set_drvdata(func, NULL);
+	devobj_deinit(dvobj);
 
-		dvobj = NULL;
-	}
-exit:
-	return dvobj;
+	return NULL;
 }
 
 static void sdio_dvobj_deinit(struct sdio_func *func)
-- 
2.51.0

[PATCH v3 2/4] staging: rtl8723bs: cleanup return in sdio_init()

[Omer El Idrissi]

Make sdio_init() return errno from sdio_enable_func or
sdio_set_block_size instead of _SUCCESS/_FAIL vendor-defined
macros.

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 358eac0837cf..01b5d8a70072 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -131,9 +131,7 @@ static u32 sdio_init(struct dvobj_priv *dvobj)
 release:
 	sdio_release_host(func);
 
-	if (err)
-		return _FAIL;
-	return _SUCCESS;
+	return err;
 }
 
 static void sdio_deinit(struct dvobj_priv *dvobj)
-- 
2.51.0

[PATCH v3 3/4] staging: rtl8723bs: clean up callees for sdio_init function

[Omer El Idrissi]

Let rtw_resume_process_normal return errno returned by sdio_init
instead of -1. sdio_dvobj_init returns NULL on error so leave that
as is.

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/os_intfs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
index 7ba689f2dfc8..e943dcea1a21 100644
--- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
+++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
@@ -1135,10 +1135,10 @@ static int rtw_resume_process_normal(struct adapter *padapter)
 	pwrpriv = adapter_to_pwrctl(padapter);
 	pmlmepriv = &padapter->mlmepriv;
 	/*  interface init */
-	/* if (sdio_init(adapter_to_dvobj(padapter)) != _SUCCESS) */
-	if ((padapter->intf_init) && (padapter->intf_init(adapter_to_dvobj(padapter)) != _SUCCESS)) {
-		ret = -1;
-		goto exit;
+	if (padapter->intf_init) {
+		ret = padapter->intf_init(adapter_to_dvobj(padapter));
+		if (ret)
+			goto exit;
 	}
 	rtw_hal_disable_interrupt(padapter);
 	/* if (sdio_alloc_irq(adapter_to_dvobj(padapter)) != _SUCCESS) */
-- 
2.51.0

[PATCH v3 4/4] staging: rtl8723bs: Choose slightly more readable error check

[Omer El Idrissi]

Let sdio_dvobj_init use a slightly more readable and conventional
error check for sdio_init().

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 01b5d8a70072..d0feb28b7043 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -155,6 +155,7 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 {
 	struct dvobj_priv *dvobj = NULL;
 	struct sdio_data *psdio;
+	int ret;
 
 	dvobj = devobj_init();
 	if (!dvobj)
@@ -165,7 +166,8 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 	psdio = &dvobj->intf_data;
 	psdio->func = func;
 
-	if (sdio_init(dvobj) != _SUCCESS)
+	ret = sdio_init(dvobj);
+	if (ret)
 		goto free_dvobj;
 
 	rtw_reset_continual_io_error(dvobj);
-- 
2.51.0

[PATCH v3 0/4] clean up error handling

[Omer El Idrissi]

This series lets sdio_init() propagate standard kernel error codes 
instead of returning _SUCCESS/_FAIL.  There are two callers for this
function.  rtw_resume_process_normal() already returns negative values 
but the caller doesn't check for errors so changing this doesn't
affect anything.  sdio_dvobj_init() returns NULL on
error so leave that as-is.

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
v3:
- Add one commit that lets sdio_dvobj_init() use more readable error
  handling
- Add v3 and v2 tags

v2:
- Add one commit that lets rtw_resume_process_normal cleanly check
  errors and return errno instead of using vendor-defined slop

v1:
- Use direct returns in sdio_dvobj_init()
- Cleanup return in sdio_init()

Omer El Idrissi (4):
  staging: rtl8723bs: use direct returns in sdio_dvobj_init()
  staging: rtl8723bs: cleanup return in sdio_init()
  staging: rtl8723bs: clean up callees for sdio_init function
  staging: rtl8723bs: Choose slightly more readable error check

 drivers/staging/rtl8723bs/os_dep/os_intfs.c  |  8 +++----
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 25 ++++++++------------
 2 files changed, 14 insertions(+), 19 deletions(-)

-- 
2.51.0

[PATCH v3 1/4] staging: rtl8723bs: use direct returns in sdio_dvobj_init()

[Omer El Idrissi]

Make sdio_dvobj_init() use direct returns

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index d664e254912c..358eac0837cf 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -155,13 +155,12 @@ static void sdio_deinit(struct dvobj_priv *dvobj)
 }
 static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 {
-	int status = _FAIL;
 	struct dvobj_priv *dvobj = NULL;
 	struct sdio_data *psdio;
 
 	dvobj = devobj_init();
 	if (!dvobj)
-		goto exit;
+		return NULL;
 
 	sdio_set_drvdata(func, dvobj);
 
@@ -172,18 +171,14 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 		goto free_dvobj;
 
 	rtw_reset_continual_io_error(dvobj);
-	status = _SUCCESS;
 
-free_dvobj:
-	if (status != _SUCCESS && dvobj) {
-		sdio_set_drvdata(func, NULL);
+	return dvobj;
 
-		devobj_deinit(dvobj);
+free_dvobj:
+	sdio_set_drvdata(func, NULL);
+	devobj_deinit(dvobj);
 
-		dvobj = NULL;
-	}
-exit:
-	return dvobj;
+	return NULL;
 }
 
 static void sdio_dvobj_deinit(struct sdio_func *func)
-- 
2.51.0

[PATCH v3 2/4] staging: rtl8723bs: cleanup return in sdio_init()

[Omer El Idrissi]

Make sdio_init() return errno from sdio_enable_func or
sdio_set_block_size instead of _SUCCESS/_FAIL vendor-defined
macros.

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 358eac0837cf..01b5d8a70072 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -131,9 +131,7 @@ static u32 sdio_init(struct dvobj_priv *dvobj)
 release:
 	sdio_release_host(func);
 
-	if (err)
-		return _FAIL;
-	return _SUCCESS;
+	return err;
 }
 
 static void sdio_deinit(struct dvobj_priv *dvobj)
-- 
2.51.0

[PATCH v3 3/4] staging: rtl8723bs: clean up callees for sdio_init function

[Omer El Idrissi]

Let rtw_resume_process_normal return errno returned by sdio_init
instead of -1. sdio_dvobj_init returns NULL on error so leave that
as is.

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/os_intfs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
index 7ba689f2dfc8..e943dcea1a21 100644
--- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
+++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
@@ -1135,10 +1135,10 @@ static int rtw_resume_process_normal(struct adapter *padapter)
 	pwrpriv = adapter_to_pwrctl(padapter);
 	pmlmepriv = &padapter->mlmepriv;
 	/*  interface init */
-	/* if (sdio_init(adapter_to_dvobj(padapter)) != _SUCCESS) */
-	if ((padapter->intf_init) && (padapter->intf_init(adapter_to_dvobj(padapter)) != _SUCCESS)) {
-		ret = -1;
-		goto exit;
+	if (padapter->intf_init) {
+		ret = padapter->intf_init(adapter_to_dvobj(padapter));
+		if (ret)
+			goto exit;
 	}
 	rtw_hal_disable_interrupt(padapter);
 	/* if (sdio_alloc_irq(adapter_to_dvobj(padapter)) != _SUCCESS) */
-- 
2.51.0

[PATCH v3 4/4] staging: rtl8723bs: Choose slightly more readable error check

[Omer El Idrissi]

Let sdio_dvobj_init use a slightly more readable and conventional
error check for sdio_init().

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 01b5d8a70072..d0feb28b7043 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -155,6 +155,7 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 {
 	struct dvobj_priv *dvobj = NULL;
 	struct sdio_data *psdio;
+	int ret;
 
 	dvobj = devobj_init();
 	if (!dvobj)
@@ -165,7 +166,8 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 	psdio = &dvobj->intf_data;
 	psdio->func = func;
 
-	if (sdio_init(dvobj) != _SUCCESS)
+	ret = sdio_init(dvobj);
+	if (ret)
 		goto free_dvobj;
 
 	rtw_reset_continual_io_error(dvobj);
-- 
2.51.0

Re: [PATCH v3 0/4] clean up error handling

[Dan Carpenter]

Sorry, I'm going to ask that you send these one last time. It should
just be 2 patches.  patch 1 stays the same, and patches 2, 3 and 4 get
squashed into the same patch.

The thing is that right now we're changing sdio_init() and then
cleaning up the callers.  We should change it and clean up the callers
all in one go.  One thing per patch means "One complete thing" not
"half a thing per patch".  It makes it easier to review when it's done
in one go, because otherwise, when I'm reviewing patch 2, I have to
check all the callers anyway.

regards,
dan carpenter

Re: [PATCH v3 0/4] clean up error handling

[Greg KH]

On Thu, Mar 26, 2026 at 09:16:22AM +0100, Omer El Idrissi wrote:
> This series lets sdio_init() propagate standard kernel error codes 
> instead of returning _SUCCESS/_FAIL.  There are two callers for this
> function.  rtw_resume_process_normal() already returns negative values 
> but the caller doesn't check for errors so changing this doesn't
> affect anything.  sdio_dvobj_init() returns NULL on
> error so leave that as-is.
> 
> Signed-off-by: Omer El Idrissi <omer.e.idriss@gmail.com>

You sent v3 twice, was that on purpose?

confused,

greg k-h

Re: [PATCH v3 0/4] clean up error handling

[Omer]

Nope.
The signed-off-by tag was off by one letter. so i fixed that and resent it.

Best,
Omer El Idrissi

On Thu, Mar 26, 2026 at 11:37 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Mar 26, 2026 at 09:16:22AM +0100, Omer El Idrissi wrote:
> > This series lets sdio_init() propagate standard kernel error codes
> > instead of returning _SUCCESS/_FAIL.  There are two callers for this
> > function.  rtw_resume_process_normal() already returns negative values
> > but the caller doesn't check for errors so changing this doesn't
> > affect anything.  sdio_dvobj_init() returns NULL on
> > error so leave that as-is.
> >
> > Signed-off-by: Omer El Idrissi <omer.e.idriss@gmail.com>
>
> You sent v3 twice, was that on purpose?
>
> confused,
>
> greg k-h

Re: [PATCH v3 0/4] clean up error handling

[Greg KH]

On Thu, Mar 26, 2026 at 12:10:36PM +0100, Omer wrote:
> Nope.

Please do not top-post :(

> The signed-off-by tag was off by one letter. so i fixed that and resent it.

Then that would be a new version, not the same version twice, otherwise
how would we know which to pick?

Please remember, some of us get 1000+ emails a day to deal with, and
when reviewing patches for a subsystem, have to handle hundreds of them
at the same time.  Think about what you would want the messages to look
like if you were on the recieving end.

As a bit of help, why not take some time to review other patches on the
staging list to reduce the overall load and to get more familiar with
how the process works?

thanks,

greg k-h