[PATCH 2/2] netdev/fec: fix performance impact from mdio poll operation

[Eric Miao]

On Wed, Apr 14, 2010 at 6:57 AM, Eric Miao <eric.miao at canonical.com> wrote:
> On Wed, Apr 14, 2010 at 5:37 AM, Bryan Wu <bryan.wu at canonical.com> wrote:
>> On 04/13/2010 12:09 PM, Andy Whitcroft wrote:
>>> On Thu, Apr 08, 2010 at 02:44:59PM +0800, Bryan Wu wrote:
>>>> BugLink: http://bugs.launchpad.net/bugs/546649
>>>> BugLink: http://bugs.launchpad.net/bugs/457878
>>>>
>>>> After introducing phylib supporting, users experienced performace drop. That is
>>>> because of the mdio polling operation of phylib. Use msleep to replace the busy
>>>> waiting cpu_relax() and remove the warning message.
>>>>
>>>> Signed-off-by: Bryan Wu<bryan.wu at canonical.com>
>>>> ---
>>>>   drivers/net/fec.c |   45 +++++++++++++++++++++------------------------
>>>>   1 files changed, 21 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
>>>> index 53240d3..2280373 100644
>>>> --- a/drivers/net/fec.c
>>>> +++ b/drivers/net/fec.c
>>>> @@ -216,7 +216,7 @@ static void fec_stop(struct net_device *dev);
>>>>   #define FEC_MMFR_TA                (2<<  16)
>>>>   #define FEC_MMFR_DATA(v)   (v&  0xffff)
>>>>
>>>> -#define FEC_MII_TIMEOUT             10000
>>>> +#define FEC_MII_TIMEOUT             10
>>>>
>>>>   /* Transmitter timeout */
>>>>   #define TX_TIMEOUT (2 * HZ)
>>>> @@ -624,13 +624,29 @@ spin_unlock:
>>>>   /*
>>>>    * NOTE: a MII transaction is during around 25 us, so polling it...
>>>>    */
>>>> -static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
>>>> +static int fec_enet_mdio_poll(struct fec_enet_private *fep)
>>>>   {
>>>> -    struct fec_enet_private *fep = bus->priv;
>>>>      int timeout = FEC_MII_TIMEOUT;
>>>>
>>>>      fep->mii_timeout = 0;
>>>>
>>>> +    /* wait for end of transfer */
>>>> +    while (!(readl(fep->hwp + FEC_IEVENT)&  FEC_ENET_MII)) {
>>>> +            msleep(1);
>>>> +            if (timeout--<  0) {
>>>> +                    fep->mii_timeout = 1;
>>>> +                    break;
>>>> +            }
>>>> +    }
>>>> +
>
> This is ugly, I'd suggest wait_event_timeout(), though the response
> time will be worse instead.
>
> msleep(1) is almost equivalent on a system with HZ == 100.
>

Typo, sorry. I mean:

msleep(1) is almost equivalent to msleep(10) on a system with HZ == 100. Won't
be too much response improvement.