* [UPDATED][NET-NEXT PATCH 0/4] Minor bnx2 patches [not found] <20080918063933.GA8027@verge.net.au> @ 2008-09-18 16:57 ` Benjamin Li 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 1/4] bnx2: In bnx2_set_mac_link() return void rather then int Benjamin Li 2008-09-18 17:04 ` [UPDATED][NET-NEXT PATCH 0/4] Minor bnx2 patches Jeff Garzik 0 siblings, 2 replies; 12+ messages in thread From: Benjamin Li @ 2008-09-18 16:57 UTC (permalink / raw) To: Jeff Garzik; +Cc: benli, netdev@vger.kernel.org Thanks for the comments Simon. I have integrated the changes that you have suggested. I have reposted the entire patch set in numbered form so that it could be merged in easier. Thanks again. -Ben > @@ -7514,7 +7514,6 @@ bnx2_init_board(struct pci_dev *pdev, struct net_device *dev) > > > > bp->stats_ticks = USEC_PER_SEC & BNX2_HC_STATS_TICKS_HC_STAT_TICKS; > > > > - bp->timer_interval = HZ; > > bp->current_interval = HZ; > > Would it make sense to set current_interval to BNX2_TIMER_INTERVAL? Changed. > > > > diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h > > index cb47c98..e534c1d 100644 > > --- a/drivers/net/bnx2.h > > +++ b/drivers/net/bnx2.h > > @@ -6701,7 +6701,7 @@ struct bnx2 { > > > > /* End of fields used in the performance code paths. */ > > > > - int timer_interval; > > +#define BNX2_TIMER_INTERVAL HZ > > Perhaps its just me, but would it be nicer to define BNX2_TIMER_INTERVAL > somewhere other than in the middle of this structure? > Moved outside of the bnx2 structure. -------------------- Benjamin Li (4): bnx2: In bnx2_set_mac_link() return void rather then int bnx2: Update MODULE_DESCRIPTION to include the 5716 bnx2: Remove name field from bnx2 structure bnx2: Remove timer_interval field from the bnx2 structure drivers/net/bnx2.c | 22 +++++++++------------- drivers/net/bnx2.h | 5 ++--- 2 files changed, 11 insertions(+), 16 deletions(-) ^ permalink raw reply [flat|nested] 12+ messages in thread
* [UPDATED][NET-NEXT PATCH 1/4] bnx2: In bnx2_set_mac_link() return void rather then int 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 0/4] Minor bnx2 patches Benjamin Li @ 2008-09-18 16:57 ` Benjamin Li 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 Benjamin Li 2008-09-18 23:38 ` [UPDATED][NET-NEXT PATCH 1/4] bnx2: In bnx2_set_mac_link() return void rather then int David Miller 2008-09-18 17:04 ` [UPDATED][NET-NEXT PATCH 0/4] Minor bnx2 patches Jeff Garzik 1 sibling, 2 replies; 12+ messages in thread From: Benjamin Li @ 2008-09-18 16:57 UTC (permalink / raw) To: Jeff Garzik; +Cc: benli, netdev@vger.kernel.org bnx2_set_mac_link() doesn't need to return any error codes. And all the callers don't check the return code. It is safe to change the return type to a void. Signed-off-by: Benjamin Li <benli@broadcom.com> Signed-off-by: Michael Chan <mchan@broadcom.com> --- drivers/net/bnx2.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c index 2486a65..192a24e 100644 --- a/drivers/net/bnx2.c +++ b/drivers/net/bnx2.c @@ -1127,7 +1127,7 @@ bnx2_init_all_rx_contexts(struct bnx2 *bp) } } -static int +static void bnx2_set_mac_link(struct bnx2 *bp) { u32 val; @@ -1193,8 +1193,6 @@ bnx2_set_mac_link(struct bnx2 *bp) if (CHIP_NUM(bp) == CHIP_NUM_5709) bnx2_init_all_rx_contexts(bp); - - return 0; } static void -- 1.6.0.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 1/4] bnx2: In bnx2_set_mac_link() return void rather then int Benjamin Li @ 2008-09-18 16:57 ` Benjamin Li 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 3/4] bnx2: Remove name field from bnx2 structure Benjamin Li ` (2 more replies) 2008-09-18 23:38 ` [UPDATED][NET-NEXT PATCH 1/4] bnx2: In bnx2_set_mac_link() return void rather then int David Miller 1 sibling, 3 replies; 12+ messages in thread From: Benjamin Li @ 2008-09-18 16:57 UTC (permalink / raw) To: Jeff Garzik; +Cc: benli, netdev@vger.kernel.org Signed-off-by: Benjamin Li <benli@broadcom.com> Signed-off-by: Michael Chan <mchan@broadcom.com> --- drivers/net/bnx2.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c index 192a24e..a78b664 100644 --- a/drivers/net/bnx2.c +++ b/drivers/net/bnx2.c @@ -69,7 +69,7 @@ static char version[] __devinitdata = "Broadcom NetXtreme II Gigabit Ethernet Driver " DRV_MODULE_NAME " v" DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")\n"; MODULE_AUTHOR("Michael Chan <mchan@broadcom.com>"); -MODULE_DESCRIPTION("Broadcom NetXtreme II BCM5706/5708/5709 Driver"); +MODULE_DESCRIPTION("Broadcom NetXtreme II BCM5706/5708/5709/5716 Driver"); MODULE_LICENSE("GPL"); MODULE_VERSION(DRV_MODULE_VERSION); -- 1.6.0.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [UPDATED][NET-NEXT PATCH 3/4] bnx2: Remove name field from bnx2 structure 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 Benjamin Li @ 2008-09-18 16:57 ` Benjamin Li 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 4/4] bnx2: Remove timer_interval field from the " Benjamin Li 2008-09-18 23:40 ` [UPDATED][NET-NEXT PATCH 3/4] bnx2: Remove name field from " David Miller 2008-09-18 22:29 ` [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 David Miller 2008-09-18 23:39 ` David Miller 2 siblings, 2 replies; 12+ messages in thread From: Benjamin Li @ 2008-09-18 16:57 UTC (permalink / raw) To: Jeff Garzik; +Cc: benli, netdev@vger.kernel.org The name of the board is only used during the initialization of the adapter. We can save the space of a pointer by not storing this information. Signed-off-by: Benjamin Li <benli@broadcom.com> Signed-off-by: Michael Chan <mchan@broadcom.com> --- drivers/net/bnx2.c | 3 +-- drivers/net/bnx2.h | 2 -- 2 files changed, 1 insertions(+), 4 deletions(-) diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c index a78b664..8417766 100644 --- a/drivers/net/bnx2.c +++ b/drivers/net/bnx2.c @@ -7718,7 +7718,6 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) memcpy(dev->dev_addr, bp->mac_addr, 6); memcpy(dev->perm_addr, bp->mac_addr, 6); - bp->name = board_info[ent->driver_data].name; dev->features |= NETIF_F_IP_CSUM | NETIF_F_SG; if (CHIP_NUM(bp) == CHIP_NUM_5709) @@ -7745,7 +7744,7 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) printk(KERN_INFO "%s: %s (%c%d) %s found at mem %lx, " "IRQ %d, node addr %s\n", dev->name, - bp->name, + board_info[ent->driver_data].name, ((CHIP_ID(bp) & 0xf000) >> 12) + 'A', ((CHIP_ID(bp) & 0x0ff0) >> 4), bnx2_bus_string(bp, str), diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h index c3c579f..cb47c98 100644 --- a/drivers/net/bnx2.h +++ b/drivers/net/bnx2.h @@ -6701,8 +6701,6 @@ struct bnx2 { /* End of fields used in the performance code paths. */ - char *name; - int timer_interval; int current_interval; struct timer_list timer; -- 1.6.0.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [UPDATED][NET-NEXT PATCH 4/4] bnx2: Remove timer_interval field from the bnx2 structure 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 3/4] bnx2: Remove name field from bnx2 structure Benjamin Li @ 2008-09-18 16:57 ` Benjamin Li 2008-09-18 23:40 ` David Miller 2008-09-18 23:40 ` [UPDATED][NET-NEXT PATCH 3/4] bnx2: Remove name field from " David Miller 1 sibling, 1 reply; 12+ messages in thread From: Benjamin Li @ 2008-09-18 16:57 UTC (permalink / raw) To: Jeff Garzik; +Cc: benli, netdev@vger.kernel.org The timer_interval field is only assigned once, and never reassigned. We can safely replace all instances of the timer_interval with a constant value. Signed-off-by: Benjamin Li <benli@broadcom.com> Signed-off-by: Michael Chan <mchan@broadcom.com> --- drivers/net/bnx2.c | 13 ++++++------- drivers/net/bnx2.h | 3 ++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c index 8417766..883e0a7 100644 --- a/drivers/net/bnx2.c +++ b/drivers/net/bnx2.c @@ -5598,7 +5598,7 @@ bnx2_5706_serdes_timer(struct bnx2 *bp) } else if ((bp->link_up == 0) && (bp->autoneg & AUTONEG_SPEED)) { u32 bmcr; - bp->current_interval = bp->timer_interval; + bp->current_interval = BNX2_TIMER_INTERVAL; bnx2_read_phy(bp, bp->mii_bmcr, &bmcr); @@ -5627,7 +5627,7 @@ bnx2_5706_serdes_timer(struct bnx2 *bp) bp->phy_flags &= ~BNX2_PHY_FLAG_PARALLEL_DETECT; } } else - bp->current_interval = bp->timer_interval; + bp->current_interval = BNX2_TIMER_INTERVAL; if (check_link) { u32 val; @@ -5672,11 +5672,11 @@ bnx2_5708_serdes_timer(struct bnx2 *bp) } else { bnx2_disable_forced_2g5(bp); bp->serdes_an_pending = 2; - bp->current_interval = bp->timer_interval; + bp->current_interval = BNX2_TIMER_INTERVAL; } } else - bp->current_interval = bp->timer_interval; + bp->current_interval = BNX2_TIMER_INTERVAL; spin_unlock(&bp->phy_lock); } @@ -7514,8 +7514,7 @@ bnx2_init_board(struct pci_dev *pdev, struct net_device *dev) bp->stats_ticks = USEC_PER_SEC & BNX2_HC_STATS_TICKS_HC_STAT_TICKS; - bp->timer_interval = HZ; - bp->current_interval = HZ; + bp->current_interval = BNX2_TIMER_INTERVAL; bp->phy_addr = 1; @@ -7605,7 +7604,7 @@ bnx2_init_board(struct pci_dev *pdev, struct net_device *dev) bp->req_flow_ctrl = FLOW_CTRL_RX | FLOW_CTRL_TX; init_timer(&bp->timer); - bp->timer.expires = RUN_AT(bp->timer_interval); + bp->timer.expires = RUN_AT(BNX2_TIMER_INTERVAL); bp->timer.data = (unsigned long) bp; bp->timer.function = bnx2_timer; diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h index cb47c98..682b8f0 100644 --- a/drivers/net/bnx2.h +++ b/drivers/net/bnx2.h @@ -6654,6 +6654,8 @@ struct bnx2_napi { struct bnx2_tx_ring_info tx_ring; }; +#define BNX2_TIMER_INTERVAL HZ + struct bnx2 { /* Fields used in the tx and intr/napi performance paths are grouped */ /* together in the beginning of the structure. */ @@ -6701,7 +6703,6 @@ struct bnx2 { /* End of fields used in the performance code paths. */ - int timer_interval; int current_interval; struct timer_list timer; struct work_struct reset_task; -- 1.6.0.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [UPDATED][NET-NEXT PATCH 4/4] bnx2: Remove timer_interval field from the bnx2 structure 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 4/4] bnx2: Remove timer_interval field from the " Benjamin Li @ 2008-09-18 23:40 ` David Miller 0 siblings, 0 replies; 12+ messages in thread From: David Miller @ 2008-09-18 23:40 UTC (permalink / raw) To: benli; +Cc: jeff, netdev From: "Benjamin Li" <benli@broadcom.com> Date: Thu, 18 Sep 2008 09:57:13 -0700 > The timer_interval field is only assigned once, and never reassigned. > We can safely replace all instances of the timer_interval with a > constant value. > > Signed-off-by: Benjamin Li <benli@broadcom.com> > Signed-off-by: Michael Chan <mchan@broadcom.com> Also applied, thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [UPDATED][NET-NEXT PATCH 3/4] bnx2: Remove name field from bnx2 structure 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 3/4] bnx2: Remove name field from bnx2 structure Benjamin Li 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 4/4] bnx2: Remove timer_interval field from the " Benjamin Li @ 2008-09-18 23:40 ` David Miller 1 sibling, 0 replies; 12+ messages in thread From: David Miller @ 2008-09-18 23:40 UTC (permalink / raw) To: benli; +Cc: jeff, netdev From: "Benjamin Li" <benli@broadcom.com> Date: Thu, 18 Sep 2008 09:57:12 -0700 > The name of the board is only used during the initialization of > the adapter. We can save the space of a pointer by not storing > this information. > > Signed-off-by: Benjamin Li <benli@broadcom.com> > Signed-off-by: Michael Chan <mchan@broadcom.com> Applied. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 Benjamin Li 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 3/4] bnx2: Remove name field from bnx2 structure Benjamin Li @ 2008-09-18 22:29 ` David Miller 2008-09-18 23:39 ` David Miller 2 siblings, 0 replies; 12+ messages in thread From: David Miller @ 2008-09-18 22:29 UTC (permalink / raw) To: benli; +Cc: jeff, netdev I've seen these patches posted at least 3 other times already. Please be patient, we saw it the first 3 times, you need not submit them again. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 Benjamin Li 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 3/4] bnx2: Remove name field from bnx2 structure Benjamin Li 2008-09-18 22:29 ` [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 David Miller @ 2008-09-18 23:39 ` David Miller 2 siblings, 0 replies; 12+ messages in thread From: David Miller @ 2008-09-18 23:39 UTC (permalink / raw) To: benli; +Cc: jeff, netdev From: "Benjamin Li" <benli@broadcom.com> Date: Thu, 18 Sep 2008 09:57:11 -0700 > Signed-off-by: Benjamin Li <benli@broadcom.com> > Signed-off-by: Michael Chan <mchan@broadcom.com> Applied. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [UPDATED][NET-NEXT PATCH 1/4] bnx2: In bnx2_set_mac_link() return void rather then int 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 1/4] bnx2: In bnx2_set_mac_link() return void rather then int Benjamin Li 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 Benjamin Li @ 2008-09-18 23:38 ` David Miller 1 sibling, 0 replies; 12+ messages in thread From: David Miller @ 2008-09-18 23:38 UTC (permalink / raw) To: benli; +Cc: jeff, netdev From: "Benjamin Li" <benli@broadcom.com> Date: Thu, 18 Sep 2008 09:57:10 -0700 > bnx2_set_mac_link() doesn't need to return any error codes. And > all the callers don't check the return code. It is safe to > change the return type to a void. > > Signed-off-by: Benjamin Li <benli@broadcom.com> > Signed-off-by: Michael Chan <mchan@broadcom.com> Applied. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [UPDATED][NET-NEXT PATCH 0/4] Minor bnx2 patches 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 0/4] Minor bnx2 patches Benjamin Li 2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 1/4] bnx2: In bnx2_set_mac_link() return void rather then int Benjamin Li @ 2008-09-18 17:04 ` Jeff Garzik 2008-09-18 22:30 ` David Miller 1 sibling, 1 reply; 12+ messages in thread From: Jeff Garzik @ 2008-09-18 17:04 UTC (permalink / raw) To: Benjamin Li; +Cc: netdev@vger.kernel.org, David Miller Benjamin Li wrote: > Thanks for the comments Simon. I have integrated the changes that you have suggested. I have reposted the entire patch set in numbered form so that it could be merged in easier. > > Thanks again. > > -Ben > >> @@ -7514,7 +7514,6 @@ bnx2_init_board(struct pci_dev *pdev, struct net_device *dev) >>> bp->stats_ticks = USEC_PER_SEC & BNX2_HC_STATS_TICKS_HC_STAT_TICKS; >>> >>> - bp->timer_interval = HZ; >>> bp->current_interval = HZ; >> Would it make sense to set current_interval to BNX2_TIMER_INTERVAL? > > Changed. > >>> diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h >>> index cb47c98..e534c1d 100644 >>> --- a/drivers/net/bnx2.h >>> +++ b/drivers/net/bnx2.h >>> @@ -6701,7 +6701,7 @@ struct bnx2 { >>> >>> /* End of fields used in the performance code paths. */ >>> >>> - int timer_interval; >>> +#define BNX2_TIMER_INTERVAL HZ >> Perhaps its just me, but would it be nicer to define BNX2_TIMER_INTERVAL >> somewhere other than in the middle of this structure? >> > Moved outside of the bnx2 structure. > > -------------------- > > Benjamin Li (4): > bnx2: In bnx2_set_mac_link() return void rather then int > bnx2: Update MODULE_DESCRIPTION to include the 5716 > bnx2: Remove name field from bnx2 structure > bnx2: Remove timer_interval field from the bnx2 structure > > drivers/net/bnx2.c | 22 +++++++++------------- > drivers/net/bnx2.h | 5 ++--- > 2 files changed, 11 insertions(+), 16 deletions(-) Patches 1-4 Acked-by: Jeff Garzik <jgarzik@redhat.com> Unlike most net drivers, as a special case, davem usually applies patches to this net driver (and tg3), rather than me. So I'll let him pick these up (CC'd). ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [UPDATED][NET-NEXT PATCH 0/4] Minor bnx2 patches 2008-09-18 17:04 ` [UPDATED][NET-NEXT PATCH 0/4] Minor bnx2 patches Jeff Garzik @ 2008-09-18 22:30 ` David Miller 0 siblings, 0 replies; 12+ messages in thread From: David Miller @ 2008-09-18 22:30 UTC (permalink / raw) To: jeff; +Cc: benli, netdev From: Jeff Garzik <jeff@garzik.org> Date: Thu, 18 Sep 2008 13:04:02 -0400 > Patches 1-4 Acked-by: Jeff Garzik <jgarzik@redhat.com> > > Unlike most net drivers, as a special case, davem usually applies patches to this net driver (and tg3), rather than me. So I'll let him pick these up (CC'd). Oh I see, this was an update, sigh. Ok, I'll look at these and integrate them later as needed. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2008-09-18 23:41 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20080918063933.GA8027@verge.net.au>
2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 0/4] Minor bnx2 patches Benjamin Li
2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 1/4] bnx2: In bnx2_set_mac_link() return void rather then int Benjamin Li
2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 Benjamin Li
2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 3/4] bnx2: Remove name field from bnx2 structure Benjamin Li
2008-09-18 16:57 ` [UPDATED][NET-NEXT PATCH 4/4] bnx2: Remove timer_interval field from the " Benjamin Li
2008-09-18 23:40 ` David Miller
2008-09-18 23:40 ` [UPDATED][NET-NEXT PATCH 3/4] bnx2: Remove name field from " David Miller
2008-09-18 22:29 ` [UPDATED][NET-NEXT PATCH 2/4] bnx2: Update MODULE_DESCRIPTION to include the 5716 David Miller
2008-09-18 23:39 ` David Miller
2008-09-18 23:38 ` [UPDATED][NET-NEXT PATCH 1/4] bnx2: In bnx2_set_mac_link() return void rather then int David Miller
2008-09-18 17:04 ` [UPDATED][NET-NEXT PATCH 0/4] Minor bnx2 patches Jeff Garzik
2008-09-18 22:30 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).