Re: [PATCH v3] net: add Faraday FTMAC100 10/100 Ethernet driver

Dear Joe,

On Fri, Jan 21, 2011 at 2:46 PM, Joe Perches [off-list ref] wrote:

On Fri, 2011-01-21 at 13:03 +0800, Po-Yu Chuang wrote:

quoted

quoted

Is it useful to retry the NORXBUF case?

The idea is that if I miss packet finished interrupts (then rx buffers used up),
I should retrieve the received packets ASAP to free buffers to HW.
Maybe this is really unnecessary.
I am not quite sure, but I'll do your advice now.

I wasn't giving advice, just asking a question.
Your concept makes sense to me.

I see. So I will leave it as is.

quoted

quoted

quoted

+     if (status & FTMAC100_INT_NORXBUF) {
+             /* RX buffer unavailable */
+             if (net_ratelimit())
+                     netdev_info(netdev, "INT_NORXBUF\n");
+
+             netdev->stats.rx_over_errors++;
+     }

Perhaps this "if (status & FTMAC100_INT_NORXBUF)" block should be
moved into the test block above it before the retry?

Since status is not changed in the function, it does not matter where
the test is.
But I agree that it is better to handle error cases earlier.

This wasn't so much a handle error case early, but
a suggestion that
       if (status & (foo | bar)) {
               ...
       }
       if (status & bar) {
               ...
       }
should be
       if (status & (foo | bar)) {
               ...
               if (status & bar) {
                       ...
               }
       }

so that when the first test fails, a known
subset of the first test isn't tested again.

Understand. Thanks.

best regards,
Po-Yu Chuang