* [PATCH v21 1/4] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator
2026-11-12 12:44 [PATCH v21 0/4] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
@ 2026-11-12 12:44 ` Lakshay Piplani
2026-11-12 12:54 ` sashiko-bot
2026-11-12 12:44 ` [PATCH v21 2/4] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Lakshay Piplani @ 2026-11-12 12:44 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, broonie, mfd, lee,
Frank.Li, lgirdwood
Cc: vikash.bansal, priyanka.jain, aman.kumarpandey, Lakshay Piplani
From: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
Add core MFD support for the NXP P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841)
family of multiport I3C hub devices. These devices connect to a host via
I3C/I2C/SMBus and expose multiple downstream target ports.
All variants share the same register interface. Read the implemented
target-port count from DEV_CAPAB, so no per-compatible driver data is
required.
Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
Signed-off-by: Vikash Bansal <vikash.bansal@nxp.com>
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v21:
- Rebased onto v7.4-rc2; no functional changes
Changes in v20:
- No change
Changes in v19:
- Retain only nxp,p3h2840 in the OF match table because the other
variants use it as their fallback compatible
Changes in v18:
- Rebased onto v7.3-rc1; no intended driver behaviour changes
Changes in v17:
- No change
Changes in v16:
- Use a named initializer for struct i2c_device_id
({ .name = "nxp-i3c-hub" })
- Keep the shared header in include/linux/mfd/ (struct p3h2x4x is shared by
the mfd parent and its regulator/i3c-hub children). The i3c_hub_priv
back-pointer is retained for IBI routing, with documented publish/clear
ordering
Changes in v15:
- Read the device capability register to determine whether the device has
four or eight target ports
- Store the detected target-port count in the shared MFD data
- Add OF match entries for all P3H2440, P3H2441, P3H2840 and P3H2841
variants
- Add a shared hub-context pointer for use by the IBI handler without
overwriting the MFD parent's driver data
Changes in v14:
- Clean up MFD driver comments, formatting, variable names and author
information
- Define and document the I3C hub DCR value in hexadecimal form
- Use direct regmap bit-width values instead of local macros
- Add a shared protected_reg_lock for protected register accesses across
P3H2X4X child drivers
Changes in v13:
- Use i3c_device helpers (i3cdev_to_dev()) instead of direct struct access
to maintain API abstraction
- Avoid including internal I3C headers and use public device headers instead
- Ensure proper device matching by relying on manufacturer ID checks in probe
Changes in v12:
- No change, added Reviewed-By tag
Changes in v11:
- Use MFD_CELL_NAME() for child device registration
- Rename local variables for consistency
- Rename driver names to follow subsystem conventions:
- Use '-' instead of '_' in driver names
- Drop the "_drv" suffix from driver names
Changes in v10:
- Drop redundant is_p3h2x4x_in_i3c flag
Changes in v9:
- Renamed macros to follow consistent uppercase naming conventions
- Made REGMAP selects in the P3H2X4X MFD Kconfig conditional,
to avoid I3C/I2C dependency issues
Changes in v8:
- No change
Changes in v7:
- Use new config I3C_OR_I2C
Changes in v6:
- No change
Changes in v5:
- Corrected the ordering in the Makefile and Kconfig for MFD_P3H2X4X
- Updated dev_err_probe() for regmap_init failure.
- Updated module description
Changes in v4:
- Split the driver into three separate patches(mfd, regulator and I3C hub)
- Added support for NXP P3H2x4x MFD functionality
---
---
MAINTAINERS | 2 +
drivers/mfd/Kconfig | 13 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/p3h2840.c | 164 ++++++++++++++++++++++++++++++++++++
include/linux/i3c/device.h | 2 +
include/linux/mfd/p3h2840.h | 41 +++++++++
6 files changed, 223 insertions(+)
create mode 100644 drivers/mfd/p3h2840.c
create mode 100644 include/linux/mfd/p3h2840.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 93abbb315065..bd2d20454057 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20242,6 +20242,8 @@ L: linux-kernel@vger.kernel.org
L: linux-i3c@lists.infradead.org
S: Maintained
F: Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
+F: drivers/mfd/p3h2840.c
+F: include/linux/mfd/p3h2840.h
NXP PF5300/PF5301/PF5302 PMIC REGULATOR DEVICE DRIVER
M: Woodrow Douglass <wdouglass@carnegierobotics.com>
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 87ed55c155f0..680d81a0e572 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -631,6 +631,19 @@ config MFD_MX25_TSADC
i.MX25 processors. They consist of a conversion queue for general
purpose ADC and a queue for Touchscreens.
+config MFD_P3H2X4X
+ tristate "NXP P3H2X4X I3C Hub Device"
+ depends on I3C_OR_I2C
+ select MFD_CORE
+ select REGMAP_I3C if I3C
+ select REGMAP_I2C if I2C
+ help
+ Enable Support for NXP P3H244x/P3H284x I3C HUB device using I3C/I2C
+ communication interface.
+
+ This driver provides support for I3C hub and regulator, each subdriver
+ can be enabled independently depending on the required functionality.
+
config MFD_PF1550
tristate "NXP PF1550 PMIC Support"
depends on I2C=y && OF
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f8101d2a9ce9..f3780584e97e 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -122,6 +122,7 @@ obj-$(CONFIG_MFD_MC13XXX) += mc13xxx-core.o
obj-$(CONFIG_MFD_MC13XXX_SPI) += mc13xxx-spi.o
obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
+obj-$(CONFIG_MFD_P3H2X4X) += p3h2840.o
obj-$(CONFIG_MFD_PF1550) += pf1550.o
obj-$(CONFIG_MFD_NCT6694) += nct6694.o
diff --git a/drivers/mfd/p3h2840.c b/drivers/mfd/p3h2840.c
new file mode 100644
index 000000000000..c48984482148
--- /dev/null
+++ b/drivers/mfd/p3h2840.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025-2026 NXP
+ *
+ * Authors:
+ * Aman Kumar Pandey <aman.kumarpandey@nxp.com>
+ * Vikash Bansal <vikash.bansal@nxp.com>
+ * Lakshay Piplani <lakshay.piplani@nxp.com>
+ *
+ * NXP P3H2x4x multi-port I3C hub.
+ */
+#include <linux/i2c.h>
+#include <linux/i3c/device.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/p3h2840.h>
+#include <linux/regmap.h>
+
+static const struct mfd_cell p3h2x4x_devs[] = {
+ MFD_CELL_NAME("p3h2x4x-regulator"),
+ MFD_CELL_NAME("p3h2x4x-i3c-hub"),
+};
+
+static const struct regmap_config p3h2x4x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0xFF,
+};
+
+/* Read port count from the device capability register (4- or 8-port variant). */
+static int p3h2x4x_read_num_target_ports(struct device *dev,
+ struct p3h2x4x *ddata)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(ddata->regmap, P3H2X4X_DEV_CAPAB, &val);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to read device capability\n");
+
+ ddata->num_target_ports = (val & P3H2X4X_TARGET_PORT_COUNT) ?
+ P3H2X4X_TARGET_PORTS_8 : P3H2X4X_TARGET_PORTS_4;
+
+ return 0;
+}
+
+static int p3h2x4x_device_probe_i3c(struct i3c_device *i3cdev)
+{
+ struct device *dev = i3cdev_to_dev(i3cdev);
+ struct i3c_device_info devinfo;
+ struct p3h2x4x *ddata;
+ int ret;
+
+ i3c_device_get_info(i3cdev, &devinfo);
+ if (I3C_PID_MANUF_ID(devinfo.pid) != I3C_MANUF_ID_NXP)
+ return -ENODEV;
+
+ ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ ret = devm_mutex_init(dev, &ddata->protected_reg_lock);
+ if (ret)
+ return ret;
+
+ i3cdev_set_drvdata(i3cdev, ddata);
+
+ ddata->regmap = devm_regmap_init_i3c(i3cdev, &p3h2x4x_regmap_config);
+ if (IS_ERR(ddata->regmap))
+ return dev_err_probe(dev, PTR_ERR(ddata->regmap),
+ "Failed to register HUB regmap\n");
+
+ /* The hub child driver retrieves information from i3cdev. */
+ ddata->i3cdev = i3cdev;
+
+ ret = p3h2x4x_read_num_target_ports(dev, ddata);
+ if (ret)
+ return ret;
+
+ ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
+ p3h2x4x_devs, ARRAY_SIZE(p3h2x4x_devs),
+ NULL, 0, NULL);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to add sub devices\n");
+
+ return 0;
+}
+
+static int p3h2x4x_device_probe_i2c(struct i2c_client *client)
+{
+ struct p3h2x4x *ddata;
+ int ret;
+
+ ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ ret = devm_mutex_init(&client->dev, &ddata->protected_reg_lock);
+ if (ret)
+ return ret;
+
+ i2c_set_clientdata(client, ddata);
+
+ ddata->regmap = devm_regmap_init_i2c(client, &p3h2x4x_regmap_config);
+ if (IS_ERR(ddata->regmap))
+ return dev_err_probe(&client->dev, PTR_ERR(ddata->regmap),
+ "Failed to register HUB regmap\n");
+
+ ddata->i3cdev = NULL;
+
+ ret = p3h2x4x_read_num_target_ports(&client->dev, ddata);
+ if (ret)
+ return ret;
+
+ ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO,
+ p3h2x4x_devs, ARRAY_SIZE(p3h2x4x_devs),
+ NULL, 0, NULL);
+ if (ret)
+ return dev_err_probe(&client->dev, ret, "Failed to add sub devices\n");
+
+ return 0;
+}
+
+static const struct i3c_device_id p3h2x4x_i3c_ids[] = {
+ I3C_CLASS(I3C_DCR_HUB, NULL),
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(i3c, p3h2x4x_i3c_ids);
+
+static const struct i2c_device_id p3h2x4x_i2c_id_table[] = {
+ { .name = "nxp-i3c-hub" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, p3h2x4x_i2c_id_table);
+
+static const struct of_device_id p3h2x4x_i2c_of_match[] = {
+ { .compatible = "nxp,p3h2840", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, p3h2x4x_i2c_of_match);
+
+static struct i3c_driver p3h2x4x_i3c = {
+ .driver = {
+ .name = "p3h2x4x-i3c",
+ },
+ .probe = p3h2x4x_device_probe_i3c,
+ .id_table = p3h2x4x_i3c_ids,
+};
+
+static struct i2c_driver p3h2x4x_i2c = {
+ .driver = {
+ .name = "p3h2x4x-i2c",
+ .of_match_table = p3h2x4x_i2c_of_match,
+ },
+ .probe = p3h2x4x_device_probe_i2c,
+ .id_table = p3h2x4x_i2c_id_table,
+};
+module_i3c_i2c_driver(p3h2x4x_i3c, &p3h2x4x_i2c);
+
+MODULE_AUTHOR("Aman Kumar Pandey <aman.kumarpandey@nxp.com>");
+MODULE_AUTHOR("Vikash Bansal <vikash.bansal@nxp.com>");
+MODULE_AUTHOR("Lakshay Piplani <lakshay.piplani@nxp.com>");
+MODULE_DESCRIPTION("NXP P3H2X4X I3C HUB multi function driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
index e30ea2cb13cb..24b8819db6ef 100644
--- a/include/linux/i3c/device.h
+++ b/include/linux/i3c/device.h
@@ -86,9 +86,11 @@ struct i3c_xfer {
/**
* enum i3c_dcr - I3C DCR values
* @I3C_DCR_GENERIC_DEVICE: generic I3C device
+ * @I3C_DCR_HUB: I3C hub device
*/
enum i3c_dcr {
I3C_DCR_GENERIC_DEVICE = 0,
+ I3C_DCR_HUB = 0xC2,
};
#define I3C_PID_MANUF_ID(pid) (((pid) & GENMASK_ULL(47, 33)) >> 33)
diff --git a/include/linux/mfd/p3h2840.h b/include/linux/mfd/p3h2840.h
new file mode 100644
index 000000000000..eb4792de5440
--- /dev/null
+++ b/include/linux/mfd/p3h2840.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2025-2026 NXP
+ */
+
+#ifndef _LINUX_MFD_P3H2840_H
+#define _LINUX_MFD_P3H2840_H
+
+#include <linux/bits.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+
+/* Device Information Registers */
+#define P3H2X4X_DEV_CAPAB 0x0a
+#define P3H2X4X_TARGET_PORT_COUNT BIT(3)
+
+/* Downstream target port counts per variant. */
+#define P3H2X4X_TARGET_PORTS_4 4
+#define P3H2X4X_TARGET_PORTS_8 8
+
+/* Device Configuration Registers */
+#define P3H2X4X_DEV_REG_PROTECTION_CODE 0x10
+#define P3H2X4X_REGISTERS_LOCK_CODE 0x00
+#define P3H2X4X_REGISTERS_UNLOCK_CODE 0x69
+#define P3H2X4X_CP1_REGISTERS_UNLOCK_CODE 0x6a
+
+#define I3C_MANUF_ID_NXP 0x011b
+
+struct p3h2x4x_i3c_hub_dev;
+
+struct p3h2x4x {
+ struct i3c_device *i3cdev;
+ struct regmap *regmap;
+ /* Number of downstream target ports (4 or 8). */
+ u8 num_target_ports;
+ /* Serializes protected register unlock/lock sequences across MFD children. */
+ struct mutex protected_reg_lock;
+ /* Hub context for the IBI handler to reach hub state via the parent i3cdev. */
+ struct p3h2x4x_i3c_hub_dev *i3c_hub_priv;
+};
+#endif /* _LINUX_MFD_P3H2840_H */
--
2.25.1
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v21 1/4] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator
2026-11-12 12:44 ` [PATCH v21 1/4] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
@ 2026-11-12 12:54 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-12 12:54 UTC (permalink / raw)
To: Lakshay Piplani, linux-i3c, mfd; +Cc: Frank.Li, Alexandre Belloni, lee
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] mfd: p3h2x4x: duplicated probe logic and unaddressed naming
feedback
--
commit dafcc1e1067d996fcab7a86770124695c8bbf434
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator
This commit adds core MFD support for the NXP P3H2x4x family of multiport
I3C hub devices over I3C and I2C.
> diff --git a/drivers/mfd/p3h2840.c b/drivers/mfd/p3h2840.c
> new file mode 100644
> index 0000000000000..c489844821487
> --- /dev/null
> +++ b/drivers/mfd/p3h2840.c
[ ... ]
> +static int p3h2x4x_device_probe_i3c(struct i3c_device *i3cdev)
> +{
> + struct device *dev = i3cdev_to_dev(i3cdev);
> + struct i3c_device_info devinfo;
> + struct p3h2x4x *ddata;
> + int ret;
> +
> + i3c_device_get_info(i3cdev, &devinfo);
> + if (I3C_PID_MANUF_ID(devinfo.pid) != I3C_MANUF_ID_NXP)
> + return -ENODEV;
> +
> + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return -ENOMEM;
> +
> + ret = devm_mutex_init(dev, &ddata->protected_reg_lock);
> + if (ret)
> + return ret;
> +
> + i3cdev_set_drvdata(i3cdev, ddata);
> +
> + ddata->regmap = devm_regmap_init_i3c(i3cdev, &p3h2x4x_regmap_config);
> + if (IS_ERR(ddata->regmap))
> + return dev_err_probe(dev, PTR_ERR(ddata->regmap),
> + "Failed to register HUB regmap\n");
> +
> + /* The hub child driver retrieves information from i3cdev. */
> + ddata->i3cdev = i3cdev;
> +
> + ret = p3h2x4x_read_num_target_ports(dev, ddata);
> + if (ret)
> + return ret;
> +
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
> + p3h2x4x_devs, ARRAY_SIZE(p3h2x4x_devs),
> + NULL, 0, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to add sub devices\n");
> +
> + return 0;
> +}
> +
> +static int p3h2x4x_device_probe_i2c(struct i2c_client *client)
> +{
> + struct p3h2x4x *ddata;
> + int ret;
> +
> + ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return -ENOMEM;
> +
> + ret = devm_mutex_init(&client->dev, &ddata->protected_reg_lock);
> + if (ret)
> + return ret;
> +
> + i2c_set_clientdata(client, ddata);
> +
> + ddata->regmap = devm_regmap_init_i2c(client, &p3h2x4x_regmap_config);
> + if (IS_ERR(ddata->regmap))
> + return dev_err_probe(&client->dev, PTR_ERR(ddata->regmap),
> + "Failed to register HUB regmap\n");
> +
> + ddata->i3cdev = NULL;
> +
> + ret = p3h2x4x_read_num_target_ports(&client->dev, ddata);
> + if (ret)
> + return ret;
> +
> + ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO,
> + p3h2x4x_devs, ARRAY_SIZE(p3h2x4x_devs),
> + NULL, 0, NULL);
> + if (ret)
> + return dev_err_probe(&client->dev, ret, "Failed to add sub devices\n");
> +
> + return 0;
> +}
> +
> +static const struct i3c_device_id p3h2x4x_i3c_ids[] = {
> + I3C_CLASS(I3C_DCR_HUB, NULL),
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(i3c, p3h2x4x_i3c_ids);
> +
> +static const struct i2c_device_id p3h2x4x_i2c_id_table[] = {
> + { .name = "nxp-i3c-hub" },
> + { /* sentinel */ }
> +};
[ ... ]
> +MODULE_DESCRIPTION("NXP P3H2X4X I3C HUB multi function driver");
> +MODULE_LICENSE("GPL");
[Severity: Low]
As reported by Lee Jones on v7 [1], could the common initialization sequence
in p3h2x4x_device_probe_i3c() and p3h2x4x_device_probe_i2c() be factored into
a shared helper function so the bus-specific probe functions only handle bus
validation and regmap initialization?
In addition, p3h2x4x_i2c_id_table uses the generic name "nxp-i3c-hub" instead
of a device-specific name, MODULE_DESCRIPTION still refers to a "multi
function driver", and the file names p3h2840.c and p3h2840.h do not match
CONFIG_MFD_P3H2X4X and the p3h2x4x symbol prefix.
[1] https://lore.kernel.org/r/20260326115848.GL1141718@google.com
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112124437.3367252-1-lakshay.piplani@nxp.com?part=1
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v21 2/4] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
2026-11-12 12:44 [PATCH v21 0/4] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
2026-11-12 12:44 ` [PATCH v21 1/4] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
@ 2026-11-12 12:44 ` Lakshay Piplani
2026-11-12 12:56 ` sashiko-bot
2026-11-12 12:44 ` [PATCH v21 3/4] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
2026-11-12 12:44 ` [PATCH v21 4/4] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
3 siblings, 1 reply; 10+ messages in thread
From: Lakshay Piplani @ 2026-11-12 12:44 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, broonie, mfd, lee,
Frank.Li, lgirdwood
Cc: vikash.bansal, priyanka.jain, aman.kumarpandey, Lakshay Piplani
From: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
The NXP P3H2x4x family integrates on-die regulators alongside I3C hub
functionality. This driver registers the regulators using the MFD
framework and exposes them via the regulator subsystem.
Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
Signed-off-by: Vikash Bansal <vikash.bansal@nxp.com>
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
---
Changes in v21:
- Rebased onto v7.4-rc2; no functional changes
Changes in v20:
- No change
Changes in v19:
- No change
Changes in v18:
- Rebased onto v7.3-rc1; no intended driver behaviour changes
Changes in v17:
- Add .enable_time = 5000 through P3H2X4X_LDO_ENABLE_TIME_US to all four
LDO descriptors, so the regulator core applies the post-enable
settling delay
Changes in v16:
- Mark the p3h2x4x_regulators[] descriptor array as const
- Keep the standard MFD dev_get_drvdata(pdev->dev.parent) parent-context
access and the shared include/linux/mfd/ header (used only by the parent
and its children)
Changes in v15:
- No change
Changes in v14:
- Use the shared MFD protected_reg_lock instead of a regulator-private mutex
to serialize protected register access across P3H2X4X child drivers
- Rework protected register handling into explicit unprotect/protect helpers
and restore the original protection state after each regulator operation
- Ensure protected registers are restored even if enable, disable or
set_voltage_sel fails
Changes in v13:
- Serialize register unlock/modify/lock sequences using a device-level mutex to
avoid races between regulators.
- Ensure proper module auto-loading by adding platform device ID table.
- Improve error handling in register guard paths to avoid operating on locked hub.
Changes in v12:
- No change
Changes in v11:
- No change, added Reviewed-By tag
Changes in v10:
- No change
Changes in v9:
- No change
Changes in v8:
- No change
Changes in v7:
- No change, added Reviewed-By tag
Changes in v6:
- Use DEFINE_LOCK_GUARD_1 for reg lock/unlock
Changes in v5:
- Updated dev_err_probe() for regmap_init failure.
- Updated module description
Changes in v4:
- Split the driver into three separate patches (mfd, regulator and I3C hub)
- Introduced driver for on-die regulators in NXP P3H2x4x I3C hub
---
MAINTAINERS | 1 +
drivers/regulator/Kconfig | 10 +
drivers/regulator/Makefile | 1 +
drivers/regulator/p3h2840_i3c_hub_regulator.c | 289 ++++++++++++++++++
4 files changed, 301 insertions(+)
create mode 100644 drivers/regulator/p3h2840_i3c_hub_regulator.c
diff --git a/MAINTAINERS b/MAINTAINERS
index bd2d20454057..0bfac3696bd2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20243,6 +20243,7 @@ L: linux-i3c@lists.infradead.org
S: Maintained
F: Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
F: drivers/mfd/p3h2840.c
+F: drivers/regulator/p3h2840_i3c_hub_regulator.c
F: include/linux/mfd/p3h2840.h
NXP PF5300/PF5301/PF5302 PMIC REGULATOR DEVICE DRIVER
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index be6b72daa76f..bea5946434c9 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1039,6 +1039,16 @@ config REGULATOR_NEX10000UB
This driver supports the NEX10000UB single inductor - dual output
power supply specifically designed for display panels.
+config REGULATOR_P3H2X4X
+ tristate "NXP P3H2X4X regulator support"
+ depends on MFD_P3H2X4X
+ help
+ This driver provides support for the voltage regulators of the
+ P3H244x/P3H284x multi-function I3C Hub device.
+
+ Say M here if you want to include support for this regulator as
+ a module. The module will be named "p3h2840_i3c_hub_regulator".
+
config REGULATOR_PALMAS
tristate "TI Palmas PMIC Regulators"
depends on MFD_PALMAS
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 6aabcc54382a..b6f62dc69978 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -130,6 +130,7 @@ obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o
obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
obj-$(CONFIG_REGULATOR_QCOM_USB_VBUS) += qcom_usb_vbus-regulator.o
+obj-$(CONFIG_REGULATOR_P3H2X4X) += p3h2840_i3c_hub_regulator.o
obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
obj-$(CONFIG_REGULATOR_PCA9450) += pca9450-regulator.o
obj-$(CONFIG_REGULATOR_PF0900) += pf0900-regulator.o
diff --git a/drivers/regulator/p3h2840_i3c_hub_regulator.c b/drivers/regulator/p3h2840_i3c_hub_regulator.c
new file mode 100644
index 000000000000..244f2a4f70f3
--- /dev/null
+++ b/drivers/regulator/p3h2840_i3c_hub_regulator.c
@@ -0,0 +1,289 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025-2026 NXP
+ * NXP P3H2X4X regulator driver file contain functions for enable/disable regulator
+ * and voltage set/get.
+ */
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/mfd/p3h2840.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+
+#define P3H2X4X_LDO_AND_PULLUP_CONF 0x19
+#define P3H2X4X_LDO_ENABLE_DISABLE_MASK GENMASK(3, 0)
+#define P3H2X4X_CP0_EN_LDO BIT(0)
+#define P3H2X4X_CP1_EN_LDO BIT(1)
+#define P3H2X4X_TP0145_EN_LDO BIT(2)
+#define P3H2X4X_TP2367_EN_LDO BIT(3)
+
+#define P3H2X4X_NET_OPER_MODE_CONF 0x15
+#define P3H2X4X_VCCIO_LDO_CONF 0x16
+#define P3H2X4X_CP0_VCCIO_LDO_VOLTAGE_MASK GENMASK(1, 0)
+#define P3H2X4X_CP0_VCCIO_LDO_VOLTAGE(x) \
+ FIELD_PREP(P3H2X4X_CP0_VCCIO_LDO_VOLTAGE_MASK, x)
+#define P3H2X4X_CP1_VCCIO_LDO_VOLTAGE_MASK GENMASK(3, 2)
+#define P3H2X4X_CP1_VCCIO_LDO_VOLTAGE(x) \
+ FIELD_PREP(P3H2X4X_CP1_VCCIO_LDO_VOLTAGE_MASK, x)
+#define P3H2X4X_TP0145_VCCIO_LDO_VOLTAGE_MASK GENMASK(5, 4)
+#define P3H2X4X_TP0145_VCCIO_LDO_VOLTAGE(x) \
+ FIELD_PREP(P3H2X4X_TP0145_VCCIO_LDO_VOLTAGE_MASK, x)
+#define P3H2X4X_TP2367_VCCIO_LDO_VOLTAGE_MASK GENMASK(7, 6)
+#define P3H2X4X_TP2367_VCCIO_LDO_VOLTAGE(x) \
+ FIELD_PREP(P3H2X4X_TP2367_VCCIO_LDO_VOLTAGE_MASK, x)
+#define P3H2X4X_LDO_COUNT 4
+#define P3H2X4X_LDO_ENABLE_TIME_US 5000
+
+struct p3h2x4x_regulator_dev {
+ struct regulator_dev *rp3h2x4x_dev[P3H2X4X_LDO_COUNT];
+ struct p3h2x4x *p3h2x4x;
+ struct regmap *regmap;
+};
+
+struct p3h2x4x_reg_state {
+ unsigned int orig;
+ bool restore;
+};
+
+static void p3h2x4x_reg_guard_enter(struct regulator_dev *rdev)
+{
+ struct p3h2x4x_regulator_dev *priv = rdev_get_drvdata(rdev);
+
+ mutex_lock(&priv->p3h2x4x->protected_reg_lock);
+}
+
+static void p3h2x4x_reg_guard_exit(struct regulator_dev *rdev)
+{
+ struct p3h2x4x_regulator_dev *priv = rdev_get_drvdata(rdev);
+
+ mutex_unlock(&priv->p3h2x4x->protected_reg_lock);
+}
+
+DEFINE_LOCK_GUARD_1(p3h2x4x_reg, struct regulator_dev,
+ p3h2x4x_reg_guard_enter(_T->lock),
+ p3h2x4x_reg_guard_exit(_T->lock));
+
+static int p3h2x4x_reg_unprotect(struct regulator_dev *rdev,
+ struct p3h2x4x_reg_state *state)
+{
+ int ret;
+
+ state->restore = false;
+
+ ret = regmap_read(rdev->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ &state->orig);
+ if (ret)
+ return ret;
+
+ if (state->orig == P3H2X4X_REGISTERS_UNLOCK_CODE)
+ return 0;
+
+ ret = regmap_write(rdev->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_UNLOCK_CODE);
+ if (ret)
+ return ret;
+
+ state->restore = true;
+
+ return 0;
+}
+
+static int p3h2x4x_reg_protect(struct regulator_dev *rdev,
+ struct p3h2x4x_reg_state *state)
+{
+ if (!state->restore)
+ return 0;
+
+ return regmap_write(rdev->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ state->orig);
+}
+
+static int p3h2x4x_regulator_enable(struct regulator_dev *rdev)
+{
+ struct p3h2x4x_reg_state state;
+ int ret, ret2;
+
+ guard(p3h2x4x_reg)(rdev);
+
+ ret = p3h2x4x_reg_unprotect(rdev, &state);
+ if (ret)
+ return ret;
+
+ ret = regulator_enable_regmap(rdev);
+ ret2 = p3h2x4x_reg_protect(rdev, &state);
+
+ return ret ? ret : ret2;
+}
+
+static int p3h2x4x_regulator_disable(struct regulator_dev *rdev)
+{
+ struct p3h2x4x_reg_state state;
+ int ret, ret2;
+
+ guard(p3h2x4x_reg)(rdev);
+
+ ret = p3h2x4x_reg_unprotect(rdev, &state);
+ if (ret)
+ return ret;
+
+ ret = regulator_disable_regmap(rdev);
+ ret2 = p3h2x4x_reg_protect(rdev, &state);
+
+ return ret ? ret : ret2;
+}
+
+static int p3h2x4x_regulator_set_voltage_sel(struct regulator_dev *rdev,
+ unsigned int sel)
+{
+ struct p3h2x4x_reg_state state;
+ int ret, ret2;
+
+ guard(p3h2x4x_reg)(rdev);
+
+ ret = p3h2x4x_reg_unprotect(rdev, &state);
+ if (ret)
+ return ret;
+
+ ret = regulator_set_voltage_sel_regmap(rdev, sel);
+ ret2 = p3h2x4x_reg_protect(rdev, &state);
+
+ return ret ? ret : ret2;
+}
+
+static const struct regulator_ops p3h2x4x_ldo_ops = {
+ .list_voltage = regulator_list_voltage_table,
+ .map_voltage = regulator_map_voltage_iterate,
+ .set_voltage_sel = p3h2x4x_regulator_set_voltage_sel,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .enable = p3h2x4x_regulator_enable,
+ .disable = p3h2x4x_regulator_disable,
+ .is_enabled = regulator_is_enabled_regmap,
+};
+
+static const unsigned int p3h2x4x_voltage_table[] = {
+ 1000000,
+ 1100000,
+ 1200000,
+ 1800000,
+};
+
+static const struct regulator_desc p3h2x4x_regulators[] = {
+ {
+ .name = "ldo-cp0",
+ .of_match = of_match_ptr("ldo-cp0"),
+ .regulators_node = of_match_ptr("regulators"),
+ .volt_table = p3h2x4x_voltage_table,
+ .n_voltages = ARRAY_SIZE(p3h2x4x_voltage_table),
+ .ops = &p3h2x4x_ldo_ops,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .enable_reg = P3H2X4X_LDO_AND_PULLUP_CONF,
+ .enable_mask = P3H2X4X_CP0_EN_LDO,
+ .enable_time = P3H2X4X_LDO_ENABLE_TIME_US,
+ .vsel_reg = P3H2X4X_VCCIO_LDO_CONF,
+ .vsel_mask = P3H2X4X_CP0_VCCIO_LDO_VOLTAGE_MASK,
+ },
+ {
+ .name = "ldo-cp1",
+ .of_match = of_match_ptr("ldo-cp1"),
+ .regulators_node = of_match_ptr("regulators"),
+ .volt_table = p3h2x4x_voltage_table,
+ .n_voltages = ARRAY_SIZE(p3h2x4x_voltage_table),
+ .ops = &p3h2x4x_ldo_ops,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .enable_reg = P3H2X4X_LDO_AND_PULLUP_CONF,
+ .enable_mask = P3H2X4X_CP1_EN_LDO,
+ .enable_time = P3H2X4X_LDO_ENABLE_TIME_US,
+ .vsel_reg = P3H2X4X_VCCIO_LDO_CONF,
+ .vsel_mask = P3H2X4X_CP1_VCCIO_LDO_VOLTAGE_MASK,
+ },
+ {
+ .name = "ldo-tpg0",
+ .of_match = of_match_ptr("ldo-tpg0"),
+ .regulators_node = of_match_ptr("regulators"),
+ .volt_table = p3h2x4x_voltage_table,
+ .n_voltages = ARRAY_SIZE(p3h2x4x_voltage_table),
+ .ops = &p3h2x4x_ldo_ops,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .enable_reg = P3H2X4X_LDO_AND_PULLUP_CONF,
+ .enable_mask = P3H2X4X_TP0145_EN_LDO,
+ .enable_time = P3H2X4X_LDO_ENABLE_TIME_US,
+ .vsel_reg = P3H2X4X_VCCIO_LDO_CONF,
+ .vsel_mask = P3H2X4X_TP0145_VCCIO_LDO_VOLTAGE_MASK,
+ },
+ {
+ .name = "ldo-tpg1",
+ .of_match = of_match_ptr("ldo-tpg1"),
+ .regulators_node = of_match_ptr("regulators"),
+ .volt_table = p3h2x4x_voltage_table,
+ .n_voltages = ARRAY_SIZE(p3h2x4x_voltage_table),
+ .ops = &p3h2x4x_ldo_ops,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .enable_reg = P3H2X4X_LDO_AND_PULLUP_CONF,
+ .enable_mask = P3H2X4X_TP2367_EN_LDO,
+ .enable_time = P3H2X4X_LDO_ENABLE_TIME_US,
+ .vsel_reg = P3H2X4X_VCCIO_LDO_CONF,
+ .vsel_mask = P3H2X4X_TP2367_VCCIO_LDO_VOLTAGE_MASK,
+ },
+};
+
+static int p3h2x4x_regulator_probe(struct platform_device *pdev)
+{
+ struct p3h2x4x *p3h2x4x = dev_get_drvdata(pdev->dev.parent);
+ struct p3h2x4x_regulator_dev *p3h2x4x_regulator;
+ struct regulator_config rcfg = { };
+ struct device *dev = &pdev->dev;
+ struct regulator_dev *rdev;
+ int i;
+
+ p3h2x4x_regulator = devm_kzalloc(dev, sizeof(*p3h2x4x_regulator), GFP_KERNEL);
+ if (!p3h2x4x_regulator)
+ return -ENOMEM;
+
+ p3h2x4x_regulator->p3h2x4x = p3h2x4x;
+ p3h2x4x_regulator->regmap = p3h2x4x->regmap;
+
+ platform_set_drvdata(pdev, p3h2x4x_regulator);
+
+ device_set_of_node_from_dev(dev, dev->parent);
+
+ rcfg.dev = dev;
+ rcfg.regmap = p3h2x4x_regulator->regmap;
+ rcfg.driver_data = p3h2x4x_regulator;
+
+ for (i = 0; i < ARRAY_SIZE(p3h2x4x_regulators); i++) {
+ rdev = devm_regulator_register(&pdev->dev, &p3h2x4x_regulators[i], &rcfg);
+ if (IS_ERR(rdev))
+ return dev_err_probe(dev, PTR_ERR(rdev), "Failed to register %s\n",
+ p3h2x4x_regulators[i].name);
+ p3h2x4x_regulator->rp3h2x4x_dev[i] = rdev;
+ }
+ return 0;
+}
+
+static const struct platform_device_id p3h2x4x_regulator_id[] = {
+ { .name = "p3h2x4x-regulator" },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, p3h2x4x_regulator_id);
+
+static struct platform_driver p3h2x4x_regulator_driver = {
+ .driver = {
+ .name = "p3h2x4x-regulator",
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+ .probe = p3h2x4x_regulator_probe,
+ .id_table = p3h2x4x_regulator_id,
+};
+module_platform_driver(p3h2x4x_regulator_driver);
+
+MODULE_AUTHOR("Aman Kumar Pandey <aman.kumarpandey@nxp.com>");
+MODULE_AUTHOR("Vikash Bansal <vikash.bansal@nxp.com>");
+MODULE_AUTHOR("Lakshay Piplani <lakshay.piplani@nxp.com>");
+MODULE_DESCRIPTION("NXP P3H2X4X I3C HUB Regulator driver");
+MODULE_LICENSE("GPL");
--
2.25.1
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v21 2/4] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
2026-11-12 12:44 ` [PATCH v21 2/4] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
@ 2026-11-12 12:56 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-12 12:56 UTC (permalink / raw)
To: Lakshay Piplani, linux-i3c, mfd; +Cc: Frank.Li, Alexandre Belloni, lee
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] regulator: p3h2x4x: OF node leak in p3h2x4x_regulator_probe()
--
commit 0c22fd7becea5c3dad0894cb83a63e3d27c04c45
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
This commit adds a regulator driver for the four on-die LDOs in the NXP
P3H2x4x I3C hub family, registered as a child of the MFD device.
> diff --git a/drivers/regulator/p3h2840_i3c_hub_regulator.c b/drivers/regulator/p3h2840_i3c_hub_regulator.c
> new file mode 100644
> index 0000000000000..244f2a4f70f38
> --- /dev/null
> +++ b/drivers/regulator/p3h2840_i3c_hub_regulator.c
[ ... ]
> +static int p3h2x4x_regulator_probe(struct platform_device *pdev)
> +{
> + struct p3h2x4x *p3h2x4x = dev_get_drvdata(pdev->dev.parent);
> + struct p3h2x4x_regulator_dev *p3h2x4x_regulator;
> + struct regulator_config rcfg = { };
> + struct device *dev = &pdev->dev;
> + struct regulator_dev *rdev;
> + int i;
> +
> + p3h2x4x_regulator = devm_kzalloc(dev, sizeof(*p3h2x4x_regulator), GFP_KERNEL);
> + if (!p3h2x4x_regulator)
> + return -ENOMEM;
> +
> + p3h2x4x_regulator->p3h2x4x = p3h2x4x;
> + p3h2x4x_regulator->regmap = p3h2x4x->regmap;
> +
> + platform_set_drvdata(pdev, p3h2x4x_regulator);
> +
> + device_set_of_node_from_dev(dev, dev->parent);
[Severity: Medium]
Does calling device_set_of_node_from_dev() here leak a reference on
dev->parent->of_node when pdev is unregistered?
The MFD cell for p3h2x4x-regulator does not set of_compatible or swnode, so
mfd_add_device() allocates pdev with dev.fwnode set to NULL.
device_set_of_node_from_dev() takes a reference on dev->parent->of_node and
assigns dev->of_node without updating dev->fwnode, whereas
platform_device_release() drops the firmware node reference via
fwnode_handle_put(pa->pdev.dev.fwnode).
Using platform_device_set_of_node_from_dev(pdev, dev->parent) sets both
dev.of_node and dev.fwnode so platform_device_release() balances the
reference.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112124437.3367252-1-lakshay.piplani@nxp.com?part=2
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v21 3/4] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
2026-11-12 12:44 [PATCH v21 0/4] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
2026-11-12 12:44 ` [PATCH v21 1/4] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
2026-11-12 12:44 ` [PATCH v21 2/4] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
@ 2026-11-12 12:44 ` Lakshay Piplani
2026-11-12 13:15 ` sashiko-bot
2026-11-12 12:44 ` [PATCH v21 4/4] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
3 siblings, 1 reply; 10+ messages in thread
From: Lakshay Piplani @ 2026-11-12 12:44 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, broonie, mfd, lee,
Frank.Li, lgirdwood
Cc: vikash.bansal, priyanka.jain, aman.kumarpandey, Lakshay Piplani
From: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
Add I3C hub functionality for the NXP P3H2x4x family of multiport hubs.
These devices support downstream target ports that can be configured
as I3C or SMBus.
This driver enables:
- I3C/I2C communication between host and hub
- Transparent communication with downstream devices
- Target port configuration (I3C/SMBus)
P3H2440/P3H2441 support 4 target ports.
P3H2840/P3H2841 support 8 target ports.
Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
Signed-off-by: Vikash Bansal <vikash.bansal@nxp.com>
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v21:
- Restrict both DT parsing passes to I3C/SMBus port nodes and reject
duplicate ports
- Reserve all DT-assigned I3C addresses before starting DAA
- Preserve routing callbacks during registration and serialize route
updates, committing state only on success
- Preserve pending receive and overflow flags during SMBus transfers
Changes in v20:
- Add the generic I3C hub MAINTAINERS entries moved from patch 6
Changes in v19:
- No change
Changes in v18:
- Rebased onto v7.3-rc1; no intended driver behaviour changes
Changes in v17:
- Remove the fixed post-enable delay; regulator settling is now handled
by the regulator core
- Use a named initializer for the platform_device_id table
- Document why I3C target ports are connected once and left connected,
making their per-operation enable and disable callbacks no-ops
- Reference the exported i3c_hub_master_ops object directly
Changes in v16:
- Do not ignore regulator-enable failures: p3h2x4x_configure_ldo() now
returns via dev_err_probe() for any error other than -ENODEV (optional
supply), instead of only warning
- Widen the SMBus poll deadline: floor the timeout at the datasheet
SDA-stuck/SCL-low recovery window (P3H2X4X_SMBUS_SCL_LOW_RECOVERY_US,
50 ms) via max(xfer_us, ...), so an in-spec SDA-stuck/SCL-low bus recovery
no longer causes spurious timeouts. The transfer-time estimate now only
drives the poll interval, not the give-up deadline
- Use i2c_get_dma_safe_msg_buf() and i2c_put_dma_safe_msg_buf() for
controller-agent read transfers before passing the buffer to
regmap_bulk_read() over I3C
- Document the i3c_hub_priv publish/clear ordering: publish before IBI is
enabled and clear it via devm_add_action_or_reset() so, on unwind,
the pointer is cleared only after IBI is disabled and freed
- Convert the SMBus register-protection sequence to scoped_guard() to avoid
mixing goto- and scope-based cleanup in one function
- Keep the virtual controllers in the devm-managed hub struct; teardown is
ordered so i3c_master_unregister() runs (via devm action) before the
memory is freed
Changes in v15:
- Use the target-port count detected by the MFD parent and only configure
registers and ports implemented by the selected device variant
- Validate target-port indices against the detected number of ports
- Preserve the MFD parent's driver data and publish the hub context through
the shared MFD structure, with managed cleanup
- Correct the SMBus transfer timeout calculation for 400 kHz operation
- Use I2C adapter quirks to enforce the maximum read and write payload
lengths
Changes in v14:
- Replace temporary parent dev->of_node reassignment with
i3c_master_register_fwnode()
- Use the shared MFD protected_reg_lock for protected hub configuration
writes
- Fix SMBus polling interval calculation to avoid oversleeping the computed
transaction timeout
- Change SMBus transfer loop counters from u8 to int
- Clean up already registered SMBus adapters on adapter allocation or
registration failure
Changes in v13:
- Fix SMBus transaction handling by replacing fixed delay with polling (read_poll_timeout)
to avoid premature reads and data corruption
- Fix DT and of_node handling: prevent duplicate target-port node leaks and avoid corrupting
parent dev->of_node by restoring it after registration
- Add proper cleanup using devm actions (relock registers, release DT nodes, unregister adapters)
and fix minor comment mismatch
Changes in v12:
- Fix target-port configuration register updates
- Correct default pull-up and drive-strength values
- Improve OF node and SMBus adapter cleanup
- Remove dead code and simplify cleanup by relying on devm-managed resources
Changes in v11:
- Fix IBI resource cleanup on error paths
- Fix adapter unregister cleanup handling
Changes in v10:
- Split SMBus target/slave mode support, including IBI and MCTP receive
handling, into a separate patch
Changes in v9:
- Added CONFIG_I2C_SLAVE guards where necessary to avoid build issues
when I2C slave support is disabled.
Changes in v8:
- No change
Changes in v7:
- Remove CONFIG_I2C_SLAVE guards
- Use Kernel API find_closest instead of custom helper
- Use devm_regulator_get_enable_optional()
- Fix kernel-doc warnings
Changes in v6:
- Remove generic I3C code and keep reg dependent code only.
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 | 3 +
drivers/i3c/Kconfig | 1 +
drivers/i3c/Makefile | 1 +
drivers/i3c/hub/Kconfig | 11 +
drivers/i3c/hub/Makefile | 4 +
drivers/i3c/hub/p3h2840_i3c_hub.h | 342 ++++++++++++++++++
drivers/i3c/hub/p3h2840_i3c_hub_common.c | 432 +++++++++++++++++++++++
drivers/i3c/hub/p3h2840_i3c_hub_i3c.c | 162 +++++++++
drivers/i3c/hub/p3h2840_i3c_hub_smbus.c | 352 ++++++++++++++++++
9 files changed, 1308 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
diff --git a/MAINTAINERS b/MAINTAINERS
index 0bfac3696bd2..8978ab188de7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20242,8 +20242,11 @@ L: linux-kernel@vger.kernel.org
L: linux-i3c@lists.infradead.org
S: Maintained
F: Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
+F: drivers/i3c/hub.c
+F: drivers/i3c/hub/*
F: drivers/mfd/p3h2840.c
F: drivers/regulator/p3h2840_i3c_hub_regulator.c
+F: include/linux/i3c/hub.h
F: include/linux/mfd/p3h2840.h
NXP PF5300/PF5301/PF5302 PMIC REGULATOR DEVICE DRIVER
diff --git a/drivers/i3c/Kconfig b/drivers/i3c/Kconfig
index 7536f3740c94..81d5ebd8025c 100644
--- a/drivers/i3c/Kconfig
+++ b/drivers/i3c/Kconfig
@@ -35,6 +35,7 @@ config I3C_HUB
Say Y here if your platform includes an I3C hub device
+source "drivers/i3c/hub/Kconfig"
endif # I3C
config I3C_OR_I2C
diff --git a/drivers/i3c/Makefile b/drivers/i3c/Makefile
index 9ddee56a6338..2950820db9ea 100644
--- a/drivers/i3c/Makefile
+++ b/drivers/i3c/Makefile
@@ -3,3 +3,4 @@ i3c-y := device.o master.o
obj-$(CONFIG_I3C) += i3c.o
obj-$(CONFIG_I3C) += master/
obj-$(CONFIG_I3C_HUB) += hub.o
+obj-$(CONFIG_I3C_HUB) += hub/
diff --git a/drivers/i3c/hub/Kconfig b/drivers/i3c/hub/Kconfig
new file mode 100644
index 000000000000..969bb1f637dd
--- /dev/null
+++ b/drivers/i3c/hub/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2025 NXP
+config P3H2X4X_I3C_HUB
+ tristate "NXP P3H2X4X I3C HUB support"
+ depends on MFD_P3H2X4X
+ select I3C_HUB
+ help
+ This enables support for NXP P3H244x/P3H284x I3C HUB. These hubs
+ connect to a host via I3C/I2C/SMBus and allow communication with
+ multiple downstream peripherals. Say Y or M here to enable the
+ P3H2x4x I3C HUB driver.
diff --git a/drivers/i3c/hub/Makefile b/drivers/i3c/hub/Makefile
new file mode 100644
index 000000000000..9dbd8a7b4184
--- /dev/null
+++ b/drivers/i3c/hub/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2025 NXP
+p3h2840_i3c_hub-y := p3h2840_i3c_hub_common.o p3h2840_i3c_hub_i3c.o p3h2840_i3c_hub_smbus.o
+obj-$(CONFIG_P3H2X4X_I3C_HUB) += p3h2840_i3c_hub.o
diff --git a/drivers/i3c/hub/p3h2840_i3c_hub.h b/drivers/i3c/hub/p3h2840_i3c_hub.h
new file mode 100644
index 000000000000..5eecc972514b
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub.h
@@ -0,0 +1,342 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2025-2026 NXP
+ * Private definitions for the NXP P3H2X4X I3C hub driver.
+ */
+
+#ifndef P3H2840_I3C_HUB_H
+#define P3H2840_I3C_HUB_H
+
+#include <linux/bitfield.h>
+#include <linux/i2c.h>
+#include <linux/i3c/device.h>
+#include <linux/i3c/hub.h>
+#include <linux/i3c/master.h>
+#include <linux/regulator/consumer.h>
+#include <linux/regmap.h>
+
+/* I3C HUB REGISTERS */
+
+/* Device Information Registers */
+#define P3H2X4X_DEV_INFO_0 0x00
+#define P3H2X4X_DEV_INFO_1 0x01
+#define P3H2X4X_PID_5 0x02
+#define P3H2X4X_PID_4 0x03
+#define P3H2X4X_PID_3 0x04
+#define P3H2X4X_PID_2 0x05
+#define P3H2X4X_PID_1 0x06
+#define P3H2X4X_PID_0 0x07
+#define P3H2X4X_BCR 0x08
+#define P3H2X4X_DCR 0x09
+#define P3H2X4X_DEV_CAPAB 0x0a
+#define P3H2X4X_DEV_REV 0x0b
+
+/* Device Configuration Registers */
+#define P3H2X4X_CP_CONF 0x11
+#define P3H2X4X_TP_ENABLE 0x12
+
+#define P3H2X4X_DEV_CONF 0x13
+#define P3H2X4X_IO_STRENGTH 0x14
+#define P3H2X4X_TP0145_IO_STRENGTH_MASK GENMASK(1, 0)
+#define P3H2X4X_TP0145_IO_STRENGTH(x) \
+ FIELD_PREP(P3H2X4X_TP0145_IO_STRENGTH_MASK, x)
+#define P3H2X4X_TP2367_IO_STRENGTH_MASK GENMASK(3, 2)
+#define P3H2X4X_TP2367_IO_STRENGTH(x) \
+ FIELD_PREP(P3H2X4X_TP2367_IO_STRENGTH_MASK, x)
+#define P3H2X4X_CP0_IO_STRENGTH_MASK GENMASK(5, 4)
+#define P3H2X4X_CP0_IO_STRENGTH(x) \
+ FIELD_PREP(P3H2X4X_CP0_IO_STRENGTH_MASK, x)
+#define P3H2X4X_CP1_IO_STRENGTH_MASK GENMASK(7, 6)
+#define P3H2X4X_CP1_IO_STRENGTH(x) \
+ FIELD_PREP(P3H2X4X_CP1_IO_STRENGTH_MASK, x)
+#define P3H2X4X_IO_STRENGTH_MASK GENMASK(7, 0)
+
+#define P3H2X4X_TP_IO_MODE_CONF 0x17
+#define P3H2X4X_TP_SMBUS_AGNT_EN 0x18
+
+#define P3H2X4X_LDO_AND_PULLUP_CONF 0x19
+
+#define P3H2X4X_TP0145_PULLUP_CONF_MASK GENMASK(7, 6)
+#define P3H2X4X_TP0145_PULLUP_CONF(x) \
+ FIELD_PREP(P3H2X4X_TP0145_PULLUP_CONF_MASK, x)
+#define P3H2X4X_TP2367_PULLUP_CONF_MASK GENMASK(5, 4)
+#define P3H2X4X_TP2367_PULLUP_CONF(x) \
+ FIELD_PREP(P3H2X4X_TP2367_PULLUP_CONF_MASK, x)
+#define P3H2X4X_PULLUP_CONF_MASK GENMASK(7, 4)
+
+#define P3H2X4X_CP_IBI_CONF 0x1a
+
+#define P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG 0x1b
+
+#define P3H2X4X_IBI_MDB_CUSTOM 0x1c
+#define P3H2X4X_JEDEC_CONTEXT_ID 0x1d
+#define P3H2X4X_TP_GPIO_MODE_EN 0x1e
+
+/* Device Status and IBI Registers */
+#define P3H2X4X_DEV_AND_IBI_STS 0x20
+#define P3H2X4X_TP_SMBUS_AGNT_IBI_STS 0x21
+#define P3H2X4X_SMBUS_AGENT_EVENT_FLAG_STATUS BIT(4)
+
+/* Controller Port Control/Status Registers */
+#define P3H2X4X_CP_MUX_SET 0x38
+#define P3H2X4X_CONTROLLER_PORT_MUX_REQ BIT(0)
+#define P3H2X4X_CP_MUX_STS 0x39
+#define P3H2X4X_CONTROLLER_PORT_MUX_CONNECTION_STATUS BIT(0)
+
+/* Target Ports Control Registers */
+#define P3H2X4X_TP_SMBUS_AGNT_TRANS_START 0x50
+#define P3H2X4X_TP_NET_CON_CONF 0x51
+
+#define P3H2X4X_TP_PULLUP_EN 0x53
+
+#define P3H2X4X_TP_SCL_OUT_EN 0x54
+#define P3H2X4X_TP_SDA_OUT_EN 0x55
+#define P3H2X4X_TP_SCL_OUT_LEVEL 0x56
+#define P3H2X4X_TP_SDA_OUT_LEVEL 0x57
+#define P3H2X4X_TP_IN_DETECT_MODE_CONF 0x58
+#define P3H2X4X_TP_SCL_IN_DETECT_IBI_EN 0x59
+#define P3H2X4X_TP_SDA_IN_DETECT_IBI_EN 0x5a
+
+/* Target Ports Status Registers */
+#define P3H2X4X_TP_SCL_IN_LEVEL_STS 0x60
+#define P3H2X4X_TP_SDA_IN_LEVEL_STS 0x61
+#define P3H2X4X_TP_SCL_IN_DETECT_FLG 0x62
+#define P3H2X4X_TP_SDA_IN_DETECT_FLG 0x63
+
+/* SMBus Agent Configuration and Status Registers */
+#define P3H2X4X_TP0_SMBUS_AGNT_STS 0x64
+#define P3H2X4X_TP1_SMBUS_AGNT_STS 0x65
+#define P3H2X4X_TP2_SMBUS_AGNT_STS 0x66
+#define P3H2X4X_TP3_SMBUS_AGNT_STS 0x67
+#define P3H2X4X_TP4_SMBUS_AGNT_STS 0x68
+#define P3H2X4X_TP5_SMBUS_AGNT_STS 0x69
+#define P3H2X4X_TP6_SMBUS_AGNT_STS 0x6a
+#define P3H2X4X_TP7_SMBUS_AGNT_STS 0x6b
+#define P3H2X4X_ONCHIP_TD_AND_SMBUS_AGNT_CONF 0x6c
+
+/* buf receive flag set */
+#define P3H2X4X_TARGET_BUF_CA_TF BIT(0)
+#define P3H2X4X_TARGET_BUF_0_RECEIVE BIT(1)
+#define P3H2X4X_TARGET_BUF_1_RECEIVE BIT(2)
+#define P3H2X4X_TARGET_BUF_0_1_RECEIVE GENMASK(2, 1)
+#define P3H2X4X_TARGET_BUF_OVRFL BIT(3)
+#define BUF_RECEIVED_FLAG_MASK GENMASK(3, 1)
+#define BUF_RECEIVED_FLAG_TF_MASK GENMASK(3, 0)
+
+#define P3H2X4X_TARGET_AGENT_LOCAL_DEV 0x11
+#define P3H2X4X_TARGET_BUFF_0_PAGE 0x12
+#define P3H2X4X_TARGET_BUFF_1_PAGE 0x13
+
+/* Special Function Registers */
+#define P3H2X4X_LDO_AND_CPSEL_STS 0x79
+#define P3H2X4X_CP_SDA1_LEVEL BIT(7)
+#define P3H2X4X_CP_SCL1_LEVEL BIT(6)
+
+#define P3H2X4X_CP_SEL_PIN_INPUT_CODE_MASK GENMASK(5, 4)
+#define P3H2X4X_CP_SEL_PIN_INPUT_CODE_GET(x) \
+ (((x) & P3H2X4X_CP_SEL_PIN_INPUT_CODE_MASK) >> 4)
+#define P3H2X4X_CP_SDA1_SCL1_PINS_CODE_MASK GENMASK(7, 6)
+#define P3H2X4X_CP_SDA1_SCL1_PINS_CODE_GET(x) \
+ (((x) & P3H2X4X_CP_SDA1_SCL1_PINS_CODE_MASK) >> 6)
+#define P3H2X4X_VCCIO1_PWR_GOOD BIT(3)
+#define P3H2X4X_VCCIO0_PWR_GOOD BIT(2)
+#define P3H2X4X_CP1_VCCIO_PWR_GOOD BIT(1)
+#define P3H2X4X_CP0_VCCIO_PWR_GOOD BIT(0)
+
+#define P3H2X4X_BUS_RESET_SCL_TIMEOUT 0x7a
+#define P3H2X4X_ONCHIP_TD_PROTO_ERR_FLG 0x7b
+#define P3H2X4X_DEV_CMD 0x7c
+#define P3H2X4X_ONCHIP_TD_STS 0x7d
+#define P3H2X4X_ONCHIP_TD_ADDR_CONF 0x7e
+#define P3H2X4X_PAGE_PTR 0x7f
+
+/* Paged Transaction Registers */
+#define P3H2X4X_CONTROLLER_BUFFER_PAGE 0x10
+#define P3H2X4X_CONTROLLER_AGENT_BUFF 0x80
+#define P3H2X4X_CONTROLLER_AGENT_BUFF_DATA 0x84
+
+#define P3H2X4X_TARGET_BUFF_LENGTH 0x80
+#define P3H2X4X_TARGET_BUFF_ADDRESS 0x81
+#define P3H2X4X_TARGET_BUFF_DATA 0x82
+
+#define P3H2X4X_TP_MAX_COUNT 0x08
+#define P3H2X4X_CP_MAX_COUNT 0x02
+#define P3H2X4X_TP_LOCAL_DEV 0x08
+
+/* LDO Disable/Enable DT settings */
+#define P3H2X4X_LDO_VOLT_1_0V 0x00
+#define P3H2X4X_LDO_VOLT_1_1V 0x01
+#define P3H2X4X_LDO_VOLT_1_2V 0x02
+#define P3H2X4X_LDO_VOLT_1_8V 0x03
+
+#define P3H2X4X_LDO_DISABLED 0x00
+#define P3H2X4X_LDO_ENABLED 0x01
+
+#define P3H2X4X_IBI_DISABLED 0x00
+#define P3H2X4X_IBI_ENABLED 0x01
+
+#define P3H2X4X_TP_PULLUP_DISABLED 0x00
+#define P3H2X4X_TP_PULLUP_ENABLED 0x01
+
+#define ONE_BYTE_SIZE 0x01
+
+/* holding SDA low when both SMBus Target Agent received data buffers are full.
+ * This feature can be used as a flow-control mechanism for MCTP applications to
+ * avoid MCTP transmitters on Target Ports time out when the SMBus agent buffers
+ * are not serviced in time by upstream controller and only receives write message
+ * from its downstream ports.
+ * SMBUS_AGENT_TX_RX_LOOPBACK_EN/TARGET_AGENT_BUF_FULL_SDA_LOW_EN
+ */
+
+#define P3H2X4X_TARGET_AGENT_DFT_IBI_CONF 0x20
+#define P3H2X4X_TARGET_AGENT_DFT_IBI_CONF_MASK 0x21
+
+/* Transaction status checking mask */
+#define P3H2X4X_SMBUS_TRANSACTION_FINISH_FLAG 1
+#define P3H2X4X_SMBUS_CNTRL_STATUS_TXN_SHIFT 4
+
+#define P3H2X4X_SMBUS_CNTRL_STATUS_TXN_OK 0
+#define P3H2X4X_SMBUS_CNTRL_STATUS_TXN_ADDR_NAK 1
+#define P3H2X4X_SMBUS_CNTRL_STATUS_TXN_DATA_NAK 2
+#define P3H2X4X_SMBUS_CNTRL_STATUS_TXN_WTR_NAK 3
+#define P3H2X4X_SMBUS_CNTRL_STATUS_TXN_SYNC_RCV 4
+#define P3H2X4X_SMBUS_CNTRL_STATUS_TXN_SYNC_RCVCLR 5
+#define P3H2X4X_SMBUS_CNTRL_STATUS_TXN_FAULT 6
+#define P3H2X4X_SMBUS_CNTRL_STATUS_TXN_ARB_LOSS 7
+#define P3H2X4X_SMBUS_CNTRL_STATUS_TXN_SCL_TO 8
+
+#define P3H2X4X_TP_TRANSACTION_CODE_MASK 0xf0
+
+/* SMBus transaction types fields */
+#define P3H2X4X_SMBUS_400kHz BIT(2)
+
+/* SMBus polling */
+#define P3H2X4X_SMBUS_POLL_COUNT 10
+#define P3H2X4X_SMBUS_POLL_INTERVAL_MIN_US 20
+#define P3H2X4X_SMBUS_POLL_INTERVAL_MAX_US 150
+
+/* Hub buffer size */
+#define P3H2X4X_CONTROLLER_BUFFER_SIZE 88
+#define P3H2X4X_TARGET_BUFFER_SIZE 80
+#define P3H2X4X_SMBUS_DESCRIPTOR_SIZE 4
+#define P3H2X4X_SMBUS_PAYLOAD_SIZE \
+ (P3H2X4X_CONTROLLER_BUFFER_SIZE - P3H2X4X_SMBUS_DESCRIPTOR_SIZE)
+#define P3H2X4X_SMBUS_TARGET_PAYLOAD_SIZE (P3H2X4X_TARGET_BUFFER_SIZE - 2)
+
+/*
+ * At 400 kHz, one 9-bit I2C byte takes 22.5 us. Round this
+ * up to 23 us per payload byte and add 100 us for the address
+ * byte, bus overhead and controller processing time. This models
+ * the time a healthy transaction takes and is used to derive the
+ * poll interval, not the overall give-up deadline.
+ */
+#define P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(x) ((23 * (x)) + 100)
+
+/*
+ * On an SDA-stuck condition the SMBus Controller Agent can legitimately
+ * hold SCL low for up to 35 ms during bus recovery (datasheet 8.11.2,
+ * Table 21). Floor the overall poll deadline above that so the recovery,
+ * status posting and the status read-back complete before we give up and
+ * report a spurious timeout.
+ */
+#define P3H2X4X_SMBUS_SCL_LOW_RECOVERY_US 50000
+
+#define P3H2X4X_NO_PAGE_PER_TP 4
+
+#define P3H2X4X_MAX_PAYLOAD_LEN 2
+#define P3H2X4X_NUM_SLOTS 6
+
+#define P3H2X4X_HUB_ID 0
+
+#define P3H2X4X_SET_BIT(n) BIT(n)
+
+#define P3H2X4X_TP_MASK GENMASK(P3H2X4X_TP_MAX_COUNT - 1, 0)
+
+#define P3H2X4X_DFT_TP_PULLUP_OHMS 500
+#define P3H2X4X_DFT_IO_STRENGTH_OHMS 20
+
+enum p3h2x4x_tp {
+ TP_0,
+ TP_1,
+ TP_2,
+ TP_3,
+ TP_4,
+ TP_5,
+ TP_6,
+ TP_7,
+};
+
+enum p3h2x4x_rcv_buf {
+ RCV_BUF_0,
+ RCV_BUF_1,
+ RCV_BUF_OF,
+};
+
+enum p3h2x4x_tp_mode {
+ P3H2X4X_TP_MODE_I3C,
+ P3H2X4X_TP_MODE_SMBUS,
+};
+
+struct tp_configuration {
+ bool pullup_en;
+ bool ibi_en;
+ bool always_enable;
+ enum p3h2x4x_tp_mode mode;
+};
+
+struct hub_configuration {
+ int tp0145_pullup;
+ int tp2367_pullup;
+ int cp0_io_strength;
+ int cp1_io_strength;
+ int tp0145_io_strength;
+ int tp2367_io_strength;
+ struct tp_configuration tp_config[P3H2X4X_TP_MAX_COUNT];
+};
+
+struct tp_bus {
+ bool is_registered; /* bus was registered in the framework. */
+ u8 tp_mask;
+ u8 tp_port;
+ struct mutex port_mutex; /* per port mutex */
+ struct device_node *of_node;
+ struct i2c_client *tp_smbus_client;
+ struct i2c_adapter *tp_smbus_adapter;
+ struct i3c_hub_controller hub_controller;
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub;
+};
+
+struct p3h2x4x_i3c_hub_dev {
+ struct device *dev;
+ struct regmap *regmap;
+ struct mutex etx_mutex; /* all port mutex */
+ struct i3c_device *i3cdev;
+ struct i2c_client *i2c_client;
+ struct hub_configuration hub_config;
+ struct tp_bus tp_bus[P3H2X4X_TP_MAX_COUNT];
+ struct i3c_hub *hub;
+};
+
+/**
+ * p3h2x4x_unregister_smbus_adapters() - unregister SMBus adapters
+ * @hub: P3H2x4x hub device
+ */
+void p3h2x4x_unregister_smbus_adapters(struct p3h2x4x_i3c_hub_dev *hub);
+
+/**
+ * p3h2x4x_tp_smbus_algo - add i2c adapter for target port configured as SMBus.
+ * @p3h2x4x_i3c_hub: P3H2x4x hub device.
+ *
+ * Return: 0 in case of success, negative error code on failure.
+ */
+int p3h2x4x_tp_smbus_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub);
+
+/**
+ * p3h2x4x_tp_i3c_algo - register i3c controller for target port configured as I3C.
+ * @p3h2x4x_i3c_hub: P3H2x4x hub device.
+ *
+ * Return: 0 in case of success, negative error code on failure.
+ */
+int p3h2x4x_tp_i3c_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub);
+
+#endif /* P3H2840_I3C_HUB_H */
diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_common.c b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
new file mode 100644
index 000000000000..937491657fb3
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
@@ -0,0 +1,432 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025-2026 NXP
+ * This P3H2X4X driver file implements functions for Hub probe and DT parsing.
+ */
+
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/p3h2840.h>
+#include <linux/util_macros.h>
+
+#include "p3h2840_i3c_hub.h"
+
+/* LDO voltage DT settings */
+#define P3H2X4X_DT_LDO_VOLT_1_0V 1000000
+#define P3H2X4X_DT_LDO_VOLT_1_1V 1100000
+#define P3H2X4X_DT_LDO_VOLT_1_2V 1200000
+#define P3H2X4X_DT_LDO_VOLT_1_8V 1800000
+
+static const int p3h2x4x_pullup_tbl[] = {
+ 250, 500, 1000, 2000
+};
+
+static const int p3h2x4x_io_strength_tbl[] = {
+ 20, 30, 40, 50
+};
+
+static u8 p3h2x4x_pullup_dt_to_reg(int dt_value)
+{
+ return find_closest(dt_value, p3h2x4x_pullup_tbl,
+ ARRAY_SIZE(p3h2x4x_pullup_tbl));
+}
+
+static u8 p3h2x4x_io_strength_dt_to_reg(int dt_value)
+{
+ return find_closest(dt_value, p3h2x4x_io_strength_tbl,
+ ARRAY_SIZE(p3h2x4x_io_strength_tbl));
+}
+
+static int p3h2x4x_configure_pullup(struct device *dev)
+{
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = dev_get_drvdata(dev);
+ u8 pullup;
+
+ pullup = P3H2X4X_TP0145_PULLUP_CONF(p3h2x4x_pullup_dt_to_reg
+ (p3h2x4x_i3c_hub->hub_config.tp0145_pullup));
+
+ pullup |= P3H2X4X_TP2367_PULLUP_CONF(p3h2x4x_pullup_dt_to_reg
+ (p3h2x4x_i3c_hub->hub_config.tp2367_pullup));
+
+ return regmap_update_bits(p3h2x4x_i3c_hub->regmap, P3H2X4X_LDO_AND_PULLUP_CONF,
+ P3H2X4X_PULLUP_CONF_MASK, pullup);
+}
+
+static int p3h2x4x_configure_io_strength(struct device *dev)
+{
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = dev_get_drvdata(dev);
+ u8 io_strength;
+
+ io_strength = P3H2X4X_CP0_IO_STRENGTH(p3h2x4x_io_strength_dt_to_reg
+ (p3h2x4x_i3c_hub->hub_config.cp0_io_strength));
+
+ io_strength |= P3H2X4X_CP1_IO_STRENGTH(p3h2x4x_io_strength_dt_to_reg
+ (p3h2x4x_i3c_hub->hub_config.cp1_io_strength));
+
+ io_strength |= P3H2X4X_TP0145_IO_STRENGTH(p3h2x4x_io_strength_dt_to_reg
+ (p3h2x4x_i3c_hub->hub_config.tp0145_io_strength));
+
+ io_strength |= P3H2X4X_TP2367_IO_STRENGTH(p3h2x4x_io_strength_dt_to_reg
+ (p3h2x4x_i3c_hub->hub_config.tp2367_io_strength));
+
+ return regmap_update_bits(p3h2x4x_i3c_hub->regmap, P3H2X4X_IO_STRENGTH,
+ P3H2X4X_IO_STRENGTH_MASK, io_strength);
+}
+
+static int p3h2x4x_configure_ldo(struct device *dev)
+{
+ static const char * const supplies[] = {
+ "vcc1",
+ "vcc2",
+ "vcc3",
+ "vcc4"
+ };
+ int ret, i;
+
+ for (i = 0; i < ARRAY_SIZE(supplies); i++) {
+ ret = devm_regulator_get_enable_optional(dev, supplies[i]);
+ if (ret && ret != -ENODEV)
+ return dev_err_probe(dev, ret, "Failed to enable %s\n",
+ supplies[i]);
+ }
+
+ return 0;
+}
+
+static int p3h2x4x_configure_tp(struct device *dev)
+{
+ struct p3h2x4x_i3c_hub_dev *hub = dev_get_drvdata(dev);
+ struct p3h2x4x *p3h2x4x = dev_get_drvdata(dev->parent);
+ u8 mode = 0, smbus = 0, pullup = 0, target_port = 0;
+ u8 tp_mask;
+ int tp, ret;
+
+ for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
+ pullup |= hub->hub_config.tp_config[tp].pullup_en ? P3H2X4X_SET_BIT(tp) : 0;
+ mode |= (hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C) ?
+ P3H2X4X_SET_BIT(tp) : 0;
+ smbus |= (hub->hub_config.tp_config[tp].mode == P3H2X4X_TP_MODE_SMBUS) ?
+ P3H2X4X_SET_BIT(tp) : 0;
+ target_port |= (hub->tp_bus[tp].tp_mask == P3H2X4X_SET_BIT(tp)) ?
+ hub->tp_bus[tp].tp_mask : 0;
+ }
+
+ /* Only touch the bits for the target ports this variant provides. */
+ tp_mask = GENMASK(p3h2x4x->num_target_ports - 1, 0);
+
+ ret = regmap_update_bits(hub->regmap, P3H2X4X_TP_PULLUP_EN, tp_mask, pullup);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(hub->regmap, P3H2X4X_TP_IO_MODE_CONF, tp_mask, mode);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(hub->regmap, P3H2X4X_TP_SMBUS_AGNT_EN, tp_mask, smbus);
+ if (ret)
+ return ret;
+
+ if (target_port & ~smbus) {
+ ret = regmap_write(hub->regmap, P3H2X4X_CP_MUX_SET,
+ P3H2X4X_CONTROLLER_PORT_MUX_REQ);
+ if (ret)
+ return ret;
+ }
+
+ return regmap_update_bits(hub->regmap, P3H2X4X_TP_ENABLE, tp_mask, target_port);
+}
+
+static int p3h2x4x_configure_hw(struct device *dev)
+{
+ struct p3h2x4x_i3c_hub_dev *hub = dev_get_drvdata(dev);
+ struct p3h2x4x *p3h2x4x = dev_get_drvdata(dev->parent);
+ int ret, ret2;
+
+ ret = p3h2x4x_configure_ldo(dev);
+ if (ret)
+ return ret;
+
+ /* Protect the unlock-modify-lock sequence with the shared MFD lock */
+ scoped_guard(mutex, &p3h2x4x->protected_reg_lock) {
+ ret = regmap_write(hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_UNLOCK_CODE);
+ if (ret)
+ break;
+
+ ret = p3h2x4x_configure_pullup(dev);
+ if (!ret)
+ ret = p3h2x4x_configure_io_strength(dev);
+ if (!ret)
+ ret = p3h2x4x_configure_tp(dev);
+
+ ret2 = regmap_write(hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_LOCK_CODE);
+ if (!ret && ret2)
+ ret = ret2;
+ }
+
+ return ret;
+}
+
+/*
+ * Only "i3c" and "smbus" children describe target ports. Any other child
+ * (for example the MFD "regulators" container) must be skipped, so that an
+ * unrelated node carrying a "reg" property cannot occupy a target port slot.
+ */
+static bool p3h2x4x_is_target_port_node(const struct device_node *node)
+{
+ return of_node_name_eq(node, "i3c") ||
+ of_node_name_eq(node, "smbus");
+}
+
+static void p3h2x4x_get_target_port_dt_conf(struct device *dev,
+ const struct device_node *node)
+{
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = dev_get_drvdata(dev);
+ struct p3h2x4x *p3h2x4x = dev_get_drvdata(dev->parent);
+ u64 tp_port;
+
+ for_each_available_child_of_node_scoped(node, dev_node) {
+ if (!p3h2x4x_is_target_port_node(dev_node))
+ continue;
+
+ if (of_property_read_reg(dev_node, 0, &tp_port, NULL))
+ continue;
+
+ if (tp_port < p3h2x4x->num_target_ports) {
+ if (p3h2x4x_i3c_hub->tp_bus[tp_port].of_node) {
+ dev_warn(dev, "Duplicate target port %llu in DT\n", tp_port);
+ continue;
+ }
+
+ p3h2x4x_i3c_hub->tp_bus[tp_port].of_node = of_node_get(dev_node);
+ p3h2x4x_i3c_hub->tp_bus[tp_port].tp_mask = P3H2X4X_SET_BIT(tp_port);
+ p3h2x4x_i3c_hub->tp_bus[tp_port].p3h2x4x_i3c_hub = p3h2x4x_i3c_hub;
+ p3h2x4x_i3c_hub->tp_bus[tp_port].tp_port = tp_port;
+ }
+ }
+}
+
+static int p3h2x4x_parse_tp_dt_settings(struct device *dev,
+ const struct device_node *node,
+ struct tp_configuration tp_config[])
+{
+ struct p3h2x4x *p3h2x4x = dev_get_drvdata(dev->parent);
+ u8 seen = 0;
+ u64 id;
+ int ret;
+
+ for_each_available_child_of_node_scoped(node, tp_node) {
+ enum p3h2x4x_tp_mode mode;
+
+ if (!p3h2x4x_is_target_port_node(tp_node))
+ continue;
+
+ if (of_node_name_eq(tp_node, "i3c"))
+ mode = P3H2X4X_TP_MODE_I3C;
+ else
+ mode = P3H2X4X_TP_MODE_SMBUS;
+
+ ret = of_property_read_reg(tp_node, 0, &id, NULL);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to read reg for %pOF\n",
+ tp_node);
+
+ if (id >= p3h2x4x->num_target_ports)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid target port index %llu\n",
+ id);
+
+ /* Two children claiming one port leave the mode ambiguous. */
+ if (seen & BIT(id))
+ return dev_err_probe(dev, -EINVAL,
+ "Duplicate target port index %llu\n",
+ id);
+
+ seen |= BIT(id);
+
+ tp_config[id].mode = mode;
+ tp_config[id].pullup_en =
+ of_property_read_bool(tp_node, "nxp,pullup-enable");
+ }
+
+ return 0;
+}
+
+static int p3h2x4x_get_hub_dt_conf(struct device *dev,
+ const struct device_node *node)
+{
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = dev_get_drvdata(dev);
+
+ of_property_read_u32(node, "nxp,tp0145-pullup-ohms",
+ &p3h2x4x_i3c_hub->hub_config.tp0145_pullup);
+ of_property_read_u32(node, "nxp,tp2367-pullup-ohms",
+ &p3h2x4x_i3c_hub->hub_config.tp2367_pullup);
+ of_property_read_u32(node, "nxp,cp0-io-strength-ohms",
+ &p3h2x4x_i3c_hub->hub_config.cp0_io_strength);
+ of_property_read_u32(node, "nxp,cp1-io-strength-ohms",
+ &p3h2x4x_i3c_hub->hub_config.cp1_io_strength);
+ of_property_read_u32(node, "nxp,tp0145-io-strength-ohms",
+ &p3h2x4x_i3c_hub->hub_config.tp0145_io_strength);
+ of_property_read_u32(node, "nxp,tp2367-io-strength-ohms",
+ &p3h2x4x_i3c_hub->hub_config.tp2367_io_strength);
+
+ return p3h2x4x_parse_tp_dt_settings(dev, node,
+ p3h2x4x_i3c_hub->hub_config.tp_config);
+}
+
+static void p3h2x4x_default_configuration(struct device *dev)
+{
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = dev_get_drvdata(dev);
+ int tp_count;
+
+ p3h2x4x_i3c_hub->hub_config.tp0145_pullup = P3H2X4X_DFT_TP_PULLUP_OHMS;
+ p3h2x4x_i3c_hub->hub_config.tp2367_pullup = P3H2X4X_DFT_TP_PULLUP_OHMS;
+ p3h2x4x_i3c_hub->hub_config.cp0_io_strength = P3H2X4X_DFT_IO_STRENGTH_OHMS;
+ p3h2x4x_i3c_hub->hub_config.cp1_io_strength = P3H2X4X_DFT_IO_STRENGTH_OHMS;
+ p3h2x4x_i3c_hub->hub_config.tp0145_io_strength = P3H2X4X_DFT_IO_STRENGTH_OHMS;
+ p3h2x4x_i3c_hub->hub_config.tp2367_io_strength = P3H2X4X_DFT_IO_STRENGTH_OHMS;
+
+ for (tp_count = 0; tp_count < P3H2X4X_TP_MAX_COUNT; ++tp_count)
+ p3h2x4x_i3c_hub->hub_config.tp_config[tp_count].mode = P3H2X4X_TP_MODE_I3C;
+}
+
+static void p3h2x4x_unregister_smbus_adapters_action(void *data)
+{
+ p3h2x4x_unregister_smbus_adapters(data);
+}
+
+static void p3h2x4x_put_target_port_of_nodes(void *data)
+{
+ struct p3h2x4x_i3c_hub_dev *hub = data;
+ int tp;
+
+ for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; tp++) {
+ of_node_put(hub->tp_bus[tp].of_node);
+ hub->tp_bus[tp].of_node = NULL;
+ }
+}
+
+static void p3h2x4x_clear_i3c_hub_priv(void *data)
+{
+ struct p3h2x4x *p3h2x4x = data;
+
+ /* Drop the IBI handler backpointer; see the ordering note at the registration site. */
+ p3h2x4x->i3c_hub_priv = NULL;
+}
+
+static int p3h2x4x_i3c_hub_probe(struct platform_device *pdev)
+{
+ struct p3h2x4x *p3h2x4x = dev_get_drvdata(pdev->dev.parent);
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub;
+ struct device *dev = &pdev->dev;
+ struct device_node *node;
+ int ret, i;
+
+ p3h2x4x_i3c_hub = devm_kzalloc(dev, sizeof(*p3h2x4x_i3c_hub), GFP_KERNEL);
+ if (!p3h2x4x_i3c_hub)
+ return -ENOMEM;
+
+ p3h2x4x_i3c_hub->regmap = p3h2x4x->regmap;
+ p3h2x4x_i3c_hub->dev = dev;
+
+ platform_set_drvdata(pdev, p3h2x4x_i3c_hub);
+ device_set_of_node_from_dev(dev, dev->parent);
+
+ p3h2x4x_default_configuration(dev);
+
+ ret = devm_mutex_init(dev, &p3h2x4x_i3c_hub->etx_mutex);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < P3H2X4X_TP_MAX_COUNT; i++) {
+ ret = devm_mutex_init(dev, &p3h2x4x_i3c_hub->tp_bus[i].port_mutex);
+ if (ret)
+ return ret;
+ }
+
+ /* get hub node from DT */
+ node = dev_of_node(dev);
+ if (!node)
+ return dev_err_probe(dev, -ENODEV, "No Device Tree entry found\n");
+
+ ret = p3h2x4x_get_hub_dt_conf(dev, node);
+ if (ret)
+ return ret;
+
+ p3h2x4x_get_target_port_dt_conf(dev, node);
+
+ ret = devm_add_action_or_reset(dev,
+ p3h2x4x_put_target_port_of_nodes,
+ p3h2x4x_i3c_hub);
+ if (ret)
+ return ret;
+
+ ret = p3h2x4x_configure_hw(dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to configure the HUB\n");
+
+ /* Register virtual I3C master controllers for I3C target ports */
+ if (p3h2x4x->i3cdev) {
+ p3h2x4x_i3c_hub->i3cdev = p3h2x4x->i3cdev;
+ /*
+ * Publish the hub context in the MFD parent struct rather than
+ * via i3cdev_set_drvdata(), which would overwrite the parent's
+ * drvdata (struct p3h2x4x) that the IBI handler and other MFD
+ * callbacks rely on. Publish it before p3h2x4x_tp_i3c_algo()
+ * enables IBI, since the IBI handler dereferences it.
+ */
+ p3h2x4x->i3c_hub_priv = p3h2x4x_i3c_hub;
+
+ /*
+ * Register the clear action before enabling IBI so that, on the
+ * devm LIFO unwind (probe failure or removal), the pointer is
+ * cleared only after IBI has been disabled and freed.
+ */
+ ret = devm_add_action_or_reset(dev, p3h2x4x_clear_i3c_hub_priv,
+ p3h2x4x);
+ if (ret)
+ return ret;
+
+ ret = p3h2x4x_tp_i3c_algo(p3h2x4x_i3c_hub);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register i3c bus\n");
+ }
+
+ /* Register virtual I2C adapters for SMBus target ports */
+ ret = p3h2x4x_tp_smbus_algo(p3h2x4x_i3c_hub);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to add i2c adapter\n");
+
+ ret = devm_add_action_or_reset(dev,
+ p3h2x4x_unregister_smbus_adapters_action,
+ p3h2x4x_i3c_hub);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static const struct platform_device_id p3h2x4x_i3c_hub_id[] = {
+ { .name = "p3h2x4x-i3c-hub" },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, p3h2x4x_i3c_hub_id);
+
+static struct platform_driver p3h2x4x_i3c_hub_driver = {
+ .driver = {
+ .name = "p3h2x4x-i3c-hub",
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+ .probe = p3h2x4x_i3c_hub_probe,
+ .id_table = p3h2x4x_i3c_hub_id,
+};
+module_platform_driver(p3h2x4x_i3c_hub_driver);
+
+MODULE_AUTHOR("Aman Kumar Pandey <aman.kumarpandey@nxp.com>");
+MODULE_AUTHOR("Vikash Bansal <vikash.bansal@nxp.com>");
+MODULE_AUTHOR("Lakshay Piplani <lakshay.piplani@nxp.com>");
+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..a070be32ffed
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025-2026 NXP
+ * This P3H2X4X driver file contain functions for I3C virtual Bus creation, connect/disconnect
+ * hub network and read/write.
+ */
+#include <linux/i3c/hub.h>
+#include <linux/mfd/p3h2840.h>
+#include <linux/regmap.h>
+
+#include "p3h2840_i3c_hub.h"
+
+static inline struct tp_bus *
+p3h2x4x_bus_from_controller(struct i3c_master_controller *controller)
+{
+ struct i3c_hub_controller *hub_controller;
+
+ hub_controller = container_of(controller, struct i3c_hub_controller, controller);
+
+ return container_of(hub_controller, struct tp_bus, hub_controller);
+}
+
+/*
+ * I3C target ports share the parent controller's dynamic-address domain.
+ * Keep always_enable false while registering the virtual controllers so
+ * these callbacks continue to connect and disconnect their target ports.
+ *
+ * After all I3C virtual controllers are registered, program the combined
+ * route mask before setting always_enable for the corresponding ports.
+ * These ports then remain connected, avoiding per-operation route writes.
+ *
+ * The generic hub core calls these callbacks with hub->lock held. Hold
+ * the same mutex across the route write and flag updates so callbacks
+ * cannot observe always_enable before the route is successfully programmed.
+ */
+static void p3h2x4x_hub_enable_port(struct i3c_master_controller *controller)
+{
+ struct tp_bus *bus = p3h2x4x_bus_from_controller(controller);
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = bus->p3h2x4x_i3c_hub;
+
+ if (p3h2x4x_i3c_hub->hub_config.tp_config[bus->tp_port].always_enable)
+ return;
+
+ regmap_set_bits(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP_NET_CON_CONF, bus->tp_mask);
+}
+
+static void p3h2x4x_hub_disable_port(struct i3c_master_controller *controller)
+{
+ struct tp_bus *bus = p3h2x4x_bus_from_controller(controller);
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = bus->p3h2x4x_i3c_hub;
+
+ if (p3h2x4x_i3c_hub->hub_config.tp_config[bus->tp_port].always_enable)
+ return;
+
+ regmap_clear_bits(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP_NET_CON_CONF, bus->tp_mask);
+}
+
+static const struct i3c_hub_ops p3h2x4x_hub_ops = {
+ .enable_port = p3h2x4x_hub_enable_port,
+ .disable_port = p3h2x4x_hub_disable_port,
+};
+
+static void p3h2x4x_unregister_i3c_master(void *data)
+{
+ struct i3c_master_controller *controller = data;
+
+ i3c_master_unregister(controller);
+}
+
+/**
+ * p3h2x4x_tp_i3c_algo - Register I3C virtual masters for I3C target ports.
+ * @p3h2x4x_hub: p3h2x4x device structure.
+ * Return: 0 in case of success, negative error code on failure.
+ */
+int p3h2x4x_tp_i3c_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_hub)
+{
+ struct i3c_master_controller *parent = i3c_dev_get_master(p3h2x4x_hub->i3cdev->desc);
+ struct p3h2x4x *p3h2x4x = dev_get_drvdata(p3h2x4x_hub->dev->parent);
+ u8 tp, ntwk_mask = 0;
+ int ret;
+
+ p3h2x4x_hub->hub = devm_kzalloc(p3h2x4x_hub->dev,
+ sizeof(*p3h2x4x_hub->hub),
+ GFP_KERNEL);
+ if (!p3h2x4x_hub->hub)
+ return -ENOMEM;
+
+ i3c_hub_init(p3h2x4x_hub->hub,
+ &p3h2x4x_hub_ops,
+ p3h2x4x_hub->i3cdev);
+
+ /*
+ * Reserve every port's assigned addresses before any registration, as
+ * registration runs DAA on the shared parent and would otherwise let
+ * one port consume a later port's address.
+ */
+ for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
+ if (!p3h2x4x_hub->tp_bus[tp].of_node ||
+ p3h2x4x_hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C)
+ continue;
+
+ struct i3c_hub_controller *hub_controller =
+ &p3h2x4x_hub->tp_bus[tp].hub_controller;
+ struct i3c_master_controller *controller = &hub_controller->controller;
+
+ hub_controller->parent = parent;
+ hub_controller->hub = p3h2x4x_hub->hub;
+
+ dev_set_drvdata(&controller->dev, hub_controller);
+
+ ret = i3c_hub_reserve_parent_addrslots_from_dt(hub_controller,
+ p3h2x4x_hub->tp_bus[tp].of_node);
+ if (ret)
+ return ret;
+ }
+
+ for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
+ if (!p3h2x4x_hub->tp_bus[tp].of_node ||
+ p3h2x4x_hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C)
+ continue;
+
+ struct i3c_master_controller *controller =
+ &p3h2x4x_hub->tp_bus[tp].hub_controller.controller;
+
+ ret = i3c_master_register_fwnode(controller,
+ p3h2x4x_hub->dev,
+ of_fwnode_handle(p3h2x4x_hub->tp_bus[tp].of_node),
+ &i3c_hub_master_ops,
+ false);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(p3h2x4x_hub->dev,
+ p3h2x4x_unregister_i3c_master,
+ controller);
+ if (ret)
+ return ret;
+
+ ntwk_mask |= p3h2x4x_hub->tp_bus[tp].tp_mask;
+ p3h2x4x_hub->tp_bus[tp].is_registered = true;
+ }
+
+ /*
+ * Take the routing lock used by the hub callbacks so they cannot
+ * observe always_enable before the route is programmed.
+ */
+ mutex_lock(&p3h2x4x_hub->hub->lock);
+
+ ret = regmap_write(p3h2x4x_hub->regmap,
+ P3H2X4X_TP_NET_CON_CONF, ntwk_mask);
+ if (!ret) {
+ for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
+ if (!(ntwk_mask & p3h2x4x_hub->tp_bus[tp].tp_mask))
+ continue;
+
+ p3h2x4x_hub->hub_config.tp_config[tp].always_enable = true;
+ }
+ }
+ mutex_unlock(&p3h2x4x_hub->hub->lock);
+
+ return ret;
+}
diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
new file mode 100644
index 000000000000..247d0f4883b7
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
@@ -0,0 +1,352 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025-2026 NXP
+ * This P3H2X4X driver file contain functions for SMBus/I2C virtual Bus creation and read/write.
+ */
+#include <linux/mfd/p3h2840.h>
+#include <linux/regmap.h>
+
+#include "p3h2840_i3c_hub.h"
+
+enum p3h2x4x_smbus_desc_idx {
+ P3H2X4X_DESC_ADDR,
+ P3H2X4X_DESC_TYPE,
+ P3H2X4X_DESC_WRITE_LEN,
+ P3H2X4X_DESC_READ_LEN,
+};
+
+static int p3h2x4x_read_smbus_transaction_status(struct p3h2x4x_i3c_hub_dev *hub,
+ u8 target_port_status,
+ u8 data_length)
+{
+ unsigned int xfer_us, timeout_us, sleep_us;
+ u32 status_read;
+ u8 status;
+ int ret;
+
+ xfer_us = P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(data_length);
+ sleep_us = clamp(xfer_us / P3H2X4X_SMBUS_POLL_COUNT,
+ P3H2X4X_SMBUS_POLL_INTERVAL_MIN_US,
+ P3H2X4X_SMBUS_POLL_INTERVAL_MAX_US);
+
+ /*
+ * Floor the deadline at the SDA-stuck recovery window so an in-spec
+ * bus recovery (agent holding SCL low up to 35 ms) plus status
+ * posting and read-back completes before we return a timeout.
+ */
+ timeout_us = max(xfer_us, P3H2X4X_SMBUS_SCL_LOW_RECOVERY_US);
+
+ ret = regmap_read_poll_timeout(hub->regmap, target_port_status,
+ status_read,
+ status_read & P3H2X4X_SMBUS_TRANSACTION_FINISH_FLAG,
+ sleep_us,
+ timeout_us);
+ if (ret)
+ return ret;
+
+ status = (u8)status_read;
+
+ status = (status & P3H2X4X_TP_TRANSACTION_CODE_MASK)
+ >> P3H2X4X_SMBUS_CNTRL_STATUS_TXN_SHIFT;
+
+ switch (status) {
+ case P3H2X4X_SMBUS_CNTRL_STATUS_TXN_OK:
+ return 0;
+ case P3H2X4X_SMBUS_CNTRL_STATUS_TXN_ADDR_NAK:
+ return -ENXIO;
+ case P3H2X4X_SMBUS_CNTRL_STATUS_TXN_DATA_NAK:
+ return -EIO;
+ case P3H2X4X_SMBUS_CNTRL_STATUS_TXN_SCL_TO:
+ return -ETIMEDOUT;
+ case P3H2X4X_SMBUS_CNTRL_STATUS_TXN_ARB_LOSS:
+ return -EAGAIN;
+ default:
+ return -EIO;
+ }
+}
+
+/*
+ * p3h2x4x_tp_i2c_xfer_msg() - This starts a SMBus write transaction by writing a descriptor
+ * and a message to the p3h2x4x registers. Controller buffer page is determined by multiplying the
+ * target port index by four and adding the base page number to it.
+ */
+static int p3h2x4x_tp_i2c_xfer_msg(struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub,
+ struct i2c_msg *xfers,
+ u8 target_port,
+ int nxfers_i, u8 rw)
+{
+ u8 controller_buffer_page = P3H2X4X_CONTROLLER_BUFFER_PAGE + 4 * target_port;
+ u8 target_port_status = P3H2X4X_TP0_SMBUS_AGNT_STS + target_port;
+ u8 desc[P3H2X4X_SMBUS_DESCRIPTOR_SIZE] = { 0 };
+ u8 transaction_type = P3H2X4X_SMBUS_400kHz;
+ int write_length, read_length;
+ u8 addr = xfers[nxfers_i].addr;
+ u8 rw_address = 2 * addr;
+ int ret, ret2;
+
+ if (rw == 2) { /* write and read */
+ write_length = xfers[nxfers_i].len;
+ read_length = xfers[nxfers_i + 1].len;
+ } else if (rw == 1) {
+ rw_address |= P3H2X4X_SET_BIT(0);
+ write_length = 0;
+ read_length = xfers[nxfers_i].len;
+ } else {
+ write_length = xfers[nxfers_i].len;
+ read_length = 0;
+ }
+
+ desc[P3H2X4X_DESC_ADDR] = rw_address;
+ if (rw == 2)
+ desc[P3H2X4X_DESC_TYPE] = transaction_type | P3H2X4X_SET_BIT(0);
+ else
+ desc[P3H2X4X_DESC_TYPE] = transaction_type;
+ desc[P3H2X4X_DESC_WRITE_LEN] = write_length;
+ desc[P3H2X4X_DESC_READ_LEN] = read_length;
+
+ /*
+ * The status register is write-1-to-clear and shared with the target
+ * path. Clear only the transaction-finish flag, as the receive and
+ * overflow flags may report packets the target path has not read yet.
+ */
+ ret = regmap_write(p3h2x4x_i3c_hub->regmap, target_port_status,
+ P3H2X4X_TARGET_BUF_CA_TF);
+ if (ret)
+ goto out;
+
+ ret = regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_PAGE_PTR, controller_buffer_page);
+ if (ret)
+ goto out;
+
+ ret = regmap_bulk_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_CONTROLLER_AGENT_BUFF,
+ desc, P3H2X4X_SMBUS_DESCRIPTOR_SIZE);
+ if (ret)
+ goto out;
+
+ if (!(rw % 2) && xfers[nxfers_i].len) {
+ ret = regmap_bulk_write(p3h2x4x_i3c_hub->regmap,
+ P3H2X4X_CONTROLLER_AGENT_BUFF_DATA,
+ xfers[nxfers_i].buf, xfers[nxfers_i].len);
+ if (ret)
+ goto out;
+ }
+
+ ret = regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP_SMBUS_AGNT_TRANS_START,
+ p3h2x4x_i3c_hub->tp_bus[target_port].tp_mask);
+ if (ret)
+ goto out;
+
+ ret = p3h2x4x_read_smbus_transaction_status(p3h2x4x_i3c_hub,
+ target_port_status,
+ (write_length + read_length));
+ if (ret)
+ goto out;
+
+ if (rw) {
+ if (rw == 2)
+ nxfers_i += 1;
+
+ if (xfers[nxfers_i].len) {
+ u8 *dma_buf = i2c_get_dma_safe_msg_buf(&xfers[nxfers_i], 1);
+
+ if (!dma_buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ ret = regmap_bulk_read(p3h2x4x_i3c_hub->regmap,
+ P3H2X4X_CONTROLLER_AGENT_BUFF_DATA + write_length,
+ dma_buf, xfers[nxfers_i].len);
+ i2c_put_dma_safe_msg_buf(dma_buf, &xfers[nxfers_i], !ret);
+ if (ret)
+ goto out;
+ }
+ }
+out:
+ ret2 = regmap_write(p3h2x4x_i3c_hub->regmap,
+ P3H2X4X_PAGE_PTR, 0x00);
+ if (!ret && ret2)
+ ret = ret2;
+
+ return ret;
+}
+
+/*
+ * This function will be called whenever you call I2C read, write APIs like
+ * i2c_master_send(), i2c_master_recv() etc.
+ */
+static s32 p3h2x4x_tp_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+ int ret_sum = 0, ret, msg_count;
+ u8 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++) {
+ rw = (msgs[msg_count].flags & I2C_M_RD) ? 1 : 0;
+ if (!rw) {
+ /* If a write message is immediately followed by a read message to
+ * the same address, consider combining them into a single transaction.
+ */
+ if (msg_count + 1 < num &&
+ msgs[msg_count].addr == msgs[msg_count + 1].addr &&
+ (msgs[msg_count + 1].flags & I2C_M_RD)) {
+ if (msgs[msg_count].len + msgs[msg_count + 1].len >
+ P3H2X4X_SMBUS_PAYLOAD_SIZE)
+ return -EINVAL;
+
+ 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;
+}
+
+static u32 p3h2x4x_tp_smbus_funcs(struct i2c_adapter *adapter)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+static const struct i2c_adapter_quirks p3h2x4x_tp_i2c_quirks = {
+ .max_read_len = P3H2X4X_SMBUS_PAYLOAD_SIZE,
+ .max_write_len = P3H2X4X_SMBUS_PAYLOAD_SIZE,
+};
+
+/*
+ * I2C algorithm Structure
+ */
+static struct i2c_algorithm p3h2x4x_tp_i2c_algorithm = {
+ .master_xfer = p3h2x4x_tp_i2c_xfer,
+ .functionality = p3h2x4x_tp_smbus_funcs,
+};
+
+void p3h2x4x_unregister_smbus_adapters(struct p3h2x4x_i3c_hub_dev *hub)
+{
+ struct p3h2x4x *p3h2x4x = dev_get_drvdata(hub->dev->parent);
+ u8 tp;
+
+ for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
+ if (!hub->tp_bus[tp].tp_smbus_adapter)
+ continue;
+
+ i2c_del_adapter(hub->tp_bus[tp].tp_smbus_adapter);
+
+ guard(mutex)(&hub->etx_mutex);
+ hub->tp_bus[tp].tp_smbus_adapter = NULL;
+ hub->tp_bus[tp].is_registered = false;
+ }
+}
+
+/**
+ * p3h2x4x_tp_smbus_algo - Register I2C adapters for SMBus target ports.
+ * @hub: p3h2x4x device structure.
+ * Return: 0 in case of success, negative error code on failure.
+ */
+int p3h2x4x_tp_smbus_algo(struct p3h2x4x_i3c_hub_dev *hub)
+{
+ struct p3h2x4x *p3h2x4x = dev_get_drvdata(hub->dev->parent);
+ int ret, ret2;
+ u8 tp;
+
+ scoped_guard(mutex, &p3h2x4x->protected_reg_lock) {
+ ret = regmap_write(hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_UNLOCK_CODE);
+ if (ret)
+ break;
+
+ ret = regmap_write(hub->regmap, P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG,
+ P3H2X4X_IBI_DISABLED);
+
+ ret2 = regmap_write(hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_LOCK_CODE);
+ if (!ret && ret2)
+ ret = ret2;
+ }
+ if (ret)
+ return ret;
+
+ for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
+ if (!hub->tp_bus[tp].of_node ||
+ hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_SMBUS)
+ continue;
+
+ /* Allocate adapter */
+ struct i2c_adapter *smbus_adapter =
+ devm_kzalloc(hub->dev, sizeof(*smbus_adapter), GFP_KERNEL);
+ if (!smbus_adapter) {
+ p3h2x4x_unregister_smbus_adapters(hub);
+ return -ENOMEM;
+ }
+
+ /* Initialize adapter */
+ smbus_adapter->owner = THIS_MODULE;
+ smbus_adapter->class = I2C_CLASS_HWMON;
+ smbus_adapter->algo = &p3h2x4x_tp_i2c_algorithm;
+ smbus_adapter->quirks = &p3h2x4x_tp_i2c_quirks;
+ smbus_adapter->dev.parent = hub->dev;
+ smbus_adapter->dev.of_node = hub->tp_bus[tp].of_node;
+ snprintf(smbus_adapter->name, sizeof(smbus_adapter->name),
+ "p3h2x4x-i3c-hub.tp-port-%d", tp);
+
+ i2c_set_adapdata(smbus_adapter, &hub->tp_bus[tp]);
+
+ /*
+ * Publish the callback-visible state before i2c_add_adapter(),
+ * which can synchronously probe a DT slave and invoke
+ * reg_slave() that inspects is_registered/tp_smbus_client and
+ * sets ibi_en. Seeding defaults here keeps reg_slave()'s view
+ * consistent and avoids clobbering its ibi_en update. Do not
+ * hold etx_mutex across the call, since reg_slave() also takes it.
+ */
+ scoped_guard(mutex, &hub->etx_mutex) {
+ hub->tp_bus[tp].tp_smbus_adapter = smbus_adapter;
+ hub->tp_bus[tp].tp_smbus_client = NULL;
+ hub->tp_bus[tp].is_registered = true;
+ hub->hub_config.tp_config[tp].ibi_en = false;
+ }
+
+ /* Register adapter */
+ ret = i2c_add_adapter(smbus_adapter);
+ if (ret) {
+ scoped_guard(mutex, &hub->etx_mutex) {
+ hub->tp_bus[tp].is_registered = false;
+ hub->tp_bus[tp].tp_smbus_adapter = NULL;
+ }
+ p3h2x4x_unregister_smbus_adapters(hub);
+ return ret;
+ }
+ }
+
+ /*
+ * Configure the SMBus Target Agents to hold SDA low when both of a
+ * port's received-data buffers are full. This provides flow control
+ * for MCTP: it prevents MCTP transmitters on the target ports from
+ * timing out when the upstream controller does not service the agent
+ * buffers in time and the port only receives write messages.
+ */
+ ret = regmap_update_bits(hub->regmap, P3H2X4X_ONCHIP_TD_AND_SMBUS_AGNT_CONF,
+ P3H2X4X_TARGET_AGENT_DFT_IBI_CONF_MASK,
+ P3H2X4X_TARGET_AGENT_DFT_IBI_CONF);
+ if (ret) {
+ p3h2x4x_unregister_smbus_adapters(hub);
+ return ret;
+ }
+
+ return 0;
+}
--
2.25.1
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v21 3/4] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
2026-11-12 12:44 ` [PATCH v21 3/4] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
@ 2026-11-12 13:15 ` sashiko-bot
2026-11-12 17:14 ` Frank Li
0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-11-12 13:15 UTC (permalink / raw)
To: Lakshay Piplani, linux-i3c, mfd; +Cc: Frank.Li, Alexandre Belloni, lee
Thank you for your contribution! Sashiko AI review found 5 potential
issues to consider:
- [High] i3c: hub: p3h2x4x: uninitialized P3H2X4X_TP_NET_CON_CONF breaks
port isolation
- [High] i3c: hub: p3h2x4x: parent DAA reconciler clobbers DT address
reservations
- [High] i3c: hub: p3h2x4x: broken SMBus functionality in
p3h2x4x_tp_smbus_funcs()
- [High] i3c: hub: p3h2x4x: ports disconnected during probe in
p3h2x4x_tp_i3c_algo()
- [Medium] i3c: hub: p3h2x4x: combined transfer check in
p3h2x4x_tp_i2c_xfer()
--
commit 9562a1f0e3515b7b354e8a193339c5c2e1630c85
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
This commit adds the I3C hub driver for the NXP P3H2x4x family, supporting
downstream target ports configured as virtual I3C buses or SMBus adapters.
> 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 0000000000000..a070be32ffed3
> --- /dev/null
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> @@ -0,0 +1,162 @@
[ ... ]
> +static void p3h2x4x_hub_enable_port(struct i3c_master_controller *controller)
> +{
> + struct tp_bus *bus = p3h2x4x_bus_from_controller(controller);
> + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = bus->p3h2x4x_i3c_hub;
> +
> + if (p3h2x4x_i3c_hub->hub_config.tp_config[bus->tp_port].always_enable)
> + return;
> +
> + regmap_set_bits(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP_NET_CON_CONF, bus->tp_mask);
> +}
> +
> +static void p3h2x4x_hub_disable_port(struct i3c_master_controller *controller)
> +{
> + struct tp_bus *bus = p3h2x4x_bus_from_controller(controller);
> + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = bus->p3h2x4x_i3c_hub;
> +
> + if (p3h2x4x_i3c_hub->hub_config.tp_config[bus->tp_port].always_enable)
> + return;
> +
> + regmap_clear_bits(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP_NET_CON_CONF, bus->tp_mask);
> +}
[Severity: High]
Does leaving P3H2X4X_TP_NET_CON_CONF uninitialized before the registration
loop break per-port isolation?
p3h2x4x_hub_enable_port() and p3h2x4x_hub_disable_port() only set and clear
bus->tp_mask, and neither p3h2x4x_configure_tp() nor p3h2x4x_tp_i3c_algo()
clears P3H2X4X_TP_NET_CON_CONF beforehand or on teardown. If any bits are
already set in P3H2X4X_TP_NET_CON_CONF (for example on driver rebind, or on
P3H2441/P3H2841 where target ports default to connected at power-on),
multiple ports stay bridged onto the internal network during
i3c_master_register_fwnode(), exposing other ports' devices to SETDASA and
ENTDAA meant for a single port.
[ ... ]
> + for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
> + if (!p3h2x4x_hub->tp_bus[tp].of_node ||
> + p3h2x4x_hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C)
> + continue;
> +
> + struct i3c_hub_controller *hub_controller =
> + &p3h2x4x_hub->tp_bus[tp].hub_controller;
> + struct i3c_master_controller *controller = &hub_controller->controller;
> +
> + hub_controller->parent = parent;
> + hub_controller->hub = p3h2x4x_hub->hub;
> +
> + dev_set_drvdata(&controller->dev, hub_controller);
> +
> + ret = i3c_hub_reserve_parent_addrslots_from_dt(hub_controller,
> + p3h2x4x_hub->tp_bus[tp].of_node);
> + if (ret)
> + return ret;
> + }
> +
> + for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
> + if (!p3h2x4x_hub->tp_bus[tp].of_node ||
> + p3h2x4x_hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C)
> + continue;
> +
> + struct i3c_master_controller *controller =
> + &p3h2x4x_hub->tp_bus[tp].hub_controller.controller;
> +
> + ret = i3c_master_register_fwnode(controller,
> + p3h2x4x_hub->dev,
> + of_fwnode_handle(p3h2x4x_hub->tp_bus[tp].of_node),
> + &i3c_hub_master_ops,
> + false);
[Severity: High]
Won't i3c_master_reconcile_dyn_addrs() on the parent bus clobber the
reservations made by i3c_hub_reserve_parent_addrslots_from_dt()?
i3c_hub_reserve_parent_addrslots_from_dt() marks each child's
assigned-address on parent->bus as I3C_ADDR_SLOT_I3C_DEV without a matching
descriptor in parent->bus.devs.i3c. When the first port runs
i3c_master_register_fwnode() -> ... -> i3c_hub_do_daa() ->
i3c_master_do_daa(parent), i3c_master_reconcile_dyn_addrs(parent) resets
every I3C_ADDR_SLOT_I3C_DEV slot not in parent->bus.devs.i3c to
I3C_ADDR_SLOT_FREE and probes it:
drivers/i3c/master.c:i3c_master_reconcile_dyn_addrs() {
...
if (status != I3C_ADDR_SLOT_I3C_DEV || test_bit(addr, dev_dyn_addrs))
continue;
i3c_bus_set_addr_slot_status(&master->bus, addr, I3C_ADDR_SLOT_FREE);
/* Try to add the device, but probe to see if it is really present */
__i3c_master_add_i3c_dev_locked(master, addr, true);
...
}
For later ports that are still disconnected, the probe sees no device and
leaves the slot I3C_ADDR_SLOT_FREE, wiping out their reservations. For
devices on the connected port that already got their address via SETDASA on
controller->bus, the probe succeeds and registers a duplicate i3c_device on
parent->bus.
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(p3h2x4x_hub->dev,
> + p3h2x4x_unregister_i3c_master,
> + controller);
> + if (ret)
> + return ret;
> +
> + ntwk_mask |= p3h2x4x_hub->tp_bus[tp].tp_mask;
> + p3h2x4x_hub->tp_bus[tp].is_registered = true;
> + }
> +
> + /*
> + * Take the routing lock used by the hub callbacks so they cannot
> + * observe always_enable before the route is programmed.
> + */
> + mutex_lock(&p3h2x4x_hub->hub->lock);
> +
> + ret = regmap_write(p3h2x4x_hub->regmap,
> + P3H2X4X_TP_NET_CON_CONF, ntwk_mask);
> + if (!ret) {
> + for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
> + if (!(ntwk_mask & p3h2x4x_hub->tp_bus[tp].tp_mask))
> + continue;
> +
> + p3h2x4x_hub->hub_config.tp_config[tp].always_enable = true;
> + }
> + }
> + mutex_unlock(&p3h2x4x_hub->hub->lock);
[Severity: High]
Does keeping always_enable false until all ports are registered leave target
ports disconnected while downstream device drivers probe?
CPU0 kworker (parent->wq)
p3h2x4x_tp_i3c_algo()
i3c_master_register_fwnode()
...
i3c_hub_do_daa()
i3c_master_do_daa(parent)
queue_work(..., &parent->reg_work)
i3c_hub_disable_port(controller)
regmap_clear_bits(..., tp_mask)
i3c_master_reg_work_fn()
device_add(&i3cdev->dev)
<target driver probe>
i3c_device_do_priv_xfers()
<-- port is disconnected
/* next port's i3c_master_register_fwnode() */
regmap_write(..., P3H2X4X_TP_NET_CON_CONF, ntwk_mask)
Devices discovered via ENTDAA are queued on parent->reg_work and use
parent->ops directly without calling p3h2x4x_hub_enable_port(), so their
probes fail if they run before P3H2X4X_TP_NET_CON_CONF is written at the
end of p3h2x4x_tp_i3c_algo(). Likewise, for statically described devices on
controller->bus that call i3c_device_enable_ibi() during
i3c_master_register_fwnode(), i3c_hub_enable_ibi() disconnects the port on
exit, dropping IBIs until all remaining ports finish registering.
> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> new file mode 100644
> index 0000000000000..247d0f4883b7c
> --- /dev/null
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> @@ -0,0 +1,352 @@
[ ... ]
> +static s32 p3h2x4x_tp_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{
> + int ret_sum = 0, ret, msg_count;
> + u8 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++) {
> + rw = (msgs[msg_count].flags & I2C_M_RD) ? 1 : 0;
> + if (!rw) {
> + /* If a write message is immediately followed by a read message to
> + * the same address, consider combining them into a single transaction.
> + */
> + if (msg_count + 1 < num &&
> + msgs[msg_count].addr == msgs[msg_count + 1].addr &&
> + (msgs[msg_count + 1].flags & I2C_M_RD)) {
> + if (msgs[msg_count].len + msgs[msg_count + 1].len >
> + P3H2X4X_SMBUS_PAYLOAD_SIZE)
> + return -EINVAL;
[Severity: Medium]
Should this fall back to sending the write and read as separate transactions
(or set max_comb_1st_msg_len and max_comb_2nd_msg_len in
p3h2x4x_tp_i2c_quirks) instead of returning -EINVAL?
Because p3h2x4x_tp_i2c_quirks advertises max_read_len = 84 and
max_write_len = 84 with no combined limit, callers such as regmap-i2c issue
a 1- or 2-byte register write followed by an 84-byte read; both messages
pass i2c_check_for_quirks(), but their sum exceeds
P3H2X4X_SMBUS_PAYLOAD_SIZE (84) and fails here with -EINVAL.
> +
> + 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;
> +}
> +
> +static u32 p3h2x4x_tp_smbus_funcs(struct i2c_adapter *adapter)
> +{
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BLOCK_DATA;
> +}
[Severity: High]
Does advertising I2C_FUNC_SMBUS_BLOCK_DATA without handling I2C_M_RECV_LEN
in p3h2x4x_tp_i2c_xfer_msg() corrupt SMBus block reads and leak stack memory?
Because p3h2x4x_tp_i2c_algorithm has no .smbus_xfer callback,
i2c_smbus_read_block_data() and I2C_SMBUS_BLOCK_DATA read ioctls go through
i2c_smbus_xfer_emulated(), which sets msg[1].flags |= I2C_M_RECV_LEN and
msg[1].len = 1. p3h2x4x_tp_i2c_xfer_msg() ignores I2C_M_RECV_LEN and reads
only 1 byte (the block length N) into msg[1].buf[0], after which
i2c_smbus_xfer_emulated() copies N unwritten bytes from msg[1].buf[1..N] to
the caller (zeroes from the bounce buffer, or uninitialized stack bytes from
msgbuf1 if kzalloc() in i2c_smbus_try_get_dmabuf() fails).
Also, omitting I2C_FUNC_SMBUS_EMUL leaves standard byte, word, and I2C-block
SMBus operations unadvertised, preventing SMBus and hwmon drivers from using
the adapter.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112124437.3367252-1-lakshay.piplani@nxp.com?part=3
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v21 3/4] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
2026-11-12 13:15 ` sashiko-bot
@ 2026-11-12 17:14 ` Frank Li
0 siblings, 0 replies; 10+ messages in thread
From: Frank Li @ 2026-11-12 17:14 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, linux-i3c, mfd, Frank.Li, Alexandre Belloni, lee
On Thu, Nov 12, 2026 at 01:15:21PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential
> issues to consider:
> - [High] i3c: hub: p3h2x4x: uninitialized P3H2X4X_TP_NET_CON_CONF breaks
> port isolation
> - [High] i3c: hub: p3h2x4x: parent DAA reconciler clobbers DT address
> reservations
> - [High] i3c: hub: p3h2x4x: broken SMBus functionality in
> p3h2x4x_tp_smbus_funcs()
> - [High] i3c: hub: p3h2x4x: ports disconnected during probe in
> p3h2x4x_tp_i3c_algo()
> - [Medium] i3c: hub: p3h2x4x: combined transfer check in
> p3h2x4x_tp_i2c_xfer()
> --
>
> commit 9562a1f0e3515b7b354e8a193339c5c2e1630c85
> Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
>
> i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
>
> This commit adds the I3C hub driver for the NXP P3H2x4x family, supporting
> downstream target ports configured as virtual I3C buses or SMBus adapters.
>
> > 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 0000000000000..a070be32ffed3
> > --- /dev/null
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> > @@ -0,0 +1,162 @@
>
> [ ... ]
>
> > +static void p3h2x4x_hub_enable_port(struct i3c_master_controller *controller)
> > +{
> > + struct tp_bus *bus = p3h2x4x_bus_from_controller(controller);
> > + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = bus->p3h2x4x_i3c_hub;
> > +
> > + if (p3h2x4x_i3c_hub->hub_config.tp_config[bus->tp_port].always_enable)
> > + return;
> > +
> > + regmap_set_bits(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP_NET_CON_CONF, bus->tp_mask);
> > +}
> > +
> > +static void p3h2x4x_hub_disable_port(struct i3c_master_controller *controller)
> > +{
> > + struct tp_bus *bus = p3h2x4x_bus_from_controller(controller);
> > + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = bus->p3h2x4x_i3c_hub;
> > +
> > + if (p3h2x4x_i3c_hub->hub_config.tp_config[bus->tp_port].always_enable)
> > + return;
> > +
> > + regmap_clear_bits(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP_NET_CON_CONF, bus->tp_mask);
> > +}
>
> [Severity: High]
> Does leaving P3H2X4X_TP_NET_CON_CONF uninitialized before the registration
> loop break per-port isolation?
>
> p3h2x4x_hub_enable_port() and p3h2x4x_hub_disable_port() only set and clear
> bus->tp_mask, and neither p3h2x4x_configure_tp() nor p3h2x4x_tp_i3c_algo()
> clears P3H2X4X_TP_NET_CON_CONF beforehand or on teardown. If any bits are
> already set in P3H2X4X_TP_NET_CON_CONF (for example on driver rebind, or on
> P3H2441/P3H2841 where target ports default to connected at power-on),
> multiple ports stay bridged onto the internal network during
> i3c_master_register_fwnode(), exposing other ports' devices to SETDASA and
> ENTDAA meant for a single port.
Suggest disable all port at probe functions.
>
> [ ... ]
>
> > + for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
> > + if (!p3h2x4x_hub->tp_bus[tp].of_node ||
> > + p3h2x4x_hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C)
> > + continue;
> > +
> > + struct i3c_hub_controller *hub_controller =
> > + &p3h2x4x_hub->tp_bus[tp].hub_controller;
> > + struct i3c_master_controller *controller = &hub_controller->controller;
> > +
> > + hub_controller->parent = parent;
> > + hub_controller->hub = p3h2x4x_hub->hub;
> > +
> > + dev_set_drvdata(&controller->dev, hub_controller);
> > +
> > + ret = i3c_hub_reserve_parent_addrslots_from_dt(hub_controller,
> > + p3h2x4x_hub->tp_bus[tp].of_node);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
> > + if (!p3h2x4x_hub->tp_bus[tp].of_node ||
> > + p3h2x4x_hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C)
> > + continue;
> > +
> > + struct i3c_master_controller *controller =
> > + &p3h2x4x_hub->tp_bus[tp].hub_controller.controller;
> > +
> > + ret = i3c_master_register_fwnode(controller,
> > + p3h2x4x_hub->dev,
> > + of_fwnode_handle(p3h2x4x_hub->tp_bus[tp].of_node),
> > + &i3c_hub_master_ops,
> > + false);
>
> [Severity: High]
> Won't i3c_master_reconcile_dyn_addrs() on the parent bus clobber the
> reservations made by i3c_hub_reserve_parent_addrslots_from_dt()?
You should set I3C_ADDR_SLOT_EXT_DESIRED status in
i3c_hub_reserve_parent_addrslots_from_dt(), assigned-address should mark
as I3C_ADDR_SLOT_EXT_DESIRED to avoid other i3c device use it.
>
> i3c_hub_reserve_parent_addrslots_from_dt() marks each child's
> assigned-address on parent->bus as I3C_ADDR_SLOT_I3C_DEV without a matching
> descriptor in parent->bus.devs.i3c. When the first port runs
> i3c_master_register_fwnode() -> ... -> i3c_hub_do_daa() ->
> i3c_master_do_daa(parent), i3c_master_reconcile_dyn_addrs(parent) resets
> every I3C_ADDR_SLOT_I3C_DEV slot not in parent->bus.devs.i3c to
> I3C_ADDR_SLOT_FREE and probes it:
>
> drivers/i3c/master.c:i3c_master_reconcile_dyn_addrs() {
> ...
> if (status != I3C_ADDR_SLOT_I3C_DEV || test_bit(addr, dev_dyn_addrs))
> continue;
> i3c_bus_set_addr_slot_status(&master->bus, addr, I3C_ADDR_SLOT_FREE);
> /* Try to add the device, but probe to see if it is really present */
> __i3c_master_add_i3c_dev_locked(master, addr, true);
> ...
> }
>
> For later ports that are still disconnected, the probe sees no device and
> leaves the slot I3C_ADDR_SLOT_FREE, wiping out their reservations. For
> devices on the connected port that already got their address via SETDASA on
> controller->bus, the probe succeeds and registers a duplicate i3c_device on
> parent->bus.
>
> > + if (ret)
> > + return ret;
> > +
> > + ret = devm_add_action_or_reset(p3h2x4x_hub->dev,
> > + p3h2x4x_unregister_i3c_master,
> > + controller);
> > + if (ret)
> > + return ret;
> > +
> > + ntwk_mask |= p3h2x4x_hub->tp_bus[tp].tp_mask;
> > + p3h2x4x_hub->tp_bus[tp].is_registered = true;
> > + }
> > +
> > + /*
> > + * Take the routing lock used by the hub callbacks so they cannot
> > + * observe always_enable before the route is programmed.
> > + */
> > + mutex_lock(&p3h2x4x_hub->hub->lock);
> > +
> > + ret = regmap_write(p3h2x4x_hub->regmap,
> > + P3H2X4X_TP_NET_CON_CONF, ntwk_mask);
> > + if (!ret) {
> > + for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
> > + if (!(ntwk_mask & p3h2x4x_hub->tp_bus[tp].tp_mask))
> > + continue;
> > +
> > + p3h2x4x_hub->hub_config.tp_config[tp].always_enable = true;
> > + }
> > + }
> > + mutex_unlock(&p3h2x4x_hub->hub->lock);
>
> [Severity: High]
> Does keeping always_enable false until all ports are registered leave target
> ports disconnected while downstream device drivers probe?
yes,
>
> CPU0 kworker (parent->wq)
> p3h2x4x_tp_i3c_algo()
> i3c_master_register_fwnode()
> ...
> i3c_hub_do_daa()
> i3c_master_do_daa(parent)
> queue_work(..., &parent->reg_work)
> i3c_hub_disable_port(controller)
> regmap_clear_bits(..., tp_mask)
> i3c_master_reg_work_fn()
> device_add(&i3cdev->dev)
> <target driver probe>
> i3c_device_do_priv_xfers()
> <-- port is disconnected
> /* next port's i3c_master_register_fwnode() */
> regmap_write(..., P3H2X4X_TP_NET_CON_CONF, ntwk_mask)
>
> Devices discovered via ENTDAA are queued on parent->reg_work and use
> parent->ops directly without calling p3h2x4x_hub_enable_port(), so their
> probes fail if they run before P3H2X4X_TP_NET_CON_CONF is written at the
> end of p3h2x4x_tp_i3c_algo(). Likewise, for statically described devices on
> controller->bus that call i3c_device_enable_ibi() during
> i3c_master_register_fwnode(), i3c_hub_enable_ibi() disconnects the port on
> exit, dropping IBIs until all remaining ports finish registering.
>
> > diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> > new file mode 100644
> > index 0000000000000..247d0f4883b7c
> > --- /dev/null
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> > @@ -0,0 +1,352 @@
>
> [ ... ]
>
> > +static s32 p3h2x4x_tp_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> > +{
> > + int ret_sum = 0, ret, msg_count;
> > + u8 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++) {
> > + rw = (msgs[msg_count].flags & I2C_M_RD) ? 1 : 0;
> > + if (!rw) {
> > + /* If a write message is immediately followed by a read message to
> > + * the same address, consider combining them into a single transaction.
> > + */
> > + if (msg_count + 1 < num &&
> > + msgs[msg_count].addr == msgs[msg_count + 1].addr &&
> > + (msgs[msg_count + 1].flags & I2C_M_RD)) {
> > + if (msgs[msg_count].len + msgs[msg_count + 1].len >
> > + P3H2X4X_SMBUS_PAYLOAD_SIZE)
> > + return -EINVAL;
>
> [Severity: Medium]
> Should this fall back to sending the write and read as separate transactions
> (or set max_comb_1st_msg_len and max_comb_2nd_msg_len in
> p3h2x4x_tp_i2c_quirks) instead of returning -EINVAL?
>
> Because p3h2x4x_tp_i2c_quirks advertises max_read_len = 84 and
> max_write_len = 84 with no combined limit, callers such as regmap-i2c issue
can set such limit to simplify the case?
> a 1- or 2-byte register write followed by an 84-byte read; both messages
> pass i2c_check_for_quirks(), but their sum exceeds
> P3H2X4X_SMBUS_PAYLOAD_SIZE (84) and fails here with -EINVAL.
>
> > +
> > + 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;
> > +}
> > +
> > +static u32 p3h2x4x_tp_smbus_funcs(struct i2c_adapter *adapter)
> > +{
> > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BLOCK_DATA;
> > +}
>
> [Severity: High]
> Does advertising I2C_FUNC_SMBUS_BLOCK_DATA without handling I2C_M_RECV_LEN
> in p3h2x4x_tp_i2c_xfer_msg() corrupt SMBus block reads and leak stack memory?
>
Please check this one.
Frank
> Because p3h2x4x_tp_i2c_algorithm has no .smbus_xfer callback,
> i2c_smbus_read_block_data() and I2C_SMBUS_BLOCK_DATA read ioctls go through
> i2c_smbus_xfer_emulated(), which sets msg[1].flags |= I2C_M_RECV_LEN and
> msg[1].len = 1. p3h2x4x_tp_i2c_xfer_msg() ignores I2C_M_RECV_LEN and reads
> only 1 byte (the block length N) into msg[1].buf[0], after which
> i2c_smbus_xfer_emulated() copies N unwritten bytes from msg[1].buf[1..N] to
> the caller (zeroes from the bounce buffer, or uninitialized stack bytes from
> msgbuf1 if kzalloc() in i2c_smbus_try_get_dmabuf() fails).
>
> Also, omitting I2C_FUNC_SMBUS_EMUL leaves standard byte, word, and I2C-block
> SMBus operations unadvertised, preventing SMBus and hwmon drivers from using
> the adapter.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261112124437.3367252-1-lakshay.piplani@nxp.com?part=3
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v21 4/4] i3c: hub: p3h2x4x: Add SMBus slave mode support
2026-11-12 12:44 [PATCH v21 0/4] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
` (2 preceding siblings ...)
2026-11-12 12:44 ` [PATCH v21 3/4] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
@ 2026-11-12 12:44 ` Lakshay Piplani
2026-11-12 13:07 ` sashiko-bot
3 siblings, 1 reply; 10+ messages in thread
From: Lakshay Piplani @ 2026-11-12 12:44 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, broonie, mfd, lee,
Frank.Li, lgirdwood
Cc: vikash.bansal, priyanka.jain, aman.kumarpandey, Lakshay Piplani
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. Add CONFIG_I2C_SLAVE
to support the receive path and forward the received payloads to the
registered I2C slave client through i2c_slave_event().
Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
Signed-off-by: Vikash Bansal <vikash.bansal@nxp.com>
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v21:
- Handle both receive-buffer flags independently, including on overflow
- Acknowledge only the overflow flag to preserve buffer-refill events and
report the affected port
- Initialize optional IBI support before acquiring the routing mutex
Changes in v20:
- No change
Changes in v19:
- No change
Changes in v18:
- Rebased onto v7.3-rc1; no intended driver behaviour changes
Changes in v17:
- No change
Changes in v16:
- Always clear the software slave state on unregister: even if the hardware
IBI-disable write fails, log the error but still set bus->tp_smbus_client = NULL
and return 0, so a later IBI cannot deref a dangling client and call a freed slave_cb
- Use a heap DMA-safe buffer for the SMBus-agent RX path
instead of a stack buffer passed to regmap_bulk_read() over I3C
- Advertise I2C_FUNC_SLAVE when CONFIG_I2C_SLAVE is enabled, and reject slave
registration with -EOPNOTSUPP unless an I3C upstream device and working IBI
path are available (hub->i3cdev and hub->ibi_ready)
- Validate the IBI payload length and bound the target-port loop by
num_target_ports
Changes in v15:
- Retrieve the hub context through the shared MFD data instead of replacing
the I3C device's parent driver data
- Rework SMBus slave registration and unregistration to use the shared
protected-register lock
- Report protected-register relock failures separately from the original
enable or disable operation
Changes in v14:
- Clear receive-buffer flags even on SMBus receive error paths to avoid
repeated IBI storms
- Decode receive-buffer status using FIELD_GET()
- Fix overflow status value and explicitly clear overflow after reading both
target buffers
Changes in v13:
- Make IBI setup optional and robust: avoid probe failure when IBI is unsupported and add proper
cleanup using devm actions
- Fix SMBus slave receive path: avoid over-clearing buffer status, handle unregistered ports,
and ensure correct event delivery
- Improve safety by adding proper locking around shared state
Changes in v12:
- Add devm cleanup for IBI request/enable path
- Fix NULL pointer dereference before tp_smbus_client check
- Clear tp_smbus_client before disabling SMBus-agent IBI in unreg_slave()
Changes in v11:
- Improve SMBus slave mode payload validation and parsing
Changes in v10:
- Split SMBus slave mode support into a separate patch
---
---
drivers/i3c/hub/p3h2840_i3c_hub.h | 14 ++
drivers/i3c/hub/p3h2840_i3c_hub_i3c.c | 51 ++++
drivers/i3c/hub/p3h2840_i3c_hub_smbus.c | 298 +++++++++++++++++++++++-
3 files changed, 362 insertions(+), 1 deletion(-)
diff --git a/drivers/i3c/hub/p3h2840_i3c_hub.h b/drivers/i3c/hub/p3h2840_i3c_hub.h
index 5eecc972514b..d477b0f8ec27 100644
--- a/drivers/i3c/hub/p3h2840_i3c_hub.h
+++ b/drivers/i3c/hub/p3h2840_i3c_hub.h
@@ -314,6 +314,10 @@ struct p3h2x4x_i3c_hub_dev {
struct i2c_client *i2c_client;
struct hub_configuration hub_config;
struct tp_bus tp_bus[P3H2X4X_TP_MAX_COUNT];
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ bool ibi_ready;
+ u8 *slave_rx_buffer;
+#endif
struct i3c_hub *hub;
};
@@ -339,4 +343,14 @@ int p3h2x4x_tp_smbus_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub);
*/
int p3h2x4x_tp_i3c_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub);
+/**
+ * p3h2x4x_ibi_handler - IBI handler.
+ * @i3cdev: i3c device.
+ * @payload: two byte IBI payload data.
+ */
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+void p3h2x4x_ibi_handler(struct i3c_device *i3cdev,
+ const struct i3c_ibi_payload *payload);
+#endif
+
#endif /* P3H2840_I3C_HUB_H */
diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
index a070be32ffed..31d088a49b9e 100644
--- a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
@@ -10,6 +10,14 @@
#include "p3h2840_i3c_hub.h"
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static const struct i3c_ibi_setup p3h2x4x_ibireq = {
+ .handler = p3h2x4x_ibi_handler,
+ .max_payload_len = P3H2X4X_MAX_PAYLOAD_LEN,
+ .num_slots = P3H2X4X_NUM_SLOTS,
+};
+#endif
+
static inline struct tp_bus *
p3h2x4x_bus_from_controller(struct i3c_master_controller *controller)
{
@@ -67,6 +75,16 @@ static void p3h2x4x_unregister_i3c_master(void *data)
i3c_master_unregister(controller);
}
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static void p3h2x4x_free_ibi(void *data)
+{
+ struct i3c_device *i3cdev = data;
+
+ i3c_device_disable_ibi(i3cdev);
+ i3c_device_free_ibi(i3cdev);
+}
+#endif
+
/**
* p3h2x4x_tp_i3c_algo - Register I3C virtual masters for I3C target ports.
* @p3h2x4x_hub: p3h2x4x device structure.
@@ -140,6 +158,39 @@ int p3h2x4x_tp_i3c_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_hub)
p3h2x4x_hub->tp_bus[tp].is_registered = true;
}
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ p3h2x4x_hub->slave_rx_buffer = devm_kzalloc(p3h2x4x_hub->dev,
+ P3H2X4X_SMBUS_TARGET_PAYLOAD_SIZE,
+ GFP_KERNEL);
+ if (!p3h2x4x_hub->slave_rx_buffer)
+ return -ENOMEM;
+
+ ret = i3c_device_request_ibi(p3h2x4x_hub->i3cdev, &p3h2x4x_ibireq);
+ if (ret) {
+ dev_warn(p3h2x4x_hub->dev,
+ "IBI not available, SMBus slave mode disabled\n");
+ p3h2x4x_hub->ibi_ready = false;
+ } else {
+ ret = i3c_device_enable_ibi(p3h2x4x_hub->i3cdev);
+ if (ret) {
+ i3c_device_free_ibi(p3h2x4x_hub->i3cdev);
+ dev_warn(p3h2x4x_hub->dev,
+ "Failed to enable IBI, SMBus slave mode disabled\n");
+ p3h2x4x_hub->ibi_ready = false;
+ } else {
+ p3h2x4x_hub->ibi_ready = true;
+
+ ret = devm_add_action_or_reset(p3h2x4x_hub->dev,
+ p3h2x4x_free_ibi,
+ p3h2x4x_hub->i3cdev);
+ if (ret) {
+ p3h2x4x_hub->ibi_ready = false;
+ return ret;
+ }
+ }
+ }
+#endif
+
/*
* Take the routing lock used by the hub callbacks so they cannot
* observe always_enable before the route is programmed.
diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
index 247d0f4883b7..02b50d1eb938 100644
--- a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
@@ -3,6 +3,7 @@
* Copyright 2025-2026 NXP
* This P3H2X4X driver file contain functions for SMBus/I2C virtual Bus creation and read/write.
*/
+#include <linux/i3c/device.h>
#include <linux/mfd/p3h2840.h>
#include <linux/regmap.h>
@@ -15,6 +16,171 @@ enum p3h2x4x_smbus_desc_idx {
P3H2X4X_DESC_READ_LEN,
};
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static void p3h2x4x_read_smbus_agent_rx_buf(struct i3c_device *i3cdev, enum p3h2x4x_rcv_buf rfbuf,
+ enum p3h2x4x_tp tp)
+{
+ struct p3h2x4x *p3h2x4x = i3cdev_get_drvdata(i3cdev);
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub;
+ u8 target_buffer_page, flag_clear, temp = 0, i, addr;
+ u32 packet_len, slave_address;
+ struct i2c_client *client;
+ u8 *slave_rx_buffer;
+ int ret;
+
+ if (!p3h2x4x || !p3h2x4x->i3c_hub_priv)
+ return;
+
+ p3h2x4x_i3c_hub = p3h2x4x->i3c_hub_priv;
+ slave_rx_buffer = p3h2x4x_i3c_hub->slave_rx_buffer;
+
+ switch (rfbuf) {
+ case RCV_BUF_0:
+ target_buffer_page = P3H2X4X_TARGET_BUFF_0_PAGE;
+ flag_clear = P3H2X4X_TARGET_BUF_0_RECEIVE;
+ break;
+ case RCV_BUF_1:
+ target_buffer_page = P3H2X4X_TARGET_BUFF_1_PAGE;
+ flag_clear = P3H2X4X_TARGET_BUF_1_RECEIVE;
+ break;
+ default:
+ return;
+ }
+
+ target_buffer_page += P3H2X4X_NO_PAGE_PER_TP * tp;
+
+ ret = regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_PAGE_PTR, target_buffer_page);
+ if (ret)
+ goto ibi_err;
+
+ /* read buffer length */
+ ret = regmap_read(p3h2x4x_i3c_hub->regmap, P3H2X4X_TARGET_BUFF_LENGTH, &packet_len);
+ if (ret)
+ goto ibi_err;
+
+ if (packet_len)
+ packet_len = packet_len - 1;
+
+ if (packet_len > P3H2X4X_SMBUS_TARGET_PAYLOAD_SIZE) {
+ dev_err(&i3cdev->dev, "Received message too big for p3h2x4x buffer\n");
+ goto ibi_err;
+ }
+
+ /* read slave address */
+ ret = regmap_read(p3h2x4x_i3c_hub->regmap, P3H2X4X_TARGET_BUFF_ADDRESS, &slave_address);
+ if (ret)
+ goto ibi_err;
+
+ /* read data */
+ if (packet_len) {
+ ret = regmap_bulk_read(p3h2x4x_i3c_hub->regmap, P3H2X4X_TARGET_BUFF_DATA,
+ slave_rx_buffer, packet_len);
+ if (ret)
+ goto ibi_err;
+ }
+
+ client = p3h2x4x_i3c_hub->tp_bus[tp].tp_smbus_client;
+ if (!client)
+ goto ibi_err;
+
+ /* notify slave driver about received data */
+ if ((client->addr & 0x7f) == (slave_address >> 1)) {
+ addr = slave_address >> 1;
+ i2c_slave_event(client,
+ I2C_SLAVE_WRITE_REQUESTED, &addr);
+ for (i = 0; i < packet_len; i++) {
+ temp = slave_rx_buffer[i];
+ i2c_slave_event(client,
+ I2C_SLAVE_WRITE_RECEIVED, &temp);
+ }
+ i2c_slave_event(client, I2C_SLAVE_STOP, &temp);
+ }
+
+ibi_err:
+ regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_PAGE_PTR, 0x00);
+
+ regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP0_SMBUS_AGNT_STS + tp, flag_clear);
+}
+
+/**
+ * p3h2x4x_ibi_handler - IBI handler.
+ * @i3cdev: i3c device.
+ * @payload: two byte IBI payload data.
+ *
+ */
+void p3h2x4x_ibi_handler(struct i3c_device *i3cdev,
+ const struct i3c_ibi_payload *payload)
+{
+ struct p3h2x4x *p3h2x4x = i3cdev_get_drvdata(i3cdev);
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub;
+ u8 payload_byte_one, payload_byte_two;
+ u32 target_port_status;
+ const u8 *data;
+ int ret, i;
+
+ if (!payload || payload->len < P3H2X4X_MAX_PAYLOAD_LEN)
+ return;
+
+ data = payload->data;
+ payload_byte_one = data[0];
+
+ if (!(payload_byte_one & P3H2X4X_SMBUS_AGENT_EVENT_FLAG_STATUS))
+ return;
+
+ p3h2x4x_i3c_hub = p3h2x4x ? p3h2x4x->i3c_hub_priv : NULL;
+
+ if (!p3h2x4x_i3c_hub || !p3h2x4x_i3c_hub->regmap)
+ return;
+
+ payload_byte_two = data[1];
+ guard(mutex)(&p3h2x4x_i3c_hub->etx_mutex);
+
+ for (i = 0; i < p3h2x4x->num_target_ports; ++i) {
+ if (!((payload_byte_two >> i) & 0x01))
+ continue;
+
+ if (!p3h2x4x_i3c_hub->tp_bus[i].is_registered) {
+ dev_dbg(&i3cdev->dev, "IBI for unregistered SMBus port %u\n", i);
+ regmap_write(p3h2x4x_i3c_hub->regmap,
+ P3H2X4X_TP0_SMBUS_AGNT_STS + i,
+ BUF_RECEIVED_FLAG_TF_MASK);
+ continue;
+ }
+
+ ret = regmap_read(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP0_SMBUS_AGNT_STS + i,
+ &target_port_status);
+ if (ret) {
+ dev_err(&i3cdev->dev, "target port read status failed %d\n", ret);
+ continue;
+ }
+
+ if (target_port_status & P3H2X4X_TARGET_BUF_CA_TF)
+ regmap_write(p3h2x4x_i3c_hub->regmap,
+ P3H2X4X_TP0_SMBUS_AGNT_STS + i,
+ P3H2X4X_TARGET_BUF_CA_TF);
+
+ /* Service the receive flags independently of the overflow flag. */
+ if (target_port_status & P3H2X4X_TARGET_BUF_0_RECEIVE)
+ p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_0, i);
+
+ if (target_port_status & P3H2X4X_TARGET_BUF_1_RECEIVE)
+ p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_1, i);
+
+ if (target_port_status & P3H2X4X_TARGET_BUF_OVRFL) {
+ /*
+ * The helpers handle receive-flag acknowledgment. Clear only
+ * overflow here, as a released buffer may already have refilled.
+ */
+ regmap_write(p3h2x4x_i3c_hub->regmap,
+ P3H2X4X_TP0_SMBUS_AGNT_STS + i,
+ P3H2X4X_TARGET_BUF_OVRFL);
+ dev_err(&i3cdev->dev,
+ "SMBus receive buffer overflow on port %d\n", i);
+ }
+ }
+}
+#endif
+
static int p3h2x4x_read_smbus_transaction_status(struct p3h2x4x_i3c_hub_dev *hub,
u8 target_port_status,
u8 data_length)
@@ -220,8 +386,134 @@ static s32 p3h2x4x_tp_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, i
static u32 p3h2x4x_tp_smbus_funcs(struct i2c_adapter *adapter)
{
- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BLOCK_DATA;
+ u32 funcs = I2C_FUNC_I2C | I2C_FUNC_SMBUS_BLOCK_DATA;
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ struct tp_bus *bus = i2c_get_adapdata(adapter);
+ struct p3h2x4x_i3c_hub_dev *hub = bus->p3h2x4x_i3c_hub;
+
+ /*
+ * Only advertise slave support when the upstream IBI path is usable.
+ * Otherwise reg_slave() returns -EOPNOTSUPP while functionality()
+ * reports I2C_FUNC_SLAVE, which is inconsistent for callers.
+ */
+ if (hub->ibi_ready)
+ funcs |= I2C_FUNC_SLAVE;
+#endif
+
+ return funcs;
+}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static int p3h2x4x_tp_i2c_reg_slave(struct i2c_client *slave)
+{
+ struct tp_bus *bus = i2c_get_adapdata(slave->adapter);
+ struct p3h2x4x_i3c_hub_dev *hub = bus->p3h2x4x_i3c_hub;
+ struct p3h2x4x *p3h2x4x = dev_get_drvdata(hub->dev->parent);
+ int relock_ret = 0;
+ int ret = 0;
+
+ guard(mutex)(&hub->etx_mutex);
+
+ if (!hub->i3cdev || !hub->ibi_ready)
+ return -EOPNOTSUPP;
+
+ if (bus->tp_smbus_client)
+ return -EBUSY;
+
+ scoped_guard(mutex, &p3h2x4x->protected_reg_lock) {
+ /* Unlock access to protected registers */
+ ret = regmap_write(hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_UNLOCK_CODE);
+ if (ret)
+ break;
+
+ ret = regmap_set_bits(hub->regmap,
+ P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG,
+ bus->tp_mask);
+
+ /* Lock access to protected registers */
+ relock_ret = regmap_write(hub->regmap,
+ P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_LOCK_CODE);
+ }
+
+ /*
+ * A relock failure does not undo the IBI enable, so report it
+ * separately and keep the callback result tied to the IBI operation.
+ */
+ if (relock_ret)
+ dev_err(hub->dev,
+ "failed to restore protected register lock: %d\n",
+ relock_ret);
+
+ if (ret)
+ return ret;
+
+ /*
+ * Publish the software state only after the hardware IBI has been
+ * enabled successfully.
+ */
+ bus->tp_smbus_client = slave;
+ hub->hub_config.tp_config[bus->tp_port].ibi_en = true;
+
+ return 0;
+}
+
+static int p3h2x4x_tp_i2c_unreg_slave(struct i2c_client *slave)
+{
+ struct tp_bus *bus = i2c_get_adapdata(slave->adapter);
+ struct p3h2x4x_i3c_hub_dev *hub = bus->p3h2x4x_i3c_hub;
+ struct p3h2x4x *p3h2x4x = dev_get_drvdata(hub->dev->parent);
+ int relock_ret = 0;
+ int ret = 0;
+
+ guard(mutex)(&hub->etx_mutex);
+
+ if (bus->tp_smbus_client != slave)
+ return -EINVAL;
+
+ scoped_guard(mutex, &p3h2x4x->protected_reg_lock) {
+ /* Unlock access to protected registers */
+ ret = regmap_write(hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_UNLOCK_CODE);
+ if (ret)
+ break;
+
+ ret = regmap_clear_bits(hub->regmap,
+ P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG,
+ bus->tp_mask);
+
+ /* Lock access to protected registers */
+ relock_ret = regmap_write(hub->regmap,
+ P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_LOCK_CODE);
+ }
+
+ if (relock_ret)
+ dev_err(hub->dev,
+ "failed to restore protected register lock: %d\n",
+ relock_ret);
+
+ if (ret)
+ dev_err(hub->dev,
+ "failed to disable SMBus agent IBI on port %u: %d\n",
+ bus->tp_port, ret);
+
+ /*
+ * Slave backend removal cannot be rolled back. Detach the
+ * callback-visible state even if disabling the hardware IBI failed,
+ * and return 0 so the core completes teardown; the failure is still
+ * visible in the log above. etx_mutex serializes this with the IBI
+ * handler, so later events are acknowledged without invoking the
+ * removed client.
+ */
+ bus->tp_smbus_client = NULL;
+ hub->hub_config.tp_config[bus->tp_port].ibi_en = false;
+
+ return 0;
}
+#endif
static const struct i2c_adapter_quirks p3h2x4x_tp_i2c_quirks = {
.max_read_len = P3H2X4X_SMBUS_PAYLOAD_SIZE,
@@ -233,6 +525,10 @@ static const struct i2c_adapter_quirks p3h2x4x_tp_i2c_quirks = {
*/
static struct i2c_algorithm p3h2x4x_tp_i2c_algorithm = {
.master_xfer = p3h2x4x_tp_i2c_xfer,
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ .reg_slave = p3h2x4x_tp_i2c_reg_slave,
+ .unreg_slave = p3h2x4x_tp_i2c_unreg_slave,
+#endif
.functionality = p3h2x4x_tp_smbus_funcs,
};
--
2.25.1
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v21 4/4] i3c: hub: p3h2x4x: Add SMBus slave mode support
2026-11-12 12:44 ` [PATCH v21 4/4] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
@ 2026-11-12 13:07 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-12 13:07 UTC (permalink / raw)
To: Lakshay Piplani, linux-i3c, mfd; +Cc: Frank.Li, Alexandre Belloni, lee
Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] i3c: hub: p3h2x4x: empty rx buffer not rejected in
p3h2x4x_read_smbus_agent_rx_buf()
- [Medium] i3c: hub: p3h2x4x: stale rx flags not cleared in
p3h2x4x_tp_i2c_reg_slave()
- [Low] i3c: hub: p3h2x4x: missing I2C_CLIENT_TEN check in
p3h2x4x_tp_i2c_reg_slave()
--
commit c82ca6aaf774f0de048a84b1a2c414e57c266eef
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 target ports,
handling receive events via I3C IBI and forwarding received payloads to
registered I2C slave clients.
> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> index 247d0f4883b7c..02b50d1eb938d 100644
> --- a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> @@ -3,6 +3,7 @@
> * Copyright 2025-2026 NXP
> * This P3H2X4X driver file contain functions for SMBus/I2C virtual Bus creation and read/write.
> */
> +#include <linux/i3c/device.h>
> #include <linux/mfd/p3h2840.h>
> #include <linux/regmap.h>
>
> @@ -15,6 +16,171 @@ enum p3h2x4x_smbus_desc_idx {
> P3H2X4X_DESC_READ_LEN,
> };
>
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +static void p3h2x4x_read_smbus_agent_rx_buf(struct i3c_device *i3cdev, enum p3h2x4x_rcv_buf rfbuf,
> + enum p3h2x4x_tp tp)
> +{
> + struct p3h2x4x *p3h2x4x = i3cdev_get_drvdata(i3cdev);
> + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub;
> + u8 target_buffer_page, flag_clear, temp = 0, i, addr;
> + u32 packet_len, slave_address;
> + struct i2c_client *client;
> + u8 *slave_rx_buffer;
> + int ret;
> +
> + if (!p3h2x4x || !p3h2x4x->i3c_hub_priv)
> + return;
> +
> + p3h2x4x_i3c_hub = p3h2x4x->i3c_hub_priv;
> + slave_rx_buffer = p3h2x4x_i3c_hub->slave_rx_buffer;
> +
> + switch (rfbuf) {
> + case RCV_BUF_0:
> + target_buffer_page = P3H2X4X_TARGET_BUFF_0_PAGE;
> + flag_clear = P3H2X4X_TARGET_BUF_0_RECEIVE;
> + break;
> + case RCV_BUF_1:
> + target_buffer_page = P3H2X4X_TARGET_BUFF_1_PAGE;
> + flag_clear = P3H2X4X_TARGET_BUF_1_RECEIVE;
> + break;
> + default:
> + return;
> + }
> +
> + target_buffer_page += P3H2X4X_NO_PAGE_PER_TP * tp;
> +
> + ret = regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_PAGE_PTR, target_buffer_page);
> + if (ret)
> + goto ibi_err;
> +
> + /* read buffer length */
> + ret = regmap_read(p3h2x4x_i3c_hub->regmap, P3H2X4X_TARGET_BUFF_LENGTH, &packet_len);
> + if (ret)
> + goto ibi_err;
> +
> + if (packet_len)
> + packet_len = packet_len - 1;
[Severity: Medium]
Should p3h2x4x_read_smbus_agent_rx_buf() jump to ibi_err when packet_len is
0?
P3H2X4X_TARGET_BUFF_LENGTH includes the 1-byte target address, so a valid
zero-payload transfer has length 1 before decrementing. When
P3H2X4X_TARGET_BUFF_LENGTH reads as 0 (for example, on an aborted transfer
or empty buffer), falling through with packet_len == 0 reads a stale
P3H2X4X_TARGET_BUFF_ADDRESS from a prior transaction and delivers spurious
I2C_SLAVE_WRITE_REQUESTED and I2C_SLAVE_STOP events to the slave client.
[ ... ]
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +static int p3h2x4x_tp_i2c_reg_slave(struct i2c_client *slave)
> +{
> + struct tp_bus *bus = i2c_get_adapdata(slave->adapter);
> + struct p3h2x4x_i3c_hub_dev *hub = bus->p3h2x4x_i3c_hub;
> + struct p3h2x4x *p3h2x4x = dev_get_drvdata(hub->dev->parent);
> + int relock_ret = 0;
> + int ret = 0;
> +
> + guard(mutex)(&hub->etx_mutex);
> +
> + if (!hub->i3cdev || !hub->ibi_ready)
> + return -EOPNOTSUPP;
> +
> + if (bus->tp_smbus_client)
> + return -EBUSY;
[Severity: Low]
Should p3h2x4x_tp_i2c_reg_slave() reject 10-bit addressed clients
(slave->flags & I2C_CLIENT_TEN) with -EAFNOSUPPORT?
i2c_slave_register() skips 7-bit address validation when I2C_CLIENT_TEN is
set and delegates the check to reg_slave(). Without it, registration
succeeds and p3h2x4x_read_smbus_agent_rx_buf() truncates client->addr to its
low 7 bits when matching against P3H2X4X_TARGET_BUFF_ADDRESS.
> +
> + scoped_guard(mutex, &p3h2x4x->protected_reg_lock) {
> + /* Unlock access to protected registers */
> + ret = regmap_write(hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
> + P3H2X4X_REGISTERS_UNLOCK_CODE);
> + if (ret)
> + break;
> +
> + ret = regmap_set_bits(hub->regmap,
> + P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG,
> + bus->tp_mask);
> +
> + /* Lock access to protected registers */
> + relock_ret = regmap_write(hub->regmap,
> + P3H2X4X_DEV_REG_PROTECTION_CODE,
> + P3H2X4X_REGISTERS_LOCK_CODE);
> + }
[Severity: Medium]
Does p3h2x4x_tp_i2c_reg_slave() need to clear BUF_RECEIVED_FLAG_MASK in
P3H2X4X_TP0_SMBUS_AGNT_STS + bus->tp_port before enabling the port IBI?
p3h2x4x_configure_tp() enables P3H2X4X_TP_SMBUS_AGNT_EN at probe time, and
p3h2x4x_tp_i2c_unreg_slave() only masks the port bit in
P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG. Any SMBus write arriving on the port
while no slave is registered fills the target buffers and sets the receive
and overflow status flags without triggering p3h2x4x_ibi_handler(). Leaving
those flags set across registration either delivers stale payloads to the
new slave client or leaves both target buffers permanently occupied if an
IBI requires a fresh status transition.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112124437.3367252-1-lakshay.piplani@nxp.com?part=4
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 10+ messages in thread