Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <1590e1abf3991c4b9023173bddee5b9e912d2c47.camel@philpotter.co.uk>
Subject: Re: [PATCH RFC 3/3] staging: r8188eu: add error argument to
 read_macreg
From:   Phillip Potter <phil@philpotter.co.uk>
To:     Pavel Skripkin <paskripkin@gmail.com>
Cc:     linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
        Larry.Finger@lwfinger.net, gregkh@linuxfoundation.org,
        straube.linux@gmail.com, fmdefrancesco@gmail.com
Date:   Sat, 21 Aug 2021 00:18:00 +0100
In-Reply-To: <44a5a4ab2eae8e0deb969028a5488ea892008c8d.1629479152.git.paskripkin@gmail.com>
References: <cover.1629479152.git.paskripkin@gmail.com>
         <44a5a4ab2eae8e0deb969028a5488ea892008c8d.1629479152.git.paskripkin@gmail.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.40.3 (3.40.3-1.fc34) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Fri, 2021-08-20 at 20:07 +0300, Pavel Skripkin wrote:
> Since read_macreg() calls rtw_read*() internally we should tell
> callers about an error on the read side.
> 
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
>  drivers/staging/r8188eu/core/rtw_mp.c    | 9 ++++-----
>  drivers/staging/r8188eu/include/rtw_mp.h | 2 +-
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_mp.c
> b/drivers/staging/r8188eu/core/rtw_mp.c
> index 601a1fd5d4e7..6bbea1cc364a 100644
> --- a/drivers/staging/r8188eu/core/rtw_mp.c
> +++ b/drivers/staging/r8188eu/core/rtw_mp.c
> @@ -7,20 +7,19 @@
>  #include "../include/odm_precomp.h"
>  #include "../include/rtl8188e_hal.h"
>  
> -u32 read_macreg(struct adapter *padapter, u32 addr, u32 sz)
> +u32 read_macreg(struct adapter *padapter, u32 addr, u32 sz, int
> *error)

Dear Pavel,

Correct me if I'm wrong, but this read_macreg function seems to be
completely unused by the rest of the driver. Rather than changing the
signature to do error handling, maybe it would be better to just remove
it?

That is just my view though, would be interested to see what others
think - perhaps it could come in handy at some point.

Regards,
Phil