On Tue, May 04, 2021 at 07:56:35PM +0200, Greg Kroah-Hartman wrote: >On Tue, May 04, 2021 at 05:45:37PM +0000, Bryan Brattlof wrote: >> Sorry for the spam Greg I dropped the mailing lists from the first >> email. :( >> >> On Tue, May 04, 2021 at 06:17:15PM +0200, Greg Kroah-Hartman wrote: >> >On Tue, May 04, 2021 at 04:07:48PM +0000, Bryan Brattlof wrote: >> >> <snip> >> >> >> @@ -139,12 +139,11 @@ static u32 sdio_init(struct dvobj_priv *dvobj) >> >> psdio_data->tx_block_mode = 1; >> >> psdio_data->rx_block_mode = 1; >> >> >> >> + return err; >> >> + >> >> release: >> >> sdio_release_host(func); >> >> - >> >> - if (err) >> >> - return _FAIL; >> >> - return _SUCCESS; >> >> + return err; >> >> } >> > >> >You just changed the logic here, are you SURE that was ok to do? >> > >> >> I can't say my brain didn't bleed a little trying to keep this straight >> in my head while walking through this. (For what ever reason my brain >> sees negative integers as False) ¯\_(ツ)_/¯ >> >> Both the sdio_enable_func() and sdio_set_block_size() will return a >> negative integer if they fail, which we evaluate as True, allowing us to >> jump to release, free the card and propagate the error backwards. >> >> If everything worked, we'll skip all the jumps until we get to the first >> 'return err;' statement, returning our 0 for success. >> >> Inside sdio_dvobj_init() if we see 'anything below 0' (This probably >> should be changed to 'anything True') we jump to free_dvobj where we >> free the dvobj and return NULL >> >> If I've looked at this long enough I don't thing I changed the logic. >> >> Hopefully. :) > >So you need to document this really well, showing that the function >whose error you changed, is being evaluated here now differently too. > Sounds good. Ill update the commit log > >> >> static void sdio_deinit(struct dvobj_priv *dvobj) >> >> @@ -186,7 +185,7 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func) >> >> psdio = &dvobj->intf_data; >> >> psdio->func = func; >> >> >> >> - if (sdio_init(dvobj) != _SUCCESS) >> >> + if (sdio_init(dvobj) < 0) >> >> goto free_dvobj; >> >> >> >> rtw_reset_continual_io_error(dvobj); >> >> >> >> base-commit: 9ccce092fc64d19504fa54de4fd659e279cc92e7 >> >> -- >> >> git-series 0.9.1 >> >> >> >> >> > >> >And that's all to remove the need for these crazy error values? If so, >> >why not also remove the #defines for them as well? >> > >> >> I might have over sold this patch. :) >> >> There are quite a few functions like this still here that need to be >> converted before we can get rid of the _SUCCESS and _FAIL definitions. >> >> Would it be better if I bundled these up in a series? > >Do it one function "call-chain" at a time, and yes, a series would be >great. > Thanks Greg! I'll bundle this up into a series. -- ~Bryan > >thanks, > >greg k-h
