linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bryan Brattlof <hello@bryanbrattlof.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: rtl8723bs: use generic kernel error codes
Date: Tue, 04 May 2021 17:45:37 +0000	[thread overview]
Message-ID: <20210504174530.3kog7zq6tuk3wnlk@bryanbrattlof.com> (raw)
In-Reply-To: <YJFziyZHnbsWdTFn@kroah.com>

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. :)

>
>>
>>  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?

--
~Bryan

>
>thanks,
>
>greg k-h


  reply	other threads:[~2021-05-04 17:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-04 16:07 [PATCH] staging: rtl8723bs: use generic kernel error codes Bryan Brattlof
2021-05-04 16:17 ` Greg Kroah-Hartman
2021-05-04 17:45   ` Bryan Brattlof [this message]
2021-05-04 17:56     ` Greg Kroah-Hartman
2021-05-04 18:52       ` Bryan Brattlof

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210504174530.3kog7zq6tuk3wnlk@bryanbrattlof.com \
    --to=hello@bryanbrattlof.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).