Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+515 −36
Open

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2703e2f
drivers: smbus: provide packet-error-checking (pec) routines
cfriedt Sep 28, 2025
4483c38
tests: drivers: smbus: add packet error correction (pec) testsuite
cfriedt Oct 3, 2025
f665447
drivers: smbus: stm32: support packet-error-checking (pec)
cfriedt Sep 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions drivers/smbus/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ config SMBUS_INIT_PRIORITY
help
SMBus device driver initialization priority.

config SMBUS_SOFT_PEC
bool "SMBus software PEC support"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

select CRC
help
Enable software Packet Error Checking (PEC) support.

These generic functions can be used by SMBus drivers for transceivers that do not support
PEC in hardware.

module = SMBUS
module-str = smbus
Expand Down Expand Up @@ -83,6 +91,7 @@ menuconfig SMBUS_STM32
depends on DT_HAS_ST_STM32_SMBUS_ENABLED
depends on I2C_STM32
select PINCTRL
select SMBUS_SOFT_PEC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

@cfriedt cfriedt Oct 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

help
Enable STM32 SMBus driver.

Expand Down
75 changes: 75 additions & 0 deletions drivers/smbus/smbus_utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
#include "smbus_utils.h"

#include <zephyr/logging/log.h>
#include <zephyr/sys/__assert.h>
#include <zephyr/sys/crc.h>
#include <zephyr/sys/util.h>

LOG_MODULE_REGISTER(smbus_utils, CONFIG_SMBUS_LOG_LEVEL);

Expand Down Expand Up @@ -41,3 +44,75 @@ void smbus_loop_alert_devices(const struct device *dev, sys_slist_t *callbacks)
smbus_fire_callbacks(callbacks, dev, address);
}
}

#if defined(CONFIG_SMBUS_SOFT_PEC)

uint8_t smbus_pec_num_msgs(uint32_t flags, uint8_t num_msgs)
{
__ASSERT_NO_MSG(num_msgs != 0);

if ((flags & SMBUS_MODE_PEC) == 0) {
return num_msgs - 1;
}

return num_msgs;
}

uint8_t smbus_pec(uint16_t addr, const struct i2c_msg *msgs, uint8_t num_msgs)
{
uint8_t pec = 0;
uint8_t prior_direction = 0;
uint8_t addr8 = addr & BIT_MASK(7);

for (uint8_t i = 0; i < num_msgs; i++) {
/* When direction changes, there is a repeated start byte. */
uint8_t start_byte;
uint8_t direction = msgs[i].flags & I2C_MSG_RW_MASK;

if ((i == 0) || (direction != prior_direction)) {
prior_direction = direction;
start_byte = (addr8 << 1) | direction;
pec = crc8_ccitt(pec, &start_byte, sizeof(start_byte));
}

pec = crc8_ccitt(pec, msgs[i].buf, msgs[i].len);
}

return pec;
}

void smbus_write_prepare_pec(uint32_t flags, uint16_t addr, struct i2c_msg *msgs, uint8_t num_msgs)
{
if ((flags & SMBUS_MODE_PEC) == 0) {
return;
}

__ASSERT_NO_MSG(msgs != NULL);
__ASSERT_NO_MSG(num_msgs != 0);
__ASSERT_NO_MSG(msgs[num_msgs - 1].buf != NULL);

msgs[num_msgs - 1].buf[0] = smbus_pec(addr, msgs, num_msgs - 1);
}

int smbus_read_check_pec(uint32_t flags, uint16_t addr, const struct i2c_msg *msgs,
uint8_t num_msgs)
{
if ((flags & SMBUS_MODE_PEC) == 0) {
return 0;
}

__ASSERT_NO_MSG(num_msgs != 0);
__ASSERT_NO_MSG(msgs != NULL);
__ASSERT_NO_MSG(msgs[num_msgs - 1].buf != NULL);

uint8_t reported_pec = msgs[num_msgs - 1].buf[0];
uint8_t computed_pec = smbus_pec(addr, msgs, num_msgs - 1);

if (reported_pec != computed_pec) {
return -EIO;
}

return 0;
}

#endif /* defined(CONFIG_SMBUS_SOFT_PEC) */
85 changes: 85 additions & 0 deletions drivers/smbus/smbus_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
#ifndef ZEPHYR_DRIVERS_SMBUS_SMBUS_UTILS_H_
#define ZEPHYR_DRIVERS_SMBUS_SMBUS_UTILS_H_

