| Message ID | 20190508112842.11654-11-alexandru.ardelean@analog.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | treewide: fix match_string() helper when array size | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (8150a153c013aa2dd1ffae43370b89ac1347a7fb) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 30 lines checked |
On Wed, 2019-05-08 at 15:20 +0300, Dan Carpenter wrote: > > > On Wed, May 08, 2019 at 02:28:35PM +0300, Alexandru Ardelean wrote: > > -static const char * const phy_types[] = { > > - "emmc 5.0 phy", > > - "emmc 5.1 phy" > > -}; > > - > > enum xenon_phy_type_enum { > > EMMC_5_0_PHY, > > EMMC_5_1_PHY, > > NR_PHY_TYPES > > There is no need for NR_PHY_TYPES now so you could remove that as well. > I thought the same. The only reason to keep NR_PHY_TYPES, is for potential future patches, where it would be just 1 addition enum xenon_phy_type_enum { EMMC_5_0_PHY, EMMC_5_1_PHY, + EMMC_5_2_PHY, NR_PHY_TYPES } Depending on style/preference of how to do enums (allow comma on last enum or not allow comma on last enum value), adding new enum values woudl be 2 additions + 1 deletion lines. enum xenon_phy_type_enum { EMMC_5_0_PHY, - EMMC_5_1_PHY + EMM C_5_1_PHY, + EMMC_5_2_PHY } Either way (leave NR_PHY_TYPES or remove NR_PHY_TYPES) is fine from my side. Thanks Alex > regards, > dan carpenter >
On Wed, 2019-05-08 at 16:26 +0300, Alexandru Ardelean wrote: > On Wed, 2019-05-08 at 15:20 +0300, Dan Carpenter wrote: > > > > > > On Wed, May 08, 2019 at 02:28:35PM +0300, Alexandru Ardelean wrote: > > > -static const char * const phy_types[] = { > > > - "emmc 5.0 phy", > > > - "emmc 5.1 phy" > > > -}; > > > - > > > enum xenon_phy_type_enum { > > > EMMC_5_0_PHY, > > > EMMC_5_1_PHY, > > > NR_PHY_TYPES > > > > There is no need for NR_PHY_TYPES now so you could remove that as well. > > > > I thought the same. > The only reason to keep NR_PHY_TYPES, is for potential future patches, > where it would be just 1 addition > > enum xenon_phy_type_enum { > EMMC_5_0_PHY, > EMMC_5_1_PHY, > + EMMC_5_2_PHY, > NR_PHY_TYPES > } > > Depending on style/preference of how to do enums (allow comma on last > enum > or not allow comma on last enum value), adding new enum values woudl be 2 > additions + 1 deletion lines. > > enum xenon_phy_type_enum { > EMMC_5_0_PHY, > - EMMC_5_1_PHY > + EMM > C_5_1_PHY, > + EMMC_5_2_PHY > } > > Either way (leave NR_PHY_TYPES or remove NR_PHY_TYPES) is fine from my > side. > Preference on this ? If no objection [nobody insists] I would keep. I don't feel strongly about it [dropping NR_PHY_TYPES or not]. Thanks Alex > Thanks > Alex > > > regards, > > dan carpenter > >
On Fri, May 10, 2019 at 09:13:26AM +0000, Ardelean, Alexandru wrote: > On Wed, 2019-05-08 at 16:26 +0300, Alexandru Ardelean wrote: > > On Wed, 2019-05-08 at 15:20 +0300, Dan Carpenter wrote: > > > > > > > > > On Wed, May 08, 2019 at 02:28:35PM +0300, Alexandru Ardelean wrote: > > > > -static const char * const phy_types[] = { > > > > - "emmc 5.0 phy", > > > > - "emmc 5.1 phy" > > > > -}; > > > > - > > > > enum xenon_phy_type_enum { > > > > EMMC_5_0_PHY, > > > > EMMC_5_1_PHY, > > > > NR_PHY_TYPES > > > > > > There is no need for NR_PHY_TYPES now so you could remove that as well. > > > > > > > I thought the same. > > The only reason to keep NR_PHY_TYPES, is for potential future patches, > > where it would be just 1 addition > > > > enum xenon_phy_type_enum { > > EMMC_5_0_PHY, > > EMMC_5_1_PHY, > > + EMMC_5_2_PHY, > > NR_PHY_TYPES > > } > > > > Depending on style/preference of how to do enums (allow comma on last > > enum > > or not allow comma on last enum value), adding new enum values woudl be 2 > > additions + 1 deletion lines. > > > > enum xenon_phy_type_enum { > > EMMC_5_0_PHY, > > - EMMC_5_1_PHY > > + EMM > > C_5_1_PHY, > > + EMMC_5_2_PHY > > } > > > > Either way (leave NR_PHY_TYPES or remove NR_PHY_TYPES) is fine from my > > side. > > > > Preference on this ? > If no objection [nobody insists] I would keep. > > I don't feel strongly about it [dropping NR_PHY_TYPES or not]. If you end up resending the series could you remove it, but if not then it's not worth it. regards, dan carpenter
diff --git a/drivers/mmc/host/sdhci-xenon-phy.c b/drivers/mmc/host/sdhci-xenon-phy.c index 59b7a6cac995..2a9206867fe1 100644 --- a/drivers/mmc/host/sdhci-xenon-phy.c +++ b/drivers/mmc/host/sdhci-xenon-phy.c @@ -135,17 +135,17 @@ struct xenon_emmc_phy_regs { u32 logic_timing_val; }; -static const char * const phy_types[] = { - "emmc 5.0 phy", - "emmc 5.1 phy" -}; - enum xenon_phy_type_enum { EMMC_5_0_PHY, EMMC_5_1_PHY, NR_PHY_TYPES }; +static const char * const phy_types[NR_PHY_TYPES] = { + [EMMC_5_0_PHY] = "emmc 5.0 phy", + [EMMC_5_1_PHY] = "emmc 5.1 phy" +}; + enum soc_pad_ctrl_type { SOC_PAD_SD, SOC_PAD_FIXED_1_8V, @@ -821,7 +821,7 @@ static int xenon_add_phy(struct device_node *np, struct sdhci_host *host, struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host); int ret; - priv->phy_type = __match_string(phy_types, NR_PHY_TYPES, phy_name); + priv->phy_type = match_string(phy_types, phy_name); if (priv->phy_type < 0) { dev_err(mmc_dev(host->mmc), "Unable to determine PHY name %s. Use default eMMC 5.1 PHY\n",
The change is also cosmetic, but it also does a tighter coupling between the enums & the string values. This way, the ARRAY_SIZE(phy_types) that is implicitly done in the match_string() macro is also a bit safer. Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- drivers/mmc/host/sdhci-xenon-phy.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)