Hi Greg,
I have tested the original FEC driver with IRQF_DISABLED and it works fine.

***** NOTE *****
When I refer to "fec tests" I mean to perform several attacks to the board
by downloading and uploading files thru BOA web server, simultaneously.
****************

However, I'd like to propose to replace the while() statement in
fec_enet_interrupt() for
a simple if() because, since we are disabling INT's, a highly loaded network
would
make the ISR to keep running for a possibly not desired long period of time
(think in, not
real time, but maybe in audio applications?? - don't know). Actually I don't
know whether setting
IRQF_DISABLED and a single if() statement would also affect the overall
performance. Disabling nested
interrupts may lead to a poor system performance for specific applications.
My last FEC driver is a little bit different from the first I dumped in the
mailing list: as you
said in the last email, here, besides of acknowledging the interrupts after
attending them, here
I attend only to 1 interrupt at a time.

Since fec_enet_interrupt is quite short, I show the whole modified function
below:

static irqreturn_t
fec_enet_interrupt(int irq, void * dev_id)
{
    struct    net_device *dev = dev_id;
    volatile fec_t    *fecp;
    uint    int_events;
    int handled = 0;

    fecp = (volatile fec_t*)dev->base_addr;

    /* Get the interrupt events that caused us to be here.
    */
    /* Not a while() statement, but an if() one */
    if ((int_events = (fecp->fec_ievent &
(FEC_ENET_RXF|FEC_ENET_TXF|FEC_ENET_MII))) != 0) {

        /* Handle receive event in its own function.
        */
        if (int_events & FEC_ENET_RXF) {
            handled = 1;
            fec_enet_rx(dev);
            fecp->fec_ievent = FEC_ENET_RXF;
            return IRQ_RETVAL(handled);            /* Return now */
        }

        /* Transmit OK, or non-fatal error. Update the buffer
           descriptors. FEC handles all errors, we just discover
           them as part of the transmit process.
        */
        if (int_events & FEC_ENET_TXF) {
            handled = 1;
            fec_enet_tx(dev);
            fecp->fec_ievent = FEC_ENET_TXF;
            return IRQ_RETVAL(handled);            /* Return now */
        }

        if (int_events & FEC_ENET_MII) {
            handled = 1;
            fec_enet_mii(dev);
            fecp->fec_ievent = FEC_ENET_MII;
            return IRQ_RETVAL(handled);            /* Return now */
        }
}

    return IRQ_RETVAL(handled);
}


This version of fec_enet_interrupt() works fine to me, but I must say that
performance in network
communications is notably worsen.

Thus, I would like to know if you agree to make a patch that requests ints
with IRQF_DISABLED
but attending the interrupt with an if() statement instead of a while().
It's just only to balance
the overall system performance. The function would stay as follows (again,
IRQF_DISABLED should be
set when calling request_irq):


static irqreturn_t
fec_enet_interrupt(int irq, void * dev_id)
{
    struct    net_device *dev = dev_id;
    volatile fec_t    *fecp;
    uint    int_events;
    int handled = 0;

    fecp = (volatile fec_t*)dev->base_addr;

    /* Get the interrupt events that caused us to be here.
    */
    if ((int_events = fecp->fec_ievent) != 0) {
        fecp->fec_ievent = int_events;

        /* Handle receive event in its own function.
        */
        if (int_events & FEC_ENET_RXF) {
            handled = 1;
            fec_enet_rx(dev);
        }

        /* Transmit OK, or non-fatal error. Update the buffer
           descriptors. FEC handles all errors, we just discover
           them as part of the transmit process.
        */
        if (int_events & FEC_ENET_TXF) {
            handled = 1;
            fec_enet_tx(dev);
        }

        if (int_events & FEC_ENET_MII) {
            handled = 1;
            fec_enet_mii(dev);
        }

    }
    return IRQ_RETVAL(handled);
}

I have also tested this code and it performs the same as the version with
the while()
statement. However this way may be better for specific applications that
require
quick interrupt processing.


Regards.
Claude
_______________________________________________
uClinux-dev mailing list
[email protected]
http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
This message was resent by [email protected]
To unsubscribe see:
http://mailman.uclinux.org/mailman/options/uclinux-dev

Reply via email to