[v2] staging: rtl8723bs: change error handling to use standard errno

[PATCH v2] staging: rtl8723bs: change error handling to use standard errno

Posted by Omer El Idrissi 1 week, 6 days ago

Replace non-standard return values with descriptive linux kernel error
codes in probe path.
Also replace the variable name 'status' with 'ret' to be more
consistent with other kernel code.

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>

---
v2:
- Return -ENODEV instead of -ENOMEM when sdio_dvobj_init fails

 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 30 ++++++++++++--------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index d664e254912c..47189c849b05 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -345,38 +345,44 @@ static int rtw_drv_init(
 	struct sdio_func *func,
 	const struct sdio_device_id *id)
 {
-	int status = _FAIL;
+	int ret;
 	struct adapter *if1 = NULL;
 	struct dvobj_priv *dvobj;
 
 	dvobj = sdio_dvobj_init(func);
-	if (!dvobj)
+	if (!dvobj) {
+		ret = -ENODEV;
 		goto exit;
+	}
 
 	if1 = rtw_sdio_if1_init(dvobj, id);
-	if (!if1)
+	if (!if1) {
+		ret = -ENOMEM;
 		goto free_dvobj;
+	}
 
 	/* dev_alloc_name && register_netdev */
-	status = rtw_drv_register_netdev(if1);
-	if (status != _SUCCESS)
+	ret = rtw_drv_register_netdev(if1);
+	if (ret) {
+		ret = -EIO;
 		goto free_if1;
+	}
 
-	status = sdio_alloc_irq(dvobj);
-	if (status != _SUCCESS)
+	ret = sdio_alloc_irq(dvobj);
+	if (ret) {
+		ret = -EIO;
 		goto free_if1;
-
-	status = _SUCCESS;
+	}
 
 free_if1:
-	if (status != _SUCCESS && if1)
+	if (ret && if1)
 		rtw_sdio_if1_deinit(if1);
 
 free_dvobj:
-	if (status != _SUCCESS)
+	if (ret)
 		sdio_dvobj_deinit(func);
 exit:
-	return status == _SUCCESS ? 0 : -ENODEV;
+	return ret;
 }
 
 static void rtw_dev_remove(struct sdio_func *func)
-- 
2.51.0

Re: [PATCH v2] staging: rtl8723bs: change error handling to use standard errno

Posted by Dan Carpenter 1 week, 6 days ago

