| Message ID | 1460838941-1251989-1-git-send-email-arnd@arndb.de |
|---|---|
| State | Accepted |
| Delegated to: | Jeff Kirsher |
| Headers | show |
From: Arnd Bergmann <arnd@arndb.de> Date: Sat, 16 Apr 2016 22:35:08 +0200 > The newly added x550em_a support causes a link failure on ARM because of > an overly long time passed into udelay(): > > ERROR: "__bad_udelay" [drivers/net/ethernet/intel/ixgbe/ixgbe.ko] undefined! > > There are multiple variants of the ixgbe_acquire_swfw_sync_*() function, > and the other ones all use msleep(), so we can safely assume that all > callers are allowed to sleep, which makes msleep() a better replacement > than mdelay(). > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Fixes: 49425dfc7451 ("ixgbe: Add support for x550em_a 10G MAC type") I'm assuming Jeff will pick this up.
- Most others David Miller <davem@davemloft.net> wrote: > From: Arnd Bergmann <arnd@arndb.de> > Date: Sat, 16 Apr 2016 22:35:08 +0200 > >> The newly added x550em_a support causes a link failure on ARM because of >> an overly long time passed into udelay(): >> >> ERROR: "__bad_udelay" [drivers/net/ethernet/intel/ixgbe/ixgbe.ko] >> undefined! >> >> There are multiple variants of the ixgbe_acquire_swfw_sync_*() function, >> and the other ones all use msleep(), so we can safely assume that all >> callers are allowed to sleep, which makes msleep() a better replacement >> than mdelay(). >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> Fixes: 49425dfc7451 ("ixgbe: Add support for x550em_a 10G MAC type") > > I'm assuming Jeff will pick this up. Actually, I'm not at all sure that any of these are safe to sleep, since I think PHY accesses can be made by ethtool which holds the rtnl lock. -- Mark Rustad, MRustad@gmail.com
> -----Original Message----- > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On > Behalf Of Arnd Bergmann > Sent: Saturday, April 16, 2016 1:35 PM > To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com> > Cc: Arnd Bergmann <arnd@arndb.de>; linux-kernel@vger.kernel.org; intel- > wired-lan@lists.osuosl.org; netdev@vger.kernel.org > Subject: [Intel-wired-lan] [PATCH] ixgbe: use msleep for long delays > > The newly added x550em_a support causes a link failure on ARM because of > an overly long time passed into udelay(): > > ERROR: "__bad_udelay" [drivers/net/ethernet/intel/ixgbe/ixgbe.ko] > undefined! > > There are multiple variants of the ixgbe_acquire_swfw_sync_*() function, > and the other ones all use msleep(), so we can safely assume that all callers > are allowed to sleep, which makes msleep() a better replacement than > mdelay(). > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Fixes: 49425dfc7451 ("ixgbe: Add support for x550em_a 10G MAC type") > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Tested-by: Andrew Bowers <andrewx.bowers@intel.com> Able to cross-compile driver for ARM architecture without sleep errors or warnings from compiler
AndrewX <andrewx.bowers@intel.com> wrote: >> -----Original Message----- >> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On >> Behalf Of Arnd Bergmann >> Sent: Saturday, April 16, 2016 1:35 PM >> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com> >> Cc: Arnd Bergmann <arnd@arndb.de>; linux-kernel@vger.kernel.org; intel- >> wired-lan@lists.osuosl.org; netdev@vger.kernel.org >> Subject: [Intel-wired-lan] [PATCH] ixgbe: use msleep for long delays >> >> The newly added x550em_a support causes a link failure on ARM because of >> an overly long time passed into udelay(): >> >> ERROR: "__bad_udelay" [drivers/net/ethernet/intel/ixgbe/ixgbe.ko] >> undefined! >> >> There are multiple variants of the ixgbe_acquire_swfw_sync_*() function, >> and the other ones all use msleep(), so we can safely assume that all >> callers >> are allowed to sleep, which makes msleep() a better replacement than >> mdelay(). >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> Fixes: 49425dfc7451 ("ixgbe: Add support for x550em_a 10G MAC type") >> --- >> drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Tested-by: Andrew Bowers <andrewx.bowers@intel.com> > Able to cross-compile driver for ARM architecture without sleep errors or > warnings from compiler > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan@lists.osuosl.org > http://lists.osuosl.org/mailman/listinfo/intel-wired-lan Was this functionally tested? I am concerned that we can get sleeping while atomic errors from ethtool operations with this in place. Compiling successfully does not test that. -- Mark Rustad, Networking Division, Intel Corporation
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c index c71e93ed4451..21c0fce47e15 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c @@ -2765,7 +2765,7 @@ static s32 ixgbe_acquire_swfw_sync_x550em_a(struct ixgbe_hw *hw, u32 mask) ixgbe_release_swfw_sync_X540(hw, hmask); if (status != IXGBE_ERR_TOKEN_RETRY) return status; - udelay(FW_PHY_TOKEN_DELAY * 1000); + msleep(FW_PHY_TOKEN_DELAY); } return status;
The newly added x550em_a support causes a link failure on ARM because of an overly long time passed into udelay(): ERROR: "__bad_udelay" [drivers/net/ethernet/intel/ixgbe/ixgbe.ko] undefined! There are multiple variants of the ixgbe_acquire_swfw_sync_*() function, and the other ones all use msleep(), so we can safely assume that all callers are allowed to sleep, which makes msleep() a better replacement than mdelay(). Signed-off-by: Arnd Bergmann <arnd@arndb.de> Fixes: 49425dfc7451 ("ixgbe: Add support for x550em_a 10G MAC type") --- drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)