#include <errno.h>
#include <stdint.h>

#include <zephyr/device.h>
#include <zephyr/drivers/i2c.h>
#include <zephyr/drivers/smbus.h>
#include <zephyr/sys/slist.h>

Expand Down Expand Up @@ -107,4 +110,86 @@ static inline void smbus_init_callback(struct smbus_callback *callback,
*/
void smbus_loop_alert_devices(const struct device *dev, sys_slist_t *callbacks);

#if defined(CONFIG_SMBUS_SOFT_PEC) || defined(__DOXYGEN__)

/**
* @brief Compute the number of messages required for an SMBus transaction
*
* If @p flags indicates that the transaction requires packet error checking (PEC),
* the number of messages is equal to @p num_msgs, otherwise the number of messages
* required is @p num_msgs - 1, since a PEC byte is not required.
*
* Callers are expected to allocated an array of @ref i2c_msg objects to hold a number of
* i2c messages, including one message dedicated to the PEC byte, whether or not PEC is
* being used.
*
* @note This function is intended for SMBus drivers that do not have hardware PEC support and
* requires software PEC calculation enabled (see @kconfig{CONFIG_SMBUS_SOFT_PEC}).
*
* @param flags SMBus flags.
* @param num_msgs Number of allocated messages.
* @return The number of required messages.
*/
uint8_t smbus_pec_num_msgs(uint32_t flags, uint8_t num_msgs);

/**
* @brief Compute the packet error checking (PEC) byte for an SMBus transaction
*
* @note This function is intended for SMBus drivers that do not have hardware PEC support and
* therefore must rely on software PEC calculation (see @kconfig{CONFIG_SMBUS_SOFT_PEC}).
*
* @note At this time, only 7-bit addresses are supported in @p addr.
*
* @param addr The address of the target device.
* @param msgs Array of @ref i2c_msg that make up the transaction.
* @param num_msgs Number of messages in the transaction.
* @return the computed PEC byte.
*/
uint8_t smbus_pec(uint16_t addr, const struct i2c_msg *msgs, uint8_t num_msgs);

/**
* @brief Prepare the packet error checking (PEC) byte for an SMBus write transaction
*
* If the @p flags bitmask contains @ref SMBUS_MODE_PEC (i.e. PEC is enabled), the number of
* messages is equal to @p num_msgs, otherwise the number of messages required is @p num_msgs - 1,
* since a PEC byte is not required.
*
* @note This function is intended for SMBus drivers that do not have hardware PEC support and
* requires software PEC calculation enabled (see @kconfig{CONFIG_SMBUS_SOFT_PEC}).
*
* @note At this time, only 7-bit addresses are supported in @p addr.
*
* @param flags SMBus flags.
* @param addr The address of the target device.
* @param msgs Array of @ref i2c_msg objects that make up the transaction.
* @param num_msgs Number of messages in the transaction.
*/
void smbus_write_prepare_pec(uint32_t flags, uint16_t addr, struct i2c_msg *msgs, uint8_t num_msgs);

/**
* @brief Check the packet error checking (PEC) byte for an SMBus read transaction
*
* If the @p flags bitmask contains @ref SMBUS_MODE_PEC (i.e. PEC is enabled), the number of
* messages is equal to @p num_msgs, otherwise the number of messages required is
* @p num_msgs - 1, since a PEC byte is not required.
*
* When PEC is not enabled, this function returns 0.
*
* @note This function is intended for SMBus drivers that do not have hardware PEC support and
* requires software PEC calculation enabled (see @kconfig{CONFIG_SMBUS_SOFT_PEC}).
*
* @note At this time, only 7-bit addresses are supported in @p addr.
*
* @param flags SMBus flags.
* @param addr The address of the target device.
* @param msgs Array of @ref i2c_msg objects that make up the transaction.
* @param num_msgs Number of messages in the transaction.
* @retval 0 on success.
* @retval -EIO if the PEC byte does not match the computed value.
*/
int smbus_read_check_pec(uint32_t flags, uint16_t addr, const struct i2c_msg *msgs,
uint8_t num_msgs);

#endif /* defined(CONFIG_SMBUS_SOFT_PEC) || defined(__DOXYGEN__) */

#endif /* ZEPHYR_DRIVERS_SMBUS_SMBUS_UTILS_H_ */