From: Casey Leedom <leedom@chelsio•com>
To: Stephen Hemminger <shemminger@linux-foundation•org>
Cc: Dimitris Michailidis <dm@chelsio•com>,
Ben Hutchings <bhutchings@solarflare•com>,
David Miller <davem@davemloft•net>,
netdev@vger•kernel.org
Subject: Re: [PATCH net-next] cxgb4: don't hold RTNL during ethtool phys_id
Date: Wed, 6 Apr 2011 17:20:29 -0700 [thread overview]
Message-ID: <201104061720.30219.leedom@chelsio.com> (raw)
In-Reply-To: <20110406170929.6e427b36@nehalam>
| From: Stephen Hemminger <shemminger@linux-foundation•org>
| Date: Wednesday, April 06, 2011 05:09 pm
|
| The Chelsio cxgb4 drivers implement blinking in a unique way by
| waiting on the mailbox. This patch cleans it up slightly by no longer
| holding the system wide network configuration lock during the process.
|
| The patch also uses correct semantics for the time argument
| which is supposed to be in seconds; and zero is supposed
| to signify infinite blinking.
|
| This is still a bad firmware interface design for this
| since it means the board is basically hung while doing the blink.
| But fixing it correctly would require hardware and firmware
| documentation. With that information the device could be converted
| to the new set_phys_id.
|
| Compile tested only.
|
| Signed-off-by: Stephen Hemminger <shemminger@vyatta•com>
Are you assuming that the firmware won't respond with a command completion
until the LED blinking is complete? If so, that's a bad assumption. The
firmware runs as an asynchronous real-time OS. The LED blinking simply becomes
a thread of activity within the OS and the command completes immediately.
Casey
| ---
| drivers/net/cxgb4/cxgb4_main.c | 17 ++++++++++++++---
| drivers/net/cxgb4vf/cxgb4vf_main.c | 21 +++++++++++++++++++--
| 2 files changed, 33 insertions(+), 5 deletions(-)
|
| --- a/drivers/net/cxgb4/cxgb4_main.c 2011-04-06 16:49:02.045648800 -0700
| +++ b/drivers/net/cxgb4/cxgb4_main.c 2011-04-06 17:00:59.508851692 -0700
| @@ -1339,12 +1339,23 @@ static int restart_autoneg(struct net_de
| static int identify_port(struct net_device *dev, u32 data)
| {
| struct adapter *adap = netdev2adap(dev);
| + int rc;
| + unsigned long blinks;
|
| if (data == 0)
| - data = 2; /* default to 2 seconds */
| + blinks = UINT_MAX;
| + else
| + blinks = 2*data + data/2;
|
| - return t4_identify_port(adap, adap->fn, netdev2pinfo(dev)->viid,
| - data * 5);
| + /* Don't block networking updates while blink is in progress */
| + dev_hold(dev);
| + rtnl_unlock();
| +
| + rc = t4_identify_port(adap, adap->fn, netdev2pinfo(dev)->viid,
| + blinks);
| + rtnl_lock();
| + dev_put(dev);
| + return rc;
| }
|
| static unsigned int from_fw_linkcaps(unsigned int type, unsigned int caps)
| --- a/drivers/net/cxgb4vf/cxgb4vf_main.c 2011-04-06 16:49:09.989728600
| -0700 +++ b/drivers/net/cxgb4vf/cxgb4vf_main.c 2011-04-06
| 17:02:38.609846223 -0700 @@ -43,6 +43,7 @@
| #include <linux/etherdevice.h>
| #include <linux/debugfs.h>
| #include <linux/ethtool.h>
| +#include <linux/rtnetlink.h>
|
| #include "t4vf_common.h"
| #include "t4vf_defs.h"
| @@ -1352,11 +1353,27 @@ static int cxgb4vf_set_rx_csum(struct ne
| /*
| * Identify the port by blinking the port's LED.
| */
| -static int cxgb4vf_phys_id(struct net_device *dev, u32 id)
| +static int cxgb4vf_phys_id(struct net_device *dev, u32 data)
| {
| struct port_info *pi = netdev_priv(dev);
| + int rc;
| + unsigned int blinks;
|
| - return t4vf_identify_port(pi->adapter, pi->viid, 5);
| + if (data == 0)
| + blinks = UINT_MAX;
| + else
| + blinks = 2*data + data/2;
| +
| + /* Don't block networking updates while blink is in progress */
| + dev_hold(dev);
| + rtnl_unlock();
| +
| + rc = t4vf_identify_port(pi->adapter, pi->viid, blinks);
| +
| + rtnl_lock();
| + dev_put(dev);
| +
| + return rc;
| }
|
| /*
next prev parent reply other threads:[~2011-04-07 0:21 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-07 0:09 [PATCH net-next] cxgb4: don't hold RTNL during ethtool phys_id Stephen Hemminger
2011-04-07 0:20 ` Casey Leedom [this message]
2011-04-07 0:33 ` Stephen Hemminger
2011-04-07 8:18 ` Dimitris Michailidis
2011-04-07 0:35 ` Ben Hutchings
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201104061720.30219.leedom@chelsio.com \
--to=leedom@chelsio$(echo .)com \
--cc=bhutchings@solarflare$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=dm@chelsio$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=shemminger@linux-foundation$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox