Message ID | 1373988107-305-2-git-send-email-dmitry@broadcom.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, 2013-07-16 at 18:21 +0300, Dmitry Kravkov wrote: > This prevent second statistics query be sent before first one is complete. > This is required since two outstanding queries may cause FW assert. Please run sparse on your code before submitting patches. > diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c [] > @@ -1591,10 +1591,16 @@ void bnx2x_stats_init(struct bnx2x *bp) > { > int /*abs*/port = BP_PORT(bp); > int mb_idx = BP_FW_MB_IDX(bp); > + __le16 init_fw_counter = le16_to_cpu(0xFFFF); __le16 init_fw_counter = cpu_to_le16(0xffff); > + struct stats_counter *counters = &bp->fw_stats_data->storm_counters; > > bp->stats_pending = 0; > bp->executer_idx = 0; > bp->stats_counter = 0; > + counters->xstats_counter = init_fw_counter; > + counters->tstats_counter = init_fw_counter; > + counters->ustats_counter = init_fw_counter; > + counters->cstats_counter = init_fw_counter; It might be better to remove init_fw_counter from stack and use a #define or just use cpu_to_le16(0xffff) in each init. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 16, 2013 at 7:24 PM, Joe Perches <joe@perches.com> wrote: > On Tue, 2013-07-16 at 18:21 +0300, Dmitry Kravkov wrote: >> This prevent second statistics query be sent before first one is complete. >> This is required since two outstanding queries may cause FW assert. > > Please run sparse on your code before submitting patches. > >> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c > [] >> @@ -1591,10 +1591,16 @@ void bnx2x_stats_init(struct bnx2x *bp) >> { >> int /*abs*/port = BP_PORT(bp); >> int mb_idx = BP_FW_MB_IDX(bp); >> + __le16 init_fw_counter = le16_to_cpu(0xFFFF); > > __le16 init_fw_counter = cpu_to_le16(0xffff); > Thanks Joe, for catching this >> + struct stats_counter *counters = &bp->fw_stats_data->storm_counters; >> >> bp->stats_pending = 0; >> bp->executer_idx = 0; >> bp->stats_counter = 0; >> + counters->xstats_counter = init_fw_counter; >> + counters->tstats_counter = init_fw_counter; >> + counters->ustats_counter = init_fw_counter; >> + counters->cstats_counter = init_fw_counter; > > It might be better to remove init_fw_counter from > stack and use a #define or just use cpu_to_le16(0xffff) > in each init. I'm not feel well for using same value in each init, probably is better to use multiple assignment like this: counters->xstats_counter = counters->tstats_counter = counters->ustats_counter = counters->cstats_counter = cpu_to_le16(0xffff); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2013-07-16 at 21:14 +0300, Dmitry Kravkov wrote: > On Tue, Jul 16, 2013 at 7:24 PM, Joe Perches <joe@perches.com> wrote: > > It might be better to remove init_fw_counter from > > stack and use a #define or just use cpu_to_le16(0xffff) > > in each init. > > I'm not feel well for using same value in each init, probably is better to use > multiple assignment like this: > counters->xstats_counter = > counters->tstats_counter = > counters->ustats_counter = > counters->cstats_counter = cpu_to_le16(0xffff); Your choice though maybe here's a couple of things to consider: I do think aligning all the counter variables useful when reading so count->xstats_counter = count->tstats_counter = count->ustats_counter = count->cstats_counter = foo; would be better. Unfortunately grep wouldn't show that actual initialization where: count->xstats_counter = foo; count->tstats_counter = foo; count->ustats_counter = foo; count->cstats_counter = foo; would and the compiler would do the same thing as the a = b = c = d = foo; case. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c index 98366ab..69c4cb6 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c @@ -1591,10 +1591,16 @@ void bnx2x_stats_init(struct bnx2x *bp) { int /*abs*/port = BP_PORT(bp); int mb_idx = BP_FW_MB_IDX(bp); + __le16 init_fw_counter = le16_to_cpu(0xFFFF); + struct stats_counter *counters = &bp->fw_stats_data->storm_counters; bp->stats_pending = 0; bp->executer_idx = 0; bp->stats_counter = 0; + counters->xstats_counter = init_fw_counter; + counters->tstats_counter = init_fw_counter; + counters->ustats_counter = init_fw_counter; + counters->cstats_counter = init_fw_counter; /* port and func stats for management */ if (!BP_NOMCP(bp)) {