Messages in this thread | | | Date | Wed, 27 May 2020 09:40:35 -0700 | From | Nathan Chancellor <> | Subject | Re: [PATCH] phy: intel: fix enum type mismatch warning |
| |
On Wed, May 27, 2020 at 03:45:06PM +0200, Arnd Bergmann wrote: > clang points out that a local variable is initialized with > an enum value of the wrong type: > > drivers/phy/intel/phy-intel-combo.c:202:34: error: implicit conversion from enumeration type 'enum intel_phy_mode' to different enumeration type 'enum intel_combo_mode' [-Werror,-Wenum-conversion] > enum intel_combo_mode cb_mode = PHY_PCIE_MODE; > ~~~~~~~ ^~~~~~~~~~~~~ > > From reading the code, it seems that this was not only the > wrong type, but not even supposed to be a code path that can > happen in practice. > > Change the code to have no default phy mode but instead return an > error for invalid input. > > Fixes: ac0a95a3ea78 ("phy: intel: Add driver support for ComboPhy") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/phy/intel/phy-intel-combo.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/phy/intel/phy-intel-combo.c b/drivers/phy/intel/phy-intel-combo.c > index c2a35be4cdfb..04f7b0d08742 100644 > --- a/drivers/phy/intel/phy-intel-combo.c > +++ b/drivers/phy/intel/phy-intel-combo.c > @@ -199,7 +199,7 @@ static int intel_cbphy_pcie_dis_pad_refclk(struct intel_cbphy_iphy *iphy) > > static int intel_cbphy_set_mode(struct intel_combo_phy *cbphy) > { > - enum intel_combo_mode cb_mode = PHY_PCIE_MODE; > + enum intel_combo_mode cb_mode; > enum aggregated_mode aggr = cbphy->aggr_mode; > struct device *dev = cbphy->dev; > enum intel_phy_mode mode; > @@ -224,6 +224,8 @@ static int intel_cbphy_set_mode(struct intel_combo_phy *cbphy) > > cb_mode = SATA0_SATA1_MODE; > break; > + default: > + return -EINVAL; > } > > ret = regmap_write(cbphy->hsiocfg, REG_COMBO_MODE(cbphy->bid), cb_mode); > -- > 2.26.2 >
I sent an almost identical patch:
https://lore.kernel.org/lkml/20200523035043.3305846-1-natechancellor@gmail.com/
I left out the default case since clang warns when a switch on an enum does not handle all the values (compile time scream) versus a run time scream like yours.
I don't have a preference for either so:
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
| |