Xilinx Temac Timer ?

John Linn John.Linn at xilinx.com
Thu Mar 27 06:48:34 EST 2008

Hi Kevin,

I didn't write the code but I know the driver somewhat.

I think the intention of stopping the timer is to prevent the reentrancy
as the comment says because there is a function, gmii_poll, that is
setup on the timer to go read the phy registers to see if anything
changed in the phy. 

Stopping the timer prevents a phy read from happening in gmii_poll in
the middle of the ioctl phy read which could hose things up.

I don't see why you couldn't change that timer stop to some other form
of synchronization/mutual exclusion so that the phy reads don't collide.

As I look at it, it appears to me the spinlock should provide the
synchronization needed without stopping the timer, but maybe I'm missing
something. I CCed John Bonesio as he's the guy that developed this code
I believe and maybe he'll have more insight here.


-----Original Message-----
From: linuxppc-embedded-bounces+john.linn=xilinx.com at ozlabs.org
[mailto:linuxppc-embedded-bounces+john.linn=xilinx.com at ozlabs.org] On
Behalf Of khollan
Sent: Wednesday, March 26, 2008 11:03 AM
To: linuxppc-embedded at ozlabs.org
Subject: Xilinx Temac Timer ?


What is the purpose of stopping the timer in the ioctl call to read a
register?  This is the code:

    case SIOCGMIIREG:   /* Read GMII PHY register. */
    case SIOCDEVPRIVATE + 1:    /* for binary compat, remove in 2.5 */
        if (data->phy_id > 31 || data->reg_num > 31)
            return -ENXIO;

        /* Stop the PHY timer to prevent reentrancy. */
        spin_lock_irqsave(&XTE_spinlock, flags);

        ret = XTemac_PhyRead(&lp->Emac, data->phy_id,
                       data->reg_num, &data->val_out);

        /* Start the PHY timer up again. */
        lp->phy_timer.expires = jiffies + 2 * HZ;
        spin_unlock_irqrestore(&XTE_spinlock, flags);
        if (ret != XST_SUCCESS) {
                   "%s: XTemac: could not read from PHY, error=%d.\n",
                   dev->name, ret);
            return -EBUSY;
        return 0;

I ask because I have an application that needs to read a Phy register
the timer has started to see if a link is present.  This causes a kernel
when trying to run the del_timer_sync function because there is not a
running timer.  I would like to know if it is safe to remove the timer
and add but keep the spin lock.

Thanks for your help

View this message in context:
Sent from the linuxppc-embedded mailing list archive at Nabble.com.

Linuxppc-embedded mailing list
Linuxppc-embedded at ozlabs.org

More information about the Linuxppc-embedded mailing list