Re: [PATCH v19 8/8] i3c: hub: p3h2x4x: Add SMBus slave mode support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] i3c: p3h2x4x: lost SMBus payloads in p3h2x4x_ibi_handler() due to status flag race
--

commit a382968ee6c58bbb9c6a6ee774af81bb351f2f46
Author: Lakshay Piplani <lakshay.piplani@nxp.com>

i3c: hub: p3h2x4x: Add SMBus slave mode support

This commit adds SMBus slave mode support for the P3H2x4x hub SMBus target
ports. The hub SMBus slave agent can now receive downstream payloads into
target buffers and report receive events through IBI.

> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> index a3324cc365956..7fdfa79dfeb6c 100644
> --- a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
[ ... ]
> @@ -15,6 +17,178 @@ enum p3h2x4x_smbus_desc_idx {
[ ... ]
> +void p3h2x4x_ibi_handler(struct i3c_device *i3cdev,
> +			 const struct i3c_ibi_payload *payload)
> +{
[ ... ]
> +		/* process data receive buffer */
> +		switch (FIELD_GET(BUF_RECEIVED_FLAG_MASK, target_port_status)) {
[ ... ]
> +		case P3H2X4X_TARGET_BUF_OVRFL_VAL:
> +			p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_0, i);
> +			p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_1, i);
> +			regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP0_SMBUS_AGNT_STS + i,
> +				     P3H2X4X_TARGET_BUF_OVRFL);

[Severity: High]
Could this silently drop SMBus payloads due to a status flag race?

When an overflow occurs, this block sequentially reads and clears the receive
flags for both buffer 0 and buffer 1 via p3h2x4x_read_smbus_agent_rx_buf().

Once buffer 0 is read and its individual flag (bit 1) is cleared, the
hardware can immediately receive a newly arriving SMBus message into buffer 0,
setting bit 1 again.

The handler then explicitly clears the overflow condition by writing
P3H2X4X_TARGET_BUF_OVRFL (which is GENMASK(3, 1)) to the write-1-to-clear
status register. Because this mask encompasses bits 1, 2, and 3, writing it
clobbers the individual buffer receive flags that might have just been set by
new incoming messages, clearing them without reading the data.

> +			dev_err(&i3cdev->dev, "Overflow, reading buffer zero and one\n");
> +			break;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916070728.1088801-1-lakshay.piplani@nxp.com?part=8