On Mon, Apr 11, 2022 at 10:49:20PM +0200, Philipp Hortmann wrote:
> Replace macro VNSvInPortB with ioread8.
> The name of macro and the arguments use CamelCase which
> is not accepted by checkpatch.pl
>
> Since there are more than one checkpatch issue per line,
> more steps are rquired to fix.
>
> Signed-off-by: Philipp Hortmann <philipp.g.hortmann@xxxxxxxxx>
> ---
> V1 -> V2: This patch was 4/7 and is now 3/6
> V2 -> V3: Changed from rename macro arguments to replace
> macro VNSvInPortB with ioread8
> This patch was 3/6 and is now 1/7
> ---
> drivers/staging/vt6655/baseband.c | 6 +++---
> drivers/staging/vt6655/card.c | 2 +-
> drivers/staging/vt6655/device_main.c | 4 ++--
> drivers/staging/vt6655/mac.h | 26 ++++++++++----------------
> drivers/staging/vt6655/srom.c | 6 +++---
> drivers/staging/vt6655/upc.h | 3 ---
> 6 files changed, 19 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/staging/vt6655/baseband.c b/drivers/staging/vt6655/baseband.c
> index dfdb0ebf43b5..decb55e96f02 100644
> --- a/drivers/staging/vt6655/baseband.c
> +++ b/drivers/staging/vt6655/baseband.c
> @@ -1916,13 +1916,13 @@ bool bb_read_embedded(struct vnt_private *priv, unsigned char by_bb_addr,
> MACvRegBitsOn(iobase, MAC_REG_BBREGCTL, BBREGCTL_REGR);
> /* W_MAX_TIMEOUT is the timeout period */
> for (ww = 0; ww < W_MAX_TIMEOUT; ww++) {
> - VNSvInPortB(iobase + MAC_REG_BBREGCTL, &by_value);
> + by_value = ioread8(iobase + MAC_REG_BBREGCTL);
This type of change is great, but...
> @@ -597,18 +597,12 @@ do { \
> #define MACvReadEtherAddress(iobase, pbyEtherAddr) \
> do { \
> VNSvOutPortB(iobase + MAC_REG_PAGE1SEL, 1); \
> - VNSvInPortB(iobase + MAC_REG_PAR0, \
> - (unsigned char *)pbyEtherAddr); \
> - VNSvInPortB(iobase + MAC_REG_PAR0 + 1, \
> - pbyEtherAddr + 1); \
> - VNSvInPortB(iobase + MAC_REG_PAR0 + 2, \
> - pbyEtherAddr + 2); \
> - VNSvInPortB(iobase + MAC_REG_PAR0 + 3, \
> - pbyEtherAddr + 3); \
> - VNSvInPortB(iobase + MAC_REG_PAR0 + 4, \
> - pbyEtherAddr + 4); \
> - VNSvInPortB(iobase + MAC_REG_PAR0 + 5, \
> - pbyEtherAddr + 5); \
> + (*((unsigned char *)pbyEtherAddr) = ioread8(iobase + MAC_REG_PAR0)); \
Why cast one and not all? No casts should be needed. If so, the caller
is wrong.
> + (*(pbyEtherAddr + 1) = ioread8(iobase + MAC_REG_PAR0 + 1)); \
That's odd.
Ok, I guess it's an array, right? How about:
pbyEtherAddr[0] = ioread8(iobase + MAC_REG_PAR0); \
pbyEtherAddr[1] = ioread8(iobase + MAC_REG_PAR0 + 1); \
and so on?
Much simpler and easier to understand.
And then there's this odd stuff:
> #define MACvGPIOIn(iobase, pbyValue) \
> - VNSvInPortB(iobase + MAC_REG_GPIOCTL1, pbyValue)
> + *(pbyValue) = ioread8(iobase + MAC_REG_GPIOCTL1)
pbyValue? unwind this first, you should be able to just have a normal
ioread() call, right?
Or, make it a function.
thanks,
greg k-h
