Re: [PATCH v18 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] W1C race condition during SMBus overflow handling causes dropped packets due to unintended clearing of buffer receive flags.
--

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

i3c: hub: p3h2x4x: Add SMBus slave mode support

Add SMBus slave mode support for the P3H2x4x hub SMBus target ports.
The hub SMBus slave agent can 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_0_RECEIVE_VAL:
> +			p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_0, i);
> +			break;
> +		case P3H2X4X_TARGET_BUF_1_RECEIVE_VAL:
> +			p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_1, i);
> +			break;
> +		case P3H2X4X_TARGET_BUF_0_1_RECEIVE_VAL:
> +			p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_0, i);
> +			p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_1, i);
> +			break;
> +		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]
Since P3H2X4X_TARGET_BUF_OVRFL is defined in drivers/i3c/hub/p3h2840_i3c_hub.h
as GENMASK(3, 1), does this W1C (write-one-to-clear) operation inadvertently
clear bits 1 and 2 in addition to the overflow bit (bit 3)?

If a new SMBus packet arrives immediately after reading the buffers (which
would set BIT(1) again), writing GENMASK(3, 1) to this W1C status register
would clear the overflow bit but also clear the newly set receive flags.
Can this race condition cause the driver to drop incoming packets without
processing them?

> +			dev_err(&i3cdev->dev, "Overflow, reading buffer zero and one\n");
> +			break;
> +		default:
> +			regmap_write(p3h2x4x_i3c_hub->regmap,
> +				     P3H2X4X_TP0_SMBUS_AGNT_STS + i,
> +				     target_port_status & BUF_RECEIVED_FLAG_MASK);
> +			break;
> +		}
> +	}
> +}

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