lkml.org 
[lkml]   [2022]   [Jul]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 5/7] staging: vt6655: Replace MACvReceive1 with function vt6655_mac_dma_ctl
On Mon, Jul 18, 2022 at 10:00:59PM +0200, Philipp Hortmann wrote:
> > > diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
> > > index 5747de436911..129a6602f6f0 100644
> > > --- a/drivers/staging/vt6655/mac.h
> > > +++ b/drivers/staging/vt6655/mac.h
> > > @@ -537,16 +537,6 @@
> > > /*--------------------- Export Macros ------------------------------*/
> > > -#define MACvReceive1(iobase) \
> > > -do { \
> > > - unsigned long reg_value; \
> > > - reg_value = ioread32(iobase + MAC_REG_RXDMACTL1); \
> > > - if (reg_value & DMACTL_RUN) \
> > > - iowrite32(DMACTL_WAKE, iobase + MAC_REG_RXDMACTL1); \
> > > - else \
> > > - iowrite32(DMACTL_RUN, iobase + MAC_REG_RXDMACTL1); \
> > > -} while (0)
> > > -
> > > #define MACvTransmit0(iobase) \
> > > do { \
> > > unsigned long reg_value; \
> >
>
> I was asking in kernelnewbies what to do with multi line macros as
> checkpatch.pl warnings cannot be totally avoided.
>
> Greg replied to make functions out of them.
>
> Please find the full email under:
>
> https://www.mail-archive.com/kernelnewbies@kernelnewbies.org/msg22042.html
>
> In this case I really like the static function solution much more than the
> macros.

Yes. We all like the static function. It's the commit message, I'm not
so keen on.

You could have avoided the checkpatch warning with an assignment at the
start of the macro:

typeof(iobase) base = (iobase);

#define MACvReceive1(iobase) \
do { \
typeof(iobase) base = (iobase); \
unsigned long reg_value = ioread32(base + MAC_REG_RXDMACTL1); \
if (reg_value & DMACTL_RUN) \
iowrite32(DMACTL_WAKE, base + MAC_REG_RXDMACTL1); \
else \
iowrite32(DMACTL_RUN, base + MAC_REG_RXDMACTL1); \
} while (0)

It's not a *good* solution, but it works.

The means the "iobase" argument would only be executed one time.
Imagine if someone passed "iobase++" to the original function. It
would have incremented twice instead of once as expected. That's what
the checkpatch warning is saying. Nothing to do with multiple lines.

regards,
dan carpenter

\
 
 \ /
  Last update: 2022-07-19 12:07    [W:0.065 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site