From: Neil Armstrong <superna9999@gmail.com>
To: Varka Bhadram <varkabhadram@gmail.com>,
Nicolas Ferre <nicolas.ferre@atmel.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] net: macb: Separate rx and tx ring init function
Date: Fri, 18 Jul 2014 12:13:33 +0200 [thread overview]
Message-ID: <53C8F34D.9090503@gmail.com> (raw)
In-Reply-To: <53C8F280.9040507@gmail.com>
Le 18/07/2014 12:10, Varka Bhadram a écrit :
> On 07/18/2014 03:22 PM, Neil Armstrong wrote:
>> A single method is called to initialize the TX and RX
>> rings. Separate the methods into distinct ones for
>> MACB and GEM context.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@neotion.com>
>> ---
>> drivers/net/ethernet/cadence/macb.c | 33 ++++++++++++++++++++++++---------
>> drivers/net/ethernet/cadence/macb.h | 3 ++-
>> 2 files changed, 26 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>> index e9daa07..20ad483 100644
>> --- a/drivers/net/ethernet/cadence/macb.c
>> +++ b/drivers/net/ethernet/cadence/macb.c
>> @@ -1219,7 +1219,14 @@ out_err:
>> return -ENOMEM;
>> }
>>
>> -static void gem_init_rings(struct macb *bp)
>> +static void gem_init_rx_rings(struct macb *bp)
>> +{
>> + bp->rx_tail = bp->rx_prepared_head = 0;
>> +
>> + gem_rx_refill(bp);
>> +}
>> +
>> +static void gem_init_tx_rings(struct macb *bp)
>> {
>> int i;
>>
>> @@ -1229,12 +1236,10 @@ static void gem_init_rings(struct macb *bp)
>> }
>> bp->tx_ring[TX_RING_SIZE - 1].ctrl |= MACB_BIT(TX_WRAP);
>>
>> - bp->rx_tail = bp->rx_prepared_head = bp->tx_head = bp->tx_tail = 0;
>> -
>> - gem_rx_refill(bp);
>> + bp->tx_head = bp->tx_tail = 0;
>
> This is not the preferred way of doing it....
>
> multiple assignments should be avoided
Should I change it ? I only moved the lines, the logic hasn't changed.
>
>> }
>>
>> -static void macb_init_rings(struct macb *bp)
>> +static void macb_init_rx_rings(struct macb *bp)
>> {
>> int i;
>> dma_addr_t addr;
>> @@ -1247,13 +1252,20 @@ static void macb_init_rings(struct macb *bp)
>> }
>> bp->rx_ring[RX_RING_SIZE - 1].addr |= MACB_BIT(RX_WRAP);
>>
>> + bp->rx_tail = 0;
>> +}
>> +
>> +static void macb_init_tx_rings(struct macb *bp)
>> +{
>> + int i;
>> +
>> for (i = 0; i < TX_RING_SIZE; i++) {
>> bp->tx_ring[i].addr = 0;
>> bp->tx_ring[i].ctrl = MACB_BIT(TX_USED);
>> }
>> bp->tx_ring[TX_RING_SIZE - 1].ctrl |= MACB_BIT(TX_WRAP);
>>
>> - bp->rx_tail = bp->tx_head = bp->tx_tail = 0;
>> + bp->tx_head = bp->tx_tail = 0;
>
> Dto...
Idem, it was like this before.
>
>> }
>>
>> static void macb_reset_hw(struct macb *bp)
>> @@ -1554,7 +1566,8 @@ static int macb_open(struct net_device *dev)
>>
>> napi_enable(&bp->napi);
>>
>> - bp->macbgem_ops.mog_init_rings(bp);
>> + bp->macbgem_ops.mog_init_rx_rings(bp);
>> + bp->macbgem_ops.mog_init_tx_rings(bp);
>> macb_init_hw(bp);
>>
>> /* schedule a link state check */
>> @@ -1901,12 +1914,14 @@ static int __init macb_probe(struct platform_device *pdev)
>> if (macb_is_gem(bp)) {
>> bp->macbgem_ops.mog_alloc_rx_buffers = gem_alloc_rx_buffers;
>> bp->macbgem_ops.mog_free_rx_buffers = gem_free_rx_buffers;
>> - bp->macbgem_ops.mog_init_rings = gem_init_rings;
>> + bp->macbgem_ops.mog_init_rx_rings = gem_init_rx_rings;
>> + bp->macbgem_ops.mog_init_tx_rings = gem_init_tx_rings;
>> bp->macbgem_ops.mog_rx = gem_rx;
>> } else {
>> bp->macbgem_ops.mog_alloc_rx_buffers = macb_alloc_rx_buffers;
>> bp->macbgem_ops.mog_free_rx_buffers = macb_free_rx_buffers;
>> - bp->macbgem_ops.mog_init_rings = macb_init_rings;
>> + bp->macbgem_ops.mog_init_rx_rings = macb_init_rx_rings;
>> + bp->macbgem_ops.mog_init_tx_rings = macb_init_tx_rings;
>> bp->macbgem_ops.mog_rx = macb_rx;
>> }
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
>> index 51c0244..8016d08 100644
>> --- a/drivers/net/ethernet/cadence/macb.h
>> +++ b/drivers/net/ethernet/cadence/macb.h
>> @@ -550,7 +550,8 @@ struct macb;
>> struct macb_or_gem_ops {
>> int (*mog_alloc_rx_buffers)(struct macb *bp);
>> void (*mog_free_rx_buffers)(struct macb *bp);
>> - void (*mog_init_rings)(struct macb *bp);
>> + void (*mog_init_rx_rings)(struct macb *bp);
>> + void (*mog_init_tx_rings)(struct macb *bp);
>> int (*mog_rx)(struct macb *bp, int budget);
>> };
>>
>
>
Do I need to make a corrective patch for multiple assignments before ?
next prev parent reply other threads:[~2014-07-18 10:14 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-18 9:52 [PATCH 1/2] net: macb: Separate rx and tx ring init function Neil Armstrong
2014-07-18 10:10 ` Varka Bhadram
2014-07-18 10:13 ` Neil Armstrong [this message]
2014-07-18 10:15 ` Varka Bhadram
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53C8F34D.9090503@gmail.com \
--to=superna9999@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nicolas.ferre@atmel.com \
--cc=varkabhadram@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.