On Sun, Jul 24, 2022 at 05:30:55PM +0100, Phillip Potter wrote:
> Convert the rtw_pwr_wakeup function to use 0 on success and -EPERM on
> error - in all places where we handle this response, we use either -1 or
> -EPERM currently anyway, which are equivalent. Also, for other places
> along the same call chain where we are using -1, use -EPERM.
>
I can't get behind a change to -EPERM. Try to pick an appropriate
error code. I'm not going to be very strict on it, but we have to at
least *try*.
Probably, leave the return -1; lines alone. Fixing that seems like an
unrelated change. We need to do this kind of change but I got bitten by
it before so I want to avoid that next time. My reviews will hopefully
be more careful now.
> This gets the driver closer to removal of the non-standard _SUCCESS and
> _FAIL definitions, which are inverted compared to the standard in-kernel
> error code mechanism.
>
> Signed-off-by: Phillip Potter <phil@xxxxxxxxxxxxxxxx>
> ---
> drivers/staging/r8188eu/core/rtw_p2p.c | 4 +--
> drivers/staging/r8188eu/core/rtw_pwrctrl.c | 10 +++---
> drivers/staging/r8188eu/os_dep/ioctl_linux.c | 32 ++++++++++----------
> 3 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_p2p.c b/drivers/staging/r8188eu/core/rtw_p2p.c
> index c306aafa183b..bd654d4ff8b4 100644
> --- a/drivers/staging/r8188eu/core/rtw_p2p.c
> +++ b/drivers/staging/r8188eu/core/rtw_p2p.c
> @@ -1888,7 +1888,7 @@ int rtw_p2p_enable(struct adapter *padapter, enum P2P_ROLE role)
>
> if (role == P2P_ROLE_DEVICE || role == P2P_ROLE_CLIENT || role == P2P_ROLE_GO) {
> /* leave IPS/Autosuspend */
> - if (rtw_pwr_wakeup(padapter) == _FAIL) {
> + if (rtw_pwr_wakeup(padapter)) {
> ret = _FAIL;
> goto exit;
> }
Fine. The caller now changes from negative error codes to _SUCCESS/_FAIL.
Later we will transition this to normal error codes so we'll update it
to preserve the error code from rtw_pwr_wakeup() at that point.
> diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> index 930bb4aea435..e0ae0c3c51f8 100644
> --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> @@ -689,7 +689,7 @@ static int rtw_wx_set_mode(struct net_device *dev, struct iw_request_info *a,
>
>
>
> - if (_FAIL == rtw_pwr_wakeup(padapter)) {
> + if (rtw_pwr_wakeup(padapter)) {
> ret = -EPERM;
> goto exit;
> }
This code is returning negative error codes so it should preserve the
code from rtw_pwr_wakeup().
ret = rtw_pwr_wakeup(padapter);
if (ret)
goto exit;
> @@ -933,13 +933,13 @@ static int rtw_wx_set_wap(struct net_device *dev,
>
>
>
> - if (_FAIL == rtw_pwr_wakeup(padapter)) {
> - ret = -1;
> + if (rtw_pwr_wakeup(padapter)) {
> + ret = -EPERM;
Same.
> goto exit;
> }
>
> if (!padapter->bup) {
> - ret = -1;
> + ret = -EPERM;
Unrelated.
> goto exit;
> }
>
[ snip ]
> @@ -1252,13 +1252,13 @@ static int rtw_wx_set_essid(struct net_device *dev,
>
> uint ret = 0, len;
>
> - if (_FAIL == rtw_pwr_wakeup(padapter)) {
> - ret = -1;
> + if (rtw_pwr_wakeup(padapter)) {
> + ret = -EPERM;
Preserve the error code.
> goto exit;
> }
regards,
dan carpenter
