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@xxxxxxxxxx> 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@xxxxxxxxx>
> > ---
>
> 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