* [PATCH v13 0/7] Add support for NXP P3H2x4x I3C hub driver
@ 2026-07-01 6:57 Lakshay Piplani
2026-07-01 6:57 ` [PATCH v13 1/7] i3c: master: Add APIs for I3C hub support Lakshay Piplani
` (6 more replies)
0 siblings, 7 replies; 18+ messages in thread
From: Lakshay Piplani @ 2026-07-01 6:57 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, krzk+dt, robh,
conor+dt, devicetree, broonie, lee, Frank.Li, lgirdwood
Cc: vikash.bansal, priyanka.jain, aman.kumarpandey, Lakshay Piplani
This is an MFD driver integrating I3C hub and on-die regulators.
The series introduces:
- Core I3C master enhancements required for hub support
- Generic I3C hub framework
- MFD driver with regulator and I3C hub child drivers for the P3H2x4x I3C hub
Patches 1/9 and 2/9 from v11 have already been applied to i3c/next branch:
- 79c41666b397 ("i3c: master: rename i3c_master_reattach_i3c_dev() to *_locked")
https://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git/commit/?id=79c41666b397
- 8d8afa428318 ("i3c: master: Expose the APIs to support I3C hub")
https://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git/commit/?id=8d8afa428318
This v13 series is rebased on top of those commits and therefore contains
only the remaining patches, renumbered as 1/7 through 7/7.
Changes in v13:
- Fix I3C master address management in direct attach/detach paths by using i3c_master_get_i3c_addrs()
adding rollback on failure, skipping master self attach/detach, and properly releasing addresses to
avoid stale state and use-after-free issues.
- Export and document address slot helper APIs for I3C hub support.
- Reserve parent bus address slots for downstream devices with identical static and assigned addresses
by parsing target-port DT nodes prior to virtual controller registration, preventing DAA conflicts.
- Keep broadcast RSTDAA blocked with added documentation, and clarify intentional no-op callbacks
and pending TODOs (e.g., IBI slot recycle).
- Rework SMBus transaction handling to use polling instead of fixed delays
avoiding premature reads and data corruption.
- Fix DT handling issues by preventing duplicate target-port node leaks and restoring dev->of_node
after temporary modification.
- Improve resource management using devm-based cleanup for DT nodes, IBI setup, adapter unregister
and register relock paths.
- Make IBI setup optional and robust to avoid probe failures on unsupported platforms, with proper cleanup.
- Fix SMBus slave receive path by improving buffer handling, event delivery, and handling of unregistered ports.
- Strengthen concurrency handling with proper locking around shared state.
- Link to v12: https://lore.kernel.org/linux-i3c/20260617110355.1591844-1-lakshay.piplani@nxp.com/T/#u
Changes in v12:
- Rebased on i3c/next
- Dropped patches 1/9 and 2/9 from v11 as they are already applied
- Add address check in i3c_master_direct_detach_i3c_dev_locked() to skip
detach for unaddressed devices
- Drop redundant depends on I3C from config I3C_HUB
- Return -EOPNOTSUPP for unsupported I2C transfers in the generic hub ops
- Correct default pull-up and drive-strength values
- Add devm cleanup for the IBI request/enable path
- Remove dead code and simplify cleanup by relying on devm-managed resources
- Fix SMBus slave client NULL handling and unregister cleanup
- Link to v11: https://lore.kernel.org/linux-i3c/20260612111816.3688240-1-lakshay.piplani@nxp.com/T/#u
Changes in v11:
- Convert i3c_master_supports_ccc_cmd() to return bool and align
semantics with CCC support checks used by the I3C core
- Use MFD_CELL_NAME() for child device registration
- Rename driver names to follow subsystem conventions:
- Use '-' instead of '_' in driver names
- Drop the "_drv" suffix from driver names
- Fix virtual hub address reattach handling and parent bus locking
- Fix IBI request and cleanup error paths
- Improve SMBus slave mode payload validation and parsing
- Link to v10: https://lore.kernel.org/linux-i3c/20260525064209.2263045-1-lakshay.piplani@nxp.com/T/#u
Changes in v10:
- Rename i3c_master_reattach_i3c_dev() to *_locked to reflect required
bus locking
- Rename i3c_master_direct_attach_i3c_dev() and i3c_master_direct_detach_i3c_dev()
to *_locked, as these APIs must be called with the bus lock held for write
- Drop redundant is_p3h2x4x_in_i3c flag from p3h2840.h
- Remove unnecessary ibi_lock handling in request/enable/disable/free
IBI APIs
- Remove redundant parent pointer from struct i3c_hub and derive upstream
master from hub_dev
- Split SMBus target/slave mode support, including IBI and MCTP receive handling,
into a separate patch
- Link to v9: https://lore.kernel.org/linux-i3c/20260420105222.1562243-1-lakshay.piplani@nxp.com/T/#u
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
- Referenced i3c.yaml and i2c-controller.yaml for child bus nodes
- Dropped unnecessary #address-cells and #size-cells from child nodes
- Added CONFIG_I2C_SLAVE guards where necessary to avoid build errors when I2C slave support is disabled
- Link to v8: https://lore.kernel.org/linux-i3c/20260323062737.886728-1-lakshay.piplani@nxp.com/T/#u
Changes in v8:
- Add compatible in i3c example
- Link to v7: https://lore.kernel.org/linux-i3c/20260319112441.3888957-1-lakshay.piplani@nxp.com/T/#u
Changes in v7:
- Fix kernel-doc warnings across I3C core and hub code
- Rework DT binding schema and examples to pass dt_binding_check
- Update MFD Kconfig to use I3C_OR_I2C
- Convert CONFIG_I3C_HUB to tristate
- Remove unnecessary CONFIG_I2C_SLAVE guards
- Replace custom helpers with find_closest()
- Use devm_regulator_get_enable_optional()
- Link to v6: https://lore.kernel.org/linux-i3c/64c5070c-aa9e-427a-933e-91e168f0510c@kernel.org/T/#u
Changes in v6:
- Update DT binding with vendor-prefixed properties
- Add generic I3C hub support
- Remove generic code from P3H2x4x driver
- Link to v5: https://lore.kernel.org/linux-i3c/20260206120121.856471-1-aman.kumarpandey@nxp.com/T/#u
Changes in v5:
- Update supply naming and descriptions
- Improve MFD Kconfig/Makefile ordering
- Link to v4: https://lore.kernel.org/linux-i3c/20260113114529.1692213-2-aman.kumarpandey@nxp.com/T/#u
Changes in v4:
- Split driver into MFD, regulator and I3C hub parts
- Update I3C master for hub support
- Fix DT binding issues
- Link to v3: https://lore.kernel.org/linux-i3c/20250811-bittern-of-abstract-prestige-aaeda9@kuoka/T/#u
Changes in v3:
- Add MFD support for hub and regulators
- Add regulator integration
- Link to v2: https://lore.kernel.org/linux-i3c/17145d2f-5d07-4939-8381-74e27cde303c@kernel.org/T/#u
Changes in v2:
- Fix DT binding warnings
- Refine DT parsing logic
- Link to v1: https://lore.kernel.org/linux-i3c/822d6dca-b2c6-4439-ade5-219620ebc435@kernel.org/T/#u
Aman Kumar Pandey (5):
i3c: master: Add APIs for I3C hub support
dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator
regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x
i3c hub
i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
Lakshay Piplani (2):
i3c: hub: Add support for the I3C interface in the I3C hub
i3c: hub: p3h2x4x: Add SMBus slave mode support
.../devicetree/bindings/i3c/nxp,p3h2840.yaml | 291 +++++++++
MAINTAINERS | 15 +
drivers/i3c/Kconfig | 15 +
drivers/i3c/Makefile | 2 +
drivers/i3c/hub.c | 566 ++++++++++++++++++
drivers/i3c/hub/Kconfig | 11 +
drivers/i3c/hub/Makefile | 4 +
drivers/i3c/hub/p3h2840_i3c_hub.h | 338 +++++++++++
drivers/i3c/hub/p3h2840_i3c_hub_common.c | 382 ++++++++++++
drivers/i3c/hub/p3h2840_i3c_hub_i3c.c | 180 ++++++
drivers/i3c/hub/p3h2840_i3c_hub_smbus.c | 506 ++++++++++++++++
drivers/i3c/master.c | 142 ++++-
drivers/mfd/Kconfig | 13 +
drivers/mfd/Makefile | 1 +
drivers/mfd/p3h2840.c | 126 ++++
drivers/regulator/Kconfig | 10 +
drivers/regulator/Makefile | 1 +
drivers/regulator/p3h2840_i3c_hub_regulator.c | 240 ++++++++
include/linux/i3c/device.h | 1 +
include/linux/i3c/hub.h | 101 ++++
include/linux/i3c/master.h | 13 +
include/linux/mfd/p3h2840.h | 28 +
22 files changed, 2983 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
create mode 100644 drivers/i3c/hub.c
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
create mode 100644 drivers/mfd/p3h2840.c
create mode 100644 drivers/regulator/p3h2840_i3c_hub_regulator.c
create mode 100644 include/linux/i3c/hub.h
create mode 100644 include/linux/mfd/p3h2840.h
--
2.25.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v13 1/7] i3c: master: Add APIs for I3C hub support
2026-07-01 6:57 [PATCH v13 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
@ 2026-07-01 6:57 ` Lakshay Piplani
2026-07-01 7:22 ` sashiko-bot
2026-07-01 6:57 ` [PATCH v13 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support Lakshay Piplani
` (5 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Lakshay Piplani @ 2026-07-01 6:57 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, krzk+dt, robh,
conor+dt, devicetree, broonie, lee, Frank.Li, lgirdwood
Cc: vikash.bansal, priyanka.jain, aman.kumarpandey, Lakshay Piplani
From: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
Add helpers for attaching and detaching I3C devices and CCC helpers
to check CCC support and send CCC commands, and address slot helpers to
query and update I3C bus address slot state These additions prepare for
I3C hub support.
The attach and detach helpers must be called with the bus lock held in
write mode.
The new helpers are:
1) i3c_master_direct_attach_i3c_dev_locked()
2) i3c_master_direct_detach_i3c_dev_locked()
3) i3c_master_send_ccc_cmd()
4) i3c_master_supports_ccc_cmd()
5) i3c_bus_get_addr_slot_status()
6) i3c_bus_set_addr_slot_status()
Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
Changes in v13:
- Fix address handling in direct attach by using i3c_master_get_i3c_addrs() and
adding rollback on failure to prevent bus address collisions
- Fix detach path by clearing master_priv and releasing addresses to avoid use-after-free
and stale state issues
- Export address slot helper APIs and add kernel-doc for them
Changes in v12:
- Add address check in i3c_master_direct_detach_i3c_dev_locked() to skip
detach for unaddressed devices.
Changes in v11:
- Convert i3c_master_supports_ccc_cmd() to return bool and align
semantics with CCC support checks used by the I3C core
Changes in v10:
- Rename i3c_master_direct_attach_i3c_dev and i3c_master_direct_detach_i3c_dev
APIs to *_locked, as these APIs must be called with the bus lock held in
write mode
Changes in v9:
- No change
Changes in v8:
- No change
Changes in v7:
- Update commit message to clarify purpose (prepare for I3C hub support)
Changes in v6:
- Split the patch into two parts:
1) expose the existing API
2) add new APIs.
---
---
drivers/i3c/master.c | 142 ++++++++++++++++++++++++++++++++++++-
include/linux/i3c/master.h | 13 ++++
2 files changed, 152 insertions(+), 3 deletions(-)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index dbb4d8119b5f..89432a906de0 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -391,11 +391,19 @@ i3c_bus_get_addr_slot_status_mask(struct i3c_bus *bus, u16 addr, u32 mask)
return status & mask;
}
-static enum i3c_addr_slot_status
+/**
+ * i3c_bus_get_addr_slot_status() - Get I3C bus address slot status
+ * @bus: I3C bus.
+ * @addr: I3C address to query.
+ *
+ * Return: Address slot status for @addr.
+ */
+enum i3c_addr_slot_status
i3c_bus_get_addr_slot_status(struct i3c_bus *bus, u16 addr)
{
return i3c_bus_get_addr_slot_status_mask(bus, addr, I3C_ADDR_SLOT_STATUS_MASK);
}
+EXPORT_SYMBOL_GPL(i3c_bus_get_addr_slot_status);
static void i3c_bus_set_addr_slot_status_mask(struct i3c_bus *bus, u16 addr,
enum i3c_addr_slot_status status, u32 mask)
@@ -411,11 +419,18 @@ static void i3c_bus_set_addr_slot_status_mask(struct i3c_bus *bus, u16 addr,
*ptr |= ((unsigned long)status & mask) << (bitpos % BITS_PER_LONG);
}
-static void i3c_bus_set_addr_slot_status(struct i3c_bus *bus, u16 addr,
- enum i3c_addr_slot_status status)
+/**
+ * i3c_bus_set_addr_slot_status() - Set I3C bus address slot status
+ * @bus: I3C bus.
+ * @addr: I3C address to update.
+ * @status: Address slot status to set.
+ */
+void i3c_bus_set_addr_slot_status(struct i3c_bus *bus, u16 addr,
+ enum i3c_addr_slot_status status)
{
i3c_bus_set_addr_slot_status_mask(bus, addr, status, I3C_ADDR_SLOT_STATUS_MASK);
}
+EXPORT_SYMBOL_GPL(i3c_bus_set_addr_slot_status);
static bool i3c_bus_dev_addr_is_avail(struct i3c_bus *bus, u8 addr)
{
@@ -1652,6 +1667,83 @@ static int i3c_master_attach_i3c_dev(struct i3c_master_controller *master,
return 0;
}
+/**
+ * i3c_master_direct_attach_i3c_dev_locked() - Attach an I3C device to a master
+ * @master: I3C master controller to attach the device to
+ * @dev: I3C device descriptor representing the device
+ *
+ * Attach an I3C device to its master controller after reserving the device
+ * addresses on the bus. The device must belong to @master.
+ *
+ * The master device itself is not passed to the controller driver's
+ * ->attach_i3c_dev() callback.
+ *
+ * Context: Caller must hold @master->bus.lock in write mode.
+ *
+ * Return: 0 on success, or a negative error code if address reservation fails
+ * or if the master controller driver's ->attach_i3c_dev() callback fails.
+ */
+int i3c_master_direct_attach_i3c_dev_locked(struct i3c_master_controller *master,
+ struct i3c_dev_desc *dev)
+{
+ int ret;
+
+ if (WARN_ON(i3c_dev_get_master(dev) != master))
+ return -EINVAL;
+
+ ret = i3c_master_get_i3c_addrs(dev);
+ if (ret)
+ return ret;
+
+ /* Do not attach the master device itself. */
+ if (master->this != dev && master->ops->attach_i3c_dev) {
+ ret = master->ops->attach_i3c_dev(dev);
+ if (ret) {
+ i3c_master_put_i3c_addrs(dev);
+ return ret;
+ }
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(i3c_master_direct_attach_i3c_dev_locked);
+
+/**
+ * i3c_master_direct_detach_i3c_dev_locked() - Detach an I3C device from a master
+ * @dev: I3C device descriptor to detach
+ *
+ * Detach an addressed I3C device from its master controller. Devices without
+ * either a static or dynamic address are skipped because they were not attached
+ * to the controller.
+ *
+ * The master device itself is not passed to the controller driver's
+ * ->detach_i3c_dev() callback. For other devices, the callback is invoked when
+ * provided by the master controller driver.
+ *
+ * The controller-private data is cleared and the device addresses are released
+ * from the bus address slot table.
+ *
+ * Context: Caller must hold the device master's bus lock in write mode.
+ */
+void i3c_master_direct_detach_i3c_dev_locked(struct i3c_dev_desc *dev)
+{
+ struct i3c_master_controller *master = i3c_dev_get_master(dev);
+
+ /*
+ * We don't attach devices to the controller until they are
+ * addressable on the bus, so skip detach for unaddressed devices.
+ */
+ if (!dev->info.static_addr && !dev->info.dyn_addr)
+ return;
+
+ /* Do not detach the master device itself. */
+ if (master->this != dev && master->ops->detach_i3c_dev)
+ master->ops->detach_i3c_dev(dev);
+
+ dev->common.master_priv = NULL;
+ i3c_master_put_i3c_addrs(dev);
+}
+EXPORT_SYMBOL_GPL(i3c_master_direct_detach_i3c_dev_locked);
+
/**
* i3c_master_reattach_i3c_dev_locked() - reattach an I3C device with a new address
* @dev: I3C device descriptor to reattach
@@ -1815,6 +1907,50 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
}
}
+/**
+ * i3c_master_supports_ccc_cmd() - check CCC command support
+ * @master: I3C master controller
+ * @cmd: CCC command to verify
+ *
+ * Return: true if @cmd is supported, false otherwise.
+ */
+bool i3c_master_supports_ccc_cmd(struct i3c_master_controller *master,
+ const struct i3c_ccc_cmd *cmd)
+{
+ if (!master || !cmd)
+ return false;
+
+ if (!master->ops->supports_ccc_cmd)
+ return true;
+
+ return master->ops->supports_ccc_cmd(master, cmd);
+}
+EXPORT_SYMBOL_GPL(i3c_master_supports_ccc_cmd);
+
+/**
+ * i3c_master_send_ccc_cmd() - send a CCC command
+ * @master: I3C master controller issuing the command
+ * @cmd: CCC command to be sent
+ *
+ * This function sends a Common Command Code (CCC) command to devices on the
+ * I3C bus. It acquires the bus maintenance lock, executes the command, and
+ * then releases the lock to ensure safe access to the bus.
+ *
+ * Return: 0 on success, or a negative error code on failure.
+ */
+int i3c_master_send_ccc_cmd(struct i3c_master_controller *master,
+ struct i3c_ccc_cmd *cmd)
+{
+ int ret;
+
+ i3c_bus_maintenance_lock(&master->bus);
+ ret = i3c_master_send_ccc_cmd_locked(master, cmd);
+ i3c_bus_maintenance_unlock(&master->bus);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(i3c_master_send_ccc_cmd);
+
/**
* i3c_master_do_daa_ext() - Dynamic Address Assignment (extended version)
* @master: controller
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 355e9b3d9ae3..d0ad30e59b56 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -615,6 +615,13 @@ DEFINE_FREE(i3c_master_dma_unmap_single, void *,
int i3c_master_reattach_i3c_dev_locked(struct i3c_dev_desc *dev,
u8 old_dyn_addr);
+int i3c_master_direct_attach_i3c_dev_locked(struct i3c_master_controller *master,
+ struct i3c_dev_desc *dev);
+void i3c_master_direct_detach_i3c_dev_locked(struct i3c_dev_desc *dev);
+int i3c_master_send_ccc_cmd(struct i3c_master_controller *master,
+ struct i3c_ccc_cmd *cmd);
+bool i3c_master_supports_ccc_cmd(struct i3c_master_controller *master,
+ const struct i3c_ccc_cmd *cmd);
int i3c_master_set_info(struct i3c_master_controller *master,
const struct i3c_device_info *info);
@@ -739,4 +746,10 @@ void i3c_for_each_bus_locked(int (*fn)(struct i3c_bus *bus, void *data),
int i3c_register_notifier(struct notifier_block *nb);
int i3c_unregister_notifier(struct notifier_block *nb);
+enum i3c_addr_slot_status
+i3c_bus_get_addr_slot_status(struct i3c_bus *bus, u16 addr);
+
+void i3c_bus_set_addr_slot_status(struct i3c_bus *bus, u16 addr,
+ enum i3c_addr_slot_status status);
+
#endif /* I3C_MASTER_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v13 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
2026-07-01 6:57 [PATCH v13 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
2026-07-01 6:57 ` [PATCH v13 1/7] i3c: master: Add APIs for I3C hub support Lakshay Piplani
@ 2026-07-01 6:57 ` Lakshay Piplani
2026-07-01 7:13 ` sashiko-bot
2026-07-01 6:57 ` [PATCH v13 3/7] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Lakshay Piplani @ 2026-07-01 6:57 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, krzk+dt, robh,
conor+dt, devicetree, broonie, lee, Frank.Li, lgirdwood
Cc: vikash.bansal, priyanka.jain, aman.kumarpandey, Lakshay Piplani
From: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
Add bindings for the NXP P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841)
multiport I3C hub family. These devices connect to a host via
I3C/I2C/SMBus and allow communication with multiple downstream
peripherals.
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: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v13:
- Fix mismatch between example unit-address and reg property by correcting the reg value
to match the node name
Changes in v12:
- No change, added Reviewed-By tag
Changes in v11:
- No change
Changes in v10:
- No change, added Reviewed-By tag
Changes in v9:
- Referenced i3c.yaml and i2c-controller.yaml for child nodes
- Dropped unnecessary #address-cells and #size-cells from child nodes
Changes in v8:
- Add compatible in i3c example
Changes in v7:
- Fix schema validation issues
- Adjust required properties
- Add I2C example
Changes in v6:
- Use a vendor prefix for the attributes
Changes in v5:
- Removed SW properties: cp0-ldo-microvolt,cp1-ldo-microvolt,
tp0145-ldo-microvolt, tp2367-ldo-microvolt
- Changed supply entries and its descriptions
Changes in v4:
- Fixed DT binding check warning
- Removed SW properties: ibi-enable, local-dev, and always-enable
Changes in v3:
- Added MFD (Multi-Function Device) support for I3C hub and on-die regulator
- Added Regulator supply node
Changes in v2:
- Fixed DT binding check warning
- Revised logic for parsing DTS nodes
---
---
.../devicetree/bindings/i3c/nxp,p3h2840.yaml | 291 ++++++++++++++++++
MAINTAINERS | 9 +
2 files changed, 300 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
diff --git a/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
new file mode 100644
index 000000000000..f7f0752de533
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
@@ -0,0 +1,291 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2025 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i3c/nxp,p3h2840.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP P3H2X4X I3C HUB
+
+maintainers:
+ - Aman Kumar Pandey <aman.kumarpandey@nxp.com>
+ - Vikash Bansal <vikash.bansal@nxp.com>
+ - Lakshay Piplani <lakshay.piplani@nxp.com>
+
+description: |
+ P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841) is a family of multiport I3C
+ hub devices that connect to:-
+ 1. A host CPU via I3C/I2C/SMBus bus on upstream side and connect to multiple
+ peripheral devices on the downstream side.
+ 2. Have two Controller Ports which can support either
+ I2C/SMBus or I3C buses and connect to a CPU, BMC or SOC.
+ 3. P3H2840/ P3H2841 are 8 port I3C hub with eight I3C/I2C Target Port.
+ 4. P3H2440/ P3H2441 are 4 port I3C hub with four I3C/I2C Target Port.
+ Target ports can be configured as I2C/SMBus, I3C or GPIO and connect to
+ peripherals.
+
+properties:
+ compatible:
+ const: nxp,p3h2840
+
+ reg:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ assigned-address:
+ maximum: 0x7f
+
+ nxp,tp0145-pullup-ohms:
+ description:
+ Selects the pull up resistance for target Port 0/1/4/5, in ohms.
+ enum: [250, 500, 1000, 2000]
+ default: 500
+
+ nxp,tp2367-pullup-ohms:
+ description:
+ Selects the pull up resistance for target Port 2/3/6/7, in ohms.
+ enum: [250, 500, 1000, 2000]
+ default: 500
+
+ nxp,cp0-io-strength-ohms:
+ description:
+ Selects the IO drive strength for controller Port 0, in ohms.
+ enum: [20, 30, 40, 50]
+ default: 20
+
+ nxp,cp1-io-strength-ohms:
+ description:
+ Selects the IO drive strength for controller Port 1, in ohms.
+ enum: [20, 30, 40, 50]
+ default: 20
+
+ nxp,tp0145-io-strength-ohms:
+ description:
+ Selects the IO drive strength for target port 0/1/4/5, in ohms.
+ enum: [20, 30, 40, 50]
+ default: 20
+
+ nxp,tp2367-io-strength-ohms:
+ description:
+ Selects the IO drive strength for target port 2/3/6/7, in ohms.
+ enum: [20, 30, 40, 50]
+ default: 20
+
+ vcc1-supply:
+ description: Controller port 0 power supply.
+
+ vcc2-supply:
+ description: Controller port 1 power supply.
+
+ vcc3-supply:
+ description: Target port 0/1/4/5 power supply.
+
+ vcc4-supply:
+ description: Target port 2/3/6/7 power supply.
+
+ regulators:
+ type: object
+ additionalProperties: false
+
+ properties:
+ ldo-cp0:
+ type: object
+ $ref: /schemas/regulator/regulator.yaml#
+ unevaluatedProperties: false
+
+ ldo-cp1:
+ type: object
+ $ref: /schemas/regulator/regulator.yaml#
+ unevaluatedProperties: false
+
+ ldo-tpg0:
+ type: object
+ $ref: /schemas/regulator/regulator.yaml#
+ unevaluatedProperties: false
+
+ ldo-tpg1:
+ type: object
+ $ref: /schemas/regulator/regulator.yaml#
+ unevaluatedProperties: false
+
+required:
+ - reg
+
+patternProperties:
+ "^i3c@[0-7]$":
+ type: object
+ $ref: /schemas/i3c/i3c.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ description:
+ The I3C HUB Target Port number.
+ maximum: 7
+
+ nxp,pullup-enable:
+ type: boolean
+ description:
+ Enables the on-die pull-up for Target Port.
+
+ required:
+ - reg
+
+ "^(i2c|smbus)@[0-7]$":
+ type: object
+ $ref: /schemas/i2c/i2c-controller.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ description:
+ The I3C HUB Target Port number.
+ maximum: 7
+
+ nxp,pullup-enable:
+ type: boolean
+ description:
+ Enables the on-die pull-up for Target Port.
+
+ required:
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i3c {
+ #address-cells = <3>;
+ #size-cells = <0>;
+
+ hub@70,236153000c2 {
+ reg = <0x70 0x236 0x153000c2>;
+ compatible = "nxp,p3h2840";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ assigned-address = <0x50>;
+
+ nxp,tp0145-pullup-ohms = <1000>;
+ nxp,tp2367-pullup-ohms = <1000>;
+ nxp,cp0-io-strength-ohms = <50>;
+ nxp,cp1-io-strength-ohms = <50>;
+ nxp,tp0145-io-strength-ohms = <50>;
+ nxp,tp2367-io-strength-ohms = <50>;
+ vcc3-supply = <®_tpg0>;
+ vcc4-supply = <®_tpg1>;
+
+ regulators {
+ reg_cp0: ldo-cp0 {
+ regulator-name = "ldo-cp0";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ reg_cp1: ldo-cp1 {
+ regulator-name = "ldo-cp1";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ reg_tpg0: ldo-tpg0 {
+ regulator-name = "ldo-tpg0";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ reg_tpg1: ldo-tpg1 {
+ regulator-name = "ldo-tpg1";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+ };
+
+ smbus@0 {
+ reg = <0x0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ nxp,pullup-enable;
+ };
+
+ i2c@1 {
+ reg = <0x1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ nxp,pullup-enable;
+ };
+
+ i3c@2 {
+ reg = <0x2>;
+ #address-cells = <3>;
+ #size-cells = <0>;
+ nxp,pullup-enable;
+ };
+ };
+ };
+
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hub@70 {
+ reg = <0x70>;
+ compatible = "nxp,p3h2840";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ nxp,tp0145-pullup-ohms = <1000>;
+ nxp,tp2367-pullup-ohms = <1000>;
+ nxp,cp0-io-strength-ohms = <50>;
+ nxp,cp1-io-strength-ohms = <50>;
+ nxp,tp0145-io-strength-ohms = <50>;
+ nxp,tp2367-io-strength-ohms = <50>;
+ vcc3-supply = <®_tpg0_i2c>;
+ vcc4-supply = <®_tpg1_i2c>;
+
+ regulators {
+ reg_cp0_i2c: ldo-cp0 {
+ regulator-name = "ldo-cp0";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ reg_cp1_i2c: ldo-cp1 {
+ regulator-name = "ldo-cp1";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ reg_tpg0_i2c: ldo-tpg0 {
+ regulator-name = "ldo-tpg0";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ reg_tpg1_i2c: ldo-tpg1 {
+ regulator-name = "ldo-tpg1";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+ };
+
+ smbus@0 {
+ reg = <0x0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ nxp,pullup-enable;
+ };
+
+ i2c@1 {
+ reg = <0x1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ nxp,pullup-enable;
+ };
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index c2c6d79275c6..ba65ae5a008b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19303,6 +19303,15 @@ S: Maintained
F: Documentation/devicetree/bindings/ptp/nxp,ptp-netc.yaml
F: drivers/ptp/ptp_netc.c
+NXP P3H2X4X I3C-HUB DRIVER
+M: Vikash Bansal <vikash.bansal@nxp.com>
+M: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
+M: Lakshay Piplani <lakshay.piplani@nxp.com>
+L: linux-kernel@vger.kernel.org
+L: linux-i3c@lists.infradead.org
+S: Maintained
+F: Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
+
NXP PF5300/PF5301/PF5302 PMIC REGULATOR DEVICE DRIVER
M: Woodrow Douglass <wdouglass@carnegierobotics.com>
S: Maintained
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v13 3/7] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator
2026-07-01 6:57 [PATCH v13 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
2026-07-01 6:57 ` [PATCH v13 1/7] i3c: master: Add APIs for I3C hub support Lakshay Piplani
2026-07-01 6:57 ` [PATCH v13 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support Lakshay Piplani
@ 2026-07-01 6:57 ` Lakshay Piplani
2026-07-01 6:57 ` [PATCH v13 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
` (3 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Lakshay Piplani @ 2026-07-01 6:57 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, krzk+dt, robh,
conor+dt, devicetree, broonie, 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.
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 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 | 126 ++++++++++++++++++++++++++++++++++++
include/linux/i3c/device.h | 1 +
include/linux/mfd/p3h2840.h | 28 ++++++++
6 files changed, 171 insertions(+)
create mode 100644 drivers/mfd/p3h2840.c
create mode 100644 include/linux/mfd/p3h2840.h
diff --git a/MAINTAINERS b/MAINTAINERS
index ba65ae5a008b..3420701a75c5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19311,6 +19311,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 7192c9d1d268..405b50c3c77b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -617,6 +617,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 e75e8045c28a..a284b22c7b13 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..89a97f0449c4
--- /dev/null
+++ b/drivers/mfd/p3h2840.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025-2026 NXP
+ * P3H2X4X i3c hub and regulator device.
+ */
+
+#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 = P3H2X4X_REG_BITS,
+ .val_bits = P3H2X4X_VAL_BITS,
+ .max_register = 0xFF,
+};
+
+static int p3h2x4x_device_probe_i3c(struct i3c_device *i3cdev)
+{
+ struct device *dev = i3cdev_to_dev(i3cdev);
+ struct i3c_device_info info;
+ struct p3h2x4x_dev *ddata;
+ int ret;
+
+ i3c_device_get_info(i3cdev, &info);
+
+ if (I3C_PID_MANUF_ID(info.pid) != I3C_MANUF_ID_NXP)
+ return -ENODEV;
+
+ ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ 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 = 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_dev *ddata;
+ int ret;
+
+ ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ 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 = 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[] = {
+ { "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 971d53349b6f..6188082599dd 100644
--- a/include/linux/i3c/device.h
+++ b/include/linux/i3c/device.h
@@ -85,6 +85,7 @@ struct i3c_xfer {
*/
enum i3c_dcr {
I3C_DCR_GENERIC_DEVICE = 0,
+ I3C_DCR_HUB = 194,
};
#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..0f0d5b442bef
--- /dev/null
+++ b/include/linux/mfd/p3h2840.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2025-2026 NXP
+ * This header file contains register definitions and bit masks for the P3H2X4X.
+ */
+
+#ifndef _LINUX_MFD_P3H2840_H
+#define _LINUX_MFD_P3H2840_H
+
+#include <linux/types.h>
+
+/* 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
+
+/* Reg config for Regmap */
+#define P3H2X4X_REG_BITS 8
+#define P3H2X4X_VAL_BITS 8
+
+#define I3C_MANUF_ID_NXP 0x011b
+
+struct p3h2x4x_dev {
+ struct i3c_device *i3cdev;
+ struct regmap *regmap;
+};
+#endif /* _LINUX_MFD_P3H2840_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v13 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
2026-07-01 6:57 [PATCH v13 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
` (2 preceding siblings ...)
2026-07-01 6:57 ` [PATCH v13 3/7] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
@ 2026-07-01 6:57 ` Lakshay Piplani
2026-07-01 7:16 ` sashiko-bot
2026-07-01 6:57 ` [PATCH v13 5/7] i3c: hub: Add support for the I3C interface in the I3C hub Lakshay Piplani
` (2 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Lakshay Piplani @ 2026-07-01 6:57 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, krzk+dt, robh,
conor+dt, devicetree, broonie, lee, Frank.Li, lgirdwood
Cc: vikash.bansal, priyanka.jain, aman.kumarpandey, Lakshay Piplani,
Frank Li
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 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 | 240 ++++++++++++++++++
4 files changed, 252 insertions(+)
create mode 100644 drivers/regulator/p3h2840_i3c_hub_regulator.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 3420701a75c5..fcb3917aa096 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19312,6 +19312,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 d71dac9436e3..88809f493fd4 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1019,6 +1019,16 @@ config REGULATOR_MTK_DVFSRC
of Mediatek. It allows for voting on regulator state
between multiple users.
+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 35639f3115fd..46f586ccde63 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -128,6 +128,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..781622a6ff46
--- /dev/null
+++ b/drivers/regulator/p3h2840_i3c_hub_regulator.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025-2026 NXP
+ * This P3H2X4X 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
+
+struct p3h2x4x_regulator_dev {
+ struct regulator_dev *rp3h2x4x_dev[P3H2X4X_LDO_COUNT];
+ struct regmap *regmap;
+ struct mutex lock; /* Serializes register unlock/restore sequences. */
+};
+
+struct p3h2x4x_reg_state {
+ unsigned int orig;
+ bool restore;
+};
+
+static void p3h2x4x_reg_guard_enter(struct regulator_dev *rdev,
+ struct p3h2x4x_reg_state *state)
+{
+ struct p3h2x4x_regulator_dev *priv = rdev_get_drvdata(rdev);
+
+ mutex_lock(&priv->lock);
+ state->restore = false;
+
+ if (regmap_read(rdev->regmap,
+ P3H2X4X_DEV_REG_PROTECTION_CODE,
+ &state->orig)) {
+ mutex_unlock(&priv->lock);
+ return;
+ }
+
+ if (state->orig != P3H2X4X_REGISTERS_UNLOCK_CODE) {
+ regmap_write(rdev->regmap,
+ P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_UNLOCK_CODE);
+ state->restore = true;
+ }
+}
+
+static void p3h2x4x_reg_guard_exit(struct regulator_dev *rdev,
+ struct p3h2x4x_reg_state *state)
+{
+ struct p3h2x4x_regulator_dev *priv = rdev_get_drvdata(rdev);
+
+ if (state->restore)
+ regmap_write(rdev->regmap,
+ P3H2X4X_DEV_REG_PROTECTION_CODE,
+ state->orig);
+
+ mutex_unlock(&priv->lock);
+}
+
+DEFINE_LOCK_GUARD_1(p3h2x4x_reg, struct regulator_dev,
+ p3h2x4x_reg_guard_enter(_T->lock, &_T->state),
+ p3h2x4x_reg_guard_exit(_T->lock, &_T->state),
+ struct p3h2x4x_reg_state state);
+
+static int p3h2x4x_regulator_enable(struct regulator_dev *rdev)
+{
+ guard(p3h2x4x_reg)(rdev);
+ return regulator_enable_regmap(rdev);
+}
+
+static int p3h2x4x_regulator_disable(struct regulator_dev *rdev)
+{
+ guard(p3h2x4x_reg)(rdev);
+ return regulator_disable_regmap(rdev);
+}
+
+static int p3h2x4x_regulator_set_voltage_sel(struct regulator_dev *rdev,
+ unsigned int sel)
+{
+ guard(p3h2x4x_reg)(rdev);
+ return regulator_set_voltage_sel_regmap(rdev, sel);
+}
+
+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 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,
+ .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,
+ .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,
+ .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,
+ .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_dev *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, ret;
+
+ p3h2x4x_regulator = devm_kzalloc(dev, sizeof(*p3h2x4x_regulator), GFP_KERNEL);
+ if (!p3h2x4x_regulator)
+ return -ENOMEM;
+
+ ret = devm_mutex_init(dev, &p3h2x4x_regulator->lock);
+ if (ret)
+ return ret;
+
+ platform_set_drvdata(pdev, p3h2x4x_regulator);
+
+ p3h2x4x_regulator->regmap = p3h2x4x->regmap;
+ 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[] = {
+ { "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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v13 5/7] i3c: hub: Add support for the I3C interface in the I3C hub
2026-07-01 6:57 [PATCH v13 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
` (3 preceding siblings ...)
2026-07-01 6:57 ` [PATCH v13 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
@ 2026-07-01 6:57 ` Lakshay Piplani
2026-07-01 7:23 ` sashiko-bot
2026-07-01 6:57 ` [PATCH v13 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
2026-07-01 6:57 ` [PATCH v13 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
6 siblings, 1 reply; 18+ messages in thread
From: Lakshay Piplani @ 2026-07-01 6:57 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, krzk+dt, robh,
conor+dt, devicetree, broonie, lee, Frank.Li, lgirdwood
Cc: vikash.bansal, priyanka.jain, aman.kumarpandey, Lakshay Piplani
Add virtual I3C bus support for the hub and provide interface to enable
or disable downstream 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>
---
Changes in v13:
- Reserve parent I3C bus address slots for downstream devices with matching static and
assigned addresses by parsing target-port DT nodes
- Keep broadcast RSTDAA blocked and add explanatory comment
- Add TODO for empty IBI slot recycle callback
- Add comments for intentional no-op callbacks
Changes in v12:
- Drop redundant depends on I3C from config I3C_HUB
- Return -EOPNOTSUPP for unsupported I2C transfers instead of reporting
success.
Changes in v11:
- Fix virtual hub address reattach handling
- Fix parent bus locking during address reattach operations
- Improve transfer and IBI request error handling
Changes in v10:
- Remove unnecessary ibi_lock handling in request/enable/disable/free
IBI APIs
- Remove redundant parent pointer from struct i3c_hub and derive upstream
master from hub_dev
Changes in v9:
- No change
Changes in v8:
- No change
Changes in v7:
- Convert Kconfig option to tristate
- Fix signedness issue in return value
- Fix kernel-doc warnings
Changes in v6:
- Add support for the generic I3C interface in the I3C Hub
---
---
MAINTAINERS | 2 +
drivers/i3c/Kconfig | 14 +
drivers/i3c/Makefile | 1 +
drivers/i3c/hub.c | 566 ++++++++++++++++++++++++++++++++++++++++
include/linux/i3c/hub.h | 101 +++++++
5 files changed, 684 insertions(+)
create mode 100644 drivers/i3c/hub.c
create mode 100644 include/linux/i3c/hub.h
diff --git a/MAINTAINERS b/MAINTAINERS
index fcb3917aa096..a0b7fd85f8e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19311,8 +19311,10 @@ 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/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 626c54b386d5..7536f3740c94 100644
--- a/drivers/i3c/Kconfig
+++ b/drivers/i3c/Kconfig
@@ -21,6 +21,20 @@ menuconfig I3C
if I3C
source "drivers/i3c/master/Kconfig"
+
+config I3C_HUB
+ tristate "I3C Hub Support"
+ help
+ Enable support for the I3C interface in hub devices.
+
+ This option adds virtual I3C bus support for hubs by creating
+ virtual master controllers for downstream ports and forwarding
+ bus operations through the hub device. It also provides an
+ interface used by hub drivers to enable or disable downstream
+ ports during bus transactions.
+
+ Say Y here if your platform includes an I3C hub device
+
endif # I3C
config I3C_OR_I2C
diff --git a/drivers/i3c/Makefile b/drivers/i3c/Makefile
index 11982efbc6d9..9ddee56a6338 100644
--- a/drivers/i3c/Makefile
+++ b/drivers/i3c/Makefile
@@ -2,3 +2,4 @@
i3c-y := device.o master.o
obj-$(CONFIG_I3C) += i3c.o
obj-$(CONFIG_I3C) += master/
+obj-$(CONFIG_I3C_HUB) += hub.o
diff --git a/drivers/i3c/hub.c b/drivers/i3c/hub.c
new file mode 100644
index 000000000000..d29ed40b6f73
--- /dev/null
+++ b/drivers/i3c/hub.c
@@ -0,0 +1,566 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2026 NXP
+ * Generic I3C Hub core implementing virtual controller operations.
+ */
+#include <linux/i3c/device.h>
+#include <linux/i3c/hub.h>
+
+#include "internals.h"
+
+/**
+ * i3c_hub_master_bus_init() - Bind controller to hub device
+ * @controller: Virtual controller for a hub port
+ *
+ * Associates the virtual controller with the hub device descriptor so that
+ * transfers are executed through the hub on the parent bus.
+ */
+static int i3c_hub_master_bus_init(struct i3c_master_controller *controller)
+{
+ struct i3c_hub_controller *hub_controller;
+ struct i3c_hub *hub;
+
+ hub_controller = dev_get_drvdata(&controller->dev);
+ if (!hub_controller || !hub_controller->hub)
+ return -ENODEV;
+
+ hub = hub_controller->hub;
+
+ if (!hub->hub_dev)
+ return -ENODEV;
+
+ controller->this = hub->hub_dev->desc;
+ return 0;
+}
+
+static void i3c_hub_master_bus_cleanup(struct i3c_master_controller *controller)
+{
+ controller->this = NULL;
+}
+
+/*
+ * Devices behind the hub are not attached to the virtual bus.
+ *
+ * Instead, i3c_hub_master_priv_xfers() temporarily updates the hub dynamic
+ * address with the target device address before issuing the transfer. Once the
+ * transfer completes, the hub address is restored back to its original value.
+ *
+ * Therefore, no per-device attach, reattach, or detach handling is required for
+ * the virtual bus.
+ */
+static int i3c_hub_attach_i3c_dev(struct i3c_dev_desc *dev)
+{
+ return 0;
+}
+
+static int i3c_hub_reattach_i3c_dev(struct i3c_dev_desc *dev, u8 old_dyn_addr)
+{
+ return 0;
+}
+
+static void i3c_hub_detach_i3c_dev(struct i3c_dev_desc *dev)
+{
+}
+
+/**
+ * i3c_hub_do_daa() - Perform DAA via hub port
+ * @hub: Hub instance
+ * @controller: Virtual controller for a hub port
+ *
+ * Enables the port connection, performs DAA on the parent controller,
+ * then disables the connection.
+ */
+static int i3c_hub_do_daa(struct i3c_hub *hub,
+ struct i3c_master_controller *controller)
+{
+ struct i3c_master_controller *parent;
+ int ret;
+
+ if (!hub || !hub->hub_dev)
+ return -ENODEV;
+
+ parent = i3c_dev_get_master(hub->hub_dev->desc);
+ if (!parent)
+ return -ENODEV;
+
+ i3c_hub_enable_port(controller);
+ ret = i3c_master_do_daa(parent);
+ i3c_hub_disable_port(controller);
+
+ return ret;
+}
+
+static bool i3c_hub_supports_ccc_cmd(struct i3c_hub *hub,
+ const struct i3c_ccc_cmd *cmd)
+{
+ struct i3c_master_controller *parent;
+
+ if (!hub || !hub->hub_dev)
+ return false;
+
+ parent = i3c_dev_get_master(hub->hub_dev->desc);
+ if (!parent)
+ return false;
+
+ return i3c_master_supports_ccc_cmd(parent, cmd);
+}
+
+/**
+ * i3c_hub_send_ccc_cmd() - Send CCC through hub port
+ * @hub: Hub instance
+ * @controller: Virtual controller
+ * @cmd: CCC command
+ *
+ * Enables the port connection while issuing CCC on the parent controller.
+ */
+static int i3c_hub_send_ccc_cmd(struct i3c_hub *hub,
+ struct i3c_master_controller *controller,
+ struct i3c_ccc_cmd *cmd)
+{
+ struct i3c_master_controller *parent;
+ int ret;
+
+ if (!hub || !hub->hub_dev)
+ return -ENODEV;
+
+ parent = i3c_dev_get_master(hub->hub_dev->desc);
+ if (!parent)
+ return -ENODEV;
+
+ i3c_hub_enable_port(controller);
+ ret = i3c_master_send_ccc_cmd(parent, cmd);
+ i3c_hub_disable_port(controller);
+
+ return ret;
+}
+
+/**
+ * i3c_hub_master_priv_xfers() - Execute private transfers via hub
+ * @dev: Target device descriptor
+ * @xfers: Transfer array
+ * @nxfers: Number of transfers
+ * @mode: transfer mode (SDR, HDR, etc.)
+ *
+ * Handles address adjustment and forwards private transfers through the hub
+ * device.
+ */
+static int i3c_hub_master_priv_xfers(struct i3c_dev_desc *dev,
+ struct i3c_xfer *xfers,
+ int nxfers,
+ enum i3c_xfer_mode mode)
+{
+ struct i3c_master_controller *controller = i3c_dev_get_master(dev);
+ struct i3c_hub_controller *hub_controller;
+ struct i3c_dev_desc *hub_dev;
+ struct i3c_master_controller *parent;
+ u8 hub_addr, target_addr;
+ struct i3c_hub *hub;
+ int ret, xfer_ret;
+
+ hub_controller = dev_get_drvdata(&controller->dev);
+ if (!hub_controller || !hub_controller->hub)
+ return -ENODEV;
+
+ hub = hub_controller->hub;
+
+ if (!hub->hub_dev)
+ return -ENODEV;
+
+ hub_dev = hub->hub_dev->desc;
+
+ parent = i3c_dev_get_master(hub_dev);
+ if (!parent)
+ return -ENODEV;
+
+ mutex_lock(&hub->xfer_lock);
+
+ i3c_hub_enable_port(controller);
+
+ hub_addr = hub_dev->info.dyn_addr ?
+ hub_dev->info.dyn_addr : hub_dev->info.static_addr;
+
+ target_addr = dev->info.dyn_addr ?
+ dev->info.dyn_addr : dev->info.static_addr;
+
+ if (hub_addr != target_addr) {
+ down_write(&parent->bus.lock);
+ hub_dev->info.dyn_addr = target_addr;
+
+ ret = i3c_master_reattach_i3c_dev_locked(hub_dev, hub_addr);
+ if (ret) {
+ up_write(&parent->bus.lock);
+ hub_dev->info.dyn_addr = hub_addr;
+ goto disable;
+ }
+ up_write(&parent->bus.lock);
+ }
+
+ ret = i3c_device_do_xfers(hub->hub_dev, xfers, nxfers, mode);
+
+ if (hub_addr != target_addr) {
+ down_write(&parent->bus.lock);
+ hub_dev->info.dyn_addr = hub_addr;
+
+ xfer_ret = i3c_master_reattach_i3c_dev_locked(hub_dev,
+ target_addr);
+
+ up_write(&parent->bus.lock);
+ if (!ret)
+ ret = xfer_ret;
+ }
+disable:
+ i3c_hub_disable_port(controller);
+ mutex_unlock(&hub->xfer_lock);
+ return ret;
+}
+
+static int i3c_hub_attach_i2c_dev(struct i2c_dev_desc *dev)
+{
+ return -EOPNOTSUPP;
+}
+
+static void i3c_hub_detach_i2c_dev(struct i2c_dev_desc *dev)
+{
+}
+
+static int i3c_hub_i2c_xfers(struct i2c_dev_desc *dev,
+ struct i2c_msg *xfers, int nxfers)
+{
+ return -EOPNOTSUPP;
+}
+
+static int i3c_hub_master_do_daa(struct i3c_master_controller *controller)
+{
+ struct i3c_hub_controller *hub_controller;
+ struct i3c_hub *hub;
+
+ hub_controller = dev_get_drvdata(&controller->dev);
+ if (!hub_controller || !hub_controller->hub)
+ return -ENODEV;
+
+ hub = hub_controller->hub;
+
+ return i3c_hub_do_daa(hub, controller);
+}
+
+static int i3c_hub_master_send_ccc_cmd(struct i3c_master_controller *controller,
+ struct i3c_ccc_cmd *cmd)
+{
+ struct i3c_hub_controller *hub_controller;
+ struct i3c_hub *hub;
+
+ hub_controller = dev_get_drvdata(&controller->dev);
+ if (!hub_controller || !hub_controller->hub)
+ return -ENODEV;
+
+ hub = hub_controller->hub;
+
+ if (!hub->hub_dev)
+ return -ENODEV;
+
+ /*
+ * Do not forward broadcast RSTDAA through the hub. The hub itself
+ * is visible on the parent bus, so forwarding RSTDAA would also
+ * reset the hub dynamic address. Downstream RSTDAA is not supported
+ * by the hub virtual-controller model.
+ */
+ if (cmd->id == I3C_CCC_RSTDAA(true))
+ return 0;
+
+ return i3c_hub_send_ccc_cmd(hub, controller, cmd);
+}
+
+static bool i3c_hub_master_supports_ccc_cmd(struct i3c_master_controller *controller,
+ const struct i3c_ccc_cmd *cmd)
+{
+ struct i3c_hub_controller *hub_controller;
+ struct i3c_hub *hub;
+
+ hub_controller = dev_get_drvdata(&controller->dev);
+ if (!hub_controller || !hub_controller->hub)
+ return false;
+
+ hub = hub_controller->hub;
+
+ return i3c_hub_supports_ccc_cmd(hub, cmd);
+}
+
+/**
+ * i3c_hub_request_ibi() - Request IBI through parent controller
+ * @desc: Target device descriptor
+ * @req: IBI setup
+ *
+ * Temporarily updates parent controller context to request IBI for a device
+ * connected through the hub.
+ */
+static int i3c_hub_request_ibi(struct i3c_dev_desc *desc,
+ const struct i3c_ibi_setup *req)
+{
+ struct i3c_master_controller *controller = i3c_dev_get_master(desc);
+ struct i3c_hub_controller *hub_controller;
+ struct i3c_master_controller *orig_parent;
+ struct i3c_master_controller *parent;
+ struct i3c_hub *hub;
+ int ret;
+
+ hub_controller = dev_get_drvdata(&controller->dev);
+ if (!hub_controller || !hub_controller->hub)
+ return -ENODEV;
+
+ hub = hub_controller->hub;
+
+ if (!hub->hub_dev)
+ return -ENODEV;
+
+ parent = i3c_dev_get_master(hub->hub_dev->desc);
+
+ down_write(&parent->bus.lock);
+
+ orig_parent = i3c_hub_update_desc_parent(&desc->common, parent);
+
+ ret = i3c_master_direct_attach_i3c_dev_locked(parent, desc);
+ if (ret) {
+ i3c_hub_update_desc_parent(&desc->common, orig_parent);
+ up_write(&parent->bus.lock);
+ return ret;
+ }
+
+ kfree(desc->ibi);
+ desc->ibi = NULL;
+ ret = i3c_dev_request_ibi_locked(desc, req);
+ if (ret)
+ i3c_master_direct_detach_i3c_dev_locked(desc);
+
+ i3c_hub_update_desc_parent(&desc->common, orig_parent);
+
+ up_write(&parent->bus.lock);
+
+ return ret;
+}
+
+static void i3c_hub_free_ibi(struct i3c_dev_desc *desc)
+{
+ struct i3c_master_controller *controller = i3c_dev_get_master(desc);
+ struct i3c_hub_controller *hub_controller;
+ struct i3c_master_controller *orig_parent;
+ struct i3c_master_controller *parent;
+ struct i3c_hub *hub;
+
+ hub_controller = dev_get_drvdata(&controller->dev);
+ if (!hub_controller || !hub_controller->hub)
+ return;
+
+ hub = hub_controller->hub;
+
+ if (!hub->hub_dev)
+ return;
+
+ parent = i3c_dev_get_master(hub->hub_dev->desc);
+
+ i3c_hub_enable_port(controller);
+
+ down_write(&parent->bus.lock);
+ orig_parent = i3c_hub_update_desc_parent(&desc->common, parent);
+ i3c_dev_free_ibi_locked(desc);
+ i3c_master_direct_detach_i3c_dev_locked(desc);
+ i3c_hub_update_desc_parent(&desc->common, orig_parent);
+ up_write(&parent->bus.lock);
+
+ i3c_hub_disable_port(controller);
+}
+
+/**
+ * i3c_hub_enable_ibi() - Enable IBI via hub port
+ * @desc: Target device descriptor
+ *
+ * Enables port connection and forwards the IBI enable request to the parent
+ * controller.
+ */
+static int i3c_hub_enable_ibi(struct i3c_dev_desc *desc)
+{
+ struct i3c_master_controller *controller = i3c_dev_get_master(desc);
+ struct i3c_hub_controller *hub_controller;
+ struct i3c_master_controller *orig_parent;
+ struct i3c_master_controller *parent;
+ struct i3c_hub *hub;
+ int ret;
+
+ hub_controller = dev_get_drvdata(&controller->dev);
+ if (!hub_controller || !hub_controller->hub)
+ return -ENODEV;
+
+ hub = hub_controller->hub;
+
+ if (!hub->hub_dev)
+ return -ENODEV;
+
+ parent = i3c_dev_get_master(hub->hub_dev->desc);
+
+ i3c_hub_enable_port(controller);
+
+ orig_parent = i3c_hub_update_desc_parent(&desc->common, parent);
+
+ down_write(&parent->bus.lock);
+ ret = i3c_dev_enable_ibi_locked(desc);
+ up_write(&parent->bus.lock);
+
+ i3c_hub_update_desc_parent(&desc->common, orig_parent);
+
+ i3c_hub_disable_port(controller);
+
+ return ret;
+}
+
+/**
+ * i3c_hub_disable_ibi() - Disable IBI via hub port
+ * @desc: Target device descriptor
+ *
+ * Enables port connection and forwards the IBI disable request to the parent
+ * controller.
+ */
+static int i3c_hub_disable_ibi(struct i3c_dev_desc *desc)
+{
+ struct i3c_master_controller *controller = i3c_dev_get_master(desc);
+ struct i3c_hub_controller *hub_controller;
+ struct i3c_master_controller *orig_parent;
+ struct i3c_master_controller *parent;
+ struct i3c_hub *hub;
+ int ret;
+
+ hub_controller = dev_get_drvdata(&controller->dev);
+ if (!hub_controller || !hub_controller->hub)
+ return -ENODEV;
+
+ hub = hub_controller->hub;
+
+ if (!hub->hub_dev)
+ return -ENODEV;
+
+ parent = i3c_dev_get_master(hub->hub_dev->desc);
+
+ i3c_hub_enable_port(controller);
+
+ orig_parent = i3c_hub_update_desc_parent(&desc->common, parent);
+
+ down_write(&parent->bus.lock);
+ ret = i3c_dev_disable_ibi_locked(desc);
+ up_write(&parent->bus.lock);
+
+ i3c_hub_update_desc_parent(&desc->common, orig_parent);
+
+ i3c_hub_disable_port(controller);
+
+ return ret;
+}
+
+static void i3c_hub_recycle_ibi_slot(struct i3c_dev_desc *desc,
+ struct i3c_ibi_slot *slot)
+{
+ /*
+ * TODO: Forward IBI slot recycling to the parent controller when
+ * downstream-device IBI support is fully enabled. Current users only
+ * rely on the hub device IBI path for SMBus-agent events, so no parent
+ * hardware slot is recycled through this virtual callback currently.
+ */
+}
+
+static const struct i3c_master_controller_ops i3c_hub_master_ops_data = {
+ .bus_init = i3c_hub_master_bus_init,
+ .bus_cleanup = i3c_hub_master_bus_cleanup,
+ .attach_i3c_dev = i3c_hub_attach_i3c_dev,
+ .reattach_i3c_dev = i3c_hub_reattach_i3c_dev,
+ .detach_i3c_dev = i3c_hub_detach_i3c_dev,
+ .do_daa = i3c_hub_master_do_daa,
+ .supports_ccc_cmd = i3c_hub_master_supports_ccc_cmd,
+ .send_ccc_cmd = i3c_hub_master_send_ccc_cmd,
+ .i3c_xfers = i3c_hub_master_priv_xfers,
+ .attach_i2c_dev = i3c_hub_attach_i2c_dev,
+ .detach_i2c_dev = i3c_hub_detach_i2c_dev,
+ .i2c_xfers = i3c_hub_i2c_xfers,
+ .request_ibi = i3c_hub_request_ibi,
+ .free_ibi = i3c_hub_free_ibi,
+ .enable_ibi = i3c_hub_enable_ibi,
+ .disable_ibi = i3c_hub_disable_ibi,
+ .recycle_ibi_slot = i3c_hub_recycle_ibi_slot,
+};
+
+/**
+ * i3c_hub_init() - Initialize hub context
+ * @hub: Hub instance
+ * @ops: Vendor callbacks
+ * @hub_dev: I3C hub device
+ */
+void i3c_hub_init(struct i3c_hub *hub,
+ const struct i3c_hub_ops *ops,
+ struct i3c_device *hub_dev)
+{
+ hub->ops = ops;
+ hub->hub_dev = hub_dev;
+ mutex_init(&hub->xfer_lock);
+}
+EXPORT_SYMBOL_GPL(i3c_hub_init);
+
+const struct i3c_master_controller_ops *i3c_hub_master_ops(void)
+{
+ return &i3c_hub_master_ops_data;
+}
+EXPORT_SYMBOL_GPL(i3c_hub_master_ops);
+
+/**
+ * i3c_hub_reserve_parent_addrslots_from_dt() - Reserve child addresses in parent bus.
+ * @hubc: I3C hub controller for a target-port virtual bus.
+ * @node: Target-port bus Device Tree node.
+ *
+ * Reserve parent bus address slots for downstream I3C devices that keep the
+ * same static and assigned dynamic address, so parent DAA does not reuse them.
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+int i3c_hub_reserve_parent_addrslots_from_dt(struct i3c_hub_controller *hubc,
+ struct device_node *node)
+{
+ struct i3c_master_controller *parent = hubc->parent;
+ enum i3c_addr_slot_status status;
+ u32 assigned_addr;
+ u8 static_addr;
+ u32 reg[3];
+ int ret;
+
+ if (!parent || !node)
+ return -ENODEV;
+
+ for_each_available_child_of_node_scoped(node, child) {
+ ret = of_property_read_variable_u32_array(child, "reg", reg, 1, 3);
+ if (ret < 0)
+ continue;
+
+ ret = of_property_read_u32(child, "assigned-address", &assigned_addr);
+ if (ret)
+ continue;
+
+ static_addr = reg[0];
+
+ if (!static_addr || !assigned_addr)
+ continue;
+
+ if (static_addr != assigned_addr)
+ continue;
+
+ down_write(&parent->bus.lock);
+ status = i3c_bus_get_addr_slot_status(&parent->bus,
+ assigned_addr);
+ if (status == I3C_ADDR_SLOT_FREE)
+ i3c_bus_set_addr_slot_status(&parent->bus,
+ assigned_addr,
+ I3C_ADDR_SLOT_I3C_DEV);
+ up_write(&parent->bus.lock);
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(i3c_hub_reserve_parent_addrslots_from_dt);
+
+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("Generic I3C hub support");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/i3c/hub.h b/include/linux/i3c/hub.h
new file mode 100644
index 000000000000..aa52548bc189
--- /dev/null
+++ b/include/linux/i3c/hub.h
@@ -0,0 +1,101 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2026 NXP
+ * Generic hub definitions and helper interfaces.
+ */
+#ifndef _LINUX_I3C_HUB_H
+#define _LINUX_I3C_HUB_H
+
+#include <linux/i3c/master.h>
+
+static inline struct i3c_master_controller *
+i3c_hub_update_desc_parent(struct i3c_i2c_dev_desc *desc,
+ struct i3c_master_controller *parent)
+{
+ struct i3c_master_controller *orig_parent = desc->master;
+
+ desc->master = parent;
+ return orig_parent;
+}
+
+/**
+ * struct i3c_hub - Generic I3C hub context
+ * @ops: Vendor callbacks for port connection control
+ * @hub_dev: I3C device representing the hub on the parent bus
+ */
+struct i3c_hub {
+ const struct i3c_hub_ops *ops;
+ struct i3c_device *hub_dev;
+ struct mutex xfer_lock; /* Serializes hub transfers. */
+};
+
+struct i3c_hub_controller {
+ struct i3c_master_controller *parent;
+ struct i3c_master_controller controller;
+ struct i3c_hub *hub;
+};
+
+struct i3c_hub_ops {
+ void (*enable_port)(struct i3c_master_controller *controller);
+ void (*disable_port)(struct i3c_master_controller *controller);
+};
+
+/**
+ * i3c_hub_enable_port() - Enable hub connection for a controller
+ * @controller: Virtual controller representing a hub port
+ *
+ * Retrieves hub context from controller drvdata and invokes the vendor
+ * callback to enable the associated port connection.
+ */
+static inline void i3c_hub_enable_port(struct i3c_master_controller *controller)
+{
+ struct i3c_hub_controller *hub_controller;
+ struct i3c_hub *hub;
+
+ hub_controller = dev_get_drvdata(&controller->dev);
+ if (!hub_controller || !hub_controller->hub)
+ return;
+
+ hub = hub_controller->hub;
+
+ if (hub && hub->ops && hub->ops->enable_port)
+ hub->ops->enable_port(controller);
+}
+
+/**
+ * i3c_hub_disable_port() - Disable hub connection for a controller
+ * @controller: Virtual controller representing a hub port
+ *
+ * Retrieves hub context from controller drvdata and invokes the vendor
+ * callback to disable the associated port connection.
+ */
+static inline void i3c_hub_disable_port(struct i3c_master_controller *controller)
+{
+ struct i3c_hub_controller *hub_controller;
+ struct i3c_hub *hub;
+
+ hub_controller = dev_get_drvdata(&controller->dev);
+ if (!hub_controller || !hub_controller->hub)
+ return;
+
+ hub = hub_controller->hub;
+
+ if (hub && hub->ops && hub->ops->disable_port)
+ hub->ops->disable_port(controller);
+}
+
+/**
+ * i3c_hub_master_ops() - Return virtual controller ops for hub ports
+ *
+ * Provides i3c_master_controller_ops used by controllers created for hub
+ * ports.
+ */
+const struct i3c_master_controller_ops *i3c_hub_master_ops(void);
+
+void i3c_hub_init(struct i3c_hub *hub,
+ const struct i3c_hub_ops *ops,
+ struct i3c_device *hub_dev);
+
+int i3c_hub_reserve_parent_addrslots_from_dt(struct i3c_hub_controller *hubc,
+ struct device_node *node);
+#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v13 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
2026-07-01 6:57 [PATCH v13 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
` (4 preceding siblings ...)
2026-07-01 6:57 ` [PATCH v13 5/7] i3c: hub: Add support for the I3C interface in the I3C hub Lakshay Piplani
@ 2026-07-01 6:57 ` Lakshay Piplani
2026-07-01 7:21 ` sashiko-bot
2026-07-01 6:57 ` [PATCH v13 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
6 siblings, 1 reply; 18+ messages in thread
From: Lakshay Piplani @ 2026-07-01 6:57 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, krzk+dt, robh,
conor+dt, devicetree, broonie, 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, I2C, or SMBus.
This driver enables:
- I3C/I2C communication between host and hub
- Transparent communication with downstream devices
- Target port configuration (I3C/I2C/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>
---
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 | 1 +
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 | 325 +++++++++++++++++++
drivers/i3c/hub/p3h2840_i3c_hub_common.c | 382 +++++++++++++++++++++++
drivers/i3c/hub/p3h2840_i3c_hub_i3c.c | 134 ++++++++
drivers/i3c/hub/p3h2840_i3c_hub_smbus.c | 297 ++++++++++++++++++
9 files changed, 1156 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 a0b7fd85f8e3..718c5fca4746 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19312,6 +19312,7 @@ 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
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..f725f3e2bfbe
--- /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. The Say Y or M here to use I3C
+ HUB driver to configure I3C HUB device.
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..d922c6b3b6be
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub.h
@@ -0,0 +1,325 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2025-2026 NXP
+ * This header file contain private device structure definition.
+ */
+
+#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 GENMASK(3, 1)
+#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 P3H2X4X_TP_MODE_I3C 0x00
+#define P3H2X4X_TP_MODE_SMBUS 0x01
+#define P3H2X4X_TP_MODE_GPIO 0x02
+#define P3H2X4X_TP_MODE_I2C 0x03
+
+#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_BUFFER_STATUS_MASK 0x0f
+#define P3H2X4X_TP_TRANSACTION_CODE_MASK 0xf0
+
+/* SMBus transaction types fields */
+#define P3H2X4X_SMBUS_400kHz BIT(2)
+
+/* SMBus polling */
+#define P3H2X4X_POLLING_ROLL_PERIOD_MS 10
+
+/* 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)
+
+/* Hub SMBus transaction time */
+#define P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(x) ((20 * (x)) + 80)
+
+#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,
+};
+
+struct tp_configuration {
+ bool pullup_en;
+ bool ibi_en;
+ bool always_enable;
+ int 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..8c029dc399d3
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
@@ -0,0 +1,382 @@
+// 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 == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ if (ret && ret != -ENODEV)
+ dev_warn(dev, "Failed to enable %s (%d)\n",
+ supplies[i], ret);
+ }
+
+ /* This delay is required for the regulator to stabilize its output voltage */
+ fsleep(5000);
+
+ return 0;
+}
+
+static int p3h2x4x_configure_tp(struct device *dev)
+{
+ struct p3h2x4x_i3c_hub_dev *hub = dev_get_drvdata(dev);
+ u8 mode = 0, smbus = 0, pullup = 0, target_port = 0;
+ int tp, ret;
+
+ for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; 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;
+ }
+
+ ret = regmap_update_bits(hub->regmap, P3H2X4X_TP_PULLUP_EN, P3H2X4X_TP_MASK, pullup);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(hub->regmap, P3H2X4X_TP_IO_MODE_CONF, P3H2X4X_TP_MASK, mode);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(hub->regmap, P3H2X4X_TP_SMBUS_AGNT_EN, P3H2X4X_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, P3H2X4X_TP_MASK, target_port);
+}
+
+static int p3h2x4x_configure_hw(struct device *dev)
+{
+ int ret;
+
+ ret = p3h2x4x_configure_ldo(dev);
+ if (ret)
+ return ret;
+
+ ret = p3h2x4x_configure_pullup(dev);
+ if (ret)
+ return ret;
+
+ ret = p3h2x4x_configure_io_strength(dev);
+ if (ret)
+ return ret;
+
+ return p3h2x4x_configure_tp(dev);
+}
+
+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);
+ u64 tp_port;
+
+ for_each_available_child_of_node_scoped(node, dev_node) {
+ if (of_property_read_reg(dev_node, 0, &tp_port, NULL))
+ continue;
+
+ if (tp_port < P3H2X4X_TP_MAX_COUNT) {
+ 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 void p3h2x4x_parse_tp_dt_settings(struct device *dev,
+ const struct device_node *node,
+ struct tp_configuration tp_config[])
+{
+ u64 id;
+
+ for_each_available_child_of_node_scoped(node, tp_node) {
+ if (of_property_read_reg(tp_node, 0, &id, NULL))
+ continue;
+
+ if (id >= P3H2X4X_TP_MAX_COUNT) {
+ dev_warn(dev, "Invalid target port index found in DT: %lli\n", id);
+ continue;
+ }
+
+ if (strcmp(tp_node->name, "i3c") == 0)
+ tp_config[id].mode = P3H2X4X_TP_MODE_I3C;
+
+ if (strcmp(tp_node->name, "i2c") == 0)
+ tp_config[id].mode = P3H2X4X_TP_MODE_I2C;
+
+ if (strcmp(tp_node->name, "smbus") == 0)
+ tp_config[id].mode = P3H2X4X_TP_MODE_SMBUS;
+
+ tp_config[id].pullup_en =
+ of_property_read_bool(tp_node, "nxp,pullup-enable");
+ }
+}
+
+static void 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);
+
+ 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_lock_registers_action(void *data)
+{
+ struct p3h2x4x_i3c_hub_dev *hub = data;
+
+ regmap_write(hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_LOCK_CODE);
+}
+
+static int p3h2x4x_i3c_hub_probe(struct platform_device *pdev)
+{
+ struct p3h2x4x_dev *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");
+
+ p3h2x4x_get_hub_dt_conf(dev, node);
+ 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;
+
+ /* Unlock access to protected registers */
+ ret = regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_UNLOCK_CODE);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to unlock HUB's protected registers\n");
+
+ ret = devm_add_action_or_reset(dev, p3h2x4x_lock_registers_action,
+ 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;
+ i3cdev_set_drvdata(p3h2x4x->i3cdev, p3h2x4x_i3c_hub);
+ 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;
+
+ /* Lock access to protected registers */
+ ret = regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
+ P3H2X4X_REGISTERS_LOCK_CODE);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to lock HUB's protected registers\n");
+
+ return 0;
+}
+
+static const struct platform_device_id p3h2x4x_i3c_hub_id[] = {
+ { "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..2d069eef8f1d
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
@@ -0,0 +1,134 @@
+// 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);
+}
+
+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 master for target port who
+ * configured as i3c.
+ * @p3h2x4x_hub: p3h2x4x device structure.
+ * Return: 0 in case of success, negative error code on failur.
+ */
+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 device_node *orig_of_node;
+ 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);
+
+ for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; tp++) {
+ if (!p3h2x4x_hub->tp_bus[tp].of_node ||
+ p3h2x4x_hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C)
+ continue;
+
+ /*
+ * i3c_master_register() derives bus population from the parent device's
+ * OF node. Temporarily switch the hub device to the target-port node for
+ * registration, then restore the original node immediately.
+ */
+ orig_of_node = p3h2x4x_hub->dev->of_node;
+ p3h2x4x_hub->dev->of_node = p3h2x4x_hub->tp_bus[tp].of_node;
+
+ 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;
+
+ ret = i3c_master_register(controller,
+ p3h2x4x_hub->dev,
+ i3c_hub_master_ops(),
+ false);
+
+ p3h2x4x_hub->dev->of_node = orig_of_node;
+
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(p3h2x4x_hub->dev,
+ p3h2x4x_unregister_i3c_master,
+ controller);
+ if (ret)
+ return ret;
+
+ /* Perform DAA */
+ ret = i3c_master_do_daa(parent);
+ if (ret)
+ return ret;
+
+ ntwk_mask |= p3h2x4x_hub->tp_bus[tp].tp_mask;
+ p3h2x4x_hub->tp_bus[tp].is_registered = true;
+ p3h2x4x_hub->hub_config.tp_config[tp].always_enable = true;
+ }
+ return regmap_write(p3h2x4x_hub->regmap, P3H2X4X_TP_NET_CON_CONF, ntwk_mask);
+}
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..fd000ff63507
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
@@ -0,0 +1,297 @@
+// 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)
+{
+ u32 status_read;
+ u8 status;
+ int ret;
+
+ ret = regmap_read_poll_timeout(hub->regmap, target_port_status,
+ status_read,
+ status_read & P3H2X4X_SMBUS_TRANSACTION_FINISH_FLAG,
+ P3H2X4X_POLLING_ROLL_PERIOD_MS * 1000,
+ P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(data_length));
+ 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,
+ u8 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;
+
+ ret = regmap_write(p3h2x4x_i3c_hub->regmap, target_port_status,
+ P3H2X4X_TP_BUFFER_STATUS_MASK);
+ 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) {
+ ret = regmap_bulk_read(p3h2x4x_i3c_hub->regmap,
+ P3H2X4X_CONTROLLER_AGENT_BUFF_DATA + write_length,
+ xfers[nxfers_i].buf, xfers[nxfers_i].len);
+ 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;
+ u8 msg_count, rw;
+
+ struct tp_bus *bus = i2c_get_adapdata(adap);
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = bus->p3h2x4x_i3c_hub;
+
+ guard(mutex)(&p3h2x4x_i3c_hub->etx_mutex);
+ guard(mutex)(&bus->port_mutex);
+
+ for (msg_count = 0; msg_count < num; msg_count++) {
+ if (msgs[msg_count].len > P3H2X4X_SMBUS_PAYLOAD_SIZE) {
+ dev_err(p3h2x4x_i3c_hub->dev,
+ "Message nr. %d not sent - length over %d bytes.\n",
+ msg_count, P3H2X4X_SMBUS_PAYLOAD_SIZE);
+ return -EINVAL;
+ }
+
+ 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;
+}
+
+/*
+ * 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)
+{
+ u8 tp;
+
+ for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; tp++) {
+ if (!hub->tp_bus[tp].tp_smbus_adapter)
+ continue;
+
+ i2c_del_adapter(hub->tp_bus[tp].tp_smbus_adapter);
+ hub->tp_bus[tp].tp_smbus_adapter = NULL;
+ hub->tp_bus[tp].is_registered = false;
+ }
+}
+
+/**
+ * p3h2x4x_tp_smbus_algo - add i2c adapter for target port who
+ * configured as SMBus.
+ * @hub: p3h2x4x device structure.
+ * Return: 0 in case of success, negative error code on failur.
+ */
+int p3h2x4x_tp_smbus_algo(struct p3h2x4x_i3c_hub_dev *hub)
+{
+ int ret;
+ u8 tp;
+
+ for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; 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)
+ return -ENOMEM;
+
+ /* Initialize adapter */
+ smbus_adapter->owner = THIS_MODULE;
+ smbus_adapter->class = I2C_CLASS_HWMON;
+ smbus_adapter->algo = &p3h2x4x_tp_i2c_algorithm;
+ 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]);
+
+ /* Register adapter */
+ ret = i2c_add_adapter(smbus_adapter);
+ if (ret) {
+ p3h2x4x_unregister_smbus_adapters(hub);
+ return ret;
+ }
+
+ hub->tp_bus[tp].is_registered = true;
+ hub->hub_config.tp_config[tp].ibi_en = false;
+ hub->tp_bus[tp].tp_smbus_adapter = smbus_adapter;
+ }
+
+ /*
+ * 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.
+ */
+ 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;
+ }
+
+ ret = regmap_write(hub->regmap, P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG, P3H2X4X_IBI_DISABLED);
+ if (ret)
+ p3h2x4x_unregister_smbus_adapters(hub);
+
+ return ret;
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v13 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support
2026-07-01 6:57 [PATCH v13 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
` (5 preceding siblings ...)
2026-07-01 6:57 ` [PATCH v13 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
@ 2026-07-01 6:57 ` Lakshay Piplani
2026-07-01 7:21 ` sashiko-bot
6 siblings, 1 reply; 18+ messages in thread
From: Lakshay Piplani @ 2026-07-01 6:57 UTC (permalink / raw)
To: linux-kernel, linux-i3c, alexandre.belloni, krzk+dt, robh,
conor+dt, devicetree, broonie, 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: Lakshay Piplani <lakshay.piplani@nxp.com>
Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
Signed-off-by: Vikash Bansal <vikash.bansal@nxp.com>
---
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 | 13 ++
drivers/i3c/hub/p3h2840_i3c_hub_i3c.c | 48 +++++-
drivers/i3c/hub/p3h2840_i3c_hub_smbus.c | 209 ++++++++++++++++++++++++
3 files changed, 269 insertions(+), 1 deletion(-)
diff --git a/drivers/i3c/hub/p3h2840_i3c_hub.h b/drivers/i3c/hub/p3h2840_i3c_hub.h
index d922c6b3b6be..9fcf6db15547 100644
--- a/drivers/i3c/hub/p3h2840_i3c_hub.h
+++ b/drivers/i3c/hub/p3h2840_i3c_hub.h
@@ -297,6 +297,9 @@ 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;
+#endif
struct i3c_hub *hub;
};
@@ -322,4 +325,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 2d069eef8f1d..91ce06ee82ca 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)
{
@@ -54,6 +62,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 master for target port who
* configured as i3c.
@@ -130,5 +148,33 @@ int p3h2x4x_tp_i3c_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_hub)
p3h2x4x_hub->tp_bus[tp].is_registered = true;
p3h2x4x_hub->hub_config.tp_config[tp].always_enable = true;
}
- return regmap_write(p3h2x4x_hub->regmap, P3H2X4X_TP_NET_CON_CONF, ntwk_mask);
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ 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
+ ret = regmap_write(p3h2x4x_hub->regmap, P3H2X4X_TP_NET_CON_CONF, ntwk_mask);
+
+ return ret;
}
diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
index fd000ff63507..f752cbe35c48 100644
--- a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
@@ -15,6 +15,157 @@ 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, bool is_of)
+{
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = i3cdev_get_drvdata(i3cdev);
+ u8 slave_rx_buffer[P3H2X4X_SMBUS_TARGET_PAYLOAD_SIZE] = { 0 };
+ u8 target_buffer_page, flag_clear = 0, temp = 0, i, addr;
+ u32 packet_len, slave_address, ret;
+ struct i2c_client *client;
+
+ target_buffer_page = (((rfbuf) ? P3H2X4X_TARGET_BUFF_1_PAGE : P3H2X4X_TARGET_BUFF_0_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;
+ }
+
+ if (is_of)
+ flag_clear = BUF_RECEIVED_FLAG_TF_MASK;
+ else
+ flag_clear = (((rfbuf == RCV_BUF_0) ? P3H2X4X_TARGET_BUF_0_RECEIVE :
+ P3H2X4X_TARGET_BUF_1_RECEIVE));
+
+ 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);
+
+ if (flag_clear)
+ 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_i3c_hub_dev *p3h2x4x_i3c_hub;
+ u8 payload_byte_one, payload_byte_two;
+ u32 target_port_status, ret, i;
+ const u8 *data;
+
+ 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 = i3cdev_get_drvdata(i3cdev);
+
+ 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_TP_MAX_COUNT; ++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;
+ }
+
+ /* process data receive buffer */
+ switch (target_port_status & BUF_RECEIVED_FLAG_MASK) {
+ case P3H2X4X_TARGET_BUF_CA_TF:
+ break;
+ case P3H2X4X_TARGET_BUF_0_RECEIVE:
+ p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_0, i, false);
+ break;
+ case P3H2X4X_TARGET_BUF_1_RECEIVE:
+ p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_1, i, false);
+ break;
+ case P3H2X4X_TARGET_BUF_0_1_RECEIVE:
+ p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_0, i, false);
+ p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_1, i, false);
+ break;
+ case P3H2X4X_TARGET_BUF_OVRFL:
+ p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_0, i, false);
+ p3h2x4x_read_smbus_agent_rx_buf(i3cdev, RCV_BUF_1, i, true);
+ dev_err(&i3cdev->dev, "Overflow, reading buffer zero and one\n");
+ break;
+ default:
+ regmap_write(p3h2x4x_i3c_hub->regmap,
+ P3H2X4X_TP0_SMBUS_AGNT_STS + i,
+ BUF_RECEIVED_FLAG_TF_MASK);
+ break;
+ }
+ }
+}
+#endif
+
static int p3h2x4x_read_smbus_transaction_status(struct p3h2x4x_i3c_hub_dev *hub,
u8 target_port_status,
u8 data_length)
@@ -207,11 +358,67 @@ static u32 p3h2x4x_tp_smbus_funcs(struct i2c_adapter *adapter)
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BLOCK_DATA;
}
+#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;
+ int ret;
+
+ guard(mutex)(&hub->etx_mutex);
+
+ if (!hub->i3cdev || !hub->ibi_ready)
+ return -EOPNOTSUPP;
+
+ if (bus->tp_smbus_client)
+ return -EBUSY;
+
+ ret = regmap_set_bits(hub->regmap,
+ P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG,
+ bus->tp_mask);
+ if (ret)
+ return ret;
+
+ 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;
+ int ret;
+
+ guard(mutex)(&hub->etx_mutex);
+
+ if (bus->tp_smbus_client != slave)
+ return -EINVAL;
+
+ bus->tp_smbus_client = NULL;
+ hub->hub_config.tp_config[bus->tp_port].ibi_en = false;
+
+ ret = regmap_clear_bits(hub->regmap,
+ P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG,
+ bus->tp_mask);
+
+ if (ret)
+ return ret;
+
+ return 0;
+}
+#endif
+
/*
* I2C algorithm Structure
*/
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,
};
@@ -224,6 +431,8 @@ void p3h2x4x_unregister_smbus_adapters(struct p3h2x4x_i3c_hub_dev *hub)
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;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v13 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
2026-07-01 6:57 ` [PATCH v13 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support Lakshay Piplani
@ 2026-07-01 7:13 ` sashiko-bot
0 siblings, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-07-01 7:13 UTC (permalink / raw)
To: Lakshay Piplani
Cc: conor+dt, linux-i3c, robh, Frank.Li, Alexandre Belloni,
devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The device tree binding advertises GPIO capabilities in the hardware description but omits the required GPIO controller properties from the schema.
- [Medium] The binding schema fails to include compatible strings for all hardware variants claimed in the commit message.
--
commit 02eaa0b7ff6f65192cd318d6704be98b321762da
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
Add bindings for the NXP P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841)
multiport I3C hub family.
> diff --git a/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
[ ... ]
> +description: |
> + P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841) is a family of multiport I3C
> + hub devices that connect to:-
> + 1. A host CPU via I3C/I2C/SMBus bus on upstream side and connect to multiple
> + peripheral devices on the downstream side.
> + 2. Have two Controller Ports which can support either
> + I2C/SMBus or I3C buses and connect to a CPU, BMC or SOC.
> + 3. P3H2840/ P3H2841 are 8 port I3C hub with eight I3C/I2C Target Port.
> + 4. P3H2440/ P3H2441 are 4 port I3C hub with four I3C/I2C Target Port.
> + Target ports can be configured as I2C/SMBus, I3C or GPIO and connect to
> + peripherals.
[Severity: Medium]
The description states that target ports can be configured as GPIO.
Since unevaluatedProperties is set to false, shouldn't this schema include the
standard GPIO provider properties (like gpio-controller and #gpio-cells)
so device trees can actually configure and utilize the advertised GPIO
capabilities without triggering validation errors?
> +
> +properties:
> + compatible:
> + const: nxp,p3h2840
[Severity: Medium]
The commit message and schema description indicate support for the P3H2x4x
family, including the P3H2440, P3H2441, and P3H2841. Does this compatible block
need to be expanded from a const constraint to an enum to allow strings for
those other variants?
Also, if the 4-port variants are added, would they require different schema
constraints, such as a maximum port reg of 3 rather than 7 for the
patternProperties?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701065755.2067793-1-lakshay.piplani@nxp.com?part=2
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v13 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
2026-07-01 6:57 ` [PATCH v13 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
@ 2026-07-01 7:16 ` sashiko-bot
0 siblings, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-07-01 7:16 UTC (permalink / raw)
To: Lakshay Piplani
Cc: Alexandre Belloni, Frank.Li, conor+dt, robh, devicetree,
linux-i3c
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double-unlock and missing error propagation in `p3h2x4x_reg` lock guard on `regmap_read()` failure.
--
commit ec428ff479eee2740ff39044d74b67527f18c3e6
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
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.
> diff --git a/drivers/regulator/p3h2840_i3c_hub_regulator.c b/drivers/regulator/p3h2840_i3c_hub_regulator.c
> new file mode 100644
> index 0000000000000..781622a6ff467
> --- /dev/null
> +++ b/drivers/regulator/p3h2840_i3c_hub_regulator.c
[ ... ]
> +static void p3h2x4x_reg_guard_enter(struct regulator_dev *rdev,
> + struct p3h2x4x_reg_state *state)
> +{
> + struct p3h2x4x_regulator_dev *priv = rdev_get_drvdata(rdev);
> +
> + mutex_lock(&priv->lock);
> + state->restore = false;
> +
> + if (regmap_read(rdev->regmap,
> + P3H2X4X_DEV_REG_PROTECTION_CODE,
> + &state->orig)) {
> + mutex_unlock(&priv->lock);
> + return;
> + }
[Severity: High]
If regmap_read() fails here, we unlock the mutex and return.
Since this function is used as the lock guard constructor, the caller is not
aware of the failure. Does this allow the caller to proceed and execute the
guarded critical section without holding the lock?
For example, in p3h2x4x_regulator_enable():
guard(p3h2x4x_reg)(rdev);
return regulator_enable_regmap(rdev);
Would regulator_enable_regmap() execute concurrently without the lock if the
initialization read fails?
[ ... ]
> +static void p3h2x4x_reg_guard_exit(struct regulator_dev *rdev,
> + struct p3h2x4x_reg_state *state)
> +{
> + struct p3h2x4x_regulator_dev *priv = rdev_get_drvdata(rdev);
> +
> + if (state->restore)
> + regmap_write(rdev->regmap,
> + P3H2X4X_DEV_REG_PROTECTION_CODE,
> + state->orig);
> +
> + mutex_unlock(&priv->lock);
> +}
[Severity: High]
When the lock guard goes out of scope, p3h2x4x_reg_guard_exit() is invoked
unconditionally.
If we previously hit the regmap_read() error path in the constructor and
already unlocked the mutex, will this second call to mutex_unlock(&priv->lock)
cause a double-unlock?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701065755.2067793-1-lakshay.piplani@nxp.com?part=4
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v13 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support
2026-07-01 6:57 ` [PATCH v13 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
@ 2026-07-01 7:21 ` sashiko-bot
2026-07-01 20:01 ` Frank Li
0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-07-01 7:21 UTC (permalink / raw)
To: Lakshay Piplani
Cc: devicetree, Frank.Li, conor+dt, Alexandre Belloni, linux-i3c,
robh
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Early error paths in `p3h2x4x_read_smbus_agent_rx_buf` fail to clear the hardware buffer receive flag, resulting in an infinite In-Band Interrupt (IBI) storm.
- [High] Hardware IBI configuration is unconditionally overwritten after exposing I2C adapters, potentially silently disabling slave interrupts.
- [Medium] The `P3H2X4X_TARGET_BUF_CA_TF` case in the IBI handler is unreachable because its value is masked out by the switch expression.
--
commit f1791953bf7e067a7ec560f25d66e9566565ed9b
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.
It allows the hub SMBus slave agent to receive downstream payloads and
report receive events through In-Band Interrupts (IBI). The payloads are
then forwarded to the registered I2C slave client.
> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> index fd000ff635077..f752cbe35c482 100644
> --- a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> @@ -15,6 +15,157 @@ enum p3h2x4x_smbus_desc_idx {
[ ... ]
> +static void p3h2x4x_read_smbus_agent_rx_buf(struct i3c_device *i3cdev, enum p3h2x4x_rcv_buf rfbuf,
> + enum p3h2x4x_tp tp, bool is_of)
> +{
> + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = i3cdev_get_drvdata(i3cdev);
> + u8 slave_rx_buffer[P3H2X4X_SMBUS_TARGET_PAYLOAD_SIZE] = { 0 };
> + u8 target_buffer_page, flag_clear = 0, temp = 0, i, addr;
> + u32 packet_len, slave_address, ret;
> + struct i2c_client *client;
[ ... ]
> + /* 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;
[Severity: High]
Since flag_clear is initialized to 0, if an error path like this one jumps
to the ibi_err label early, will it skip setting flag_clear entirely?
If flag_clear remains 0, the cleanup code below won't write to the
P3H2X4X_TP0_SMBUS_AGNT_STS register to clear the hardware interrupt buffer
receive flag. Could this cause the unserviced buffer status to remain active
in the hardware and result in an continuous IBI interrupt storm?
> + }
[ ... ]
> +ibi_err:
> + regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_PAGE_PTR, 0x00);
> +
> + if (flag_clear)
> + regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP0_SMBUS_AGNT_STS + tp, flag_clear);
> +}
[ ... ]
> +void p3h2x4x_ibi_handler(struct i3c_device *i3cdev,
> + const struct i3c_ibi_payload *payload)
> +{
[ ... ]
> + 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;
> + }
> +
> + /* process data receive buffer */
> + switch (target_port_status & BUF_RECEIVED_FLAG_MASK) {
> + case P3H2X4X_TARGET_BUF_CA_TF:
[Severity: Medium]
Is this case reachable?
If BUF_RECEIVED_FLAG_MASK is defined as 0xE (GENMASK(3, 1)) and
P3H2X4X_TARGET_BUF_CA_TF is defined as 0x1 (BIT(0)), a bitwise AND with 0xE
will always clear bit 0.
Will the switch expression ever evaluate to 0x1, or does this unintentionally
fall through to the default case which clears all bits by writing
BUF_RECEIVED_FLAG_TF_MASK?
> + break;
> + case P3H2X4X_TARGET_BUF_0_RECEIVE:
[ ... ]
> +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;
> + int ret;
> +
> + guard(mutex)(&hub->etx_mutex);
> +
> + if (!hub->i3cdev || !hub->ibi_ready)
> + return -EOPNOTSUPP;
> +
> + if (bus->tp_smbus_client)
> + return -EBUSY;
> +
> + ret = regmap_set_bits(hub->regmap,
> + P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG,
> + bus->tp_mask);
> + if (ret)
> + return ret;
> +
> + bus->tp_smbus_client = slave;
> + hub->hub_config.tp_config[bus->tp_port].ibi_en = true;
> +
> + return 0;
> +}
[Severity: High]
When p3h2x4x_tp_smbus_algo() loops over target ports and exposes I2C adapters
via i2c_add_adapter(), this can trigger synchronous driver binding. A slave
driver probing could call i2c_slave_register(), which calls this function to
enable its IBI by setting a bit in P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG.
However, if p3h2x4x_tp_smbus_algo() unconditionally writes
P3H2X4X_IBI_DISABLED (0) to P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG after the adapter
registration loop finishes, will this silently overwrite and clear the
interrupt configurations just set by any concurrently probing slave drivers?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701065755.2067793-1-lakshay.piplani@nxp.com?part=7
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v13 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
2026-07-01 6:57 ` [PATCH v13 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
@ 2026-07-01 7:21 ` sashiko-bot
2026-07-01 19:56 ` Frank Li
0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-07-01 7:21 UTC (permalink / raw)
To: Lakshay Piplani
Cc: devicetree, robh, conor+dt, linux-i3c, Frank.Li,
Alexandre Belloni
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Unprotected and temporary reassignment of a live device's `of_node` creates a data race, and missing restoration on the error path causes reference count leaks and a double-free.
- [High] `regmap_read_poll_timeout` is configured with a 10ms sleep interval that vastly exceeds the actual total timeout, artificially delaying every transaction and causing spurious timeouts.
- [High] Failure to allocate memory for an I2C adapter leaves previously registered adapters active but with freed backing memory, leading to a Use-After-Free.
- [Medium] Loop counters `msg_count` and `nxfers_i` are `u8`, causing infinite loops if the transfer count `num` is >= 256.
- [Medium] Race condition between the I3C hub driver and other MFD child drivers over the shared `P3H2X4X_DEV_REG_PROTECTION_CODE` register.
--
commit 37198d19f613736872b6f1fd4f9a75597b1f0ab1
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
This patch adds I3C hub functionality for the NXP P3H2x4x family of multiport
hubs. It enables I3C/I2C communication between host and hub, downstream device
support, and target port configuration.
> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_common.c b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
> --- a/drivers/i3c/hub/p3h2840_i3c_hub_common.c
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
[ ... ]
> +static int p3h2x4x_i3c_hub_probe(struct platform_device *pdev)
> +{
> + struct p3h2x4x_dev *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;
[ ... ]
> + /* Unlock access to protected registers */
> + ret = regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
> + P3H2X4X_REGISTERS_UNLOCK_CODE);
[Severity: Medium]
Could this global unlock race with other MFD child drivers (such as the
on-die regulator)?
Because the protected configuration registers are unlocked at the start of
p3h2x4x_i3c_hub_probe() and left unlocked during the entire hardware
configuration and device discovery window (which includes sleeping in
i3c_master_do_daa()), a concurrent MFD child driver could access its own
registers and re-lock the protection code. If that happens, subsequent
hardware configuration writes in this function might fail silently.
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to unlock HUB's protected registers\n");
> +
> + ret = devm_add_action_or_reset(dev, p3h2x4x_lock_registers_action,
> + 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");
[ ... ]
> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> --- a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
[ ... ]
> +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 device_node *orig_of_node;
> + u8 tp, ntwk_mask = 0;
> + int ret;
[ ... ]
> + for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; tp++) {
> + if (!p3h2x4x_hub->tp_bus[tp].of_node ||
> + p3h2x4x_hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C)
> + continue;
> +
> + /*
> + * i3c_master_register() derives bus population from the parent device's
> + * OF node. Temporarily switch the hub device to the target-port node for
> + * registration, then restore the original node immediately.
> + */
> + orig_of_node = p3h2x4x_hub->dev->of_node;
> + p3h2x4x_hub->dev->of_node = p3h2x4x_hub->tp_bus[tp].of_node;
> +
> + 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;
[Severity: High]
Will this leak the original parent node reference and cause a double-free on
the target node if i3c_hub_reserve_parent_addrslots_from_dt() returns an
error?
If the early return is taken, dev->of_node is never restored to
orig_of_node. When the probe unwinds, the devres cleanup function
p3h2x4x_put_target_port_of_nodes() drops the target node's reference.
Later, when the device core destroys the device, it will call
of_node_put(dev->of_node) which now incorrectly points to the target node
again, causing a double-free.
Concurrently, modifying dev->of_node without synchronization creates a data
race for concurrent sysfs or uevent reads.
> +
> + ret = i3c_master_register(controller,
> + p3h2x4x_hub->dev,
> + i3c_hub_master_ops(),
> + false);
> +
> + p3h2x4x_hub->dev->of_node = orig_of_node;
[ ... ]
> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> --- a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
[ ... ]
> +static int p3h2x4x_read_smbus_transaction_status(struct p3h2x4x_i3c_hub_dev *hub,
> + u8 target_port_status,
> + u8 data_length)
> +{
> + u32 status_read;
> + u8 status;
> + int ret;
> +
> + ret = regmap_read_poll_timeout(hub->regmap, target_port_status,
> + status_read,
> + status_read & P3H2X4X_SMBUS_TRANSACTION_FINISH_FLAG,
> + P3H2X4X_POLLING_ROLL_PERIOD_MS * 1000,
> + P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(data_length));
[Severity: High]
Does this polling configuration cause spurious timeouts or severe performance
penalties?
The sleep_us parameter is set to P3H2X4X_POLLING_ROLL_PERIOD_MS * 1000,
which is 10,000us (10ms). However, the total timeout_us parameter provided
by P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(data_length) is significantly smaller
(e.g., between 100us and 2,000us depending on data length).
If the transaction is not complete on the very first read, the thread will
sleep for a full 10ms, drastically overshooting the intended timeout and
potentially returning a spurious -ETIMEDOUT depending on the state when it
wakes up.
> + if (ret)
> + return ret;
[ ... ]
> +static s32 p3h2x4x_tp_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{
> + int ret_sum = 0, ret;
> + u8 msg_count, rw;
> +
> + struct tp_bus *bus = i2c_get_adapdata(adap);
> + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = bus->p3h2x4x_i3c_hub;
> +
> + guard(mutex)(&p3h2x4x_i3c_hub->etx_mutex);
> + guard(mutex)(&bus->port_mutex);
> +
> + for (msg_count = 0; msg_count < num; msg_count++) {
[Severity: Medium]
Can this loop run infinitely if the transfer count (num) is greater than
or equal to 256?
The parameter num is a signed 32-bit integer, but msg_count is declared as
u8. If an in-kernel driver submits an I2C transfer with 256 or more
messages, msg_count will wrap around to 0 before ever reaching num, making
the loop condition perpetually true and causing a CPU lockup.
> + if (msgs[msg_count].len > P3H2X4X_SMBUS_PAYLOAD_SIZE) {
> + dev_err(p3h2x4x_i3c_hub->dev,
> + "Message nr. %d not sent - length over %d bytes.\n",
> + msg_count, P3H2X4X_SMBUS_PAYLOAD_SIZE);
> + return -EINVAL;
> + }
[ ... ]
> +int p3h2x4x_tp_smbus_algo(struct p3h2x4x_i3c_hub_dev *hub)
> +{
> + int ret;
> + u8 tp;
> +
> + for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; 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)
> + return -ENOMEM;
[Severity: High]
Will this early return on allocation failure cause a use-after-free for
previously registered I2C adapters?
In this loop, if devm_kzalloc() fails for a given port, the function returns
-ENOMEM immediately without calling p3h2x4x_unregister_smbus_adapters().
Any adapters successfully registered in earlier loop iterations will remain
active in the I2C subsystem. Because the devres unregister action
(p3h2x4x_unregister_smbus_adapters_action) is only added at the end of the
probe() function, it will not execute. However, devres will automatically
free the memory allocated by devm_kzalloc(), leaving the I2C subsystem to
dereference freed memory when it interacts with those remaining dangling
adapters.
> +
> + /* Initialize adapter */
> + smbus_adapter->owner = THIS_MODULE;
> + smbus_adapter->class = I2C_CLASS_HWMON;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701065755.2067793-1-lakshay.piplani@nxp.com?part=6
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v13 1/7] i3c: master: Add APIs for I3C hub support
2026-07-01 6:57 ` [PATCH v13 1/7] i3c: master: Add APIs for I3C hub support Lakshay Piplani
@ 2026-07-01 7:22 ` sashiko-bot
2026-07-01 18:37 ` Frank Li
0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-07-01 7:22 UTC (permalink / raw)
To: Lakshay Piplani
Cc: robh, Frank.Li, conor+dt, linux-i3c, Alexandre Belloni,
devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Exported APIs interact with controller hardware without taking a Runtime PM reference.
- [Medium] `i3c_master_supports_ccc_cmd` incorrectly returns true for masters that do not implement CCC command sending.
--
commit b5579dc49beffe04e3498fcfb290372e74bd88f0
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
i3c: master: Add APIs for I3C hub support
This commit adds helper functions for attaching and detaching I3C devices,
checking CCC support, sending CCC commands, and managing address slot states.
These additions prepare the subsystem for I3C hub support.
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index f1be38a640ca1..3e7b7f7f97cfb 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> @@ -1775,6 +1790,83 @@ static int i3c_master_attach_i3c_dev(struct i3c_master_controller *master,
> return 0;
> }
>
> +/**
> + * i3c_master_direct_attach_i3c_dev_locked() - Attach an I3C device to a master
[ ... ]
> +int i3c_master_direct_attach_i3c_dev_locked(struct i3c_master_controller *master,
> + struct i3c_dev_desc *dev)
> +{
> + int ret;
> +
> + if (WARN_ON(i3c_dev_get_master(dev) != master))
> + return -EINVAL;
> +
> + ret = i3c_master_get_i3c_addrs(dev);
> + if (ret)
> + return ret;
> +
> + /* Do not attach the master device itself. */
> + if (master->this != dev && master->ops->attach_i3c_dev) {
> + ret = master->ops->attach_i3c_dev(dev);
[Severity: High]
Should this function call i3c_master_rpm_get() to take a runtime PM
reference before invoking the controller's attach_i3c_dev callback?
Since the I3C subsystem sets pm_suspend_ignore_children(&master->dev, true),
maintaining an active runtime PM reference on the hub device does not
automatically resume the parent controller. If an I3C hub calls this on a
parent master that is runtime suspended, accessing the hardware registers
could lead to bus faults or hangs.
> + if (ret) {
> + i3c_master_put_i3c_addrs(dev);
> + return ret;
> + }
> + }
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_direct_attach_i3c_dev_locked);
> +
> +/**
> + * i3c_master_direct_detach_i3c_dev_locked() - Detach an I3C device from a master
[ ... ]
> +void i3c_master_direct_detach_i3c_dev_locked(struct i3c_dev_desc *dev)
> +{
> + struct i3c_master_controller *master = i3c_dev_get_master(dev);
> +
> + /*
> + * We don't attach devices to the controller until they are
> + * addressable on the bus, so skip detach for unaddressed devices.
> + */
> + if (!dev->info.static_addr && !dev->info.dyn_addr)
> + return;
> +
> + /* Do not detach the master device itself. */
> + if (master->this != dev && master->ops->detach_i3c_dev)
> + master->ops->detach_i3c_dev(dev);
[Severity: High]
Similar to the attach path, does this detach callback require the parent
master to be resumed?
If the parent controller is runtime suspended, invoking detach_i3c_dev
without first taking a runtime PM reference might access suspended hardware.
> +
> + dev->common.master_priv = NULL;
> + i3c_master_put_i3c_addrs(dev);
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_direct_detach_i3c_dev_locked);
[ ... ]
> @@ -2548,6 +2640,50 @@ static void i3c_master_reconcile_dyn_addrs(struct i3c_master_controller *master)
> }
> }
>
> +/**
> + * i3c_master_supports_ccc_cmd() - check CCC command support
[ ... ]
> +bool i3c_master_supports_ccc_cmd(struct i3c_master_controller *master,
> + const struct i3c_ccc_cmd *cmd)
> +{
> + if (!master || !cmd)
> + return false;
> +
> + if (!master->ops->supports_ccc_cmd)
> + return true;
[Severity: Medium]
Is it safe to default to true without verifying that the controller
actually implements master->ops->send_ccc_cmd?
If a controller lacks send_ccc_cmd, it does not support sending any CCC
commands. The core will return -EOPNOTSUPP in i3c_master_send_ccc_cmd_locked(),
but returning true here might cause a hub controller to falsely advertise
CCC support for its downstream devices.
> +
> + return master->ops->supports_ccc_cmd(master, cmd);
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_supports_ccc_cmd);
> +
> +/**
> + * i3c_master_send_ccc_cmd() - send a CCC command
[ ... ]
> +int i3c_master_send_ccc_cmd(struct i3c_master_controller *master,
> + struct i3c_ccc_cmd *cmd)
> +{
> + int ret;
> +
> + i3c_bus_maintenance_lock(&master->bus);
> + ret = i3c_master_send_ccc_cmd_locked(master, cmd);
[Severity: High]
Does this function need to take a runtime PM reference before calling
i3c_master_send_ccc_cmd_locked()?
Like the attach/detach helpers, if an I3C hub calls this on a parent master
that is suspended, this could lead to hardware access while suspended.
> + i3c_bus_maintenance_unlock(&master->bus);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_send_ccc_cmd);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701065755.2067793-1-lakshay.piplani@nxp.com?part=1
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v13 5/7] i3c: hub: Add support for the I3C interface in the I3C hub
2026-07-01 6:57 ` [PATCH v13 5/7] i3c: hub: Add support for the I3C interface in the I3C hub Lakshay Piplani
@ 2026-07-01 7:23 ` sashiko-bot
2026-07-01 19:42 ` Frank Li
0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-07-01 7:23 UTC (permalink / raw)
To: Lakshay Piplani
Cc: linux-i3c, robh, Alexandre Belloni, devicetree, conor+dt,
Frank.Li
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] `i3c_hub_request_ibi()` intentionally frees `desc->ibi` out from under its caller without cleaning up its workqueue, causing a memory leak and a double-free on failure.
- [High] `i3c_hub_free_ibi()` causes a guaranteed NULL pointer dereference in the core IBI free path by recursively clearing `dev->ibi`.
- [High] Address spoofing in `i3c_hub_master_priv_xfers` corrupts parent bus address reservations and suffers from an unprotected race window.
- [High] Missing `xfer_lock` in several hub operations allows concurrent port enabling, breaking virtual bus isolation.
- [High] `i3c_hub_free_ibi()` permanently destroys downstream device address reservations on the parent bus.
- [High] `i3c_hub_do_daa` delegates DAA entirely to the parent controller, breaking boardinfo matching and DT topology for downstream devices.
- [Medium] Acquiring `parent->bus.lock` while holding `child->bus.lock` triggers a lockdep splat due to unannotated same-class lock nesting.
--
commit 22c2a31197952a434777498d93a1ed925009d6de
Author: Lakshay Piplani <lakshay.piplani@nxp.com>
Subject: i3c: hub: Add support for the I3C interface in the I3C hub
This patch adds virtual I3C bus support for the hub and provides an
interface to enable or disable downstream ports. Virtual master controllers
are created for the downstream ports, and bus operations are forwarded
through the hub device.
> diff --git a/drivers/i3c/hub.c b/drivers/i3c/hub.c
> new file mode 100644
> index 0000000000000..d29ed40b6f735
> --- /dev/null
> +++ b/drivers/i3c/hub.c
> @@ -0,0 +1,566 @@
[ ... ]
> +static int i3c_hub_do_daa(struct i3c_hub *hub,
> + struct i3c_master_controller *controller)
> +{
> + struct i3c_master_controller *parent;
> + int ret;
> +
> + if (!hub || !hub->hub_dev)
> + return -ENODEV;
> +
> + parent = i3c_dev_get_master(hub->hub_dev->desc);
> + if (!parent)
> + return -ENODEV;
> +
> + i3c_hub_enable_port(controller);
> + ret = i3c_master_do_daa(parent);
[Severity: High]
Does delegating DAA directly to the parent controller bypass the virtual hub
bus entirely?
By calling i3c_master_do_daa(parent), it appears the parent discovers and
attaches downstream devices to the physical bus instead of the virtual
controller. Does this cause boardinfo matching to fail since the devices
are defined under the hub's device tree node?
> + i3c_hub_disable_port(controller);
> +
> + return ret;
> +}
[ ... ]
> +static int i3c_hub_send_ccc_cmd(struct i3c_hub *hub,
> + struct i3c_master_controller *controller,
> + struct i3c_ccc_cmd *cmd)
> +{
> + struct i3c_master_controller *parent;
> + int ret;
> +
> + if (!hub || !hub->hub_dev)
> + return -ENODEV;
> +
> + parent = i3c_dev_get_master(hub->hub_dev->desc);
> + if (!parent)
> + return -ENODEV;
> +
> + i3c_hub_enable_port(controller);
> + ret = i3c_master_send_ccc_cmd(parent, cmd);
[Severity: High]
Could the omission of hub->xfer_lock in this function allow concurrent port
enabling?
The lock is used to serialize private transfers, but callbacks like
i3c_hub_send_ccc_cmd(), i3c_hub_do_daa(), and i3c_hub_enable_ibi() do not
acquire it. If a private transfer runs concurrently with a CCC command,
could multiple downstream ports be enabled simultaneously and cause the hub
to broadcast traffic incorrectly?
> + i3c_hub_disable_port(controller);
> +
> + return ret;
> +}
[ ... ]
> +static int i3c_hub_master_priv_xfers(struct i3c_dev_desc *dev,
> + struct i3c_xfer *xfers,
> + int nxfers,
> + enum i3c_xfer_mode mode)
> +{
> + struct i3c_master_controller *controller = i3c_dev_get_master(dev);
> + struct i3c_hub_controller *hub_controller;
> + struct i3c_dev_desc *hub_dev;
> + struct i3c_master_controller *parent;
> + u8 hub_addr, target_addr;
> + struct i3c_hub *hub;
> + int ret, xfer_ret;
[ ... ]
> + if (hub_addr != target_addr) {
> + down_write(&parent->bus.lock);
[Severity: Medium]
Will acquiring parent->bus.lock here trigger a lockdep warning?
Since virtual hub operations are invoked by the I3C core with the child
bus's lock already held, acquiring the parent bus's lock (which belongs to
the same lock class) without nesting annotations might produce a false
positive lockdep splat. This also happens in i3c_hub_enable_ibi() and
i3c_hub_disable_ibi().
> + hub_dev->info.dyn_addr = target_addr;
> +
> + ret = i3c_master_reattach_i3c_dev_locked(hub_dev, hub_addr);
> + if (ret) {
> + up_write(&parent->bus.lock);
> + hub_dev->info.dyn_addr = hub_addr;
> + goto disable;
> + }
> + up_write(&parent->bus.lock);
> + }
> +
> + ret = i3c_device_do_xfers(hub->hub_dev, xfers, nxfers, mode);
> +
> + if (hub_addr != target_addr) {
> + down_write(&parent->bus.lock);
> + hub_dev->info.dyn_addr = hub_addr;
> +
> + xfer_ret = i3c_master_reattach_i3c_dev_locked(hub_dev,
> + target_addr);
[Severity: High]
Does this sequence permanently destroy device tree static address
reservations?
When the hub's dynamic address is altered via
i3c_master_reattach_i3c_dev_locked(), the hub's original address slot is
set to free on the parent bus, leaving it vulnerable to DAA theft since the
parent write-lock is dropped during the transfer. When the address is
restored later by passing target_addr to i3c_master_reattach_i3c_dev_locked,
this marks the downstream device's address as free, which seems to wipe out
its reservation.
> +
> + up_write(&parent->bus.lock);
> + if (!ret)
> + ret = xfer_ret;
> + }
> +disable:
> + i3c_hub_disable_port(controller);
> + mutex_unlock(&hub->xfer_lock);
> + return ret;
> +}
[ ... ]
> +static int i3c_hub_request_ibi(struct i3c_dev_desc *desc,
> + const struct i3c_ibi_setup *req)
> +{
> + struct i3c_master_controller *controller = i3c_dev_get_master(desc);
> + struct i3c_hub_controller *hub_controller;
> + struct i3c_master_controller *orig_parent;
> + struct i3c_master_controller *parent;
> + struct i3c_hub *hub;
> + int ret;
[ ... ]
> + orig_parent = i3c_hub_update_desc_parent(&desc->common, parent);
> +
> + ret = i3c_master_direct_attach_i3c_dev_locked(parent, desc);
> + if (ret) {
> + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> + up_write(&parent->bus.lock);
> + return ret;
> + }
> +
> + kfree(desc->ibi);
> + desc->ibi = NULL;
> + ret = i3c_dev_request_ibi_locked(desc, req);
[Severity: Critical]
Can freeing desc->ibi here cause a memory leak and a double-free?
If a downstream device requests an IBI, the core i3c_dev_request_ibi_locked()
allocates ibi and ibi->wq before calling this hub operation. By calling
kfree(desc->ibi) here, the outer workqueue is leaked.
Furthermore, if the recursive call to i3c_dev_request_ibi_locked() fails,
the outer caller will execute:
if (ret) {
kfree(ibi);
dev->ibi = NULL;
}
Could this result in a double-free on the original allocation?
> + if (ret)
> + i3c_master_direct_detach_i3c_dev_locked(desc);
> +
> + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> +
> + up_write(&parent->bus.lock);
> +
> + return ret;
> +}
> +
> +static void i3c_hub_free_ibi(struct i3c_dev_desc *desc)
> +{
> + struct i3c_master_controller *controller = i3c_dev_get_master(desc);
> + struct i3c_hub_controller *hub_controller;
> + struct i3c_master_controller *orig_parent;
> + struct i3c_master_controller *parent;
> + struct i3c_hub *hub;
[ ... ]
> + i3c_hub_enable_port(controller);
> +
> + down_write(&parent->bus.lock);
> + orig_parent = i3c_hub_update_desc_parent(&desc->common, parent);
> + i3c_dev_free_ibi_locked(desc);
> + i3c_master_direct_detach_i3c_dev_locked(desc);
[Severity: High]
Does reparenting the device to the upstream master before detaching it
destroy its address reservations?
The detach logic calls i3c_master_put_i3c_addrs(), which resets the device's
static and dynamic address slots to free. Because the device is temporarily
parented to the upstream master, this frees the addresses on the parent bus.
[Severity: High]
Also, does the recursive call to i3c_dev_free_ibi_locked(desc) guarantee a
NULL pointer dereference?
The core i3c_dev_free_ibi_locked() function calls master->ops->free_ibi(dev).
When this hub implementation recursively calls i3c_dev_free_ibi_locked(), it
frees the IBI and sets desc->ibi = NULL. Upon returning to the outer function,
it executes:
if (dev->ibi->wq) {
destroy_workqueue(dev->ibi->wq);
which unconditionally dereferences the newly set NULL pointer.
> + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> + up_write(&parent->bus.lock);
> +
> + i3c_hub_disable_port(controller);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701065755.2067793-1-lakshay.piplani@nxp.com?part=5
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v13 1/7] i3c: master: Add APIs for I3C hub support
2026-07-01 7:22 ` sashiko-bot
@ 2026-07-01 18:37 ` Frank Li
0 siblings, 0 replies; 18+ messages in thread
From: Frank Li @ 2026-07-01 18:37 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, robh, Frank.Li, conor+dt, linux-i3c,
Alexandre Belloni, devicetree
On Wed, Jul 01, 2026 at 07:22:30AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Exported APIs interact with controller hardware without taking a Runtime PM reference.
> - [Medium] `i3c_master_supports_ccc_cmd` incorrectly returns true for masters that do not implement CCC command sending.
> --
>
> commit b5579dc49beffe04e3498fcfb290372e74bd88f0
> Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
>
> i3c: master: Add APIs for I3C hub support
>
> This commit adds helper functions for attaching and detaching I3C devices,
> checking CCC support, sending CCC commands, and managing address slot states.
> These additions prepare the subsystem for I3C hub support.
>
> > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > index f1be38a640ca1..3e7b7f7f97cfb 100644
> > --- a/drivers/i3c/master.c
> > +++ b/drivers/i3c/master.c
> [ ... ]
> > @@ -1775,6 +1790,83 @@ static int i3c_master_attach_i3c_dev(struct i3c_master_controller *master,
> > return 0;
> > }
> >
> > +/**
> > + * i3c_master_direct_attach_i3c_dev_locked() - Attach an I3C device to a master
> [ ... ]
> > +int i3c_master_direct_attach_i3c_dev_locked(struct i3c_master_controller *master,
> > + struct i3c_dev_desc *dev)
> > +{
> > + int ret;
> > +
> > + if (WARN_ON(i3c_dev_get_master(dev) != master))
> > + return -EINVAL;
> > +
> > + ret = i3c_master_get_i3c_addrs(dev);
> > + if (ret)
> > + return ret;
> > +
> > + /* Do not attach the master device itself. */
> > + if (master->this != dev && master->ops->attach_i3c_dev) {
> > + ret = master->ops->attach_i3c_dev(dev);
>
> [Severity: High]
> Should this function call i3c_master_rpm_get() to take a runtime PM
> reference before invoking the controller's attach_i3c_dev callback?
>
> Since the I3C subsystem sets pm_suspend_ignore_children(&master->dev, true),
> maintaining an active runtime PM reference on the hub device does not
> automatically resume the parent controller. If an I3C hub calls this on a
> parent master that is runtime suspended, accessing the hardware registers
> could lead to bus faults or hangs.
>
> > + if (ret) {
> > + i3c_master_put_i3c_addrs(dev);
> > + return ret;
> > + }
> > + }
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_master_direct_attach_i3c_dev_locked);
> > +
> > +/**
> > + * i3c_master_direct_detach_i3c_dev_locked() - Detach an I3C device from a master
> [ ... ]
> > +void i3c_master_direct_detach_i3c_dev_locked(struct i3c_dev_desc *dev)
> > +{
> > + struct i3c_master_controller *master = i3c_dev_get_master(dev);
> > +
> > + /*
> > + * We don't attach devices to the controller until they are
> > + * addressable on the bus, so skip detach for unaddressed devices.
> > + */
> > + if (!dev->info.static_addr && !dev->info.dyn_addr)
> > + return;
> > +
> > + /* Do not detach the master device itself. */
> > + if (master->this != dev && master->ops->detach_i3c_dev)
> > + master->ops->detach_i3c_dev(dev);
>
> [Severity: High]
> Similar to the attach path, does this detach callback require the parent
> master to be resumed?
>
> If the parent controller is runtime suspended, invoking detach_i3c_dev
> without first taking a runtime PM reference might access suspended hardware.
>
> > +
> > + dev->common.master_priv = NULL;
> > + i3c_master_put_i3c_addrs(dev);
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_master_direct_detach_i3c_dev_locked);
> [ ... ]
> > @@ -2548,6 +2640,50 @@ static void i3c_master_reconcile_dyn_addrs(struct i3c_master_controller *master)
> > }
> > }
> >
> > +/**
> > + * i3c_master_supports_ccc_cmd() - check CCC command support
> [ ... ]
> > +bool i3c_master_supports_ccc_cmd(struct i3c_master_controller *master,
> > + const struct i3c_ccc_cmd *cmd)
> > +{
> > + if (!master || !cmd)
> > + return false;
> > +
> > + if (!master->ops->supports_ccc_cmd)
> > + return true;
>
> [Severity: Medium]
> Is it safe to default to true without verifying that the controller
> actually implements master->ops->send_ccc_cmd?
>
> If a controller lacks send_ccc_cmd, it does not support sending any CCC
> commands. The core will return -EOPNOTSUPP in i3c_master_send_ccc_cmd_locked(),
> but returning true here might cause a hub controller to falsely advertise
> CCC support for its downstream devices.
>
> > +
> > + return master->ops->supports_ccc_cmd(master, cmd);
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_master_supports_ccc_cmd);
> > +
> > +/**
> > + * i3c_master_send_ccc_cmd() - send a CCC command
> [ ... ]
> > +int i3c_master_send_ccc_cmd(struct i3c_master_controller *master,
> > + struct i3c_ccc_cmd *cmd)
> > +{
> > + int ret;
> > +
> > + i3c_bus_maintenance_lock(&master->bus);
> > + ret = i3c_master_send_ccc_cmd_locked(master, cmd);
>
> [Severity: High]
> Does this function need to take a runtime PM reference before calling
> i3c_master_send_ccc_cmd_locked()?
>
> Like the attach/detach helpers, if an I3C hub calls this on a parent master
> that is suspended, this could lead to hardware access while suspended.
This one need fixed. i3c_master_rpm_get() need be called before
i3c_bus_maintenance_lock();
Frank
>
> > + i3c_bus_maintenance_unlock(&master->bus);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_master_send_ccc_cmd);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260701065755.2067793-1-lakshay.piplani@nxp.com?part=1
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v13 5/7] i3c: hub: Add support for the I3C interface in the I3C hub
2026-07-01 7:23 ` sashiko-bot
@ 2026-07-01 19:42 ` Frank Li
0 siblings, 0 replies; 18+ messages in thread
From: Frank Li @ 2026-07-01 19:42 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, linux-i3c, robh, Alexandre Belloni, devicetree,
conor+dt, Frank.Li
On Wed, Jul 01, 2026 at 07:23:18AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
> - [Critical] `i3c_hub_request_ibi()` intentionally frees `desc->ibi` out from under its caller without cleaning up its workqueue, causing a memory leak and a double-free on failure.
> - [High] `i3c_hub_free_ibi()` causes a guaranteed NULL pointer dereference in the core IBI free path by recursively clearing `dev->ibi`.
> - [High] Address spoofing in `i3c_hub_master_priv_xfers` corrupts parent bus address reservations and suffers from an unprotected race window.
> - [High] Missing `xfer_lock` in several hub operations allows concurrent port enabling, breaking virtual bus isolation.
> - [High] `i3c_hub_free_ibi()` permanently destroys downstream device address reservations on the parent bus.
> - [High] `i3c_hub_do_daa` delegates DAA entirely to the parent controller, breaking boardinfo matching and DT topology for downstream devices.
> - [Medium] Acquiring `parent->bus.lock` while holding `child->bus.lock` triggers a lockdep splat due to unannotated same-class lock nesting.
> --
>
> commit 22c2a31197952a434777498d93a1ed925009d6de
> Author: Lakshay Piplani <lakshay.piplani@nxp.com>
> Subject: i3c: hub: Add support for the I3C interface in the I3C hub
>
> This patch adds virtual I3C bus support for the hub and provides an
> interface to enable or disable downstream ports. Virtual master controllers
> are created for the downstream ports, and bus operations are forwarded
> through the hub device.
>
> > diff --git a/drivers/i3c/hub.c b/drivers/i3c/hub.c
> > new file mode 100644
> > index 0000000000000..d29ed40b6f735
> > --- /dev/null
> > +++ b/drivers/i3c/hub.c
> > @@ -0,0 +1,566 @@
>
> [ ... ]
>
> > +static int i3c_hub_do_daa(struct i3c_hub *hub,
> > + struct i3c_master_controller *controller)
> > +{
> > + struct i3c_master_controller *parent;
> > + int ret;
> > +
> > + if (!hub || !hub->hub_dev)
> > + return -ENODEV;
> > +
> > + parent = i3c_dev_get_master(hub->hub_dev->desc);
> > + if (!parent)
> > + return -ENODEV;
> > +
> > + i3c_hub_enable_port(controller);
> > + ret = i3c_master_do_daa(parent);
>
> [Severity: High]
> Does delegating DAA directly to the parent controller bypass the virtual hub
> bus entirely?
>
> By calling i3c_master_do_daa(parent), it appears the parent discovers and
> attaches downstream devices to the physical bus instead of the virtual
> controller. Does this cause boardinfo matching to fail since the devices
> are defined under the hub's device tree node?
Add comments here: like
/*
All target devices which connect active ports of hub are tread as
connect to parent bus at same time. So keep simple, all devices dynamatic
address should be in the same address space, that means it is differences
for all target devices even it is at difference ports. So far, address
(128, need double check) is enough now.
*/
Please rephrase it.
>
> > + i3c_hub_disable_port(controller);
> > +
> > + return ret;
> > +}
>
> [ ... ]
>
> > +static int i3c_hub_send_ccc_cmd(struct i3c_hub *hub,
> > + struct i3c_master_controller *controller,
> > + struct i3c_ccc_cmd *cmd)
> > +{
> > + struct i3c_master_controller *parent;
> > + int ret;
> > +
> > + if (!hub || !hub->hub_dev)
> > + return -ENODEV;
> > +
> > + parent = i3c_dev_get_master(hub->hub_dev->desc);
> > + if (!parent)
> > + return -ENODEV;
> > +
> > + i3c_hub_enable_port(controller);
> > + ret = i3c_master_send_ccc_cmd(parent, cmd);
>
> [Severity: High]
> Could the omission of hub->xfer_lock in this function allow concurrent port
> enabling?
look like correct, you acquire it at i3c_hub_master_priv_xfers(), should
be similar here.
>
> The lock is used to serialize private transfers, but callbacks like
> i3c_hub_send_ccc_cmd(), i3c_hub_do_daa(), and i3c_hub_enable_ibi() do not
> acquire it. If a private transfer runs concurrently with a CCC command,
> could multiple downstream ports be enabled simultaneously and cause the hub
> to broadcast traffic incorrectly?
>
> > + i3c_hub_disable_port(controller);
> > +
> > + return ret;
> > +}
>
> [ ... ]
>
> > +static int i3c_hub_master_priv_xfers(struct i3c_dev_desc *dev,
> > + struct i3c_xfer *xfers,
> > + int nxfers,
> > + enum i3c_xfer_mode mode)
> > +{
> > + struct i3c_master_controller *controller = i3c_dev_get_master(dev);
> > + struct i3c_hub_controller *hub_controller;
> > + struct i3c_dev_desc *hub_dev;
> > + struct i3c_master_controller *parent;
> > + u8 hub_addr, target_addr;
> > + struct i3c_hub *hub;
> > + int ret, xfer_ret;
>
> [ ... ]
>
> > + if (hub_addr != target_addr) {
> > + down_write(&parent->bus.lock);
call i3c_bus_maintenance_lock(), move it to API.
>
> [Severity: Medium]
> Will acquiring parent->bus.lock here trigger a lockdep warning?
>
> Since virtual hub operations are invoked by the I3C core with the child
> bus's lock already held, acquiring the parent bus's lock (which belongs to
> the same lock class) without nesting annotations might produce a false
> positive lockdep splat. This also happens in i3c_hub_enable_ibi() and
> i3c_hub_disable_ibi().
>
> > + hub_dev->info.dyn_addr = target_addr;
> > +
> > + ret = i3c_master_reattach_i3c_dev_locked(hub_dev, hub_addr);
> > + if (ret) {
> > + up_write(&parent->bus.lock);
> > + hub_dev->info.dyn_addr = hub_addr;
> > + goto disable;
> > + }
> > + up_write(&parent->bus.lock);
> > + }
> > +
> > + ret = i3c_device_do_xfers(hub->hub_dev, xfers, nxfers, mode);
> > +
> > + if (hub_addr != target_addr) {
> > + down_write(&parent->bus.lock);
> > + hub_dev->info.dyn_addr = hub_addr;
> > +
> > + xfer_ret = i3c_master_reattach_i3c_dev_locked(hub_dev,
> > + target_addr);
>
> [Severity: High]
> Does this sequence permanently destroy device tree static address
> reservations?
>
> When the hub's dynamic address is altered via
> i3c_master_reattach_i3c_dev_locked(), the hub's original address slot is
> set to free on the parent bus, leaving it vulnerable to DAA theft since the
> parent write-lock is dropped during the transfer. When the address is
> restored later by passing target_addr to i3c_master_reattach_i3c_dev_locked,
> this marks the downstream device's address as free, which seems to wipe out
> its reservation.
It makes sense
i3c_bus_maintenance_lock()
i3c_master_reattach_i3c_dev_locked()
i3c_device_do_xfers_locked()
i3c_master_reattach_i3c_dev_locked()
i3c_bus_maintenance_unlock()
Another problem is previous lockdep().
int i3c_device_do_xfers(struct i3c_device *dev, struct i3c_xfer *xfers,
int nxfers, enum i3c_xfer_mode mode)
{
...
i3c_bus_normaluse_lock(dev->bus);
ret = i3c_dev_do_xfers_locked(dev->desc, xfers, nxfers, mode);
i3c_bus_normaluse_unlock(dev->bus);
...
return ret;
}
simple method is promote i3c_bus_normaluse_lock() to i3c_bus_maintenance_lock()
when dev attached to hub devices.
and also lock parent master's i3c_bus_maintenance_lock(),
So here needn't lock in hub.c
if parent master use write lock, hub->xfer_lock needn't
>
> > +
> > + up_write(&parent->bus.lock);
> > + if (!ret)
> > + ret = xfer_ret;
> > + }
> > +disable:
> > + i3c_hub_disable_port(controller);
> > + mutex_unlock(&hub->xfer_lock);
> > + return ret;
> > +}
>
> [ ... ]
>
> > +static int i3c_hub_request_ibi(struct i3c_dev_desc *desc,
> > + const struct i3c_ibi_setup *req)
> > +{
> > + struct i3c_master_controller *controller = i3c_dev_get_master(desc);
> > + struct i3c_hub_controller *hub_controller;
> > + struct i3c_master_controller *orig_parent;
> > + struct i3c_master_controller *parent;
> > + struct i3c_hub *hub;
> > + int ret;
>
> [ ... ]
>
> > + orig_parent = i3c_hub_update_desc_parent(&desc->common, parent);
> > +
> > + ret = i3c_master_direct_attach_i3c_dev_locked(parent, desc);
All IBI cap devices should be already attached to master bus when DAA? need
attach again here?
Actived port's devices logically attached to master bus?
So needn't switch port.
> > + if (ret) {
> > + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> > + up_write(&parent->bus.lock);
> > + return ret;
> > + }
> > +
> > + kfree(desc->ibi);
> > + desc->ibi = NULL;
> > + ret = i3c_dev_request_ibi_locked(desc, req);
>
> [Severity: Critical]
> Can freeing desc->ibi here cause a memory leak and a double-free?
>
> If a downstream device requests an IBI, the core i3c_dev_request_ibi_locked()
> allocates ibi and ibi->wq before calling this hub operation. By calling
> kfree(desc->ibi) here, the outer workqueue is leaked.
>
> Furthermore, if the recursive call to i3c_dev_request_ibi_locked() fails,
> the outer caller will execute:
>
> if (ret) {
> kfree(ibi);
> dev->ibi = NULL;
> }
>
> Could this result in a double-free on the original allocation?
look like make sense
Frank
>
> > + if (ret)
> > + i3c_master_direct_detach_i3c_dev_locked(desc);
> > +
> > + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> > +
> > + up_write(&parent->bus.lock);
> > +
> > + return ret;
> > +}
> > +
> > +static void i3c_hub_free_ibi(struct i3c_dev_desc *desc)
> > +{
> > + struct i3c_master_controller *controller = i3c_dev_get_master(desc);
> > + struct i3c_hub_controller *hub_controller;
> > + struct i3c_master_controller *orig_parent;
> > + struct i3c_master_controller *parent;
> > + struct i3c_hub *hub;
>
> [ ... ]
>
> > + i3c_hub_enable_port(controller);
> > +
> > + down_write(&parent->bus.lock);
> > + orig_parent = i3c_hub_update_desc_parent(&desc->common, parent);
> > + i3c_dev_free_ibi_locked(desc);
> > + i3c_master_direct_detach_i3c_dev_locked(desc);
>
> [Severity: High]
> Does reparenting the device to the upstream master before detaching it
> destroy its address reservations?
>
> The detach logic calls i3c_master_put_i3c_addrs(), which resets the device's
> static and dynamic address slots to free. Because the device is temporarily
> parented to the upstream master, this frees the addresses on the parent bus.
>
> [Severity: High]
> Also, does the recursive call to i3c_dev_free_ibi_locked(desc) guarantee a
> NULL pointer dereference?
>
> The core i3c_dev_free_ibi_locked() function calls master->ops->free_ibi(dev).
> When this hub implementation recursively calls i3c_dev_free_ibi_locked(), it
> frees the IBI and sets desc->ibi = NULL. Upon returning to the outer function,
> it executes:
>
> if (dev->ibi->wq) {
> destroy_workqueue(dev->ibi->wq);
>
> which unconditionally dereferences the newly set NULL pointer.
>
> > + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> > + up_write(&parent->bus.lock);
> > +
> > + i3c_hub_disable_port(controller);
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260701065755.2067793-1-lakshay.piplani@nxp.com?part=5
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v13 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
2026-07-01 7:21 ` sashiko-bot
@ 2026-07-01 19:56 ` Frank Li
0 siblings, 0 replies; 18+ messages in thread
From: Frank Li @ 2026-07-01 19:56 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, devicetree, robh, conor+dt, linux-i3c, Frank.Li,
Alexandre Belloni
On Wed, Jul 01, 2026 at 07:21:48AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] Unprotected and temporary reassignment of a live device's `of_node` creates a data race, and missing restoration on the error path causes reference count leaks and a double-free.
> - [High] `regmap_read_poll_timeout` is configured with a 10ms sleep interval that vastly exceeds the actual total timeout, artificially delaying every transaction and causing spurious timeouts.
> - [High] Failure to allocate memory for an I2C adapter leaves previously registered adapters active but with freed backing memory, leading to a Use-After-Free.
> - [Medium] Loop counters `msg_count` and `nxfers_i` are `u8`, causing infinite loops if the transfer count `num` is >= 256.
> - [Medium] Race condition between the I3C hub driver and other MFD child drivers over the shared `P3H2X4X_DEV_REG_PROTECTION_CODE` register.
> --
>
> commit 37198d19f613736872b6f1fd4f9a75597b1f0ab1
> Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
>
> i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
>
> This patch adds I3C hub functionality for the NXP P3H2x4x family of multiport
> hubs. It enables I3C/I2C communication between host and hub, downstream device
> support, and target port configuration.
>
> > diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_common.c b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
> > --- a/drivers/i3c/hub/p3h2840_i3c_hub_common.c
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
> [ ... ]
> > +static int p3h2x4x_i3c_hub_probe(struct platform_device *pdev)
> > +{
> > + struct p3h2x4x_dev *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;
> [ ... ]
> > + /* Unlock access to protected registers */
> > + ret = regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
> > + P3H2X4X_REGISTERS_UNLOCK_CODE);
>
> [Severity: Medium]
> Could this global unlock race with other MFD child drivers (such as the
> on-die regulator)?
>
> Because the protected configuration registers are unlocked at the start of
> p3h2x4x_i3c_hub_probe() and left unlocked during the entire hardware
> configuration and device discovery window (which includes sleeping in
> i3c_master_do_daa()), a concurrent MFD child driver could access its own
> registers and re-lock the protection code. If that happens, subsequent
> hardware configuration writes in this function might fail silently.
>
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to unlock HUB's protected registers\n");
> > +
> > + ret = devm_add_action_or_reset(dev, p3h2x4x_lock_registers_action,
> > + 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");
> [ ... ]
>
> > diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> > --- a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> [ ... ]
> > +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 device_node *orig_of_node;
> > + u8 tp, ntwk_mask = 0;
> > + int ret;
> [ ... ]
> > + for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; tp++) {
> > + if (!p3h2x4x_hub->tp_bus[tp].of_node ||
> > + p3h2x4x_hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C)
> > + continue;
> > +
> > + /*
> > + * i3c_master_register() derives bus population from the parent device's
> > + * OF node. Temporarily switch the hub device to the target-port node for
> > + * registration, then restore the original node immediately.
> > + */
i3c_master_register() use dev of node. if add API i3c_master_register_fwnode(),
pass down a children fwnode, things will be simple, and no below AI' report
issue
> > + orig_of_node = p3h2x4x_hub->dev->of_node;
> > + p3h2x4x_hub->dev->of_node = p3h2x4x_hub->tp_bus[tp].of_node;
> > +
> > + 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;
>
> [Severity: High]
> Will this leak the original parent node reference and cause a double-free on
> the target node if i3c_hub_reserve_parent_addrslots_from_dt() returns an
> error?
>
> If the early return is taken, dev->of_node is never restored to
> orig_of_node. When the probe unwinds, the devres cleanup function
> p3h2x4x_put_target_port_of_nodes() drops the target node's reference.
> Later, when the device core destroys the device, it will call
> of_node_put(dev->of_node) which now incorrectly points to the target node
> again, causing a double-free.
>
> Concurrently, modifying dev->of_node without synchronization creates a data
> race for concurrent sysfs or uevent reads.
>
> > +
> > + ret = i3c_master_register(controller,
> > + p3h2x4x_hub->dev,
> > + i3c_hub_master_ops(),
> > + false);
> > +
> > + p3h2x4x_hub->dev->of_node = orig_of_node;
> [ ... ]
>
> > diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> > --- a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> [ ... ]
> > +static int p3h2x4x_read_smbus_transaction_status(struct p3h2x4x_i3c_hub_dev *hub,
> > + u8 target_port_status,
> > + u8 data_length)
> > +{
> > + u32 status_read;
> > + u8 status;
> > + int ret;
> > +
> > + ret = regmap_read_poll_timeout(hub->regmap, target_port_status,
> > + status_read,
> > + status_read & P3H2X4X_SMBUS_TRANSACTION_FINISH_FLAG,
> > + P3H2X4X_POLLING_ROLL_PERIOD_MS * 1000,
> > + P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(data_length));
>
> [Severity: High]
> Does this polling configuration cause spurious timeouts or severe performance
> penalties?
>
> The sleep_us parameter is set to P3H2X4X_POLLING_ROLL_PERIOD_MS * 1000,
> which is 10,000us (10ms). However, the total timeout_us parameter provided
> by P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(data_length) is significantly smaller
> (e.g., between 100us and 2,000us depending on data length).
>
> If the transaction is not complete on the very first read, the thread will
> sleep for a full 10ms, drastically overshooting the intended timeout and
> potentially returning a spurious -ETIMEDOUT depending on the state when it
> wakes up.
Make sense, pleas double check it
>
> > + if (ret)
> > + return ret;
> [ ... ]
> > +static s32 p3h2x4x_tp_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> > +{
> > + int ret_sum = 0, ret;
> > + u8 msg_count, rw;
> > +
> > + struct tp_bus *bus = i2c_get_adapdata(adap);
> > + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = bus->p3h2x4x_i3c_hub;
> > +
> > + guard(mutex)(&p3h2x4x_i3c_hub->etx_mutex);
> > + guard(mutex)(&bus->port_mutex);
> > +
> > + for (msg_count = 0; msg_count < num; msg_count++) {
>
> [Severity: Medium]
> Can this loop run infinitely if the transfer count (num) is greater than
> or equal to 256?
>
> The parameter num is a signed 32-bit integer, but msg_count is declared as
> u8. If an in-kernel driver submits an I2C transfer with 256 or more
> messages, msg_count will wrap around to 0 before ever reaching num, making
> the loop condition perpetually true and causing a CPU lockup.
make sense, please check
>
> > + if (msgs[msg_count].len > P3H2X4X_SMBUS_PAYLOAD_SIZE) {
> > + dev_err(p3h2x4x_i3c_hub->dev,
> > + "Message nr. %d not sent - length over %d bytes.\n",
> > + msg_count, P3H2X4X_SMBUS_PAYLOAD_SIZE);
> > + return -EINVAL;
> > + }
> [ ... ]
> > +int p3h2x4x_tp_smbus_algo(struct p3h2x4x_i3c_hub_dev *hub)
> > +{
> > + int ret;
> > + u8 tp;
> > +
> > + for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; 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)
> > + return -ENOMEM;
>
> [Severity: High]
> Will this early return on allocation failure cause a use-after-free for
> previously registered I2C adapters?
>
> In this loop, if devm_kzalloc() fails for a given port, the function returns
> -ENOMEM immediately without calling p3h2x4x_unregister_smbus_adapters().
make sense, please check it.
Frank
>
> Any adapters successfully registered in earlier loop iterations will remain
> active in the I2C subsystem. Because the devres unregister action
> (p3h2x4x_unregister_smbus_adapters_action) is only added at the end of the
> probe() function, it will not execute. However, devres will automatically
> free the memory allocated by devm_kzalloc(), leaving the I2C subsystem to
> dereference freed memory when it interacts with those remaining dangling
> adapters.
>
> > +
> > + /* Initialize adapter */
> > + smbus_adapter->owner = THIS_MODULE;
> > + smbus_adapter->class = I2C_CLASS_HWMON;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260701065755.2067793-1-lakshay.piplani@nxp.com?part=6
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v13 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support
2026-07-01 7:21 ` sashiko-bot
@ 2026-07-01 20:01 ` Frank Li
0 siblings, 0 replies; 18+ messages in thread
From: Frank Li @ 2026-07-01 20:01 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, devicetree, Frank.Li, conor+dt,
Alexandre Belloni, linux-i3c, robh
On Wed, Jul 01, 2026 at 07:21:00AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Early error paths in `p3h2x4x_read_smbus_agent_rx_buf` fail to clear the hardware buffer receive flag, resulting in an infinite In-Band Interrupt (IBI) storm.
> - [High] Hardware IBI configuration is unconditionally overwritten after exposing I2C adapters, potentially silently disabling slave interrupts.
> - [Medium] The `P3H2X4X_TARGET_BUF_CA_TF` case in the IBI handler is unreachable because its value is masked out by the switch expression.
> --
>
> commit f1791953bf7e067a7ec560f25d66e9566565ed9b
> 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.
> It allows the hub SMBus slave agent to receive downstream payloads and
> report receive events through In-Band Interrupts (IBI). The payloads are
> then forwarded to the registered I2C slave client.
>
> > diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> > index fd000ff635077..f752cbe35c482 100644
> > --- a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> > @@ -15,6 +15,157 @@ enum p3h2x4x_smbus_desc_idx {
> [ ... ]
> > +static void p3h2x4x_read_smbus_agent_rx_buf(struct i3c_device *i3cdev, enum p3h2x4x_rcv_buf rfbuf,
> > + enum p3h2x4x_tp tp, bool is_of)
> > +{
> > + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = i3cdev_get_drvdata(i3cdev);
> > + u8 slave_rx_buffer[P3H2X4X_SMBUS_TARGET_PAYLOAD_SIZE] = { 0 };
> > + u8 target_buffer_page, flag_clear = 0, temp = 0, i, addr;
> > + u32 packet_len, slave_address, ret;
> > + struct i2c_client *client;
> [ ... ]
> > + /* 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;
>
> [Severity: High]
> Since flag_clear is initialized to 0, if an error path like this one jumps
> to the ibi_err label early, will it skip setting flag_clear entirely?
>
> If flag_clear remains 0, the cleanup code below won't write to the
> P3H2X4X_TP0_SMBUS_AGNT_STS register to clear the hardware interrupt buffer
> receive flag. Could this cause the unserviced buffer status to remain active
> in the hardware and result in an continuous IBI interrupt storm?
Is it true?
>
> > + }
> [ ... ]
> > +ibi_err:
> > + regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_PAGE_PTR, 0x00);
> > +
> > + if (flag_clear)
> > + regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP0_SMBUS_AGNT_STS + tp, flag_clear);
> > +}
> [ ... ]
> > +void p3h2x4x_ibi_handler(struct i3c_device *i3cdev,
> > + const struct i3c_ibi_payload *payload)
> > +{
> [ ... ]
> > + 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;
> > + }
> > +
> > + /* process data receive buffer */
> > + switch (target_port_status & BUF_RECEIVED_FLAG_MASK) {
> > + case P3H2X4X_TARGET_BUF_CA_TF:
>
> [Severity: Medium]
> Is this case reachable?
>
> If BUF_RECEIVED_FLAG_MASK is defined as 0xE (GENMASK(3, 1)) and
> P3H2X4X_TARGET_BUF_CA_TF is defined as 0x1 (BIT(0)), a bitwise AND with 0xE
> will always clear bit 0.
>
> Will the switch expression ever evaluate to 0x1, or does this unintentionally
> fall through to the default case which clears all bits by writing
> BUF_RECEIVED_FLAG_TF_MASK?
Make sense, please check, look like need use FIELD_GET()
Frank
>
> > + break;
> > + case P3H2X4X_TARGET_BUF_0_RECEIVE:
> [ ... ]
> > +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;
> > + int ret;
> > +
> > + guard(mutex)(&hub->etx_mutex);
> > +
> > + if (!hub->i3cdev || !hub->ibi_ready)
> > + return -EOPNOTSUPP;
> > +
> > + if (bus->tp_smbus_client)
> > + return -EBUSY;
> > +
> > + ret = regmap_set_bits(hub->regmap,
> > + P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG,
> > + bus->tp_mask);
> > + if (ret)
> > + return ret;
> > +
> > + bus->tp_smbus_client = slave;
> > + hub->hub_config.tp_config[bus->tp_port].ibi_en = true;
> > +
> > + return 0;
> > +}
>
> [Severity: High]
> When p3h2x4x_tp_smbus_algo() loops over target ports and exposes I2C adapters
> via i2c_add_adapter(), this can trigger synchronous driver binding. A slave
> driver probing could call i2c_slave_register(), which calls this function to
> enable its IBI by setting a bit in P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG.
>
> However, if p3h2x4x_tp_smbus_algo() unconditionally writes
> P3H2X4X_IBI_DISABLED (0) to P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG after the adapter
> registration loop finishes, will this silently overwrite and clear the
> interrupt configurations just set by any concurrently probing slave drivers?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260701065755.2067793-1-lakshay.piplani@nxp.com?part=7
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-07-01 20:01 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-01 6:57 [PATCH v13 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
2026-07-01 6:57 ` [PATCH v13 1/7] i3c: master: Add APIs for I3C hub support Lakshay Piplani
2026-07-01 7:22 ` sashiko-bot
2026-07-01 18:37 ` Frank Li
2026-07-01 6:57 ` [PATCH v13 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support Lakshay Piplani
2026-07-01 7:13 ` sashiko-bot
2026-07-01 6:57 ` [PATCH v13 3/7] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
2026-07-01 6:57 ` [PATCH v13 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
2026-07-01 7:16 ` sashiko-bot
2026-07-01 6:57 ` [PATCH v13 5/7] i3c: hub: Add support for the I3C interface in the I3C hub Lakshay Piplani
2026-07-01 7:23 ` sashiko-bot
2026-07-01 19:42 ` Frank Li
2026-07-01 6:57 ` [PATCH v13 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
2026-07-01 7:21 ` sashiko-bot
2026-07-01 19:56 ` Frank Li
2026-07-01 6:57 ` [PATCH v13 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
2026-07-01 7:21 ` sashiko-bot
2026-07-01 20:01 ` Frank Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox