[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aYYl3IOjC89mGM2U@lizhi-Precision-Tower-5810>
Date: Fri, 6 Feb 2026 12:33:16 -0500
From: Frank Li <Frank.li@....com>
To: Aman Kumar Pandey <aman.kumarpandey@....com>
Cc: linux-kernel@...r.kernel.org, linux-i3c@...ts.infradead.org,
alexandre.belloni@...tlin.com, krzk+dt@...nel.org, robh@...nel.org,
conor+dt@...nel.org, devicetree@...r.kernel.org, broonie@...nel.org,
lee@...nel.org, lgirdwood@...il.com, vikash.bansal@....com,
priyanka.jain@....com, shashank.rebbapragada@....com
Subject: Re: [PATCH v5 5/5] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x
I3C hub functionality
On Fri, Feb 06, 2026 at 02:01:21PM +0200, Aman Kumar Pandey wrote:
> Add I3C hub functionality for the NXP P3H2x4x family of multiport hubs.
> These devices support downstream target ports that can be configured
> as I3C, I2C, or SMBus.
>
> This driver enables:
> - I3C/I2C communication between host and hub
> - Transparent communication with downstream devices
> - Target port configuration (I3C/I2C/SMBus)
> - MCTP device support
> - In-band interrupt handling
>
> P3H2440/P3H2441 support 4 target ports;
> P3H2840/P3H2841 support 8 target ports.
>
> Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@....com>
> Signed-off-by: Vikash Bansal <vikash.bansal@....com>
>
> ---
> Changes in v5:
> - Updated supply names.
>
> Changes in v4:
> - Split the driver into three separate patches (mfd, regulator and I3C hub)
> - Added support for NXP P3H2x4x I3C hub functionality
> - Integrated hub driver with its on-die regulator
>
> Changes in v3:
> - Added MFD (Multi-Function Device) support for I3C hub and on-die regulator
>
> Changes in v2:
> - Refined coding style and incorporated review feedback
> - Updated directory structure
> - Revised logic for parsing DTS nodes
> ---
> ---
> MAINTAINERS | 1 +
> drivers/i3c/Kconfig | 1 +
> drivers/i3c/Makefile | 1 +
> drivers/i3c/hub/Kconfig | 10 +
> drivers/i3c/hub/Makefile | 4 +
> drivers/i3c/hub/p3h2840_i3c_hub.h | 340 ++++++++++++++++++
> drivers/i3c/hub/p3h2840_i3c_hub_common.c | 405 ++++++++++++++++++++++
> drivers/i3c/hub/p3h2840_i3c_hub_i3c.c | 354 +++++++++++++++++++
> drivers/i3c/hub/p3h2840_i3c_hub_smbus.c | 418 +++++++++++++++++++++++
> include/linux/i3c/device.h | 1 +
> 10 files changed, 1535 insertions(+)
> create mode 100644 drivers/i3c/hub/Kconfig
> create mode 100644 drivers/i3c/hub/Makefile
> create mode 100644 drivers/i3c/hub/p3h2840_i3c_hub.h
> create mode 100644 drivers/i3c/hub/p3h2840_i3c_hub_common.c
> create mode 100644 drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> create mode 100644 drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
>
...
> +
> +static struct platform_driver p3h2x4x_i3c_hub_driver = {
> + .driver = {
> + .name = "p3h2x4x-i3c-hub",
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + },
> + .probe = p3h2x4x_i3c_hub_probe,
> + .remove = p3h2x4x_i3c_hub_remove,
> +};
> +module_platform_driver(p3h2x4x_i3c_hub_driver);
> +
> +MODULE_AUTHOR("Aman Kumar Pandey <aman.kumarpandey@....com>");
> +MODULE_AUTHOR("vikash Bansal <vikash.bansal@....com>");
AI: Should be "Vikash Bansal" (capitalize first name)
> +MODULE_DESCRIPTION("P3H2x4x I3C HUB driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> new file mode 100644
> index 000000000000..cbc6626721fe
> --- /dev/null
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> @@ -0,0 +1,354 @@
...
> +
> +static int p3h2x4x_request_ibi(struct i3c_dev_desc *desc,
> + const struct i3c_ibi_setup *req)
> +{
> + struct i3c_master_controller *parent = get_parent_controller_from_i3c_desc(desc);
> + struct i3c_master_controller *orig_parent;
> + int ret;
> +
> + orig_parent = update_i3c_i2c_desc_parent(&desc->common, parent);
> + ret = i3c_master_direct_attach_i3c_dev(parent, desc);
> + if (ret) {
> + restore_i3c_i2c_desc_parent(&desc->common, orig_parent);
> + return ret;
> + }
> +
> + mutex_unlock(&desc->ibi_lock);
supposed caller hold ibi_lock.
provide i3c_device_request_ibi_lock() version, which have not hold ibi_lock.
unlock and lock is not good enough.
> + kfree(desc->ibi);
> + desc->ibi = NULL;
> + ret = i3c_device_request_ibi(desc->dev, req);
> + mutex_lock(&desc->ibi_lock);
> + restore_i3c_i2c_desc_parent(&desc->common, orig_parent);
> +
> + return ret;
> +}
> +
...
> +static s32 p3h2x4x_tp_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{
> + int ret_sum = 0, ret;
> + u8 msg_count, rw;
> +
> + struct tp_bus *bus = i2c_get_adapdata(adap);
> + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = bus->p3h2x4x_i3c_hub;
> +
> + guard(mutex)(&p3h2x4x_i3c_hub->etx_mutex);
> + guard(mutex)(&bus->port_mutex);
> +
> + for (msg_count = 0; msg_count < num; msg_count++) {
> + if (msgs[msg_count].len > P3H2x4x_SMBUS_PAYLOAD_SIZE) {
> + dev_err(p3h2x4x_i3c_hub->dev,
> + "Message nr. %d not sent - length over %d bytes.\n",
> + msg_count, P3H2x4x_SMBUS_PAYLOAD_SIZE);
> + continue;
> + }
> +
> + rw = msgs[msg_count].flags % 2;
> + if (!rw) {
> + /* If a read message is immediately followed by a write message to
> + * the same address, consider combining them into a single transaction.
> + */
> + if (msg_count < num && msgs[msg_count].addr == msgs[msg_count + 1].addr &&
> + (msgs[msg_count + 1].flags & I2C_M_RD)) {
AI: `msg_count < num` should be `msg_count + 1 < num` to prevent out-of-bounds access
Frank
> + rw = 2;
> + msg_count += 1;
> + ret_sum += 1;
> + }
> + }
> +
> + ret = p3h2x4x_tp_i2c_xfer_msg(p3h2x4x_i3c_hub,
> + msgs,
> + bus->tp_port,
> + (rw == 2) ? (msg_count - 1) : msg_count,
> + rw);
> + if (ret)
> + return ret;
> +
> + ret_sum++;
> + }
> + return ret_sum;
> +}
> +
...
> --
> 2.25.1
>
Powered by blists - more mailing lists