* [PATCH v12 1/7] i3c: master: Add APIs for I3C hub support
2026-06-17 11:03 [PATCH v12 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
@ 2026-06-17 11:03 ` Lakshay Piplani
2026-06-17 11:20 ` sashiko-bot
2026-06-17 11:03 ` [PATCH v12 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support Lakshay Piplani
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Lakshay Piplani @ 2026-06-17 11:03 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. These additions prepare
for I3C hub support.
The attach and detach helpers must be called with the bus lock held in
write mode.
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()
Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
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 | 108 +++++++++++++++++++++++++++++++++++++
include/linux/i3c/master.h | 7 +++
2 files changed, 115 insertions(+)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index dbb4d8119b5f..38f80617e2c0 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -1652,6 +1652,70 @@ 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
+ *
+ * This function attaches an I3C device to its master controller once the
+ * device has a valid address on the bus. Devices without an assigned address
+ * are ignored. The master device itself is never attached through this bus.
+ *
+ * Context: Caller must hold master->bus.lock in write mode.
+ *
+ * Return: 0 on success, or a negative error code if the attach operation
+ * fails in the master controller driver.
+ */
+int i3c_master_direct_attach_i3c_dev_locked(struct i3c_master_controller *master,
+ struct i3c_dev_desc *dev)
+{
+ int ret = 0;
+
+ /*
+ * We don't attach devices to the controller until they are
+ * addressable on the bus.
+ */
+
+ if (!dev->info.static_addr && !dev->info.dyn_addr)
+ return 0;
+
+ /* Do not attach the master device itself. */
+ if (master->this != dev && master->ops->attach_i3c_dev)
+ ret = master->ops->attach_i3c_dev(dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(i3c_master_direct_attach_i3c_dev_locked);
+
+/**
+ * i3c_master_direct_detach_i3c_dev_locked() - Detach an I3C device from its
+ * master controller.
+ * @dev: I3C device descriptor to be detached
+ *
+ * This function detaches an I3C device from its master controller.
+ * It ensures that the master itself is not detached. If the device is not
+ * the master and the master controller provides a detach operation,
+ * the detach callback is invoked to perform the actual removal.
+ *
+ * Context: Caller must hold master->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);
+}
+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 +1879,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..7e358b6774c5 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);
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v12 1/7] i3c: master: Add APIs for I3C hub support
2026-06-17 11:03 ` [PATCH v12 1/7] i3c: master: Add APIs for I3C hub support Lakshay Piplani
@ 2026-06-17 11:20 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-06-17 11:20 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: Frank.Li, linux-i3c, conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Bypassing core state tracking in `direct_attach` / `direct_detach` causes Use-After-Free, NULL pointer dereferences, and Memory Leaks.
- [High] Bypassing address slot reservation in `direct_attach` leads to unrecoverable I3C bus address collisions.
--
commit ce08f4a6bc0d82c966d4e3337e6b4d082a9d09d2
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
i3c: master: Add APIs for I3C hub support
This commit introduces helper functions for attaching and detaching I3C
devices, alongside CCC command helpers. These additions prepare the core
framework for handling I3C hubs.
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1775,6 +1775,70 @@ static int i3c_master_attach_i3c_dev(struct i3c_master_controller *master,
[ ... ]
> +int i3c_master_direct_attach_i3c_dev_locked(struct i3c_master_controller *master,
> + struct i3c_dev_desc *dev)
> +{
> + int ret = 0;
> +
> + /*
> + * We don't attach devices to the controller until they are
> + * addressable on the bus.
> + */
> +
> + if (!dev->info.static_addr && !dev->info.dyn_addr)
> + return 0;
> +
> + /* 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]
Does bypassing address slot reservation here lead to I3C bus collisions?
In the standard attachment flow, i3c_master_get_i3c_addrs() is called to
reserve the address slot in the controller's bus.addrslots bitmap. Because
this direct helper skips that reservation, the parent controller's address
map is never updated to reflect the newly occupied address.
Since the parent controller and the virtual hub share the physical I3C bus,
the parent might still believe the downstream device's address is free. If
a new device hot-joins, could the parent controller allocate the exact same
address, resulting in an unrecoverable address collision?
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_direct_attach_i3c_dev_locked);
> +
> +/**
> + * i3c_master_direct_detach_i3c_dev_locked() - Detach an I3C device from its
> + * master controller.
[ ... ]
> +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: Critical]
Can this lead to a use-after-free or a NULL pointer dereference by bypassing
the core state tracking?
When i3c_master_direct_detach_i3c_dev_locked() invokes the controller's
detach callback, master drivers typically free the master_priv data. However,
the dev->common.master_priv pointer is never cleared here.
If i3c_hub_free_ibi() calls this detach helper, and later a transfer is sent
via i3c_hub_master_priv_xfers():
drivers/i3c/hub.c:i3c_hub_master_priv_xfers() {
...
xfer_ret = i3c_master_priv_xfers(dev, xfers, nxfers, mode);
...
}
Will the parent controller end up dereferencing the freed master_priv
pointer?
Additionally, if a device is never explicitly attached via the direct helper
(leaving master_priv NULL), would a standard transfer immediately cause a
NULL pointer dereference when forwarded to the parent driver?
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_direct_detach_i3c_dev_locked);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v12 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
2026-06-17 11:03 [PATCH v12 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
2026-06-17 11:03 ` [PATCH v12 1/7] i3c: master: Add APIs for I3C hub support Lakshay Piplani
@ 2026-06-17 11:03 ` Lakshay Piplani
2026-06-17 11:13 ` sashiko-bot
2026-06-17 11:03 ` [PATCH v12 3/7] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Lakshay Piplani @ 2026-06-17 11:03 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 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..c080eeb0eeaa
--- /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 0x3000c2>;
+ 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] 15+ messages in thread* Re: [PATCH v12 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
2026-06-17 11:03 ` [PATCH v12 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support Lakshay Piplani
@ 2026-06-17 11:13 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-06-17 11:13 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: robh, conor+dt, devicetree, linux-i3c, Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The binding documents GPIO capability for target ports but omits the standard GPIO provider properties, violating subsystem guidelines.
- [Medium] The binding fails to include compatible strings for the 4-port variants and unconditionally allows up to 8 ports.
- [Low] The unit address in the I3C example does not match its `reg` property.
--
commit af3b77595fece3e15864b37b9b683be835a0e1eb
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
This commit adds device tree bindings for the NXP P3H2x4x multi-port I3C hub
family. It supports connecting a host via I3C/I2C/SMBus to multiple downstream
peripherals across up to 8 ports.
> diff --git a/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
> new file mode 100644
> index 0000000000000..c080eeb0eeaa8
> --- /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.
[Severity: Medium]
Since the description advertises that target ports can be configured as GPIO,
should this binding include the standard GPIO provider properties like
gpio-controller and #gpio-cells?
Because unevaluatedProperties is false and there are no child node patterns
defined for GPIO ports in patternProperties, won't adding a GPIO child node
fail schema validation?
> +
> +properties:
> + compatible:
> + const: nxp,p3h2840
[Severity: Medium]
The commit message and description state the binding supports the P3H2440
and P3H2441 4-port hubs alongside the 8-port P3H2840/P3H2841.
Should compatible strings be included for the 4-port variants here as well?
[ ... ]
> +patternProperties:
> + "^i3c@[0-7]$":
> + type: object
> + $ref: /schemas/i3c/i3c.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + description:
> + The I3C HUB Target Port number.
> + maximum: 7
[Severity: Medium]
Does unconditionally allowing a maximum port index of 7 cause issues for the
4-port variants mentioned in the description?
It seems this allows invalid configurations that define non-existent ports
(4-7) for the 4-port hardware.
[ ... ]
> +examples:
> + - |
> + i3c {
> + #address-cells = <3>;
> + #size-cells = <0>;
> +
> + hub@70,236153000c2 {
> + reg = <0x70 0x236 0x3000c2>;
[Severity: Low]
Does the unit address in this example match its reg property?
The reg property combines to a provisioned ID of 0x236003000c2, which appears
to differ from 236153000c2 in the unit address. Could this mismatch trigger a
simple_bus_reg warning during make dt_binding_check?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=2
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v12 3/7] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator
2026-06-17 11:03 [PATCH v12 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
2026-06-17 11:03 ` [PATCH v12 1/7] i3c: master: Add APIs for I3C hub support Lakshay Piplani
2026-06-17 11:03 ` [PATCH v12 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support Lakshay Piplani
@ 2026-06-17 11:03 ` Lakshay Piplani
2026-06-17 11:24 ` sashiko-bot
2026-06-17 11:03 ` [PATCH v12 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Lakshay Piplani @ 2026-06-17 11:03 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 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 | 119 ++++++++++++++++++++++++++++++++++++
include/linux/i3c/device.h | 1 +
include/linux/mfd/p3h2840.h | 26 ++++++++
6 files changed, 162 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..0a80992d6250
--- /dev/null
+++ b/drivers/mfd/p3h2840.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025-2026 NXP
+ * P3H2X4X i3c hub and regulator device.
+ */
+
+#include <linux/i2c.h>
+#include <linux/i3c/master.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 p3h2x4x_dev *ddata;
+ int ret;
+
+ ddata = devm_kzalloc(&i3cdev->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(&i3cdev->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(&i3cdev->dev, PLATFORM_DEVID_AUTO,
+ p3h2x4x_devs, ARRAY_SIZE(p3h2x4x_devs),
+ NULL, 0, NULL);
+ if (ret)
+ return dev_err_probe(&i3cdev->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..2d4fae2176ae
--- /dev/null
+++ b/include/linux/mfd/p3h2840.h
@@ -0,0 +1,26 @@
+/* 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
+
+struct p3h2x4x_dev {
+ struct i3c_device *i3cdev;
+ struct regmap *regmap;
+};
+#endif /* _LINUX_MFD_P3H2840_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v12 3/7] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator
2026-06-17 11:03 ` [PATCH v12 3/7] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
@ 2026-06-17 11:24 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-06-17 11:24 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: Frank.Li, linux-i3c, conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The I3C driver incorrectly binds to any generic I3C Hub instead of specifically to NXP P3H2x4x devices.
--
commit e3f8613aba897f64df36ad159bade0cf0e0c5cf1
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator
This commit adds core MFD support for the NXP P3H2x4x family of multiport
I3C hub devices. It registers the devices via I3C/I2C/SMBus and exposes
multiple downstream target ports and an on-die regulator.
> diff --git a/drivers/mfd/p3h2840.c b/drivers/mfd/p3h2840.c
> new file mode 100644
> index 0000000000000..0a80992d6250a
> --- /dev/null
> +++ b/drivers/mfd/p3h2840.c
[ ... ]
> +static const struct i3c_device_id p3h2x4x_i3c_ids[] = {
> + I3C_CLASS(I3C_DCR_HUB, NULL),
[Severity: High]
Does this match table inappropriately bind the driver to any generic I3C Hub
instead of specifically NXP P3H2x4x devices?
Since p3h2x4x_device_probe_i3c() does not perform manufacturer or part ID
validation, could this claim non-NXP hubs and cause child drivers to send
proprietary NXP register sequences to them?
Should this use the I3C_DEVICE() macro with MIPI_VENDOR_ID_NXP instead?
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(i3c, p3h2x4x_i3c_ids);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v12 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
2026-06-17 11:03 [PATCH v12 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
` (2 preceding siblings ...)
2026-06-17 11:03 ` [PATCH v12 3/7] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
@ 2026-06-17 11:03 ` Lakshay Piplani
2026-06-17 11:17 ` sashiko-bot
2026-06-17 11:03 ` [PATCH v12 5/7] i3c: hub: Add support for the I3C interface in the I3C hub Lakshay Piplani
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Lakshay Piplani @ 2026-06-17 11:03 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 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 | 220 ++++++++++++++++++
4 files changed, 232 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..4eb8bdba6b0a
--- /dev/null
+++ b/drivers/regulator/p3h2840_i3c_hub_regulator.c
@@ -0,0 +1,220 @@
+// 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 p3h2x4x_reg_state {
+ unsigned int orig;
+ bool restore;
+};
+
+static void p3h2x4x_reg_guard_enter(struct regulator_dev *rdev,
+ struct p3h2x4x_reg_state *state)
+{
+ state->restore = false;
+
+ if (regmap_read(rdev->regmap,
+ P3H2X4X_DEV_REG_PROTECTION_CODE,
+ &state->orig))
+ 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)
+{
+ if (state->restore)
+ regmap_write(rdev->regmap,
+ P3H2X4X_DEV_REG_PROTECTION_CODE,
+ state->orig);
+}
+
+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;
+
+ p3h2x4x_regulator = devm_kzalloc(dev, sizeof(*p3h2x4x_regulator), GFP_KERNEL);
+ if (!p3h2x4x_regulator)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, p3h2x4x_regulator);
+
+ p3h2x4x_regulator->regmap = p3h2x4x->regmap;
+ device_set_of_node_from_dev(dev, dev->parent);
+
+ rcfg.dev = dev;
+ rcfg.dev->of_node = dev->of_node;
+ 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 struct platform_driver p3h2x4x_regulator_driver = {
+ .driver = {
+ .name = "p3h2x4x-regulator",
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+ .probe = p3h2x4x_regulator_probe,
+};
+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] 15+ messages in thread* Re: [PATCH v12 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
2026-06-17 11:03 ` [PATCH v12 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
@ 2026-06-17 11:17 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-06-17 11:17 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: linux-i3c, robh, conor+dt, Frank.Li, devicetree
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Concurrent regulator operations race on the shared device-level protection register, leading to silent hardware configuration failures.
- [Medium] The driver module will not auto-load because it lacks a MODULE_ALIAS or platform_device_id table.
- [Medium] If regmap_read or the unlock sequence fails in the guard constructor, the driver proceeds with the regulator operation on locked registers.
- [Low] Typo in regulator config assignment assigns dev->of_node to itself instead of rcfg.of_node.
--
commit e68081b58005a2422d441a5611dac29f8cffe2f4
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.
> --- /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)
> +{
> + state->restore = false;
> +
> + if (regmap_read(rdev->regmap,
> + P3H2X4X_DEV_REG_PROTECTION_CODE,
> + &state->orig))
> + return;
[Severity: Medium]
If regmap_read() fails here, the function returns early and swallows the
error since the guard constructor returns void. Will this cause the caller
to proceed with regulator operations on locked hardware, potentially
resulting in silent hardware configuration failures?
If the device ACKs the transaction despite ignoring the write, wouldn't
the driver incorrectly report success to the regulator core?
> +
> + 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)
> +{
> + if (state->restore)
> + regmap_write(rdev->regmap,
> + P3H2X4X_DEV_REG_PROTECTION_CODE,
> + state->orig);
> +}
> +
> +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);
[Severity: High]
Does this guard provide mutual exclusion across the four different regulators
on the same device?
The regmap lock protects individual I/O operations, but since there is no
device-level lock held across the unlock/modify/lock sequence, can concurrent
operations on different regulators interleave?
For example, could one thread re-lock the device in p3h2x4x_reg_guard_exit()
while another thread is just about to write its configuration, causing the
hardware write to be silently ignored? Is a shared device-level mutex needed
to serialize these sequences?
[ ... ]
> +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;
> +
> + p3h2x4x_regulator = devm_kzalloc(dev, sizeof(*p3h2x4x_regulator), GFP_KERNEL);
> + if (!p3h2x4x_regulator)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, p3h2x4x_regulator);
> +
> + p3h2x4x_regulator->regmap = p3h2x4x->regmap;
> + device_set_of_node_from_dev(dev, dev->parent);
> +
> + rcfg.dev = dev;
> + rcfg.dev->of_node = dev->of_node;
[Severity: Low]
Should this be rcfg.of_node = dev->of_node? Since rcfg.dev is assigned to dev
on the previous line, this just assigns dev->of_node to itself.
> + rcfg.regmap = p3h2x4x_regulator->regmap;
> + rcfg.driver_data = p3h2x4x_regulator;
[ ... ]
> +static struct platform_driver p3h2x4x_regulator_driver = {
> + .driver = {
> + .name = "p3h2x4x-regulator",
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + },
> + .probe = p3h2x4x_regulator_probe,
> +};
> +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");
[Severity: Medium]
Since the driver is registered as a platform driver with the name
"p3h2x4x-regulator" but the module is compiled as
"p3h2840_i3c_hub_regulator.ko", will this module fail to auto-load?
Without a MODULE_ALIAS("platform:p3h2x4x-regulator") or an exported
platform_device_id table, udev may not be able to load the module when the
MFD core creates the child platform device.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v12 5/7] i3c: hub: Add support for the I3C interface in the I3C hub
2026-06-17 11:03 [PATCH v12 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
` (3 preceding siblings ...)
2026-06-17 11:03 ` [PATCH v12 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
@ 2026-06-17 11:03 ` Lakshay Piplani
2026-06-17 11:18 ` sashiko-bot
2026-06-17 11:03 ` [PATCH v12 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
2026-06-17 11:03 ` [PATCH v12 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
6 siblings, 1 reply; 15+ messages in thread
From: Lakshay Piplani @ 2026-06-17 11:03 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 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 | 483 ++++++++++++++++++++++++++++++++++++++++
include/linux/i3c/hub.h | 98 ++++++++
5 files changed, 598 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..e8744b517234
--- /dev/null
+++ b/drivers/i3c/hub.c
@@ -0,0 +1,483 @@
+// 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;
+}
+
+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;
+
+ 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);
+ up_write(&parent->bus.lock);
+ if (ret) {
+ hub_dev->info.dyn_addr = hub_addr;
+ goto disable;
+ }
+ }
+
+ 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);
+ return ret;
+}
+
+static int i3c_hub_attach_i2c_dev(struct i2c_dev_desc *dev)
+{
+ return 0;
+}
+
+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;
+
+ 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;
+
+ 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_master_direct_detach_i3c_dev_locked(desc);
+ i3c_dev_free_ibi_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)
+{
+}
+
+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;
+}
+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);
+
+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..9688a802ff10
--- /dev/null
+++ b/include/linux/i3c/hub.h
@@ -0,0 +1,98 @@
+/* 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 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);
+
+#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v12 5/7] i3c: hub: Add support for the I3C interface in the I3C hub
2026-06-17 11:03 ` [PATCH v12 5/7] i3c: hub: Add support for the I3C interface in the I3C hub Lakshay Piplani
@ 2026-06-17 11:18 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-06-17 11:18 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: robh, conor+dt, Frank.Li, devicetree, linux-i3c
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [Critical] Deterministic NULL pointer dereference in `i3c_hub_free_ibi()` due to recursive callback chain.
- [Critical] Memory leak and double-free memory corruption in `i3c_hub_request_ibi()` error paths.
- [Critical] Critical race condition and parent bus state corruption in `i3c_hub_master_priv_xfers()` due to unlocked address mutation.
- [Critical] Unimplemented `i3c_hub_recycle_ibi_slot()` permanently leaks hardware IBI slots.
- [Critical] Architectural bypass in `i3c_hub_do_daa()` attaches dynamically discovered devices to the parent bus instead of the virtual hub bus.
- [High] Calling `i3c_hub_enable_port()` outside of any locks allows concurrent operations to physically enable multiple downstream ports simultaneously, breaking electrical isolation.
- [High] Silently dropping broadcast RSTDAA commands prevents resetting of downstream devices.
- [Medium] Accepting I2C devices while unconditionally rejecting all I2C transfers violates the API contract.
--
commit 2a201a9e0e9a55b31467d874340085240b34afc3
Author: Lakshay Piplani <lakshay.piplani@nxp.com>
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.
> diff --git a/drivers/i3c/hub.c b/drivers/i3c/hub.c
> new file mode 100644
> index 0000000000000..e8744b5172340
> --- /dev/null
> +++ b/drivers/i3c/hub.c
> @@ -0,0 +1,483 @@
[ ... ]
> +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: Critical]
Since `i3c_master_do_daa()` executes the DAA sequence natively on the parent
controller, will the newly discovered devices be allocated and attached
directly to the parent bus instead of this virtual hub's bus?
If they are attached to the parent bus, won't subsequent transfers completely
bypass `i3c_hub_master_priv_xfers()`, causing communication with these
dynamically enumerated devices to fail?
[Severity: High]
Is there a risk of breaking electrical isolation here? `i3c_hub_enable_port()`
is called before acquiring any parent bus locks.
If multiple threads operate on different virtual controllers concurrently,
could they physically enable multiple downstream ports at the same time
before the bus lock is acquired?
> + 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]
Similar to `i3c_hub_do_daa()`, does calling `i3c_hub_enable_port()` without a
lock risk enabling multiple ports concurrently if multiple CCC commands are
issued on different ports?
> + 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)
> +{
[ ... ]
> + 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);
> + up_write(&parent->bus.lock);
> + if (ret) {
> + hub_dev->info.dyn_addr = hub_addr;
> + goto disable;
> + }
> + }
> +
> + ret = i3c_device_do_xfers(hub->hub_dev, xfers, nxfers, mode);
[Severity: Critical]
Does dropping `parent->bus.lock` before calling `i3c_device_do_xfers()` expose
a race condition?
If multiple downstream ports initiate transfers concurrently, could they
overwrite the shared `hub_dev->info.dyn_addr` before the transfer completes,
causing packets to be routed to the wrong physical device?
> +
> + 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: Critical]
Could passing `target_addr` as the old address corrupt the parent bus state?
`i3c_master_reattach_i3c_dev_locked()` frees the old address slot in the bus
slot map. Will this erroneously mark `target_addr` as `I3C_ADDR_SLOT_FREE`
on the parent bus?
> +
> + up_write(&parent->bus.lock);
> + if (!ret)
> + ret = xfer_ret;
> + }
> +disable:
> + i3c_hub_disable_port(controller);
> + return ret;
> +}
> +
> +static int i3c_hub_attach_i2c_dev(struct i2c_dev_desc *dev)
> +{
> + return 0;
> +}
[ ... ]
> +static int i3c_hub_i2c_xfers(struct i2c_dev_desc *dev,
> + struct i2c_msg *xfers, int nxfers)
> +{
> + return -EOPNOTSUPP;
> +}
[Severity: Medium]
Is it intentional to return `0` from `i3c_hub_attach_i2c_dev()` while rejecting
all I2C transfers with `-EOPNOTSUPP`?
Would it be safer to return an error from the attach callback to cleanly
prevent I2C driver binding if the hub does not support I2C forwarding?
[ ... ]
> +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;
> +
> + if (cmd->id == I3C_CCC_RSTDAA(true))
> + return 0;
[Severity: High]
Does silently dropping the broadcast RSTDAA command prevent downstream devices
from being correctly reset by the hub?
If the command is dropped instead of being forwarded to the downstream ports,
will devices retain their old dynamic addresses and fail to re-initialize
during hot-plug recovery?
> +
> + return i3c_hub_send_ccc_cmd(hub, controller, cmd);
> +}
[ ... ]
> +static int i3c_hub_request_ibi(struct i3c_dev_desc *desc,
> + const struct i3c_ibi_setup *req)
> +{
[ ... ]
> + 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);
[Severity: Critical]
Does freeing `desc->ibi` here leak `desc->ibi->wq`?
When the core function `i3c_dev_request_ibi_locked()` allocates `ibi`, it also
allocates `ibi->wq`.
Also, if the inner call to `i3c_dev_request_ibi_locked()` fails, this function
returns an error to the original core caller. Will the outer caller's error
path execute `kfree(ibi)` on its local variable, resulting in a double-free
since it was already freed here?
> + 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)
> +{
[ ... ]
> + i3c_hub_enable_port(controller);
> +
> + down_write(&parent->bus.lock);
> + orig_parent = i3c_hub_update_desc_parent(&desc->common, parent);
> + i3c_master_direct_detach_i3c_dev_locked(desc);
> + i3c_dev_free_ibi_locked(desc);
[Severity: Critical]
Can this recursive call to `i3c_dev_free_ibi_locked(desc)` cause a null
pointer dereference?
When the core initially calls `i3c_dev_free_ibi_locked(desc)`, it invokes
this hub callback. The inner call here successfully frees `desc->ibi` and sets
it to null.
When control returns to the outer core function, it unconditionally evaluates:
if (dev->ibi->wq) {
Will this immediately dereference the null pointer?
> + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> + up_write(&parent->bus.lock);
> +
> + i3c_hub_disable_port(controller);
> +}
[ ... ]
> +static void i3c_hub_recycle_ibi_slot(struct i3c_dev_desc *desc,
> + struct i3c_ibi_slot *slot)
> +{
> +}
[Severity: Critical]
Does leaving this function empty permanently leak hardware IBI slots?
When the core finishes processing an IBI, it returns the slot to the virtual
controller. Because this doesn't forward the recycle request to the parent
hardware controller, will the parent controller eventually exhaust its hardware
slots and permanently stop processing IBIs?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=5
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v12 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
2026-06-17 11:03 [PATCH v12 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
` (4 preceding siblings ...)
2026-06-17 11:03 ` [PATCH v12 5/7] i3c: hub: Add support for the I3C interface in the I3C hub Lakshay Piplani
@ 2026-06-17 11:03 ` Lakshay Piplani
2026-06-17 11:18 ` sashiko-bot
2026-06-17 11:03 ` [PATCH v12 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
6 siblings, 1 reply; 15+ messages in thread
From: Lakshay Piplani @ 2026-06-17 11:03 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 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 | 357 +++++++++++++++++++++++
drivers/i3c/hub/p3h2840_i3c_hub_i3c.c | 121 ++++++++
drivers/i3c/hub/p3h2840_i3c_hub_smbus.c | 295 +++++++++++++++++++
9 files changed, 1116 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..05e19bff20d5
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
@@ -0,0 +1,357 @@
+// 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) {
+ 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 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;
+ 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 = 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 struct platform_driver p3h2x4x_i3c_hub_driver = {
+ .driver = {
+ .name = "p3h2x4x-i3c-hub",
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+ .probe = p3h2x4x_i3c_hub_probe,
+};
+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..8adb235b992c
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
@@ -0,0 +1,121 @@
+// 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);
+ 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;
+
+ /* Assign DT node for this TP */
+ 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_master_register(controller,
+ p3h2x4x_hub->dev,
+ i3c_hub_master_ops(),
+ false);
+
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(p3h2x4x_hub->dev,
+ p3h2x4x_unregister_i3c_master,
+ controller);
+ if (ret)
+ return ret;
+
+ /* 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..cf664425af07
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
@@ -0,0 +1,295 @@
+// 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;
+
+ fsleep(P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(data_length));
+
+ ret = regmap_read(hub->regmap, target_port_status, &status_read);
+ 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 read message is immediately followed by a write 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] 15+ messages in thread* Re: [PATCH v12 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
2026-06-17 11:03 ` [PATCH v12 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
@ 2026-06-17 11:18 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-06-17 11:18 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: devicetree, robh, linux-i3c, Frank.Li, conor+dt
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The SMBus transaction wait logic uses a fixed sleep instead of polling the hardware completion flag, leading to premature reads of incomplete transactions and data corruption.
- [High] Overwriting the parent device's `of_node` pointer iteratively causes a memory leak of the original parent node, a double-free of the target port nodes, and state corruption.
- [Medium] Potential `of_node` reference leak in `p3h2x4x_get_target_port_dt_conf()` if multiple child nodes specify the same target port index.
- [Medium] The hub's protected registers remain permanently unlocked if the hardware configuration or bus registration fails during probe.
- [Low] Comment describes a read followed by a write, but the code implements a write followed by a read.
--
commit 1c982ecf71c6fb3b765caf255191a821a5e37cb5
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
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.
> 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 0000000000000..05e19bff20d5c
> --- /dev/null
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
[ ... ]
> +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) {
> + p3h2x4x_i3c_hub->tp_bus[tp_port].of_node = of_node_get(dev_node);
[Severity: Medium]
Could this leak the of_node reference if a malformed device tree specifies
multiple child nodes with the same target port index?
If multiple child nodes specify the same reg value, the pointer is
overwritten with a new of_node_get() result, which abandons the previous
reference.
> + p3h2x4x_i3c_hub->tp_bus[tp_port].tp_mask = P3H2X4X_SET_BIT(tp_port);
> + p3h2x4x_i3c_hub->tp_bus[tp_port].p3h2x4x_i3c_hub = p3h2x4x_i3c_hub;
> + p3h2x4x_i3c_hub->tp_bus[tp_port].tp_port = tp_port;
> + }
> + }
> +}
[ ... ]
> +static int p3h2x4x_i3c_hub_probe(struct platform_device *pdev)
> +{
[ ... ]
> + /* 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 = p3h2x4x_configure_hw(dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to configure the HUB\n");
[Severity: Medium]
Are the hub's protected registers left permanently unlocked if probe fails
during p3h2x4x_configure_hw() or bus registration?
The driver unlocks the hardware registers at the start of probe and locks
them at the very end. However, if any initialization step fails and returns
an error, the function exits immediately without locking the registers again.
Could this be mitigated by using a devm_add_action_or_reset() callback or
explicit error handling labels to ensure the registers are safely relocked?
> +
> + /* Register virtual I3C master controllers for I3C target ports */
> + if (p3h2x4x->i3cdev) {
[ ... ]
> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> new file mode 100644
> index 0000000000000..8adb235b992c0
> --- /dev/null
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
[ ... ]
> +int p3h2x4x_tp_i3c_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_hub)
> +{
[ ... ]
> + 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;
> +
> + /* Assign DT node for this TP */
> + p3h2x4x_hub->dev->of_node = p3h2x4x_hub->tp_bus[tp].of_node;
[Severity: High]
Does overwriting the parent device's of_node pointer iteratively cause a
memory leak of the original parent node and a double-free of the target port
nodes?
The loop over target ports directly mutates the shared pdev->dev.of_node
pointer which was populated in probe using device_set_of_node_from_dev().
This leaks the reference to the original parent node, and leaves
dev->of_node pointing to the last target port node.
When the driver is unbound, the driver core will drop a reference on
dev->of_node, but p3h2x4x_put_target_port_of_nodes() also drops a reference
on the same node, which can cause a use-after-free or double-free refcount
underflow. Mutating dev->of_node also creates a race with sysfs/uevent.
Should the driver assign the node to controller->dev.of_node directly
instead of mutating the parent's pointer?
> +
> + struct i3c_hub_controller *hub_controller =
> + &p3h2x4x_hub->tp_bus[tp].hub_controller;
[ ... ]
> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> new file mode 100644
> index 0000000000000..cf664425af071
> --- /dev/null
> +++ 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;
> +
> + fsleep(P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(data_length));
> +
> + ret = regmap_read(hub->regmap, target_port_status, &status_read);
[Severity: High]
Does using a hardcoded sleep based on bytes transferred instead of polling a
hardware completion status bit lead to premature reads of incomplete
transactions?
If the downstream device performs clock stretching or is slow to respond,
the transfer will take longer than the hardcoded timeout. The driver then
reads target_port_status and shifts to check the status bits.
Since an ongoing transaction leaves the status bits at 0
(P3H2X4X_SMBUS_CNTRL_STATUS_TXN_OK), the driver will incorrectly assume the
transaction has finished successfully. It will then proceed to read stale or
uninitialized memory buffers and return garbage data.
Could this be updated to use a polling loop checking the finish flag?
> + if (ret)
> + return ret;
> +
> + status = (u8)status_read;
[ ... ]
> +static s32 p3h2x4x_tp_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{
[ ... ]
> + rw = (msgs[msg_count].flags & I2C_M_RD) ? 1 : 0;
> + if (!rw) {
> + /* If a read message is immediately followed by a write message to
> + * the same address, consider combining them into a single transaction.
> + */
[Severity: Low]
This isn't a bug, but the comment describes a read followed by a write while
the code implements a write followed by a read.
The actual code block checks !rw for the first message and flags & I2C_M_RD
for the second message, which accurately handles a write message immediately
followed by a read message.
> + 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;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=6
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v12 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support
2026-06-17 11:03 [PATCH v12 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
` (5 preceding siblings ...)
2026-06-17 11:03 ` [PATCH v12 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
@ 2026-06-17 11:03 ` Lakshay Piplani
2026-06-17 11:24 ` sashiko-bot
6 siblings, 1 reply; 15+ messages in thread
From: Lakshay Piplani @ 2026-06-17 11:03 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 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 | 10 ++
drivers/i3c/hub/p3h2840_i3c_hub_i3c.c | 40 ++++-
drivers/i3c/hub/p3h2840_i3c_hub_smbus.c | 193 ++++++++++++++++++++++++
3 files changed, 242 insertions(+), 1 deletion(-)
diff --git a/drivers/i3c/hub/p3h2840_i3c_hub.h b/drivers/i3c/hub/p3h2840_i3c_hub.h
index d922c6b3b6be..7f8363170efe 100644
--- a/drivers/i3c/hub/p3h2840_i3c_hub.h
+++ b/drivers/i3c/hub/p3h2840_i3c_hub.h
@@ -322,4 +322,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 8adb235b992c..88b4a76aa264 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.
@@ -117,5 +135,25 @@ 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)
+ return ret;
+
+ ret = i3c_device_enable_ibi(p3h2x4x_hub->i3cdev);
+ if (ret) {
+ i3c_device_free_ibi(p3h2x4x_hub->i3cdev);
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(p3h2x4x_hub->dev,
+ p3h2x4x_free_ibi,
+ p3h2x4x_hub->i3cdev);
+ if (ret)
+ 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 cf664425af07..49b6d213a4a7 100644
--- a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
@@ -15,6 +15,146 @@ 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 = 0x0f, 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);
+ 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 (p3h2x4x_i3c_hub->tp_bus[i].is_registered && (payload_byte_two >> i) & 0x01) {
+ 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)
@@ -205,11 +345,64 @@ 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 (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,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v12 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support
2026-06-17 11:03 ` [PATCH v12 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
@ 2026-06-17 11:24 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-06-17 11:24 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: linux-i3c, Frank.Li, robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Unconditional IBI request causes probe failure if the upstream I3C controller lacks IBI support.
- [High] Over-clearing buffer status flags on error paths discards pending data in the receive buffers.
- [High] DMA-from-stack vulnerabilities by allocating buffers on the stack and passing them to regmap bulk operations.
- [Medium] SMBus slave registration configures IBI without verifying if the upstream connection is I3C.
- [Medium] Missing SMBus agent status clear for unregistered ports leads to IBI storms.
- [Medium] Lockless read of `is_registered` in the IBI handler races with lockless write during SMBus adapter teardown.
--
commit 4badb83d7f4fe074334808f3d555ec26e7923604
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.
The hub SMBus slave agent can receive downstream payloads into target buffers
and report receive events through IBI, which are then forwarded to the
registered I2C slave client.
> 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
[ ... ]
> @@ -117,5 +135,25 @@ 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)
> + return ret;
[Severity: High]
Could this unconditional IBI request cause the probe to fail entirely if the
upstream I3C master controller does not support In-Band Interrupts?
If i3c_device_request_ibi() returns an error like -ENOTSUPP, it looks like
it aborts the entire initialization sequence for the hub.
Does this mean the core I3C/I2C target port routing functionality would break
on systems without IBI support merely because an optional SMBus slave feature
could not be enabled?
[ ... ]
> 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
[ ... ]
> @@ -15,6 +15,146 @@ 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 = 0x0f, temp = 0, i, addr;
[Severity: High]
This isn't a bug strictly introduced by this patch, as a similar preexisting
flaw exists in p3h2x4x_tp_i2c_xfer_msg() with the desc array, but allocating
slave_rx_buffer on the stack and passing it to regmap bulk operations can
cause DMA-from-stack vulnerabilities.
Because the underlying regmap is backed by an I3C controller that may use DMA
for transfers, passing stack addresses is unsafe.
If CONFIG_VMAP_STACK is enabled, the stack memory is not guaranteed to be
physically contiguous, which could cause dma_map_single() to fail or corrupt
memory.
Should these buffers be dynamically allocated or pre-allocated in the device
structure instead?
> + u32 packet_len, slave_address, ret;
[ ... ]
> +ibi_err:
> + regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_PAGE_PTR, 0x00);
> + regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP0_SMBUS_AGNT_STS + tp, flag_clear);
> +}
[Severity: High]
Could this over-clear the buffer status flags on error paths and discard
pending data?
The local variable flag_clear is initialized to 0x0f, which matches
BUF_RECEIVED_FLAG_TF_MASK.
If an early error occurs before flag_clear is constrained to the specific port
buffer flag, it will write 0x0f to the status register.
Will this unconditionally clear all receive buffer flags, including buffer 0,
buffer 1, and overflow flags, for that port? If the other buffer contained a
valid pending message, would it be silently dropped?
> +
> +/**
> + * 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)
> +{
[ ... ]
> + guard(mutex)(&p3h2x4x_i3c_hub->etx_mutex);
> +
> + for (i = 0; i < P3H2X4X_TP_MAX_COUNT; ++i) {
> + if (p3h2x4x_i3c_hub->tp_bus[i].is_registered && (payload_byte_two >> i) & 0x01) {
[Severity: Medium]
What happens if an IBI is triggered by a port that is not registered?
Since the loop skips unregistered ports entirely, the hardware interrupt source
at P3H2X4X_TP0_SMBUS_AGNT_STS is never read or cleared.
Does this mean the underlying hardware condition remains active, causing the
hub to immediately re-assert the IBI after the handler returns?
Could this result in an infinite loop of IBIs locking up the I3C bus, and
should the event status be cleared defensively even for unregistered ports?
[Severity: Medium]
Is there a data race here involving the read of is_registered?
During driver unbind or probe failure, p3h2x4x_unregister_smbus_adapters()
executes hub->tp_bus[tp].is_registered = false; without holding etx_mutex.
Because the devm action to free the IBI executes after the SMBus teardown
action, this new handler could be running concurrently and reading
is_registered under etx_mutex.
Since the writer does not hold the mutex, should this state modification be
properly synchronized?
> + ret = regmap_read(p3h2x4x_i3c_hub->regmap, P3H2X4X_TP0_SMBUS_AGNT_STS + i,
[ ... ]
> @@ -205,11 +345,64 @@ 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 (bus->tp_smbus_client)
> + return -EBUSY;
> +
> + ret = regmap_set_bits(hub->regmap,
> + P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG,
> + bus->tp_mask);
[Severity: Medium]
Does this configure IBI without verifying if the upstream connection is I3C?
The registration enables P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG to route receive
events via IBI. However, it does not check if the hub is connected via an I3C
bus (i.e., whether hub->i3cdev is valid and an IBI handler was successfully
requested).
If the hub is connected via an I2C upstream bus, it cannot generate IBIs.
Enabling IBI generation on an I2C-connected hub would cause slave receive
events to be silently dropped since no handler is registered.
Should this function return -EOPNOTSUPP if !hub->i3cdev?
> + if (ret)
> + return ret;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=7
^ permalink raw reply [flat|nested] 15+ messages in thread