[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 5 May 2011 20:35:42 +0200
From: Michał Mirosław <mirqus@...il.com>
To: Mahesh Bandewar <maheshb@...gle.com>
Cc: Matt Carlson <mcarlson@...adcom.com>,
David Miller <davem@...emloft.net>,
netdev <netdev@...r.kernel.org>,
Michael Chan <mchan@...adcom.com>,
Tom Herbert <therbert@...gle.com>
Subject: Re: [PATCHv4 2/2] tg3: Allow ethtool to enable/disable loopback.
W dniu 5 maja 2011 19:47 użytkownik Mahesh Bandewar
<maheshb@...gle.com> napisał:
> On Wed, May 4, 2011 at 11:59 PM, Michał Mirosław <mirqus@...il.com> wrote:
>> 2011/5/5 Mahesh Bandewar <maheshb@...gle.com>:
>>> This patch adds tg3_set_features() to handle loopback mode. Currently the
>>> capability is added for the devices which support internal MAC loopback mode.
>>> So when enabled, it enables internal-MAC loopback.
[...]
>>> @@ -9485,6 +9533,15 @@ static int tg3_open(struct net_device *dev)
>>>
>>> netif_tx_start_all_queues(dev);
>>>
>>> + /*
>>> + * Reset loopback feature if it was turned on while the device was down
>>> + * to avoid and any discrepancy in features reporting.
>>> + */
>>> + if (dev->features & NETIF_F_LOOPBACK) {
>>> + dev->features &= ~NETIF_F_LOOPBACK;
>>> + dev->wanted_features &= ~NETIF_F_LOOPBACK;
>>> + }
>>> +
>>
>> if (dev->features & NETIF_F_LOOPBACK)
>> tg3_set_loopback(dev, dev->features);
>>
> Unfortunately at this stage device will not be able to set-loopback so
> I resorted to clearing the bit(s).
ndo_fix_features+ndo_set_features might be caled before ndo_open - the
first might be just from register_netdev() so even before ndo_open
callback.
>> Whatever you do, don't modify wanted_features in drivers.
> Since just clearing the 'features' would leave wanted_features in
> discrepant state, I thought this will bring it to a sane state. So
> what is a preferred way?
If you really can't do other way, driver should keep additional state
that is checked in ndo_fix_features callback.
>> BTW, similar problems (also like in previous versions) are in
>> forcedeth implementation.
> Yes, whatever we decide about the state of the wanted_features, I'll
> implement similarly for forcedeth. Which previous problem you are
> referring to? Is it the return value? There is a different kind of
> failure (error while writing the register). Since update_features()
> cant handle return value, I'm ignoring the return value. As far as the
> correct return code is concerned, I wasn't sure what is appropriate
> return code here (may be PHY_ERROR would be appropriate there) but
> again I could ignore it just the way rest of the code is ignoring.
Let's wait for this threads points to be resolved. You will probably
change the forcedeth's implementation then anyway. :-)
Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists