| Message ID | 1372563153.3875.2.camel@phoenix |
|---|---|
| State | Superseded |
| Headers | show |
Hi Axel, Thanks for your v2. On Sun, Jun 30, 2013 at 9:02 AM, Axel Lin <axel.lin@ingics.com> wrote: > Use ARRAY_SIZE instead of having similar implementation in each drivers. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > Cc: Albert Aribaud <albert.u.boot@aribaud.net> > Cc: Ben Warren <biggerbadderben@gmail.com> > Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > Cc: Joe Hershberger <joe.hershberger@ni.com> > Cc: Marek Vasut <marex@denx.de> > Cc: Mike Frysinger <vapier@gentoo.org> > Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org> > Cc: TsiChungLiew <Tsi-Chung.Liew@freescale.com> > Cc: Wolfgang Denk <wd@denx.de> > Cc: York Sun <yorksun@freescale.com> > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > --- > Hi Jagan Teki, > This is v2 per your request. > > v2: > Fix checkpatch issues, now checkpatch only shows below warnings: > a few WARNING: Avoid CamelCase > and a WARNING: line over 80 characters > #134: FILE: drivers/net/npe/IxEthDBFeatures.c:150: > + if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) { I think you can fix this warning and may be you can send a separate patch for "CamelCase:" if you want. And also.. > > Obviously, I prefer v1 for below reasons: > 1. The diff in v2 becomes bigger and makes it harder for review. > 2. The whole file in drivers/net/npe/IxEthDBFeatures.c uses 4 spaces as indent, > only the lines touched by this patch uses tab as indent (to make checkpatch happy). > This makes the code looks odd after apply v2, I don't think this is an improvement. > > Well, it's up to maintainer to pick up the version he prefer. > > Regards, > Axel > > drivers/net/ax88180.c | 2 +- > drivers/net/fsl_mcdmafec.c | 2 +- > drivers/net/lan91c96.c | 2 +- > drivers/net/mcffec.c | 2 +- > drivers/net/mcfmii.c | 2 +- > drivers/net/ne2000.c | 2 +- > drivers/net/npe/IxEthDBFeatures.c | 28 ++++++++++++++-------------- > drivers/net/npe/IxOsalIoMem.c | 3 +-- > drivers/net/npe/include/IxEthDBPortDefs.h | 2 +- > drivers/net/npe/include/IxOsalTypes.h | 2 +- > 10 files changed, 23 insertions(+), 24 deletions(-) > > diff --git a/drivers/net/ax88180.c b/drivers/net/ax88180.c > index f501768..7f0cfe5 100644 > --- a/drivers/net/ax88180.c > +++ b/drivers/net/ax88180.c > @@ -157,7 +157,7 @@ static void ax88180_mac_reset (struct eth_device *dev) > OUTW (dev, MISC_RESET_MAC, MISC); > tmpval = INW (dev, MISC); > > - for (i = 0; i < (sizeof (program_seq) / sizeof (program_seq[0])); i++) > + for (i = 0; i < ARRAY_SIZE(program_seq); i++) > OUTW (dev, program_seq[i].value, program_seq[i].offset); > } > > diff --git a/drivers/net/fsl_mcdmafec.c b/drivers/net/fsl_mcdmafec.c > index 63842cd..0e18764 100644 > --- a/drivers/net/fsl_mcdmafec.c > +++ b/drivers/net/fsl_mcdmafec.c > @@ -520,7 +520,7 @@ int mcdmafec_initialize(bd_t * bis) > u32 tmp = CONFIG_SYS_INTSRAM + 0x2000; > #endif > > - for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) { > + for (i = 0; i < ARRAY_SIZE(fec_info); i++) { > > dev = > (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE, > diff --git a/drivers/net/lan91c96.c b/drivers/net/lan91c96.c > index 11d350e..47c15c4 100644 > --- a/drivers/net/lan91c96.c > +++ b/drivers/net/lan91c96.c > @@ -779,7 +779,7 @@ static int lan91c96_detect_chip(struct eth_device *dev) > SMC_SELECT_BANK(dev, 3); > chip_id = (SMC_inw(dev, 0xA) & LAN91C96_REV_CHIPID) >> 4; > SMC_SELECT_BANK(dev, 0); > - for (r = 0; r < sizeof(supported_chips) / sizeof(struct id_type); r++) > + for (r = 0; r < ARRAY_SIZE(supported_chips); r++) > if (chip_id == supported_chips[r].id) > return r; > return 0; > diff --git a/drivers/net/mcffec.c b/drivers/net/mcffec.c > index ed7459c..7ae6320 100644 > --- a/drivers/net/mcffec.c > +++ b/drivers/net/mcffec.c > @@ -559,7 +559,7 @@ int mcffec_initialize(bd_t * bis) > u32 tmp = CONFIG_SYS_INIT_RAM_ADDR + 0x1000; > #endif > > - for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) { > + for (i = 0; i < ARRAY_SIZE(fec_info); i++) { > > dev = > (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE, > diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c > index 5e64dbd..63bc8f8 100644 > --- a/drivers/net/mcfmii.c > +++ b/drivers/net/mcfmii.c > @@ -186,7 +186,7 @@ int mii_discover_phy(struct eth_device *dev) > printf("PHY @ 0x%x pass %d\n", phyno, pass); > #endif > > - for (i = 0; (i < (sizeof(phyinfo) / sizeof(phy_info_t))) > + for (i = 0; (i < ARRAY_SIZE(phyinfo)) > && (phyinfo[i].phyid != 0); i++) { > if (phyinfo[i].phyid == phytype) { > #ifdef ET_DEBUG > diff --git a/drivers/net/ne2000.c b/drivers/net/ne2000.c > index 3939158..e6cd3e9 100644 > --- a/drivers/net/ne2000.c > +++ b/drivers/net/ne2000.c > @@ -228,7 +228,7 @@ int get_prom(u8* mac_addr, u8* base_addr) > > mdelay (10); > > - for (i = 0; i < sizeof (program_seq) / sizeof (program_seq[0]); i++) > + for (i = 0; i < ARRAY_SIZE(program_seq); i++) > n2k_outb (program_seq[i].value, program_seq[i].offset); > > PRINTK ("PROM:"); > diff --git a/drivers/net/npe/IxEthDBFeatures.c b/drivers/net/npe/IxEthDBFeatures.c > index c5b680a..d43efaa 100644 > --- a/drivers/net/npe/IxEthDBFeatures.c > +++ b/drivers/net/npe/IxEthDBFeatures.c > @@ -143,22 +143,22 @@ void ixEthDBFeatureCapabilityScan(void) > IX_ENSURE(sizeof (ixEthDBTrafficClassDefinitions) != 0, "DB: no traffic class definitions found, check IxEthDBQoS.h"); > > /* find the traffic class definition index compatible with the current NPE A functionality ID */ > - for (trafficClassDefinitionIndex = 0 ; > - trafficClassDefinitionIndex < sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]); > - trafficClassDefinitionIndex++) > - { > - if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) > - { > - /* found it */ > - break; > - } > - } > + for (trafficClassDefinitionIndex = 0; > + trafficClassDefinitionIndex < > + ARRAY_SIZE(ixEthDBTrafficClassDefinitions); > + trafficClassDefinitionIndex++) { > + if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) { > + /* found it */ > + break; > + } > + } > Above for loop along with if should be a block separable, means for (trafficClassDefinitionIndex = 0; trafficClassDefinitionIndex < ARRAY_SIZE(ixEthDBTrafficClassDefinitions); trafficClassDefinitionIndex++) { if (..........) { -- Thanks, Jagan. > /* select the default case if we went over the array boundary */ > - if (trafficClassDefinitionIndex == sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0])) > - { > - trafficClassDefinitionIndex = 0; /* the first record is the default case */ > - } > + if (trafficClassDefinitionIndex == > + ARRAY_SIZE(ixEthDBTrafficClassDefinitions)) { > + /* the first record is the default case */ > + trafficClassDefinitionIndex = 0; > + } > > /* select queue assignment structure based on the traffic class configuration index */ > queueStructureIndex = ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_QUEUE_ASSIGNMENT_INDEX]; > diff --git a/drivers/net/npe/IxOsalIoMem.c b/drivers/net/npe/IxOsalIoMem.c > index 34df92b..15a16b8 100644 > --- a/drivers/net/npe/IxOsalIoMem.c > +++ b/drivers/net/npe/IxOsalIoMem.c > @@ -66,8 +66,7 @@ ixOsalMemMapFind (UINT32 requestedAddress, > { > UINT32 mapIndex; > > - UINT32 numMapElements = > - sizeof (ixOsalGlobalMemoryMap) / sizeof (IxOsalMemoryMap); > + UINT32 numMapElements = ARRAY_SIZE(ixOsalGlobalMemoryMap); > > for (mapIndex = 0; mapIndex < numMapElements; mapIndex++) > { > diff --git a/drivers/net/npe/include/IxEthDBPortDefs.h b/drivers/net/npe/include/IxEthDBPortDefs.h > index c3acbdd..eac48cb 100644 > --- a/drivers/net/npe/include/IxEthDBPortDefs.h > +++ b/drivers/net/npe/include/IxEthDBPortDefs.h > @@ -106,7 +106,7 @@ static const IxEthDBPortDefinition ixEthDBPortDefinitions[] = > * @def IX_ETH_DB_NUMBER_OF_PORTS > * @brief number of supported ports > */ > -#define IX_ETH_DB_NUMBER_OF_PORTS (sizeof (ixEthDBPortDefinitions) / sizeof (ixEthDBPortDefinitions[0])) > +#define IX_ETH_DB_NUMBER_OF_PORTS ARRAY_SIZE(ixEthDBPortDefinitions) > > /** > * @def IX_ETH_DB_UNKNOWN_PORT > diff --git a/drivers/net/npe/include/IxOsalTypes.h b/drivers/net/npe/include/IxOsalTypes.h > index 06e71de..615c655 100644 > --- a/drivers/net/npe/include/IxOsalTypes.h > +++ b/drivers/net/npe/include/IxOsalTypes.h > @@ -93,7 +93,7 @@ typedef volatile INT32 VINT32; > > > #ifndef NUMELEMS > -#define NUMELEMS(x) (sizeof(x) / sizeof((x)[0])) > +#define NUMELEMS(x) ARRAY_SIZE(x) > #endif > > > -- > 1.8.1.2 > > >
On Mon, Jul 1, 2013 at 10:26 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote: > Hi Axel, > > Thanks for your v2. > > On Sun, Jun 30, 2013 at 9:02 AM, Axel Lin <axel.lin@ingics.com> wrote: >> Use ARRAY_SIZE instead of having similar implementation in each drivers. >> >> Signed-off-by: Axel Lin <axel.lin@ingics.com> >> Cc: Albert Aribaud <albert.u.boot@aribaud.net> >> Cc: Ben Warren <biggerbadderben@gmail.com> >> Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> >> Cc: Joe Hershberger <joe.hershberger@ni.com> >> Cc: Marek Vasut <marex@denx.de> >> Cc: Mike Frysinger <vapier@gentoo.org> >> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org> >> Cc: TsiChungLiew <Tsi-Chung.Liew@freescale.com> >> Cc: Wolfgang Denk <wd@denx.de> >> Cc: York Sun <yorksun@freescale.com> >> >> Signed-off-by: Axel Lin <axel.lin@ingics.com> >> --- >> Hi Jagan Teki, >> This is v2 per your request. >> >> v2: >> Fix checkpatch issues, now checkpatch only shows below warnings: >> a few WARNING: Avoid CamelCase >> and a WARNING: line over 80 characters >> #134: FILE: drivers/net/npe/IxEthDBFeatures.c:150: >> + if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) { > > I think you can fix this warning and may be you can send a separate > patch for "CamelCase:" > if you want. > > And also.. > >> >> Obviously, I prefer v1 for below reasons: >> 1. The diff in v2 becomes bigger and makes it harder for review. >> 2. The whole file in drivers/net/npe/IxEthDBFeatures.c uses 4 spaces as indent, >> only the lines touched by this patch uses tab as indent (to make checkpatch happy). >> This makes the code looks odd after apply v2, I don't think this is an improvement. >> >> Well, it's up to maintainer to pick up the version he prefer. >> >> Regards, >> Axel >> >> drivers/net/ax88180.c | 2 +- >> drivers/net/fsl_mcdmafec.c | 2 +- >> drivers/net/lan91c96.c | 2 +- >> drivers/net/mcffec.c | 2 +- >> drivers/net/mcfmii.c | 2 +- >> drivers/net/ne2000.c | 2 +- >> drivers/net/npe/IxEthDBFeatures.c | 28 ++++++++++++++-------------- >> drivers/net/npe/IxOsalIoMem.c | 3 +-- >> drivers/net/npe/include/IxEthDBPortDefs.h | 2 +- >> drivers/net/npe/include/IxOsalTypes.h | 2 +- >> 10 files changed, 23 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/net/ax88180.c b/drivers/net/ax88180.c >> index f501768..7f0cfe5 100644 >> --- a/drivers/net/ax88180.c >> +++ b/drivers/net/ax88180.c >> @@ -157,7 +157,7 @@ static void ax88180_mac_reset (struct eth_device *dev) >> OUTW (dev, MISC_RESET_MAC, MISC); >> tmpval = INW (dev, MISC); >> >> - for (i = 0; i < (sizeof (program_seq) / sizeof (program_seq[0])); i++) >> + for (i = 0; i < ARRAY_SIZE(program_seq); i++) >> OUTW (dev, program_seq[i].value, program_seq[i].offset); >> } >> >> diff --git a/drivers/net/fsl_mcdmafec.c b/drivers/net/fsl_mcdmafec.c >> index 63842cd..0e18764 100644 >> --- a/drivers/net/fsl_mcdmafec.c >> +++ b/drivers/net/fsl_mcdmafec.c >> @@ -520,7 +520,7 @@ int mcdmafec_initialize(bd_t * bis) >> u32 tmp = CONFIG_SYS_INTSRAM + 0x2000; >> #endif >> >> - for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) { >> + for (i = 0; i < ARRAY_SIZE(fec_info); i++) { >> >> dev = >> (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE, >> diff --git a/drivers/net/lan91c96.c b/drivers/net/lan91c96.c >> index 11d350e..47c15c4 100644 >> --- a/drivers/net/lan91c96.c >> +++ b/drivers/net/lan91c96.c >> @@ -779,7 +779,7 @@ static int lan91c96_detect_chip(struct eth_device *dev) >> SMC_SELECT_BANK(dev, 3); >> chip_id = (SMC_inw(dev, 0xA) & LAN91C96_REV_CHIPID) >> 4; >> SMC_SELECT_BANK(dev, 0); >> - for (r = 0; r < sizeof(supported_chips) / sizeof(struct id_type); r++) >> + for (r = 0; r < ARRAY_SIZE(supported_chips); r++) >> if (chip_id == supported_chips[r].id) >> return r; >> return 0; >> diff --git a/drivers/net/mcffec.c b/drivers/net/mcffec.c >> index ed7459c..7ae6320 100644 >> --- a/drivers/net/mcffec.c >> +++ b/drivers/net/mcffec.c >> @@ -559,7 +559,7 @@ int mcffec_initialize(bd_t * bis) >> u32 tmp = CONFIG_SYS_INIT_RAM_ADDR + 0x1000; >> #endif >> >> - for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) { >> + for (i = 0; i < ARRAY_SIZE(fec_info); i++) { >> >> dev = >> (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE, >> diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c >> index 5e64dbd..63bc8f8 100644 >> --- a/drivers/net/mcfmii.c >> +++ b/drivers/net/mcfmii.c >> @@ -186,7 +186,7 @@ int mii_discover_phy(struct eth_device *dev) >> printf("PHY @ 0x%x pass %d\n", phyno, pass); >> #endif >> >> - for (i = 0; (i < (sizeof(phyinfo) / sizeof(phy_info_t))) >> + for (i = 0; (i < ARRAY_SIZE(phyinfo)) >> && (phyinfo[i].phyid != 0); i++) { >> if (phyinfo[i].phyid == phytype) { >> #ifdef ET_DEBUG >> diff --git a/drivers/net/ne2000.c b/drivers/net/ne2000.c >> index 3939158..e6cd3e9 100644 >> --- a/drivers/net/ne2000.c >> +++ b/drivers/net/ne2000.c >> @@ -228,7 +228,7 @@ int get_prom(u8* mac_addr, u8* base_addr) >> >> mdelay (10); >> >> - for (i = 0; i < sizeof (program_seq) / sizeof (program_seq[0]); i++) >> + for (i = 0; i < ARRAY_SIZE(program_seq); i++) >> n2k_outb (program_seq[i].value, program_seq[i].offset); >> >> PRINTK ("PROM:"); >> diff --git a/drivers/net/npe/IxEthDBFeatures.c b/drivers/net/npe/IxEthDBFeatures.c >> index c5b680a..d43efaa 100644 >> --- a/drivers/net/npe/IxEthDBFeatures.c >> +++ b/drivers/net/npe/IxEthDBFeatures.c >> @@ -143,22 +143,22 @@ void ixEthDBFeatureCapabilityScan(void) >> IX_ENSURE(sizeof (ixEthDBTrafficClassDefinitions) != 0, "DB: no traffic class definitions found, check IxEthDBQoS.h"); >> >> /* find the traffic class definition index compatible with the current NPE A functionality ID */ >> - for (trafficClassDefinitionIndex = 0 ; >> - trafficClassDefinitionIndex < sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]); >> - trafficClassDefinitionIndex++) >> - { >> - if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) >> - { >> - /* found it */ >> - break; >> - } >> - } >> + for (trafficClassDefinitionIndex = 0; >> + trafficClassDefinitionIndex < >> + ARRAY_SIZE(ixEthDBTrafficClassDefinitions); >> + trafficClassDefinitionIndex++) { >> + if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) { >> + /* found it */ >> + break; >> + } >> + } >> > Above for loop along with if should be a block separable, means > > for (trafficClassDefinitionIndex = 0; > trafficClassDefinitionIndex < > ARRAY_SIZE(ixEthDBTrafficClassDefinitions); > trafficClassDefinitionIndex++) { > if (..........) { Previous msg sent intermediately, See my exact comment here. for (trafficClassDefinitionIndex = 0; trafficClassDefinitionIndex < ARRAY_SIZE(ixEthDBTrafficClassDefinitions); trafficClassDefinitionIndex++) { if (..........) { The above way is easy to understand where the code block starts, It just my opinion you may find the different way but it should be understandable where should code block start for a given condition statement. -- Thanks, Jagan. > > > -- > Thanks, > Jagan. > >> /* select the default case if we went over the array boundary */ >> - if (trafficClassDefinitionIndex == sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0])) >> - { >> - trafficClassDefinitionIndex = 0; /* the first record is the default case */ >> - } >> + if (trafficClassDefinitionIndex == >> + ARRAY_SIZE(ixEthDBTrafficClassDefinitions)) { >> + /* the first record is the default case */ >> + trafficClassDefinitionIndex = 0; >> + } >> >> /* select queue assignment structure based on the traffic class configuration index */ >> queueStructureIndex = ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_QUEUE_ASSIGNMENT_INDEX]; >> diff --git a/drivers/net/npe/IxOsalIoMem.c b/drivers/net/npe/IxOsalIoMem.c >> index 34df92b..15a16b8 100644 >> --- a/drivers/net/npe/IxOsalIoMem.c >> +++ b/drivers/net/npe/IxOsalIoMem.c >> @@ -66,8 +66,7 @@ ixOsalMemMapFind (UINT32 requestedAddress, >> { >> UINT32 mapIndex; >> >> - UINT32 numMapElements = >> - sizeof (ixOsalGlobalMemoryMap) / sizeof (IxOsalMemoryMap); >> + UINT32 numMapElements = ARRAY_SIZE(ixOsalGlobalMemoryMap); >> >> for (mapIndex = 0; mapIndex < numMapElements; mapIndex++) >> { >> diff --git a/drivers/net/npe/include/IxEthDBPortDefs.h b/drivers/net/npe/include/IxEthDBPortDefs.h >> index c3acbdd..eac48cb 100644 >> --- a/drivers/net/npe/include/IxEthDBPortDefs.h >> +++ b/drivers/net/npe/include/IxEthDBPortDefs.h >> @@ -106,7 +106,7 @@ static const IxEthDBPortDefinition ixEthDBPortDefinitions[] = >> * @def IX_ETH_DB_NUMBER_OF_PORTS >> * @brief number of supported ports >> */ >> -#define IX_ETH_DB_NUMBER_OF_PORTS (sizeof (ixEthDBPortDefinitions) / sizeof (ixEthDBPortDefinitions[0])) >> +#define IX_ETH_DB_NUMBER_OF_PORTS ARRAY_SIZE(ixEthDBPortDefinitions) >> >> /** >> * @def IX_ETH_DB_UNKNOWN_PORT >> diff --git a/drivers/net/npe/include/IxOsalTypes.h b/drivers/net/npe/include/IxOsalTypes.h >> index 06e71de..615c655 100644 >> --- a/drivers/net/npe/include/IxOsalTypes.h >> +++ b/drivers/net/npe/include/IxOsalTypes.h >> @@ -93,7 +93,7 @@ typedef volatile INT32 VINT32; >> >> >> #ifndef NUMELEMS >> -#define NUMELEMS(x) (sizeof(x) / sizeof((x)[0])) >> +#define NUMELEMS(x) ARRAY_SIZE(x) >> #endif >> >> >> -- >> 1.8.1.2 >> >> >>
>>> diff --git a/drivers/net/npe/IxEthDBFeatures.c b/drivers/net/npe/IxEthDBFeatures.c >>> index c5b680a..d43efaa 100644 >>> --- a/drivers/net/npe/IxEthDBFeatures.c >>> +++ b/drivers/net/npe/IxEthDBFeatures.c >>> @@ -143,22 +143,22 @@ void ixEthDBFeatureCapabilityScan(void) >>> IX_ENSURE(sizeof (ixEthDBTrafficClassDefinitions) != 0, "DB: no traffic class definitions found, check IxEthDBQoS.h"); >>> >>> /* find the traffic class definition index compatible with the current NPE A functionality ID */ >>> - for (trafficClassDefinitionIndex = 0 ; >>> - trafficClassDefinitionIndex < sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]); >>> - trafficClassDefinitionIndex++) >>> - { >>> - if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) >>> - { >>> - /* found it */ >>> - break; >>> - } >>> - } >>> + for (trafficClassDefinitionIndex = 0; >>> + trafficClassDefinitionIndex < >>> + ARRAY_SIZE(ixEthDBTrafficClassDefinitions); >>> + trafficClassDefinitionIndex++) { >>> + if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) { >>> + /* found it */ >>> + break; >>> + } >>> + } >>> >> Above for loop along with if should be a block separable, means >> >> for (trafficClassDefinitionIndex = 0; >> trafficClassDefinitionIndex < >> ARRAY_SIZE(ixEthDBTrafficClassDefinitions); >> trafficClassDefinitionIndex++) { >> if (..........) { > > Previous msg sent intermediately, > > See my exact comment here. > > for (trafficClassDefinitionIndex = 0; > trafficClassDefinitionIndex < > ARRAY_SIZE(ixEthDBTrafficClassDefinitions); > trafficClassDefinitionIndex++) { > if (..........) { > > The above way is easy to understand where the code block starts, It > just my opinion you may find the different way > but it should be understandable where should code block start for a > given condition statement. > Hi Jagan, I thogut I just proved v2 is worse than v1, but obviously you don't think so. In v1: drivers/net/npe/IxEthDBFeatures.c | 4 ++-- In v2: drivers/net/npe/IxEthDBFeatures.c | 28 ++++++++++++++-------------- The intention of this patch is really simple and clear in v1 - to use ARRAY_SIZE. Note, the checkpatch issue is not introduced by this patch. It is because the original file does not pass checkpath at all. If you do apply v2 to you should be able to see my comment in v2: The whole file in drivers/net/npe/IxEthDBFeatures.c uses 4 spaces as indent, only the lines touched by this patch uses tab as indent (to make checkpatch happy). Note: tab takes 8 spaces. The indent looks really odd after apply v2. I just cannot convince myself v2 is better. I do think we should use v1 and you can send a separate patch to fix all checkpatch warnings if you want. Regards, Axel
diff --git a/drivers/net/ax88180.c b/drivers/net/ax88180.c index f501768..7f0cfe5 100644 --- a/drivers/net/ax88180.c +++ b/drivers/net/ax88180.c @@ -157,7 +157,7 @@ static void ax88180_mac_reset (struct eth_device *dev) OUTW (dev, MISC_RESET_MAC, MISC); tmpval = INW (dev, MISC); - for (i = 0; i < (sizeof (program_seq) / sizeof (program_seq[0])); i++) + for (i = 0; i < ARRAY_SIZE(program_seq); i++) OUTW (dev, program_seq[i].value, program_seq[i].offset); } diff --git a/drivers/net/fsl_mcdmafec.c b/drivers/net/fsl_mcdmafec.c index 63842cd..0e18764 100644 --- a/drivers/net/fsl_mcdmafec.c +++ b/drivers/net/fsl_mcdmafec.c @@ -520,7 +520,7 @@ int mcdmafec_initialize(bd_t * bis) u32 tmp = CONFIG_SYS_INTSRAM + 0x2000; #endif - for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) { + for (i = 0; i < ARRAY_SIZE(fec_info); i++) { dev = (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE, diff --git a/drivers/net/lan91c96.c b/drivers/net/lan91c96.c index 11d350e..47c15c4 100644 --- a/drivers/net/lan91c96.c +++ b/drivers/net/lan91c96.c @@ -779,7 +779,7 @@ static int lan91c96_detect_chip(struct eth_device *dev) SMC_SELECT_BANK(dev, 3); chip_id = (SMC_inw(dev, 0xA) & LAN91C96_REV_CHIPID) >> 4; SMC_SELECT_BANK(dev, 0); - for (r = 0; r < sizeof(supported_chips) / sizeof(struct id_type); r++) + for (r = 0; r < ARRAY_SIZE(supported_chips); r++) if (chip_id == supported_chips[r].id) return r; return 0; diff --git a/drivers/net/mcffec.c b/drivers/net/mcffec.c index ed7459c..7ae6320 100644 --- a/drivers/net/mcffec.c +++ b/drivers/net/mcffec.c @@ -559,7 +559,7 @@ int mcffec_initialize(bd_t * bis) u32 tmp = CONFIG_SYS_INIT_RAM_ADDR + 0x1000; #endif - for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) { + for (i = 0; i < ARRAY_SIZE(fec_info); i++) { dev = (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE, diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c index 5e64dbd..63bc8f8 100644 --- a/drivers/net/mcfmii.c +++ b/drivers/net/mcfmii.c @@ -186,7 +186,7 @@ int mii_discover_phy(struct eth_device *dev) printf("PHY @ 0x%x pass %d\n", phyno, pass); #endif - for (i = 0; (i < (sizeof(phyinfo) / sizeof(phy_info_t))) + for (i = 0; (i < ARRAY_SIZE(phyinfo)) && (phyinfo[i].phyid != 0); i++) { if (phyinfo[i].phyid == phytype) { #ifdef ET_DEBUG diff --git a/drivers/net/ne2000.c b/drivers/net/ne2000.c index 3939158..e6cd3e9 100644 --- a/drivers/net/ne2000.c +++ b/drivers/net/ne2000.c @@ -228,7 +228,7 @@ int get_prom(u8* mac_addr, u8* base_addr) mdelay (10); - for (i = 0; i < sizeof (program_seq) / sizeof (program_seq[0]); i++) + for (i = 0; i < ARRAY_SIZE(program_seq); i++) n2k_outb (program_seq[i].value, program_seq[i].offset); PRINTK ("PROM:"); diff --git a/drivers/net/npe/IxEthDBFeatures.c b/drivers/net/npe/IxEthDBFeatures.c index c5b680a..d43efaa 100644 --- a/drivers/net/npe/IxEthDBFeatures.c +++ b/drivers/net/npe/IxEthDBFeatures.c @@ -143,22 +143,22 @@ void ixEthDBFeatureCapabilityScan(void) IX_ENSURE(sizeof (ixEthDBTrafficClassDefinitions) != 0, "DB: no traffic class definitions found, check IxEthDBQoS.h"); /* find the traffic class definition index compatible with the current NPE A functionality ID */ - for (trafficClassDefinitionIndex = 0 ; - trafficClassDefinitionIndex < sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]); - trafficClassDefinitionIndex++) - { - if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) - { - /* found it */ - break; - } - } + for (trafficClassDefinitionIndex = 0; + trafficClassDefinitionIndex < + ARRAY_SIZE(ixEthDBTrafficClassDefinitions); + trafficClassDefinitionIndex++) { + if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) { + /* found it */ + break; + } + } /* select the default case if we went over the array boundary */ - if (trafficClassDefinitionIndex == sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0])) - { - trafficClassDefinitionIndex = 0; /* the first record is the default case */ - } + if (trafficClassDefinitionIndex == + ARRAY_SIZE(ixEthDBTrafficClassDefinitions)) { + /* the first record is the default case */ + trafficClassDefinitionIndex = 0; + } /* select queue assignment structure based on the traffic class configuration index */ queueStructureIndex = ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_QUEUE_ASSIGNMENT_INDEX]; diff --git a/drivers/net/npe/IxOsalIoMem.c b/drivers/net/npe/IxOsalIoMem.c index 34df92b..15a16b8 100644 --- a/drivers/net/npe/IxOsalIoMem.c +++ b/drivers/net/npe/IxOsalIoMem.c @@ -66,8 +66,7 @@ ixOsalMemMapFind (UINT32 requestedAddress, { UINT32 mapIndex; - UINT32 numMapElements = - sizeof (ixOsalGlobalMemoryMap) / sizeof (IxOsalMemoryMap); + UINT32 numMapElements = ARRAY_SIZE(ixOsalGlobalMemoryMap); for (mapIndex = 0; mapIndex < numMapElements; mapIndex++) { diff --git a/drivers/net/npe/include/IxEthDBPortDefs.h b/drivers/net/npe/include/IxEthDBPortDefs.h index c3acbdd..eac48cb 100644 --- a/drivers/net/npe/include/IxEthDBPortDefs.h +++ b/drivers/net/npe/include/IxEthDBPortDefs.h @@ -106,7 +106,7 @@ static const IxEthDBPortDefinition ixEthDBPortDefinitions[] = * @def IX_ETH_DB_NUMBER_OF_PORTS * @brief number of supported ports */ -#define IX_ETH_DB_NUMBER_OF_PORTS (sizeof (ixEthDBPortDefinitions) / sizeof (ixEthDBPortDefinitions[0])) +#define IX_ETH_DB_NUMBER_OF_PORTS ARRAY_SIZE(ixEthDBPortDefinitions) /** * @def IX_ETH_DB_UNKNOWN_PORT diff --git a/drivers/net/npe/include/IxOsalTypes.h b/drivers/net/npe/include/IxOsalTypes.h index 06e71de..615c655 100644 --- a/drivers/net/npe/include/IxOsalTypes.h +++ b/drivers/net/npe/include/IxOsalTypes.h @@ -93,7 +93,7 @@ typedef volatile INT32 VINT32; #ifndef NUMELEMS -#define NUMELEMS(x) (sizeof(x) / sizeof((x)[0])) +#define NUMELEMS(x) ARRAY_SIZE(x) #endif