Misc Intel driver cleanups
Subsystems (2)
cover Misc Intel driver cleanups
These are some assorted cleanups that occurred to me over the last several days. They are compile tested only.
The first three are based on review of some patches I saw recently accepted into fm10k. We weren't doing consistent exception handling when we probably should have been. Also it looks like there are some exception paths what would have likely resulted in a kernel panic so I decided to try and address those.
The fourth is based on a suggestion I had given to Nick but he ignored. Basically e1000e_up is always returning 0, there was code that was checking for it returning non-zero so I make e1000e_up void and dropped all code that was checking it for a non-zero return value.
The last patch is a minor tweak to an existing patch in the dev-queue. Feel free to just fold it into the patch it fixes and add my signed-off-by or carry it separately. I am good either way.
---
Alexander Duyck (5):
fm10k: Cleanup MSI-X interrupts in case of failure
fm10k: Cleanup exception handling for mailbox interrupt
fm10k: Cleanup exception handling for changing queues
e1000e: Switch e1000e_up to void, drop code checking for error result
igb: Fix unused variable warning
drivers/net/ethernet/intel/e1000e/e1000.h | 2 - drivers/net/ethernet/intel/e1000e/netdev.c | 15 +---- drivers/net/ethernet/intel/fm10k/fm10k_main.c | 4 + drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 17 ++++-- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 68 ++++++++++++++++++----- drivers/net/ethernet/intel/igb/igb_main.c | 1 6 files changed, 74 insertions(+), 33 deletions(-)
--
PATCH 1/5 fm10k: Cleanup MSI-X interrupts in case of failure
If the q_vector allocation fails we should free the resources associated with the MSI-X vector table.
--- drivers/net/ethernet/intel/fm10k/fm10k_main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/net/ethernet/intel/fm10k/fm10k_main.c +3 -1
@@ -2002,8 +2002,10 @@ int fm10k_init_queueing_scheme(struct fm10k_intfc *interface)/* Allocate memory for queues */ err =fm10k_alloc_q_vectors (interface); -if (err) +if (err) { +fm10k_reset_msix_capability (interface);return err; + }/* Map rings to devices, and map devices to physical queues */ fm10k_assign_rings (interface);
Allan
PATCH 1/5 fm10k: Cleanup MSI-X interrupts in case of failure
Quoted 8 lines
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure
Quoted 7 lines
If the
q_vectorallocation fails we should free the resources associated with the MSI-X vector table.Signed-off-by Alexander Duyck <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Singh
PATCH 1/5 fm10k: Cleanup MSI-X interrupts in case of failure
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Allan, Bruce W
Sent: Wednesday, October 28, 2015 9:11 AM
To: Alexander Duyck <[email protected]>; intel-wired-lan at lists.osuosl.org
Subject: Re: [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure
Quoted 9 lines
-----Original Message-----
From: Intel-wired-lan
[mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure
Quoted 7 lines
If the
q_vectorallocation fails we should free the resources associated with the MSI-X vector table.Signed-off-by Alexander Duyck <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
PATCH 2/5 fm10k: Cleanup exception handling for mailbox interrupt
In reply to Alexander DuyckThis patch addresses two issues.
First is the fact that the fm10k_mbx_free_irq was assuming msix_entries was valid and that will not always be the case. As such we need to add a check for if it is NULL.
Second is the fact that we weren't freeing the IRQ if the mailbox API returned an error on trying to connect.
--- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +11 -2
@@ -1131,6 +1131,10 @@ void fm10k_mbx_free_irq(struct fm10k_intfc *interface)struct fm10k_hw *hw = &interface->hw;int itr_reg; +/* no mailbox IRQ to free if MSI-X is not enabled */ +if (!interface->msix_entries) +return ; +/* disconnect the mailbox */ hw->mbx.ops.disconnect (hw, &hw->mbx);
@@ -1453,10 +1457,15 @@ int fm10k_mbx_request_irq(struct fm10k_intfc *interface) err =fm10k_mbx_request_irq_pf (interface); else err =fm10k_mbx_request_irq_vf (interface); +if (err) +return err;/* connect mailbox */ -if (!err) - err = hw->mbx.ops.connect (hw, &hw->mbx); + err = hw->mbx.ops.connect (hw, &hw->mbx); + +/* if the mailbox failed to connect, then free IRQ */ +if (err) +fm10k_mbx_free_irq (interface);return err; }
Allan
PATCH 2/5 fm10k: Cleanup exception handling for mailbox interrupt
Quoted 15 lines
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception handling for mailbox interruptThis patch addresses two issues.
First is the fact that the
fm10k_mbx_free_irqwas assumingmsix_entrieswas
valid and that will not always be the case. As such we need to add a check for if it is NULL.
Quoted 7 lines
Second is the fact that we weren't freeing the IRQ if the mailbox API returned an error on trying to connect.
Signed-off-by Alexander Duyck <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
Singh
PATCH 2/5 fm10k: Cleanup exception handling for mailbox interrupt
In reply to Alexander Duyck-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: [email protected]
Subject: [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception handling for mailbox interrupt
This patch addresses two issues.
First is the fact that the fm10k_mbx_free_irq was assuming msix_entries was valid and that will not always be the case. As such we need to add a check for if it is NULL.
Second is the fact that we weren't freeing the IRQ if the mailbox API returned an error on trying to connect.
---
PATCH 3/5 fm10k: Cleanup exception handling for changing queues
In reply to Alexander DuyckThis patch is meant to cleanup the exception handling for the paths where we reset the interrupts and then reconfigure them. In all of these paths we had very different levels of exception handling. I have updated the driver so that all of the paths should result in a similar state if we fail.
Specifically the driver will now unload the mailbox interrupt, free the queue vectors and MSI-X, and then detach the interface.
In addition for any of the PCIe related resets I have added a check with the hw_ready function to just make sure the registers are in a readable state prior to reopening the interface.
--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 17 +++++-- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 55 ++++++++++++++++++----- 2 files changed, 55 insertions(+), 17 deletions(-)
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +13 -4
@@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device *dev, u8 tc) err =fm10k_init_queueing_scheme (interface);if (err) -return err; + goto err_queueing_scheme; err =fm10k_mbx_request_irq (interface);if (err) -return err; + goto err_mbx_irq; -if (netif_running (dev)) -fm10k_open (dev); + err =netif_running (dev) ?fm10k_open (dev) :0 ; +if (err) + goto err_open;/* flag to indicate SWPRI has yet to be updated */ interface->flags |=FM10K_FLAG_SWPRI_CONFIG ;return 0 ; +err_open: +fm10k_mbx_free_irq (interface); +err_mbx_irq: +fm10k_clear_queueing_scheme (interface); +err_queueing_scheme: +netif_device_detach (dev); + +return err; }static int fm10k_ioctl (struct net_device *netdev,struct ifreq *ifr,int cmd)
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +42 -13
85 lines not on this page · raw message
Allan
PATCH 3/5 fm10k: Cleanup exception handling for changing queues
Quoted 21 lines
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queuesThis patch is meant to cleanup the exception handling for the paths where we reset the interrupts and then reconfigure them. In all of these paths we had very different levels of exception handling. I have updated the driver so that all of the paths should result in a similar state if we fail.
Specifically the driver will now unload the mailbox interrupt, free the queue vectors and MSI-X, and then detach the interface.
In addition for any of the PCIe related resets I have added a check with the
hw_readyfunction to just make sure the registers are in a readable state prior to reopening the interface.
Quoted 6 lines
Signed-off-by Alexander Duyck <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 17 +++++-- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 55 ++++++++++++++++++----- 2 files changed, 55 insertions(+), 17 deletions(-)
Allan
PATCH 3/5 fm10k: Cleanup exception handling for changing queues
In reply to Alexander DuyckQuoted 90 lines
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queuesThis patch is meant to cleanup the exception handling for the paths where we reset the interrupts and then reconfigure them. In all of these paths we had very different levels of exception handling. I have updated the driver so that all of the paths should result in a similar state if we fail.
Specifically the driver will now unload the mailbox interrupt, free the queue vectors and MSI-X, and then detach the interface.
In addition for any of the PCIe related resets I have added a check with the
hw_readyfunction to just make sure the registers are in a readable state prior to reopening the interface.Signed-off-by Alexander Duyck <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 17 +++++-- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 55 ++++++++++++++++++----- 2 files changed, 55 insertions(+), 17 deletions(-)drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +13
-4@@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device *dev, u8 tc) err =fm10k_init_queueing_scheme (interface);if (err) -return err; + goto err_queueing_scheme; err =fm10k_mbx_request_irq (interface);if (err) -return err; + goto err_mbx_irq; -if (netif_running (dev)) -fm10k_open (dev); + err =netif_running (dev) ?fm10k_open (dev) :0 ; +if (err) + goto err_open;/* flag to indicate SWPRI has yet to be updated */ interface->flags |=FM10K_FLAG_SWPRI_CONFIG ;return 0 ; +err_open: +fm10k_mbx_free_irq (interface); +err_mbx_irq: +fm10k_clear_queueing_scheme (interface); +err_queueing_scheme: +netif_device_detach (dev); + +return err; }static int
fm10k_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)drivers/net/ethernet/intel/fm10k/fm10k_pci.c +7
-1@@ -185,7 +185,13 @@ static void fm10k_reinit(struct fm10k_intfc *interface) }/* reassociate interrupts */ -fm10k_mbx_request_irq (interface); + err =fm10k_mbx_request_irq (interface); +if (err) + goto err_mbx_irq; + + err =fm10k_hw_ready (interface); +if (err) + goto err_open;/* update hardware address for VFs if perm_addr has changed */ drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0
-0if (hw->mac.type == fm10k_mac_vf) {@@ -205,14 +211,23 @@ static void fm10k_reinit(struct fm10k_intfc *interface)
drivers/net/ethernet/intel/fm10k/fm10k_pci.c static void fm10k_reinit(struct fm10k_intfc · line 218
Quoted 10 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4
-3/* reset clock */ fm10k_ts_reset (interface); -if (netif_running (netdev)) -fm10k_open (netdev); - + err =netif_running (netdev) ?fm10k_open (netdev) :0 ; +if (err) + goto err_open; +
Jeff, the above blank-line separator line in Alex' original submission looks fine here but in your next-queue tree dev-queue branch there is additional whitespace (2 tabs). How did that happen?
Quoted 34 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +15
-8fm10k_iov_resume (interface->pdev); +rtnl_unlock (); + +clear_bit (__FM10K_RESETTING, &interface->state); + +return ; +err_open: +fm10k_mbx_free_irq (interface); +err_mbx_irq: +fm10k_clear_queueing_scheme (interface); reinit_err: -if (err) -netif_device_detach (netdev); +netif_device_detach (netdev);rtnl_unlock ();@@ -2130,16 +2145,22 @@ static int fm10k_resume(struct pci_dev *pdev)rtnl_lock (); err =fm10k_init_queueing_scheme (interface); -if (!err) { -fm10k_mbx_request_irq (interface); -if (netif_running (netdev)) - err =fm10k_open (netdev); - } +if (err) + goto err_queueing_scheme; -rtnl_unlock (); + err =fm10k_mbx_request_irq (interface); +if (err) + goto err_mbx_irq;
drivers/net/ethernet/intel/fm10k/fm10k_pci.c static int fm10k_resume(struct pci_dev *pdev) · line 2162
Quoted 10 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +7
-1+ err =fm10k_hw_ready (interface);if (err) -return err; + goto err_open; + + err =netif_running (netdev) ?fm10k_open (netdev) :0 ; +if (err) + goto err_open; +
Ditto here
Quoted 25 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +1
-0+rtnl_unlock ();/* assume host is not ready, to prevent race with watchdog in case we
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +8
-0* actually don't have connection to the switch @@ -2157,6 +2178,14 @@ static int fm10k_resume(struct pci_dev *pdev)netif_device_attach (netdev);return 0 ; +err_open: +fm10k_mbx_free_irq (interface); +err_mbx_irq: +fm10k_clear_queueing_scheme (interface); +err_queueing_scheme: +rtnl_unlock (); + +return err; }/** _______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan
PATCH 3/5 fm10k: Cleanup exception handling for changing queues
Quoted 93 lines · Bruce W
Quoted 93 lines
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queuesThis patch is meant to cleanup the exception handling for the paths where we reset the interrupts and then reconfigure them. In all of these paths we had very different levels of exception handling. I have updated the driver so that all of the paths should result in a similar state if we fail.
Specifically the driver will now unload the mailbox interrupt, free the queue vectors and MSI-X, and then detach the interface.
In addition for any of the PCIe related resets I have added a check with the
hw_readyfunction to just make sure the registers are in a readable state prior to reopening the interface.Signed-off-by Alexander Duyck <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 17 +++++-- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 55 ++++++++++++++++++----- 2 files changed, 55 insertions(+), 17 deletions(-)drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +13
-4@@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device *dev, u8 tc) err =fm10k_init_queueing_scheme (interface);if (err) -return err; + goto err_queueing_scheme; err =fm10k_mbx_request_irq (interface);if (err) -return err; + goto err_mbx_irq; -if (netif_running (dev)) -fm10k_open (dev); + err =netif_running (dev) ?fm10k_open (dev) :0 ; +if (err) + goto err_open;/* flag to indicate SWPRI has yet to be updated */ interface->flags |=FM10K_FLAG_SWPRI_CONFIG ;return 0 ; +err_open: +fm10k_mbx_free_irq (interface); +err_mbx_irq: +fm10k_clear_queueing_scheme (interface); +err_queueing_scheme: +netif_device_detach (dev); + +return err; }static int
fm10k_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)drivers/net/ethernet/intel/fm10k/fm10k_pci.c +7
-1@@ -185,7 +185,13 @@ static void fm10k_reinit(struct fm10k_intfc *interface) }/* reassociate interrupts */ -fm10k_mbx_request_irq (interface); + err =fm10k_mbx_request_irq (interface); +if (err) + goto err_mbx_irq; + + err =fm10k_hw_ready (interface); +if (err) + goto err_open;/* update hardware address for VFs if perm_addr has changed */ if (hw->mac.type == fm10k_mac_vf) {drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0
-0@@ -205,14 +211,23 @@ static void fm10k_reinit(struct fm10k_intfc *interface)/* reset clock */ fm10k_ts_reset (interface);
Quoted 10 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c static void fm10k_reinit(struct fm10k_intfc *interface) · line 217
Quoted 7 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4
-3-if (netif_running (netdev)) -fm10k_open (netdev); - + err =netif_running (netdev) ?fm10k_open (netdev) :0 ; +if (err) + goto err_open; +Jeff, the above blank-line separator line in Alex' original submission looks fine here but in your next-queue tree dev-queue branch there is additional whitespace (2 tabs). How did that happen?
I just went back and checked and it looks like I had included some trailing white space in my original patch. Jeff do you need me to resubmit this patch or can you clean up the two spots where it added trailing white space in the blank lines?
- Alex
Allan
PATCH 3/5 fm10k: Cleanup exception handling for changing queues
Quoted 117 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Thursday, October 29, 2015 12:39 PM
To: Allan, Bruce W; Kirsher, Jeffrey T; Alexander Duyck; intel-wired- lan at lists.osuosl.org
Subject: Re: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queuesQuoted 2 lines · Bruce W
Quoted 2 lines
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org]On
Quoted 7 lines
Quoted 7 lines
Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queuesThis patch is meant to cleanup the exception handling for the paths
where
Quoted 19 lines
Quoted 19 lines
we reset the interrupts and then reconfigure them. In all of these paths we had very different levels of exception handling. I have updated the driver so that all of the paths should result in a similar state if we fail.
Specifically the driver will now unload the mailbox interrupt, free the queue vectors and MSI-X, and then detach the interface.
In addition for any of the PCIe related resets I have added a check with the
hw_readyfunction to just make sure the registers are in a readable state prior to reopening the interface.Signed-off-by Alexander Duyck <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 17 +++++-- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 55 ++++++++++++++++++----- 2 files changed, 55 insertions(+), 17 deletions(-)Quoted 6 lines
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c int fm10k_setup_tc(struct net_device *dev,
Quoted 6 lines
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +0
-0@@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device *dev,u8
Quoted 32 lines
Quoted 32 lines
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +13
-4tc) err =fm10k_init_queueing_scheme (interface);if (err) -return err; + goto err_queueing_scheme; err =fm10k_mbx_request_irq (interface);if (err) -return err; + goto err_mbx_irq; -if (netif_running (dev)) -fm10k_open (dev); + err =netif_running (dev) ?fm10k_open (dev) :0 ; +if (err) + goto err_open;/* flag to indicate SWPRI has yet to be updated */ interface->flags |=FM10K_FLAG_SWPRI_CONFIG ;return 0 ; +err_open: +fm10k_mbx_free_irq (interface); +err_mbx_irq: +fm10k_clear_queueing_scheme (interface); +err_queueing_scheme: +netif_device_detach (dev); + +return err; }Quoted 1 line
Quoted 1 line
static int fm10k_ioctl(struct
net_device*netdev, struct ifreq *ifr, intcmd)
Quoted 26 lines
Quoted 26 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +7
-1@@ -185,7 +185,13 @@ static void fm10k_reinit(struct fm10k_intfc *interface) }/* reassociate interrupts */ -fm10k_mbx_request_irq (interface); + err =fm10k_mbx_request_irq (interface); +if (err) + goto err_mbx_irq; + + err =fm10k_hw_ready (interface); +if (err) + goto err_open;/* update hardware address for VFs if perm_addr has changed */ if (hw->mac.type == fm10k_mac_vf) {drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0
-0@@ -205,14 +211,23 @@ static void fm10k_reinit(struct fm10k_intfc *interface)/* reset clock */ fm10k_ts_reset (interface);Quoted 8 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c static void fm10k_reinit(struct fm10k_intfc *interface) · line 217
Quoted 7 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4
-3-if (netif_running (netdev)) -fm10k_open (netdev); - + err =netif_running (netdev) ?fm10k_open (netdev) :0 ; +if (err) + goto err_open; +Jeff, the above blank-line separator line in Alex' original submission looks
fine here but
Quoted 1 line
in your next-queue tree dev-queue branch there is additional whitespace
(2 tabs).
Quoted 8 lines
Quoted 1 line
How did that happen?
I just went back and checked and it looks like I had included some trailing white space in my original patch. Jeff do you need me to resubmit this patch or can you clean up the two spots where it added trailing white space in the blank lines?
- Alex
Jeff, are you going to fix the whitespace issues in this patch?
Thanks,
Bruce.
Allan
PATCH 3/5 fm10k: Cleanup exception handling for changing queues
Jeff? Ping!
Quoted 149 lines
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Allan, Bruce W
Sent: Tuesday, November 03, 2015 3:20 PM
To: Alexander Duyck; Kirsher, Jeffrey T; Alexander Duyck; intel-wired- lan at lists.osuosl.org
Subject: Re: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queuesQuoted 11 lines
-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Thursday, October 29, 2015 12:39 PM
To: Allan, Bruce W; Kirsher, Jeffrey T; Alexander Duyck; intel-wired- lan at lists.osuosl.org
Subject: Re: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queuesQuoted 2 lines · Bruce W
Quoted 2 lines
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org]
Quoted 10 lines
On
Quoted 7 lines
Quoted 7 lines
Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queuesThis patch is meant to cleanup the exception handling for the paths
where
Quoted 1 line
Quoted 1 line
we reset the interrupts and then reconfigure them. In all of these
paths
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
we had very different levels of exception handling. I have updated the driver so that all of the paths should result in a similar state if we fail.
Specifically the driver will now unload the mailbox interrupt, free the queue vectors and MSI-X, and then detach the interface.
In addition for any of the PCIe related resets I have added a check with the
hw_readyfunction to just make sure the registers are in a readable state prior to reopening the interface.Signed-off-by Alexander Duyck <[email protected]>--- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 17 +++++-- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 55 ++++++++++++++++++----- 2 files changed, 55 insertions(+), 17 deletions(-)Quoted 6 lines
Quoted 6 lines
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c int fm10k_setup_tc(struct net_device
Quoted 6 lines
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +0
-0@@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device*dev,
Quoted 69 lines
u8
Quoted 32 lines
Quoted 32 lines
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +13
-4tc) err =fm10k_init_queueing_scheme (interface);if (err) -return err; + goto err_queueing_scheme; err =fm10k_mbx_request_irq (interface);if (err) -return err; + goto err_mbx_irq; -if (netif_running (dev)) -fm10k_open (dev); + err =netif_running (dev) ?fm10k_open (dev) :0 ; +if (err) + goto err_open;/* flag to indicate SWPRI has yet to be updated */ interface->flags |=FM10K_FLAG_SWPRI_CONFIG ;return 0 ; +err_open: +fm10k_mbx_free_irq (interface); +err_mbx_irq: +fm10k_clear_queueing_scheme (interface); +err_queueing_scheme: +netif_device_detach (dev); + +return err; }Quoted 1 line
Quoted 1 line
static int fm10k_ioctl(struct
net_device*netdev, struct ifreq *ifr, intcmd)
Quoted 26 lines
Quoted 26 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +7
-1@@ -185,7 +185,13 @@ static void fm10k_reinit(struct fm10k_intfc *interface) }/* reassociate interrupts */ -fm10k_mbx_request_irq (interface); + err =fm10k_mbx_request_irq (interface); +if (err) + goto err_mbx_irq; + + err =fm10k_hw_ready (interface); +if (err) + goto err_open;/* update hardware address for VFs if perm_addr has changed */ if (hw->mac.type == fm10k_mac_vf) {drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0
-0@@ -205,14 +211,23 @@ static void fm10k_reinit(struct fm10k_intfc *interface)/* reset clock */ fm10k_ts_reset (interface);Quoted 8 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c static void fm10k_reinit(struct fm10k_intfc *interface) · line 217
Quoted 7 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4
-3-if (netif_running (netdev)) -fm10k_open (netdev); - + err =netif_running (netdev) ?fm10k_open (netdev) :0 ; +if (err) + goto err_open; +Jeff, the above blank-line separator line in Alex' original submission looks
Quoted 2 lines
fine here but
Quoted 1 line
in your next-queue tree dev-queue branch there is additional
whitespace
Quoted 9 lines
(2 tabs).
Quoted 1 line
How did that happen?
I just went back and checked and it looks like I had included some trailing white space in my original patch. Jeff do you need me to resubmit this patch or can you clean up the two spots where it added trailing white space in the blank lines?
- Alex
Jeff, are you going to fix the whitespace issues in this patch?
Thanks,
Bruce._______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan
PATCH 3/5 fm10k: Cleanup exception handling for changing queues
In reply to Alexander DuyckQuoted 115 lines · Alexander Duyck
Quoted 2 lines · Bruce W
Quoted 2 lines
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
Quoted 4 lines
Quoted 4 lines
Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanupexception
Quoted 3 lines
Quoted 3 lines
handling for changing queues
This patch is meant to cleanup the exception handling for the
paths where
Quoted 1 line
Quoted 1 line
we reset the interrupts and then reconfigure them.? In all of
these paths
Quoted 1 line
Quoted 1 line
we had very different levels of exception handling.? I have
updated the
Quoted 1 line
Quoted 1 line
driver so that all of the paths should result in a similar state
if we
Quoted 3 lines
Quoted 3 lines
fail.
Specifically the driver will now unload the mailbox interrupt,
free the
Quoted 3 lines
Quoted 3 lines
queue vectors and MSI-X, and then detach the interface.
In addition for any of the PCIe related resets I have added a
check with
Quoted 1 line
Quoted 1 line
the
hw_readyfunction to just make sure the registers are in areadable
Quoted 9 lines
Quoted 9 lines
state prior to reopening the interface.
Signed-off-by Alexander Duyck <[email protected]>---
??drivers/net/ethernet/intel/fm10k/fm10k_netdev.c|?? 17 +++++-- ??drivers/net/ethernet/intel/fm10k/fm10k_pci.c??? |?? 55
++++++++++++++++++-----
?? 2 files changed, 55 insertions(+), 17 deletions(-)Quoted 6 lines
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c int fm10k_setup_tc(struct net_device *dev,
Quoted 6 lines
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +0
-0@@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device *dev,u8
Quoted 32 lines
Quoted 32 lines
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +5
-4tc) ??????err = fm10k_init_queueing_scheme(interface); ??????if (err) -????????????return err; +????????????goto err_queueing_scheme; ??????err = fm10k_mbx_request_irq(interface); ??????if (err) -????????????return err; +????????????goto err_mbx_irq; -????if (netif_running(dev)) -????????????fm10k_open(dev); +????err = netif_running(dev) ? fm10k_open(dev) : 0; +????if (err) +????????????goto err_open;??????/* flag to indicate SWPRI has yet to be updated */ ??????interface->flags |=
FM10K_FLAG_SWPRI_CONFIG;??????return 0;
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +8
-0+err_open: +????fm10k_mbx_free_irq(interface); +err_mbx_irq: +????fm10k_clear_queueing_scheme(interface); +err_queueing_scheme: +????netif_device_detach(dev); + +????return err;?? }
Quoted 1 line
Quoted 1 line
?? static int fm10k_ioctl(struct
net_device*netdev, struct ifreq*ifr, int cmd)
Quoted 19 lines
Quoted 19 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0
-0@@ -185,7 +185,13 @@ static void fm10k_reinit(struct fm10k_intfc *interface) ??????}??????/* reassociate interrupts */ -????fm10k_mbx_request_irq(interface); +????err =
fm10k_mbx_request_irq(interface); +????if (err)
+????????????gotoerr_mbx_irq;
+
+????err =fm10k_hw_ready(interface); +????if (err)
+????????????gotoerr_open;Quoted 1 line
Quoted 1 line
??????/* update hardware address for VFs if
perm_addrhas changed*/
Quoted 6 lines
Quoted 6 lines
??????if (
hw->mac.type==fm10k_mac_vf) {drivers/net/ethernet/intel/fm10k/fm10k_pci.c +0
-0@@ -205,14 +211,23 @@ static void fm10k_reinit(struct fm10k_intfc *interface)??????/* reset clock */ ??????fm10k_ts_reset(interface);
Quoted 8 lines
drivers/net/ethernet/intel/fm10k/fm10k_pci.c
Quoted 7 lines
-????if (
netif_running(netdev)) -????????????fm10k_open(netdev);drivers/net/ethernet/intel/fm10k/fm10k_pci.c +4
-1- +????err = netif_running(netdev) ? fm10k_open(netdev) : 0; +????if (err) +????????????goto err_open; +Jeff, the above blank-line separator line in Alex' original
submission looks fine here but
Quoted 8 lines
Quoted 1 line
in your next-queue tree dev-queue branch there is additional
whitespace (2 tabs).
Quoted 1 line
How did that happen?
I just went back and checked and it looks like I had included some trailing white space in my original patch.? Jeff do you need me to resubmit this patch or can you clean up the two spots where it added trailing white space in the blank lines?
I can fix it. ?I saw it when I applied it, and purposely did not fix it
to see if validation would catch it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20151110/6cd93d05/attachment.asc>
Singh
PATCH 3/5 fm10k: Cleanup exception handling for changing queues
In reply to Alexander Duyck-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: [email protected]
Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues
This patch is meant to cleanup the exception handling for the paths where we reset the interrupts and then reconfigure them. In all of these paths we had very different levels of exception handling. I have updated the driver so that all of the paths should result in a similar state if we fail.
Specifically the driver will now unload the mailbox interrupt, free the queue vectors and MSI-X, and then detach the interface.
In addition for any of the PCIe related resets I have added a check with the hw_ready function to just make sure the registers are in a readable state prior to reopening the interface.
---
PATCH 4/5 e1000e: Switch e1000e_up to void, drop code checking for error result
In reply to Alexander DuyckThe function e1000e_up always returns 0. As such we can convert it to a void and just ignore the results. This allows us to drop some code in a couple spots as we no longer need to worry about non-zero return values.
--- drivers/net/ethernet/intel/e1000e/e1000.h | 2 +- drivers/net/ethernet/intel/e1000e/netdev.c | 15 ++++----------- 2 files changed, 5 insertions(+), 12 deletions(-)
drivers/net/ethernet/intel/e1000e/e1000.h +1 -1
@@ -480,7 +480,7 @@ extern const char e1000e_driver_version[];void e1000e_check_options (struct e1000_adapter *adapter);void e1000e_set_ethtool_ops (struct net_device *netdev); -int e1000e_up (struct e1000_adapter *adapter); +void e1000e_up (struct e1000_adapter *adapter);void e1000e_down (struct e1000_adapter *adapter,bool reset);void e1000e_reinit_locked (struct e1000_adapter *adapter);void e1000e_reset (struct e1000_adapter *adapter);
drivers/net/ethernet/intel/e1000e/netdev.c +4 -11
@@ -4143,7 +4143,7 @@ void e1000e_reset(struct e1000_adapter *adapter) } -int e1000e_up (struct e1000_adapter *adapter) +void e1000e_up (struct e1000_adapter *adapter) {struct e1000_hw *hw = &adapter->hw;
@@ -4163,8 +4163,6 @@ int e1000e_up(struct e1000_adapter *adapter)ew32 (ICS ,E1000_ICS_LSC |E1000_ICR_OTHER ); elseew32 (ICS ,E1000_ICS_LSC ); - -return 0 ; }static void e1000e_flush_descriptors (struct e1000_adapter *adapter)
@@ -6643,7 +6641,7 @@ static int e1000e_pm_runtime_resume(struct device *dev)return rc;if (netdev->flags &IFF_UP ) - rc =e1000e_up (adapter); +e1000e_up (adapter);return rc; }
@@ -6834,13 +6832,8 @@ static void e1000_io_resume(struct pci_dev *pdev)e1000_init_manageability_pt (adapter); -if (netif_running (netdev)) { -if (e1000e_up (adapter)) { -dev_err (&pdev->dev, -"can't bring device back up after reset\n" ); -return ; - } - } +if (netif_running (netdev)) +e1000e_up (adapter);netif_device_attach (netdev);
Brown
PATCH 4/5 e1000e: Switch e1000e_up to void, drop code checking for error result
Quoted 11 lines
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 5:00 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 4/5] e1000e: Switche1000e_upto void, drop code checking for error resultThe function
e1000e_upalways returns 0. As such we can convert it to a void and just ignore the results. This allows us to drop some code in a couple spots as we no longer need to worry about non-zero return values.
Quoted 6 lines
Signed-off-by Alexander Duyck <[email protected]>--- drivers/net/ethernet/intel/e1000e/e1000.h | 2 +- drivers/net/ethernet/intel/e1000e/netdev.c | 15 ++++----------- 2 files changed, 5 insertions(+), 12 deletions(-)
PATCH 5/5 igb: Fix unused variable warning
In reply to Alexander DuyckI ran into an unused variable warning when building dev-queue. It turns out that hw isn't used anymore when setting up the q_vectors since we switch from hw->hw_addr to adapter->io_addr in igb_request_irq. As such the variable can be dropped.
--- drivers/net/ethernet/intel/igb/igb_main.c | 1 - 1 file changed, 1 deletion(-)
drivers/net/ethernet/intel/igb/igb_main.c +0 -1
@@ -946,7 +946,6 @@ static void igb_configure_msix(struct igb_adapter *adapter)static int igb_request_msix (struct igb_adapter *adapter) {struct net_device *netdev = adapter->netdev; -struct e1000_hw *hw = &adapter->hw;int i, err =0 , vector =0 , free_vector =0 ; err = request_irq(adapter->msix_entries[vector].vector,
Brown
PATCH 5/5 igb: Fix unused variable warning
Quoted 11 lines
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 5:00 PM
To: intel-wired-lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 5/5] igb: Fix unused variable warningI ran into an unused variable warning when building dev-queue. It turns out that hw isn't used anymore when setting up the
q_vectorssince we switch fromhw->hw_addrtoadapter->io_addrinigb_request_irq. As such the variable can be dropped.
Quoted 6 lines
Fixes 0b3659ca297fc ("igb: improve handling of disconnected adapters")Signed-off-by Alexander Duyck <[email protected]>--- drivers/net/ethernet/intel/igb/igb_main.c | 1 - 1 file changed, 1 deletion(-)