drivers/phy/st/phy-stm32-combophy.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)
From: Arnd Bergmann <arnd@arndb.de>
FIELD_PREP() checks that a constant fits into the available bitfield,
but if one of the two lookup tables in stm32_impedance_tune() does
not find a matching entry, the index is out of range, which gcc
correctly complains about:
In file included from <command-line>:
In function 'stm32_impedance_tune',
inlined from 'stm32_combophy_pll_init' at drivers/phy/st/phy-stm32-combophy.c:247:9:
include/linux/compiler_types.h:517:38: error: call to '__compiletime_assert_447' declared with attribute error: FIELD_PREP: value too large for the field
517 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
include/linux/bitfield.h:68:3: note: in expansion of macro 'BUILD_BUG_ON_MSG'
68 | BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
115 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
| ^~~~~~~~~~~~~~~~
drivers/phy/st/phy-stm32-combophy.c:162:8: note: in expansion of macro 'FIELD_PREP'
162 | FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_VSWING, vswing_of));
| ^~~~~~~~~~
Rework this so the field value gets set inside of the loop and otherwise
set to zero.
Fixes: 47e1bb6b4ba0 ("phy: stm32: Add support for STM32MP25 COMBOPHY.")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/phy/st/phy-stm32-combophy.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/st/phy-stm32-combophy.c b/drivers/phy/st/phy-stm32-combophy.c
index 765bb34fe358..49e9fa90a681 100644
--- a/drivers/phy/st/phy-stm32-combophy.c
+++ b/drivers/phy/st/phy-stm32-combophy.c
@@ -122,6 +122,7 @@ static int stm32_impedance_tune(struct stm32_combophy *combophy)
u32 max_vswing = imp_lookup[imp_size - 1].vswing[vswing_size - 1];
u32 min_vswing = imp_lookup[0].vswing[0];
u32 val;
+ u32 regval;
if (!of_property_read_u32(combophy->dev->of_node, "st,output-micro-ohms", &val)) {
if (val < min_imp || val > max_imp) {
@@ -129,16 +130,20 @@ static int stm32_impedance_tune(struct stm32_combophy *combophy)
return -EINVAL;
}
- for (imp_of = 0; imp_of < ARRAY_SIZE(imp_lookup); imp_of++)
- if (imp_lookup[imp_of].microohm <= val)
+ regval = 0;
+ for (imp_of = 0; imp_of < ARRAY_SIZE(imp_lookup); imp_of++) {
+ if (imp_lookup[imp_of].microohm <= val) {
+ regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_OHM, imp_of);
break;
+ }
+ }
dev_dbg(combophy->dev, "Set %u micro-ohms output impedance\n",
imp_lookup[imp_of].microohm);
regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
STM32MP25_PCIEPRG_IMPCTRL_OHM,
- FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_OHM, imp_of));
+ regval);
} else {
regmap_read(combophy->regmap, SYSCFG_PCIEPRGCR, &val);
imp_of = FIELD_GET(STM32MP25_PCIEPRG_IMPCTRL_OHM, val);
@@ -150,16 +155,20 @@ static int stm32_impedance_tune(struct stm32_combophy *combophy)
return -EINVAL;
}
- for (vswing_of = 0; vswing_of < ARRAY_SIZE(imp_lookup[imp_of].vswing); vswing_of++)
- if (imp_lookup[imp_of].vswing[vswing_of] >= val)
+ regval = 0;
+ for (vswing_of = 0; vswing_of < ARRAY_SIZE(imp_lookup[imp_of].vswing); vswing_of++) {
+ if (imp_lookup[imp_of].vswing[vswing_of] >= val) {
+ regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_VSWING, vswing_of);
break;
+ }
+ }
dev_dbg(combophy->dev, "Set %u microvolt swing\n",
imp_lookup[imp_of].vswing[vswing_of]);
regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
STM32MP25_PCIEPRG_IMPCTRL_VSWING,
- FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_VSWING, vswing_of));
+ regval);
}
return 0;
--
2.39.5Hello,
Sorry, I missed this patch.
The gcc warning bellow is a false positive as the range is necessarily
between the min_imp/max_imp values, thus imp_lookup will catch, or
returned -EINVAL earlier
So maybe a more explicit way to shut-up the warning would be to use a
boolean flag with a WARN_ON, something like:
bool found;
for (imp_of = 0; imp_of < ARRAY_SIZE(imp_lookup); imp_of++) {
if (imp_lookup[imp_of].microohm <= val) {
found = true;
break;
}
}
dev_dbg(combophy->dev, "Set %u micro-ohms output impedance\n",
imp_lookup[imp_of].microohm);
if (WARN_ON(! found))
return;
regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
STM32MP25_PCIEPRG_IMPCTRL_OHM,
FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_OHM, imp_of));
Regards
Christian
On 11/11/24 11:37, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> FIELD_PREP() checks that a constant fits into the available bitfield,
> but if one of the two lookup tables in stm32_impedance_tune() does
> not find a matching entry, the index is out of range, which gcc
> correctly complains about:
>
> In file included from <command-line>:
> In function 'stm32_impedance_tune',
> inlined from 'stm32_combophy_pll_init' at drivers/phy/st/phy-stm32-combophy.c:247:9:
> include/linux/compiler_types.h:517:38: error: call to '__compiletime_assert_447' declared with attribute error: FIELD_PREP: value too large for the field
> 517 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> | ^
> include/linux/bitfield.h:68:3: note: in expansion of macro 'BUILD_BUG_ON_MSG'
> 68 | BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
> 115 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
> | ^~~~~~~~~~~~~~~~
> drivers/phy/st/phy-stm32-combophy.c:162:8: note: in expansion of macro 'FIELD_PREP'
> 162 | FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_VSWING, vswing_of));
> | ^~~~~~~~~~
>
> Rework this so the field value gets set inside of the loop and otherwise
> set to zero.
>
> Fixes: 47e1bb6b4ba0 ("phy: stm32: Add support for STM32MP25 COMBOPHY.")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/phy/st/phy-stm32-combophy.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/phy/st/phy-stm32-combophy.c b/drivers/phy/st/phy-stm32-combophy.c
> index 765bb34fe358..49e9fa90a681 100644
> --- a/drivers/phy/st/phy-stm32-combophy.c
> +++ b/drivers/phy/st/phy-stm32-combophy.c
> @@ -122,6 +122,7 @@ static int stm32_impedance_tune(struct stm32_combophy *combophy)
> u32 max_vswing = imp_lookup[imp_size - 1].vswing[vswing_size - 1];
> u32 min_vswing = imp_lookup[0].vswing[0];
> u32 val;
> + u32 regval;
>
> if (!of_property_read_u32(combophy->dev->of_node, "st,output-micro-ohms", &val)) {
> if (val < min_imp || val > max_imp) {
> @@ -129,16 +130,20 @@ static int stm32_impedance_tune(struct stm32_combophy *combophy)
> return -EINVAL;
> }
>
> - for (imp_of = 0; imp_of < ARRAY_SIZE(imp_lookup); imp_of++)
> - if (imp_lookup[imp_of].microohm <= val)
> + regval = 0;
> + for (imp_of = 0; imp_of < ARRAY_SIZE(imp_lookup); imp_of++) {
> + if (imp_lookup[imp_of].microohm <= val) {
> + regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_OHM, imp_of);
> break;
> + }
> + }
>
> dev_dbg(combophy->dev, "Set %u micro-ohms output impedance\n",
> imp_lookup[imp_of].microohm);
>
> regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
> STM32MP25_PCIEPRG_IMPCTRL_OHM,
> - FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_OHM, imp_of));
> + regval);
> } else {
> regmap_read(combophy->regmap, SYSCFG_PCIEPRGCR, &val);
> imp_of = FIELD_GET(STM32MP25_PCIEPRG_IMPCTRL_OHM, val);
> @@ -150,16 +155,20 @@ static int stm32_impedance_tune(struct stm32_combophy *combophy)
> return -EINVAL;
> }
>
> - for (vswing_of = 0; vswing_of < ARRAY_SIZE(imp_lookup[imp_of].vswing); vswing_of++)
> - if (imp_lookup[imp_of].vswing[vswing_of] >= val)
> + regval = 0;
> + for (vswing_of = 0; vswing_of < ARRAY_SIZE(imp_lookup[imp_of].vswing); vswing_of++) {
> + if (imp_lookup[imp_of].vswing[vswing_of] >= val) {
> + regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_VSWING, vswing_of);
> break;
> + }
> + }
>
> dev_dbg(combophy->dev, "Set %u microvolt swing\n",
> imp_lookup[imp_of].vswing[vswing_of]);
>
> regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
> STM32MP25_PCIEPRG_IMPCTRL_VSWING,
> - FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_VSWING, vswing_of));
> + regval);
> }
>
> return 0;
On Mon, 11 Nov 2024 11:37:02 +0100, Arnd Bergmann wrote:
> FIELD_PREP() checks that a constant fits into the available bitfield,
> but if one of the two lookup tables in stm32_impedance_tune() does
> not find a matching entry, the index is out of range, which gcc
> correctly complains about:
>
> In file included from <command-line>:
> In function 'stm32_impedance_tune',
> inlined from 'stm32_combophy_pll_init' at drivers/phy/st/phy-stm32-combophy.c:247:9:
> include/linux/compiler_types.h:517:38: error: call to '__compiletime_assert_447' declared with attribute error: FIELD_PREP: value too large for the field
> 517 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> | ^
> include/linux/bitfield.h:68:3: note: in expansion of macro 'BUILD_BUG_ON_MSG'
> 68 | BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
> 115 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
> | ^~~~~~~~~~~~~~~~
> drivers/phy/st/phy-stm32-combophy.c:162:8: note: in expansion of macro 'FIELD_PREP'
> 162 | FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_VSWING, vswing_of));
> | ^~~~~~~~~~
>
> [...]
Applied, thanks!
[1/1] phy: stm32: work around constant-value overflow assertion
commit: 2de679ecd724b823c2cb58caab8508c7eec8aefc
Best regards,
--
~Vinod
© 2016 - 2026 Red Hat, Inc.