| Message ID | 20240920185918.616302-3-wander@redhat.com |
|---|---|
| State | Under Review |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | Fixes in igbvf driver | expand |
Dear Wander, Thank you for your patch. Am 20.09.24 um 20:59 schrieb Wander Lairson Costa: > tx_queue_lock and stats_lock are declared and initialized, but never > used. Remove them. > > Signed-off-by: Wander Lairson Costa <wander@redhat.com> It’d be great if you added a Fixes: tag. > --- > drivers/net/ethernet/intel/igbvf/igbvf.h | 3 --- > drivers/net/ethernet/intel/igbvf/netdev.c | 3 --- > 2 files changed, 6 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igbvf/igbvf.h b/drivers/net/ethernet/intel/igbvf/igbvf.h > index 6ad35a00a287..ca6e44245a7b 100644 > --- a/drivers/net/ethernet/intel/igbvf/igbvf.h > +++ b/drivers/net/ethernet/intel/igbvf/igbvf.h > @@ -169,8 +169,6 @@ struct igbvf_adapter { > u16 link_speed; > u16 link_duplex; > > - spinlock_t tx_queue_lock; /* prevent concurrent tail updates */ > - > /* track device up/down/testing state */ > unsigned long state; > > @@ -220,7 +218,6 @@ struct igbvf_adapter { > /* OS defined structs */ > struct net_device *netdev; > struct pci_dev *pdev; > - spinlock_t stats_lock; /* prevent concurrent stats updates */ > > /* structs defined in e1000_hw.h */ > struct e1000_hw hw; > diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c > index 925d7286a8ee..02044aa2181b 100644 > --- a/drivers/net/ethernet/intel/igbvf/netdev.c > +++ b/drivers/net/ethernet/intel/igbvf/netdev.c > @@ -1656,12 +1656,9 @@ static int igbvf_sw_init(struct igbvf_adapter *adapter) > if (igbvf_alloc_queues(adapter)) > return -ENOMEM; > > - spin_lock_init(&adapter->tx_queue_lock); > - > /* Explicitly disable IRQ since the NIC can be in any state. */ > igbvf_irq_disable(adapter); > > - spin_lock_init(&adapter->stats_lock); > spin_lock_init(&adapter->hw.mbx_lock); > > set_bit(__IGBVF_DOWN, &adapter->state); With that addressed: Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de> Kind regards, Paul
On 9/21/24 14:52, Paul Menzel wrote: > Dear Wander, > > > Thank you for your patch. > > Am 20.09.24 um 20:59 schrieb Wander Lairson Costa: >> tx_queue_lock and stats_lock are declared and initialized, but never >> used. Remove them. >> >> Signed-off-by: Wander Lairson Costa <wander@redhat.com> > > It’d be great if you added a Fixes: tag. Alternatively you could split this series into two, and send this patch to iwl-next tree, without the fixes tag. For me this patch is just a cleanup, not a fix. > [...] > > With that addressed: > > Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de> > > > Kind regards, > > Paul
On Mon, Sep 23, 2024 at 6:04 AM Przemek Kitszel <przemyslaw.kitszel@intel.com> wrote: > > On 9/21/24 14:52, Paul Menzel wrote: > > Dear Wander, > > > > > > Thank you for your patch. > > > > Am 20.09.24 um 20:59 schrieb Wander Lairson Costa: > >> tx_queue_lock and stats_lock are declared and initialized, but never > >> used. Remove them. > >> > >> Signed-off-by: Wander Lairson Costa <wander@redhat.com> > > > > It’d be great if you added a Fixes: tag. > > Alternatively you could split this series into two, and send this patch > to iwl-next tree, without the fixes tag. For me this patch is just > a cleanup, not a fix. > > > > Should I send a new version of the patches separately? > [...] > > > > > With that addressed: > > > > Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de> > > > > > > Kind regards, > > > > Paul >
On 9/23/2024 9:46 AM, Wander Lairson Costa wrote: > On Mon, Sep 23, 2024 at 6:04 AM Przemek Kitszel > <przemyslaw.kitszel@intel.com> wrote: >> >> On 9/21/24 14:52, Paul Menzel wrote: >>> Dear Wander, >>> >>> >>> Thank you for your patch. >>> >>> Am 20.09.24 um 20:59 schrieb Wander Lairson Costa: >>>> tx_queue_lock and stats_lock are declared and initialized, but never >>>> used. Remove them. >>>> >>>> Signed-off-by: Wander Lairson Costa <wander@redhat.com> >>> >>> It’d be great if you added a Fixes: tag. >> >> Alternatively you could split this series into two, and send this patch >> to iwl-next tree, without the fixes tag. For me this patch is just >> a cleanup, not a fix. >> >>> >> > > Should I send a new version of the patches separately? The patches apply to the respective trees when split out so I can take these without a re-send. Patch 1 will need a Fixes: for it though... I'm seeing it as: 9d5c824399de ("igb: PCI-Express 82575 Gigabit Ethernet driver")? Thanks, Tony >> [...] >> >>> >>> With that addressed: >>> >>> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de> >>> >>> >>> Kind regards, >>> >>> Paul >> >
On Mon, Sep 23, 2024 at 3:44 PM Tony Nguyen <anthony.l.nguyen@intel.com> wrote: > > > > On 9/23/2024 9:46 AM, Wander Lairson Costa wrote: > > On Mon, Sep 23, 2024 at 6:04 AM Przemek Kitszel > > <przemyslaw.kitszel@intel.com> wrote: > >> > >> On 9/21/24 14:52, Paul Menzel wrote: > >>> Dear Wander, > >>> > >>> > >>> Thank you for your patch. > >>> > >>> Am 20.09.24 um 20:59 schrieb Wander Lairson Costa: > >>>> tx_queue_lock and stats_lock are declared and initialized, but never > >>>> used. Remove them. > >>>> > >>>> Signed-off-by: Wander Lairson Costa <wander@redhat.com> > >>> > >>> It’d be great if you added a Fixes: tag. > >> > >> Alternatively you could split this series into two, and send this patch > >> to iwl-next tree, without the fixes tag. For me this patch is just > >> a cleanup, not a fix. > >> > >>> > >> > > > > Should I send a new version of the patches separately? > > The patches apply to the respective trees when split out so I can take > these without a re-send. Patch 1 will need a Fixes: for it though... > > I'm seeing it as: 9d5c824399de ("igb: PCI-Express 82575 Gigabit Ethernet > driver")? > Can you add the tag when you apply the patch or should I add it? > Thanks, > Tony > > >> [...] > >> > >>> > >>> With that addressed: > >>> > >>> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de> > >>> > >>> > >>> Kind regards, > >>> > >>> Paul > >> > > >
diff --git a/drivers/net/ethernet/intel/igbvf/igbvf.h b/drivers/net/ethernet/intel/igbvf/igbvf.h index 6ad35a00a287..ca6e44245a7b 100644 --- a/drivers/net/ethernet/intel/igbvf/igbvf.h +++ b/drivers/net/ethernet/intel/igbvf/igbvf.h @@ -169,8 +169,6 @@ struct igbvf_adapter { u16 link_speed; u16 link_duplex; - spinlock_t tx_queue_lock; /* prevent concurrent tail updates */ - /* track device up/down/testing state */ unsigned long state; @@ -220,7 +218,6 @@ struct igbvf_adapter { /* OS defined structs */ struct net_device *netdev; struct pci_dev *pdev; - spinlock_t stats_lock; /* prevent concurrent stats updates */ /* structs defined in e1000_hw.h */ struct e1000_hw hw; diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c index 925d7286a8ee..02044aa2181b 100644 --- a/drivers/net/ethernet/intel/igbvf/netdev.c +++ b/drivers/net/ethernet/intel/igbvf/netdev.c @@ -1656,12 +1656,9 @@ static int igbvf_sw_init(struct igbvf_adapter *adapter) if (igbvf_alloc_queues(adapter)) return -ENOMEM; - spin_lock_init(&adapter->tx_queue_lock); - /* Explicitly disable IRQ since the NIC can be in any state. */ igbvf_irq_disable(adapter); - spin_lock_init(&adapter->stats_lock); spin_lock_init(&adapter->hw.mbx_lock); set_bit(__IGBVF_DOWN, &adapter->state);
tx_queue_lock and stats_lock are declared and initialized, but never used. Remove them. Signed-off-by: Wander Lairson Costa <wander@redhat.com> --- drivers/net/ethernet/intel/igbvf/igbvf.h | 3 --- drivers/net/ethernet/intel/igbvf/netdev.c | 3 --- 2 files changed, 6 deletions(-)