Re: [PATCH v2] net: add Faraday FTMAC100 10/100 Ethernet driver
From: Joe Perches
Date: Thu Jan 20 2011 - 03:46:40 EST
On Thu, 2011-01-20 at 13:30 +0800, Po-Yu Chuang wrote:
> One more question: how to deal with this? Add a local variable for the
> 2nd argument?
>
> ftmac100_set_receive_ring_base(priv,
> priv->descs_dma_addr + offsetof(struct ftmac100_descs, rxdes));
That's one way, another is:
ftmac100_set_receive_ring_base(priv,
priv->descs_dma_addr +
offsetof(struct ftmac100_descs, rxdes));
Another would be to change set_receive_ring_base to
just pass priv and determine the dma address there:
static void ftmac100_set_receive_ring_base(struct ftmac100 *priv)
{
dma_addr_t addr;
addr = priv->descs_dma_addr + offsetof(struct ftmac100_descs, rxdes);
iowrite32(addr, priv->base + FTMAC100_OFFSET_RXR_BADR);
}
...
ftmac100_set_receive_ring_base(priv);
Another is not to be overly adherent to 80 columns.
Pick one that suits you.
You chose to use a lot of single use, single line
functions with descriptive names that use iowrite32 or
return some flag. I probably would have just used
iowrite32 or tested the flag directly, but that's your
choice and it's perfectly fine.
There are a lot of coding choices that are readable and
good. There isn't and shouldn't be some mandate for some
specific code appearance before inclusion or acceptance.
checkpatch is just a style guide. Ignore it and ignore
me when you feel it's appropriate. I won't mind.
cheers, Joe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/