drivers/phy/st/phy-stm32-combophy.c | 84 ++++++++++++++--------------- 1 file changed, 42 insertions(+), 42 deletions(-)
phy_init can be recalled during PM resume. Thus cache the tuning values
read from the device tree.
Don't read the known default ohm value from regmap.
Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
---
v2: Fix 'val' uninitialized variable from clang build bot
drivers/phy/st/phy-stm32-combophy.c | 84 ++++++++++++++---------------
1 file changed, 42 insertions(+), 42 deletions(-)
diff --git a/drivers/phy/st/phy-stm32-combophy.c b/drivers/phy/st/phy-stm32-combophy.c
index 49e9fa90a681..5c09c4e48bd0 100644
--- a/drivers/phy/st/phy-stm32-combophy.c
+++ b/drivers/phy/st/phy-stm32-combophy.c
@@ -86,6 +86,10 @@ struct stm32_combophy {
struct clk_bulk_data clks[ARRAY_SIZE(combophy_clks)];
int num_clks;
bool have_pad_clk;
+ bool have_ssc;
+ int rx_eq;
+ u32 microohm;
+ u32 microvolt;
unsigned int type;
bool is_init;
int irq_wakeup;
@@ -112,27 +116,15 @@ static const struct clk_impedance imp_lookup[] = {
{ 3999000, { 571000, 648000, 726000, 803000 } }
};
-static int stm32_impedance_tune(struct stm32_combophy *combophy)
+static void stm32_impedance_tune(struct stm32_combophy *combophy)
{
- u8 imp_size = ARRAY_SIZE(imp_lookup);
- u8 vswing_size = ARRAY_SIZE(imp_lookup[0].vswing);
u8 imp_of, vswing_of;
- u32 max_imp = imp_lookup[0].microohm;
- u32 min_imp = imp_lookup[imp_size - 1].microohm;
- 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) {
- dev_err(combophy->dev, "Invalid value %u for output ohm\n", val);
- return -EINVAL;
- }
-
+ if (combophy->microohm) {
regval = 0;
for (imp_of = 0; imp_of < ARRAY_SIZE(imp_lookup); imp_of++) {
- if (imp_lookup[imp_of].microohm <= val) {
+ if (imp_lookup[imp_of].microohm <= combophy->microohm) {
regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_OHM, imp_of);
break;
}
@@ -145,19 +137,14 @@ static int stm32_impedance_tune(struct stm32_combophy *combophy)
STM32MP25_PCIEPRG_IMPCTRL_OHM,
regval);
} else {
- regmap_read(combophy->regmap, SYSCFG_PCIEPRGCR, &val);
- imp_of = FIELD_GET(STM32MP25_PCIEPRG_IMPCTRL_OHM, val);
+ /* default is 50 ohm */
+ imp_of = 3;
}
- if (!of_property_read_u32(combophy->dev->of_node, "st,output-vswing-microvolt", &val)) {
- if (val < min_vswing || val > max_vswing) {
- dev_err(combophy->dev, "Invalid value %u for output vswing\n", val);
- return -EINVAL;
- }
-
+ if (combophy->microvolt) {
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) {
+ if (imp_lookup[imp_of].vswing[vswing_of] >= combophy->microvolt) {
regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_VSWING, vswing_of);
break;
}
@@ -170,8 +157,6 @@ static int stm32_impedance_tune(struct stm32_combophy *combophy)
STM32MP25_PCIEPRG_IMPCTRL_VSWING,
regval);
}
-
- return 0;
}
static int stm32_combophy_pll_init(struct stm32_combophy *combophy)
@@ -197,7 +182,7 @@ static int stm32_combophy_pll_init(struct stm32_combophy *combophy)
cr1_val |= SYSCFG_COMBOPHY_CR1_REFSSPEN;
}
- if (of_property_present(combophy->dev->of_node, "st,ssc-on")) {
+ if (combophy->have_ssc) {
dev_dbg(combophy->dev, "Enabling clock with SSC\n");
cr1_mask |= SYSCFG_COMBOPHY_CR1_SSCEN;
cr1_val |= SYSCFG_COMBOPHY_CR1_SSCEN;
@@ -253,24 +238,16 @@ static int stm32_combophy_pll_init(struct stm32_combophy *combophy)
reset_control_assert(combophy->phy_reset);
if (combophy->type == PHY_TYPE_PCIE) {
- ret = stm32_impedance_tune(combophy);
- if (ret)
- goto out_iso;
+ stm32_impedance_tune(combophy);
cr1_mask |= SYSCFG_COMBOPHY_CR1_REFUSEPAD;
cr1_val |= combophy->have_pad_clk ? SYSCFG_COMBOPHY_CR1_REFUSEPAD : 0;
}
- if (!of_property_read_u32(combophy->dev->of_node, "st,rx-equalizer", &val)) {
- dev_dbg(combophy->dev, "Set RX equalizer %u\n", val);
- if (val > SYSCFG_COMBOPHY_CR4_RX0_EQ) {
- dev_err(combophy->dev, "Invalid value %u for rx0 equalizer\n", val);
- ret = -EINVAL;
- goto out_iso;
- }
-
+ if (combophy->rx_eq != -1) {
+ dev_dbg(combophy->dev, "Set RX equalizer %u\n", combophy->rx_eq);
regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR4,
- SYSCFG_COMBOPHY_CR4_RX0_EQ, val);
+ SYSCFG_COMBOPHY_CR4_RX0_EQ, combophy->rx_eq);
}
regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR1, cr1_mask, cr1_val);
@@ -314,9 +291,6 @@ static int stm32_combophy_pll_init(struct stm32_combophy *combophy)
return 0;
-out_iso:
- reset_control_deassert(combophy->phy_reset);
-
out:
regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR2,
SYSCFG_COMBOPHY_CR2_ISO_DIS, 0);
@@ -522,6 +496,12 @@ static int stm32_combophy_probe(struct platform_device *pdev)
struct stm32_combophy *combophy;
struct device *dev = &pdev->dev;
struct phy_provider *phy_provider;
+ u8 imp_size = ARRAY_SIZE(imp_lookup);
+ u8 vswing_size = ARRAY_SIZE(imp_lookup[0].vswing);
+ u32 max_imp = imp_lookup[0].microohm;
+ u32 min_imp = imp_lookup[imp_size - 1].microohm;
+ u32 max_vswing = imp_lookup[imp_size - 1].vswing[vswing_size - 1];
+ u32 min_vswing = imp_lookup[0].vswing[0];
int ret, irq;
combophy = devm_kzalloc(dev, sizeof(*combophy), GFP_KERNEL);
@@ -569,6 +549,26 @@ static int stm32_combophy_probe(struct platform_device *pdev)
combophy->irq_wakeup);
}
+ if (of_property_present(dev->of_node, "st,ssc-on"))
+ combophy->have_ssc = true;
+
+ if (!of_property_read_u32(dev->of_node, "st,rx-equalizer", &combophy->rx_eq)) {
+ if (combophy->rx_eq > SYSCFG_COMBOPHY_CR4_RX0_EQ)
+ return dev_err_probe(dev, combophy->rx_eq,
+ "Invalid value for rx0 equalizer\n");
+ } else
+ combophy->rx_eq = -1;
+
+ if (!of_property_read_u32(dev->of_node, "st,output-micro-ohms", &combophy->microohm))
+ if (combophy->microohm < min_imp || combophy->microohm > max_imp)
+ return dev_err_probe(dev, combophy->microohm,
+ "Invalid value for output ohm\n");
+
+ if (!of_property_read_u32(dev->of_node, "st,output-vswing-microvolt", &combophy->microvolt))
+ if (combophy->microvolt < min_vswing || combophy->microvolt > max_vswing)
+ return dev_err_probe(dev, combophy->microvolt,
+ "Invalid value for output vswing\n");
+
ret = devm_pm_runtime_enable(dev);
if (ret)
return dev_err_probe(dev, ret, "Failed to enable pm runtime\n");
--
2.34.1Hi Christian,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Christian-Bruel/phy-stm32-Optimize-tuning-values-from-DT/20250113-172435
base: v6.13-rc7
patch link: https://lore.kernel.org/r/20250113092001.1344151-1-christian.bruel%40foss.st.com
patch subject: [PATCH v2] phy: stm32: Optimize tuning values from DT.
config: arm-randconfig-r071-20250117 (https://download.01.org/0day-ci/archive/20250117/202501171619.0XDYDyBZ-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202501171619.0XDYDyBZ-lkp@intel.com/
New smatch warnings:
drivers/phy/st/phy-stm32-combophy.c:147 stm32_impedance_tune() error: buffer overflow 'imp_lookup' 8 <= 8 (assuming for loop doesn't break)
drivers/phy/st/phy-stm32-combophy.c:564 stm32_combophy_probe() warn: passing zero to 'dev_err_probe'
vim +147 drivers/phy/st/phy-stm32-combophy.c
31219ca5436f01 Christian Bruel 2025-01-13 119 static void stm32_impedance_tune(struct stm32_combophy *combophy)
47e1bb6b4ba098 Christian Bruel 2024-09-30 120 {
47e1bb6b4ba098 Christian Bruel 2024-09-30 121 u8 imp_of, vswing_of;
2de679ecd724b8 Arnd Bergmann 2024-11-11 122 u32 regval;
47e1bb6b4ba098 Christian Bruel 2024-09-30 123
31219ca5436f01 Christian Bruel 2025-01-13 124 if (combophy->microohm) {
2de679ecd724b8 Arnd Bergmann 2024-11-11 125 regval = 0;
2de679ecd724b8 Arnd Bergmann 2024-11-11 126 for (imp_of = 0; imp_of < ARRAY_SIZE(imp_lookup); imp_of++) {
31219ca5436f01 Christian Bruel 2025-01-13 127 if (imp_lookup[imp_of].microohm <= combophy->microohm) {
2de679ecd724b8 Arnd Bergmann 2024-11-11 128 regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_OHM, imp_of);
47e1bb6b4ba098 Christian Bruel 2024-09-30 129 break;
2de679ecd724b8 Arnd Bergmann 2024-11-11 130 }
2de679ecd724b8 Arnd Bergmann 2024-11-11 131 }
If we don't hit the break sttaement above
47e1bb6b4ba098 Christian Bruel 2024-09-30 132
47e1bb6b4ba098 Christian Bruel 2024-09-30 133 dev_dbg(combophy->dev, "Set %u micro-ohms output impedance\n",
47e1bb6b4ba098 Christian Bruel 2024-09-30 134 imp_lookup[imp_of].microohm);
^^^^^^
Then this is an out of bounds access.
47e1bb6b4ba098 Christian Bruel 2024-09-30 135
47e1bb6b4ba098 Christian Bruel 2024-09-30 136 regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
47e1bb6b4ba098 Christian Bruel 2024-09-30 137 STM32MP25_PCIEPRG_IMPCTRL_OHM,
2de679ecd724b8 Arnd Bergmann 2024-11-11 138 regval);
47e1bb6b4ba098 Christian Bruel 2024-09-30 139 } else {
31219ca5436f01 Christian Bruel 2025-01-13 140 /* default is 50 ohm */
31219ca5436f01 Christian Bruel 2025-01-13 141 imp_of = 3;
47e1bb6b4ba098 Christian Bruel 2024-09-30 142 }
47e1bb6b4ba098 Christian Bruel 2024-09-30 143
31219ca5436f01 Christian Bruel 2025-01-13 144 if (combophy->microvolt) {
2de679ecd724b8 Arnd Bergmann 2024-11-11 145 regval = 0;
2de679ecd724b8 Arnd Bergmann 2024-11-11 146 for (vswing_of = 0; vswing_of < ARRAY_SIZE(imp_lookup[imp_of].vswing); vswing_of++) {
31219ca5436f01 Christian Bruel 2025-01-13 @147 if (imp_lookup[imp_of].vswing[vswing_of] >= combophy->microvolt) {
2de679ecd724b8 Arnd Bergmann 2024-11-11 148 regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_VSWING, vswing_of);
47e1bb6b4ba098 Christian Bruel 2024-09-30 149 break;
2de679ecd724b8 Arnd Bergmann 2024-11-11 150 }
2de679ecd724b8 Arnd Bergmann 2024-11-11 151 }
47e1bb6b4ba098 Christian Bruel 2024-09-30 152
47e1bb6b4ba098 Christian Bruel 2024-09-30 153 dev_dbg(combophy->dev, "Set %u microvolt swing\n",
47e1bb6b4ba098 Christian Bruel 2024-09-30 154 imp_lookup[imp_of].vswing[vswing_of]);
47e1bb6b4ba098 Christian Bruel 2024-09-30 155
47e1bb6b4ba098 Christian Bruel 2024-09-30 156 regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
47e1bb6b4ba098 Christian Bruel 2024-09-30 157 STM32MP25_PCIEPRG_IMPCTRL_VSWING,
2de679ecd724b8 Arnd Bergmann 2024-11-11 158 regval);
47e1bb6b4ba098 Christian Bruel 2024-09-30 159 }
47e1bb6b4ba098 Christian Bruel 2024-09-30 160 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Dan,
Thanks for reporting. I think this error is a false positive.
In this situation, the impedance values that are looked-up are ordere
(the compiler probably doesn't analysis this) so the imp_looup test that
defines the imp_of must have matched.
Note that the boundaries was checked during probe.
already discussed here:
https://lore.kernel.org/all/ef1ea5da-4344-4c59-8d2b1b52533ef0cd@foss.st.com/
I'll provide a new revision to make this code more friendly with this error
Regards
Christian
On 1/17/25 09:38, Dan Carpenter wrote:
> Hi Christian,
>
> kernel test robot noticed the following build warnings:
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Christian-Bruel/phy-stm32-Optimize-tuning-values-from-DT/20250113-172435
> base: v6.13-rc7
> patch link: https://lore.kernel.org/r/20250113092001.1344151-1-christian.bruel%40foss.st.com
> patch subject: [PATCH v2] phy: stm32: Optimize tuning values from DT.
> config: arm-randconfig-r071-20250117 (https://download.01.org/0day-ci/archive/20250117/202501171619.0XDYDyBZ-lkp@intel.com/config)
> compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202501171619.0XDYDyBZ-lkp@intel.com/
>
> New smatch warnings:
> drivers/phy/st/phy-stm32-combophy.c:147 stm32_impedance_tune() error: buffer overflow 'imp_lookup' 8 <= 8 (assuming for loop doesn't break)
> drivers/phy/st/phy-stm32-combophy.c:564 stm32_combophy_probe() warn: passing zero to 'dev_err_probe'
>
> vim +147 drivers/phy/st/phy-stm32-combophy.c
>
> 31219ca5436f01 Christian Bruel 2025-01-13 119 static void stm32_impedance_tune(struct stm32_combophy *combophy)
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 120 {
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 121 u8 imp_of, vswing_of;
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 122 u32 regval;
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 123
> 31219ca5436f01 Christian Bruel 2025-01-13 124 if (combophy->microohm) {
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 125 regval = 0;
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 126 for (imp_of = 0; imp_of < ARRAY_SIZE(imp_lookup); imp_of++) {
> 31219ca5436f01 Christian Bruel 2025-01-13 127 if (imp_lookup[imp_of].microohm <= combophy->microohm) {
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 128 regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_OHM, imp_of);
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 129 break;
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 130 }
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 131 }
>
> If we don't hit the break sttaement above
>
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 132
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 133 dev_dbg(combophy->dev, "Set %u micro-ohms output impedance\n",
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 134 imp_lookup[imp_of].microohm);
> ^^^^^^
> Then this is an out of bounds access.
>
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 135
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 136 regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 137 STM32MP25_PCIEPRG_IMPCTRL_OHM,
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 138 regval);
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 139 } else {
> 31219ca5436f01 Christian Bruel 2025-01-13 140 /* default is 50 ohm */
> 31219ca5436f01 Christian Bruel 2025-01-13 141 imp_of = 3;
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 142 }
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 143
> 31219ca5436f01 Christian Bruel 2025-01-13 144 if (combophy->microvolt) {
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 145 regval = 0;
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 146 for (vswing_of = 0; vswing_of < ARRAY_SIZE(imp_lookup[imp_of].vswing); vswing_of++) {
> 31219ca5436f01 Christian Bruel 2025-01-13 @147 if (imp_lookup[imp_of].vswing[vswing_of] >= combophy->microvolt) {
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 148 regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_VSWING, vswing_of);
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 149 break;
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 150 }
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 151 }
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 152
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 153 dev_dbg(combophy->dev, "Set %u microvolt swing\n",
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 154 imp_lookup[imp_of].vswing[vswing_of]);
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 155
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 156 regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 157 STM32MP25_PCIEPRG_IMPCTRL_VSWING,
> 2de679ecd724b8 Arnd Bergmann 2024-11-11 158 regval);
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 159 }
> 47e1bb6b4ba098 Christian Bruel 2024-09-30 160 }
>
© 2016 - 2025 Red Hat, Inc.