linux-phy.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] phy: stm32: work around constant-value overflow assertion
@ 2024-11-11 10:37 Arnd Bergmann
  2024-12-08 16:58 ` Vinod Koul
  2025-01-10 12:57 ` Christian Bruel
  0 siblings, 2 replies; 3+ messages in thread
From: Arnd Bergmann @ 2024-11-11 10:37 UTC (permalink / raw)
  To: Christian Bruel, Vinod Koul, Kishon Vijay Abraham I,
	Maxime Coquelin, Alexandre Torgue
  Cc: Arnd Bergmann, Yang Li, linux-phy, linux-stm32, linux-arm-kernel,
	linux-kernel

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.5


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] phy: stm32: work around constant-value overflow assertion
  2024-11-11 10:37 [PATCH] phy: stm32: work around constant-value overflow assertion Arnd Bergmann
@ 2024-12-08 16:58 ` Vinod Koul
  2025-01-10 12:57 ` Christian Bruel
  1 sibling, 0 replies; 3+ messages in thread
From: Vinod Koul @ 2024-12-08 16:58 UTC (permalink / raw)
  To: Christian Bruel, Kishon Vijay Abraham I, Maxime Coquelin,
	Alexandre Torgue, Arnd Bergmann
  Cc: Arnd Bergmann, Yang Li, linux-phy, linux-stm32, linux-arm-kernel,
	linux-kernel


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



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] phy: stm32: work around constant-value overflow assertion
  2024-11-11 10:37 [PATCH] phy: stm32: work around constant-value overflow assertion Arnd Bergmann
  2024-12-08 16:58 ` Vinod Koul
@ 2025-01-10 12:57 ` Christian Bruel
  1 sibling, 0 replies; 3+ messages in thread
From: Christian Bruel @ 2025-01-10 12:57 UTC (permalink / raw)
  To: Arnd Bergmann, Vinod Koul, Kishon Vijay Abraham I,
	Maxime Coquelin, Alexandre Torgue
  Cc: Arnd Bergmann, Yang Li, linux-phy, linux-stm32, linux-arm-kernel,
	linux-kernel

Hello,

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;

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-01-10 14:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-11 10:37 [PATCH] phy: stm32: work around constant-value overflow assertion Arnd Bergmann
2024-12-08 16:58 ` Vinod Koul
2025-01-10 12:57 ` Christian Bruel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).