On Sat, Mar 21, 2026 at 09:17:38PM +0100, Omer El Idrissi wrote:
> Replace non-standard return values with descriptive linux kernel error
> codes in probe path.
> Also replace the variable name 'status' with 'ret' to be more
> consistent with other kernel code.
> 
> Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
> 
> ---
> v2:
> - Return -ENODEV instead of -ENOMEM when sdio_dvobj_init fails
> 
>  drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 30 ++++++++++++--------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> index d664e254912c..47189c849b05 100644
> --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> @@ -345,38 +345,44 @@ static int rtw_drv_init(
>  	struct sdio_func *func,
>  	const struct sdio_device_id *id)
>  {
> -	int status = _FAIL;
> +	int ret;
>  	struct adapter *if1 = NULL;
>  	struct dvobj_priv *dvobj;
>  
>  	dvobj = sdio_dvobj_init(func);
> -	if (!dvobj)
> +	if (!dvobj) {
> +		ret = -ENODEV;
>  		goto exit;
> +	}
>  
>  	if1 = rtw_sdio_if1_init(dvobj, id);
> -	if (!if1)
> +	if (!if1) {
> +		ret = -ENOMEM;
>  		goto free_dvobj;
> +	}
>  
>  	/* dev_alloc_name && register_netdev */
> -	status = rtw_drv_register_netdev(if1);
> -	if (status != _SUCCESS)
> +	ret = rtw_drv_register_netdev(if1);
> +	if (ret) {
> +		ret = -EIO;
>  		goto free_if1;
> +	}


rtw_drv_register_netdev() doesn't return regular error codes
but when you change:

-	if (status != _SUCCESS) {
+	if (ret) {

Then you're making it look like it does, which is confusing.

You're going about this the wrong way.  You need to start with
functions like _rtw_drv_register_netdev() which are where
the _SUCCESS returns are first returned and change those first.
You have to update the callers as you go.  It's tricky because
you have to be sure you're catching all the callers.

Then eventually, you'll get back to rtw_drv_init() and all the
functions it call will return normal error codes and you can
update that.

Do it one function at a time.  Update the function and the
callers.  Something like this:

regards,
dan carpenter

[patch] update error code for rtw_init_netdev_name()

The rtw_init_netdev_name() returns 1 on failure and 0 on success
which is non-standard.  Change it to propagate the error from
dev_alloc_name() and update the only caller.

diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
index 7ba689f2dfc8..92b52b1a2920 100644
--- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
+++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
@@ -407,9 +407,12 @@ static const struct net_device_ops rtw_netdev_ops = {
 
 int rtw_init_netdev_name(struct net_device *pnetdev, const char *ifname)
 {
-	if (dev_alloc_name(pnetdev, ifname) < 0) {
+	int ret;
+
+	ret = dev_alloc_name(pnetdev, ifname);
+	if (ret < 0) {
 		pr_err("dev_alloc_name, fail for %s\n", ifname);
-		return 1;
+		return ret;
 	}
 	netif_carrier_off(pnetdev);
 	/* rtw_netif_stop_queue(pnetdev); */
@@ -755,7 +758,8 @@ static int _rtw_drv_register_netdev(struct adapter *padapter, char *name)
 	struct net_device *pnetdev = padapter->pnetdev;
 
 	/* alloc netdev name */
-	if (rtw_init_netdev_name(pnetdev, name))
+	ret = rtw_init_netdev_name(pnetdev, name);
+	if (ret)
 		return _FAIL;
 
 	eth_hw_addr_set(pnetdev, padapter->eeprompriv.mac_addr);

Re: [PATCH v2] staging: rtl8723bs: change error handling to use standard errno

Posted by Omer 1 week, 6 days ago

Thank you for responding sir.

Most of the functions called in the probe function are only called
there so it should be easier to fix.

I will move on to do that.

On Sat, Mar 21, 2026 at 10:24 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Sat, Mar 21, 2026 at 09:17:38PM +0100, Omer El Idrissi wrote:
> > Replace non-standard return values with descriptive linux kernel error
> > codes in probe path.
> > Also replace the variable name 'status' with 'ret' to be more
> > consistent with other kernel code.
> >
> > Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
> >
> > ---
> > v2:
> > - Return -ENODEV instead of -ENOMEM when sdio_dvobj_init fails
> >
> >  drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 30 ++++++++++++--------
> >  1 file changed, 18 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > index d664e254912c..47189c849b05 100644
> > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > @@ -345,38 +345,44 @@ static int rtw_drv_init(
> >       struct sdio_func *func,
> >       const struct sdio_device_id *id)
> >  {
> > -     int status = _FAIL;
> > +     int ret;
> >       struct adapter *if1 = NULL;
> >       struct dvobj_priv *dvobj;
> >
> >       dvobj = sdio_dvobj_init(func);
> > -     if (!dvobj)
> > +     if (!dvobj) {
> > +             ret = -ENODEV;
> >               goto exit;
> > +     }
> >
> >       if1 = rtw_sdio_if1_init(dvobj, id);
> > -     if (!if1)
> > +     if (!if1) {
> > +             ret = -ENOMEM;
> >               goto free_dvobj;
> > +     }
> >
> >       /* dev_alloc_name && register_netdev */
> > -     status = rtw_drv_register_netdev(if1);
> > -     if (status != _SUCCESS)
> > +     ret = rtw_drv_register_netdev(if1);
> > +     if (ret) {
> > +             ret = -EIO;
> >               goto free_if1;
> > +     }
>
>
> rtw_drv_register_netdev() doesn't return regular error codes
> but when you change:
>
> -       if (status != _SUCCESS) {
> +       if (ret) {
>
> Then you're making it look like it does, which is confusing.
>
> You're going about this the wrong way.  You need to start with
> functions like _rtw_drv_register_netdev() which are where
> the _SUCCESS returns are first returned and change those first.
> You have to update the callers as you go.  It's tricky because
> you have to be sure you're catching all the callers.
>
> Then eventually, you'll get back to rtw_drv_init() and all the
> functions it call will return normal error codes and you can
> update that.
>
> Do it one function at a time.  Update the function and the
> callers.  Something like this:
>
> regards,
> dan carpenter
>
> [patch] update error code for rtw_init_netdev_name()
>
> The rtw_init_netdev_name() returns 1 on failure and 0 on success
> which is non-standard.  Change it to propagate the error from
> dev_alloc_name() and update the only caller.
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> index 7ba689f2dfc8..92b52b1a2920 100644
> --- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> +++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> @@ -407,9 +407,12 @@ static const struct net_device_ops rtw_netdev_ops = {
>
>  int rtw_init_netdev_name(struct net_device *pnetdev, const char *ifname)
>  {
> -       if (dev_alloc_name(pnetdev, ifname) < 0) {
> +       int ret;
> +
> +       ret = dev_alloc_name(pnetdev, ifname);
> +       if (ret < 0) {
>                 pr_err("dev_alloc_name, fail for %s\n", ifname);
> -               return 1;
> +               return ret;
>         }
>         netif_carrier_off(pnetdev);
>         /* rtw_netif_stop_queue(pnetdev); */
> @@ -755,7 +758,8 @@ static int _rtw_drv_register_netdev(struct adapter *padapter, char *name)
>         struct net_device *pnetdev = padapter->pnetdev;
>
>         /* alloc netdev name */
> -       if (rtw_init_netdev_name(pnetdev, name))
> +       ret = rtw_init_netdev_name(pnetdev, name);
> +       if (ret)
>                 return _FAIL;
>
>         eth_hw_addr_set(pnetdev, padapter->eeprompriv.mac_addr);