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@xxxxxxxxxxxxxxxxx/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
