On 7/18/22 08:07, Joe Perches wrote:
Please remember that checkpatch is a stupid little scripted tool
and the actual goal is to have readable code.
Understood.
Look a bit beyond the code and see if and how you could make the
code better.
All of these macros have the same form and logic.
That is the reason why I put them all together in one static function:
static void vt6655_mac_dma_ctl(void __iomem *iobase, u8 reg_index)
{
unsigned long reg_value;
reg_value = ioread32(iobase + reg_index);
if (reg_value & DMACTL_RUN)
iowrite32(DMACTL_WAKE, iobase + reg_index);
else
iowrite32(DMACTL_RUN, iobase + reg_index);
}
Perhaps it'd be better to use another indirect macro and define
all of these with that new macro.
Something like:
#define mac_v(iobase, reg) \
do { \
void __iomem *addr = (iobase) + (reg); \
iowrite32(ioread32(addr) & DMACTL_RUN ? DMACTL_WAKE : DMACTL_RUN,\
addr); \
} while (0)
#define MACvReceive0(iobase) mac_v(iobase, MAC_REG_RXDMACTL0)
#define MACvReceive1(iobase) mac_v(iobase, MAC_REG_RXDMACTL1)
#define MACvTransmit0(iobase) mac_v(iobase, MAC_REG_TXDMACTL0)
#define MACvTransmitAC0(iobase) mac_v(iobase, MAC_REG_AC0DMACTL)
That is an interesting solution. But for me this code is not as good
readable as my proposal. Reason is that I struggle with the function in
function with condition broken into two lines.
