drivers: smbus: stm32: support packet-error-checking (pec) #96691

cfriedt · 2025-09-28T22:50:51Z

drivers: smbus: provide packet-error-checking (pec) routines
tests: drivers: smbus: add packet error correction (pec) testsuite
drivers: smbus: stm32: support packet-error-checking (pec)

etienne-lms

Minor typos in header file inline description comments. Otherwise LGTM.

drivers/smbus/smbus_utils.h

drivers/smbus/smbus_stm32.c

mathieuchopstm · 2025-10-08T09:08:31Z

drivers/smbus/smbus_utils.h

+ * @note This function is intended for SMBus drivers that do not have hardware PEC support and
+ * therefore must rely on software PEC calculation.


Maybe these should be guarded by a Kconfig symbol then?

Woops - I still need to add some ifdefs to smbus_util.c for this

cfriedt · 2025-10-08T12:44:30Z

drivers/smbus/smbus_stm32.c

+	*count = msgs[2].len;
+	ret = smbus_read_check_pec(data->config, periph_addr, msgs, num_msgs);
+	if (ret < 0) {
+		return ret;


It would be good to investigate capturing error statistics, in this block, in a subsequent PR.

Also mentioned here
https://github.com/zephyrproject-rtos/zephyr/pull/96691/files#r2413732281

drivers/smbus/smbus_stm32.c

In the case that SMBus hardware does not automatically perform packet error checking (PEC), provide generic inline functions in `zephyr/drivers/smbus.h` that can be used by drivers to perform PEC in software. Signed-off-by: Chris Friedt <[email protected]> Signed-off-by: Andrew Lewycky <[email protected]>

Add tests for SMBus packet error correction (PEC). The two primary utilities tested are * smbus_pec(): compute the PEC byte for a Block Write * smbus_read_check_pec(): verify a PEC after * Read Byte * Read Word * Block Read Signed-off-by: Chris Friedt <[email protected]>

Add support for SMBus packet error checking (PEC) to the stm32 driver. This feature allows SMBust communication to be slightly more robust in the presence of noise, in that packet errors can be detected on the receive side. Signed-off-by: Andrew Lewycky <[email protected]> Signed-off-by: Chris Friedt <[email protected]>

cfriedt · 2025-10-08T15:20:38Z

@etienne-lms @mathieuchopstm - sorry to ask for a re-approval. Just a minor change from the previous commit (to ensure that unnecessary code is not compiled.

The difference is here
https://github.com/zephyrproject-rtos/zephyr/compare/3ae7fc69c9e9cde833d7214404d2ce783ca90fb7..f6654476ccba5e08134f890c3bb19b98c78979e9

sonarqubecloud · 2025-10-08T15:40:12Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
8.1% Duplication on New Code

See analysis details on SonarQube Cloud

etienne-lms · 2025-10-08T15:55:42Z

drivers/smbus/Kconfig

 	  SMBus device driver initialization priority.

+config SMBUS_SOFT_PEC
+	bool "SMBus software PEC support"


Is this option configurable by the project? It seems rather a driver private config.
... hmmm, i see at least the test sample needs to enable it.

It's configurable by the project. Technically could be used by any driver or other code.

mathieuchopstm · 2025-10-08T16:11:09Z

@etienne-lms @mathieuchopstm - sorry to ask for a re-approval. Just a minor change from the previous commit (to ensure that unnecessary code is not compiled.

The difference is here https://github.com/zephyrproject-rtos/zephyr/compare/3ae7fc69c9e9cde833d7214404d2ce783ca90fb7..f6654476ccba5e08134f890c3bb19b98c78979e9

No problem - as a matter of fact, I was wondering if this wasn't missing when approving 😉

erwango · 2025-10-09T13:03:47Z

@benediktibk Would you have time to have a look ?

benediktibk · 2025-10-09T16:47:40Z

drivers/smbus/Kconfig

 	depends on DT_HAS_ST_STM32_SMBUS_ENABLED
 	depends on I2C_STM32
 	select PINCTRL
+	select SMBUS_SOFT_PEC


I suppose there are two ways to look at this:

PEC is a feature, which is implemented in the STM32 SMBus driver

PEC is a capability which is required by the SMBus driver

With the select here it is more the latter one, although it should actually be the first one? But this then would also mean that everything has to be ifdef-ed in the driver itself, so I'm not sure if it is actually a good idea.

Slowly I start to understand it, you actually need this config-flag so that the utility functions for PEC (which themselves require CRC) are only pulled in when necessary?

With the select here it is more the latter one, although it should actually be the first one?

"Should" is potentially debatable. E.g. intel_pch_smbus.c implicitly supports PEC.

zephyr/drivers/smbus/intel_pch_smbus.c

Line 332 in d3e645e

if (auxs & PCH_SMBUS_AUXS_CRC_ERROR) {

The stm32 driver is the only other smbus driver in-tree.

It's almost completely platform-independent, actually, with the exception of i2c_stm32_smbalert_enable(), i2c_stm32_smbalert_disable(), and i2c_stm32_set_smbus_mode().

Personally, I would feel better if there was a uniform API, so that PEC can at least be configured by the application, but it isn't disabled by default here (the Zephyr convention), so I can see your point.

everything has to be ifdef-ed in the driver itself, so I'm not sure if it is actually a good idea.

SMBUS_SOFT_PEC could use depends on CRC instead of select, and the stm32 driver could use conditional compilation throughout.

I'm ok to go either way.

@benediktibk , @erwango , @etienne-lms - do you have preferences?

everything has to be ifdef-ed in the driver itself, so I'm not sure if it is actually a good idea.

SMBUS_SOFT_PEC could use depends on CRC instead of select, and the stm32 driver could use conditional compilation throughout.

I'm ok to go either way.

I'd go for SMBUS_SOFT_PEC select'ing CRC and the consumer drivers select'ing SMBUS_SOFT_PEC as done in PR right now. This avoids duplicating the select CRC in every driver's Kconfig and is just more sensical IMO.

cfriedt mentioned this pull request Sep 30, 2025

zephyr: patches: use smbus pec patch as it is in upstream pr tenstorrent/tt-zephyr-platforms#666

Merged

cfriedt force-pushed the stm32-smbus-pec-patch branch 3 times, most recently from e22919f to 57a4ac9 Compare October 3, 2025 15:36

cfriedt marked this pull request as ready for review October 3, 2025 15:40

cfriedt added the area: SMBus label Oct 3, 2025

zephyrbot added area: Tests Issues related to a particular existing or missing test platform: STM32 ST Micro STM32 labels Oct 3, 2025

zephyrbot requested review from FRASTM, GeorgeCGV, djiatsaf-st, erwango, etienne-lms, finikorg, gautierg-st, marwaiehm-st, mathieuchopstm and nashif October 3, 2025 15:42

zephyrbot assigned finikorg Oct 3, 2025

cfriedt assigned erwango Oct 3, 2025

cfriedt force-pushed the stm32-smbus-pec-patch branch from 57a4ac9 to d34b26a Compare October 3, 2025 15:53

etienne-lms reviewed Oct 8, 2025

View reviewed changes

drivers/smbus/smbus_utils.h Outdated Show resolved Hide resolved

drivers/smbus/smbus_utils.h Outdated Show resolved Hide resolved

drivers/smbus/smbus_stm32.c Show resolved Hide resolved

mathieuchopstm reviewed Oct 8, 2025

View reviewed changes

erwango requested a review from benediktibk October 8, 2025 09:31

cfriedt mentioned this pull request Oct 8, 2025

drivers: smbus: stm32: add support for pcall api #97188

Draft

cfriedt added this to the v4.3.0 milestone Oct 8, 2025

cfriedt commented Oct 8, 2025

View reviewed changes

cfriedt force-pushed the stm32-smbus-pec-patch branch from d34b26a to 3ae7fc6 Compare October 8, 2025 13:02

cfriedt requested review from etienne-lms and mathieuchopstm October 8, 2025 13:03

etienne-lms previously approved these changes Oct 8, 2025

View reviewed changes

drivers/smbus/smbus_stm32.c Show resolved Hide resolved

mathieuchopstm previously approved these changes Oct 8, 2025

View reviewed changes

cfriedt added 3 commits October 8, 2025 11:14

cfriedt dismissed stale reviews from mathieuchopstm and etienne-lms via f665447 October 8, 2025 15:14

cfriedt force-pushed the stm32-smbus-pec-patch branch from 3ae7fc6 to f665447 Compare October 8, 2025 15:14

cfriedt requested review from etienne-lms and mathieuchopstm October 8, 2025 15:20

etienne-lms approved these changes Oct 8, 2025

View reviewed changes

mathieuchopstm approved these changes Oct 8, 2025

View reviewed changes

benediktibk reviewed Oct 9, 2025

View reviewed changes

		* @note This function is intended for SMBus drivers that do not have hardware PEC support and
		* therefore must rely on software PEC calculation.

drivers: smbus: stm32: support packet-error-checking (pec) #96691

Are you sure you want to change the base?

drivers: smbus: stm32: support packet-error-checking (pec) #96691

Conversation

cfriedt commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cfriedt commented Oct 8, 2025

Uh oh!

sonarqubecloud bot commented Oct 8, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathieuchopstm commented Oct 8, 2025

Uh oh!

erwango commented Oct 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cfriedt Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

cfriedt commented Sep 28, 2025 •

edited

Loading

cfriedt Oct 9, 2025 •

edited

Loading