From: FUJITA Tomonori <fujita.tomonori@gmail.com>
To: jdamato@fastly.com
Cc: fujita.tomonori@gmail.com, netdev@vger.kernel.org,
andrew@lunn.ch, horms@kernel.org, kuba@kernel.org,
jiri@resnulli.us, pabeni@redhat.com, linux@armlinux.org.uk,
hfdevel@gmx.net
Subject: Re: [PATCH net-next v6 4/6] net: tn40xx: add basic Rx handling
Date: Wed, 15 May 2024 18:37:19 +0900 (JST) [thread overview]
Message-ID: <20240515.183719.2094245718391165470.fujita.tomonori@gmail.com> (raw)
In-Reply-To: <ZkJd55Y3MwsBFG2_@LQ3V64L9R2>
Hi,
Thanks for reviewing the patch!
On Mon, 13 May 2024 11:37:27 -0700
Joe Damato <jdamato@fastly.com> wrote:
> On Sun, May 12, 2024 at 05:56:09PM +0900, FUJITA Tomonori wrote:
>> This patch adds basic Rx handling. The Rx logic uses three major data
>> structures; two ring buffers with NIC and one database. One ring
>> buffer is used to send information to NIC about memory to be stored
>> packets to be received. The other is used to get information from NIC
>> about received packets. The database is used to keep the information
>> about DMA mapping. After a packet arrived, the db is used to pass the
>> packet to the network stack.
>
> I left one comment below, but had a higher level question unrelated to my
> comment below:
>
> Have you considered using the page pool for allocating/recycling RX
> buffers? It might simplify your code significantly and reduce the amount of
> code that needs to be maintained. Several drivers are using the page pool
> already, so there are many examples.
>
> My apologies if you answered this in an earlier version and I just missed
> it.
The page pool hasn't been mentioned before. I'll try it.
>> +static int tn40_poll(struct napi_struct *napi, int budget)
>> +{
>> + struct tn40_priv *priv = container_of(napi, struct tn40_priv, napi);
>> + int work_done;
>> +
>> + tn40_tx_cleanup(priv);
>> +
>> + if (!budget)
>> + return 0;
>> +
>> + work_done = tn40_rx_receive(priv, &priv->rxd_fifo0, budget);
>> + if (work_done == budget)
>> + return budget;
>> +
>> + napi_complete_done(napi, work_done);
>
> I believe the return value of napi_complete_done should be checked here and
> only if it returns true, should IRQs be re-enabled.
>
> For example:
>
> if (napi_complete_done(napi, work_done))
> tn40_enable_interrupts(priv);
>
>> + tn40_enable_interrupts(priv);
>> + return work_done;
>> +}
Ah, I messed up when I changed the poller to handle zero budget. I'll
fix in v7.
Thanks!
next prev parent reply other threads:[~2024-05-15 9:37 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-12 8:56 [PATCH net-next v6 0/6] add ethernet driver for Tehuti Networks TN40xx chips FUJITA Tomonori
2024-05-12 8:56 ` [PATCH net-next v6 1/6] net: tn40xx: add pci " FUJITA Tomonori
2024-05-12 8:56 ` [PATCH net-next v6 2/6] net: tn40xx: add register defines FUJITA Tomonori
2024-05-12 8:56 ` [PATCH net-next v6 3/6] net: tn40xx: add basic Tx handling FUJITA Tomonori
2024-05-12 8:56 ` [PATCH net-next v6 4/6] net: tn40xx: add basic Rx handling FUJITA Tomonori
2024-05-13 18:37 ` Joe Damato
2024-05-15 9:37 ` FUJITA Tomonori [this message]
2024-05-12 8:56 ` [PATCH net-next v6 5/6] net: tn40xx: add mdio bus support FUJITA Tomonori
2024-05-13 9:40 ` Naveen Mamindlapalli
2024-05-15 10:29 ` FUJITA Tomonori
2024-05-12 8:56 ` [PATCH net-next v6 6/6] net: tn40xx: add phylink support FUJITA Tomonori
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=20240515.183719.2094245718391165470.fujita.tomonori@gmail.com \
--to=fujita.tomonori@gmail.com \
--cc=andrew@lunn.ch \
--cc=hfdevel@gmx.net \
--cc=horms@kernel.org \
--cc=jdamato@fastly.com \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).