Thanks for these. Unfortunately you didn't cc me on the individual
patches so I had to go and look in the online archive.
Comments below.
On Sun, May 06 2018, Kamal Heib wrote:
> This patches fixes an compilation error and cleanup few errors reported
> by checkpatch.pl script.
>
> Cc: NeilBrown <neil@xxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>
> Kamal Heib (4):
> staging: mt7621-eth: Fix compilation error
I don't think this is a good fix. It fixes the error, but it doesn't
fix the code.
phy_ring_head and phy_ring_tail are conceptually the same type, so they
should both be dma_addr_t.
> staging: mt7621-eth: Remove unnecessary blank lines
Reviewed-by: NeilBrown <neil@xxxxxxxxxx>
> staging: mt7621-eth: Add missing blank lines after declarations
Reviewed-by: NeilBrown <neil@xxxxxxxxxx>
> staging: mt7621-eth: Alignment should match open parenthesis
I don't really like
ring->rx_dma = dma_alloc_coherent(eth->dev,
- ring->rx_ring_size * sizeof(*ring->rx_dma),
- &ring->rx_phys,
- GFP_ATOMIC | __GFP_ZERO);
+ ring->rx_ring_size *
+ sizeof(*ring->rx_dma),
+ &ring->rx_phys,
+ GFP_ATOMIC | __GFP_ZERO);
as you need to look at the end of the lines to see that some end ',' and
one ends '*'.
One solution would be to make it
ring->rx_dma = dma_alloc_coherent(eth->dev,
(ring->rx_ring_size *
sizeof(*ring->rx_dma)),
&ring->rx_phys,
GFP_ATOMIC | __GFP_ZERO);
i.e. use parentheses around the multiplication so that the
second line gets indented a bit.
I would also be happy with
ring->rx_dma = dma_alloc_coherent(
eth->dev,
ring->rx_ring_size * sizeof(*ring->rx_dma),
&ring->rx_phys,
GFP_ATOMIC | __GFP_ZERO);
If there is nothing after the open '(', then the alignment rules are
different (I think).
Note that whatever change you make in mtk_dma_rx_alloc() is also
required in mtk_pdma_tx_alloc().
Thanks,
NeilBrown
>
> drivers/staging/mt7621-eth/ethtool.c | 1 -
> drivers/staging/mt7621-eth/gsw_mt7621.c | 3 +--
> drivers/staging/mt7621-eth/mdio.c | 2 ++
> drivers/staging/mt7621-eth/mtk_eth_soc.c | 26 ++++++++++++++------------
> drivers/staging/mt7621-eth/soc_mt7621.c | 1 -
> 5 files changed, 17 insertions(+), 16 deletions(-)
>
> --
> 2.14.3
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
