* [PATCH v10 8/9] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
From: Lakshay Piplani @ 2026-05-25 6:42 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
In-Reply-To: <20260525064209.2263045-1-lakshay.piplani@nxp.com>
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 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 | 327 +++++++++++++++++++++
drivers/i3c/hub/p3h2840_i3c_hub_common.c | 347 +++++++++++++++++++++++
drivers/i3c/hub/p3h2840_i3c_hub_i3c.c | 124 ++++++++
drivers/i3c/hub/p3h2840_i3c_hub_smbus.c | 269 ++++++++++++++++++
9 files changed, 1085 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 1d4954695f84..9aec93c26f85 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19312,6 +19312,7 @@ L: linux-i3c-owner@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 65304b416bb4..74727d614492 100644
--- a/drivers/i3c/Kconfig
+++ b/drivers/i3c/Kconfig
@@ -36,6 +36,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..d69fafbac584
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub.h
@@ -0,0 +1,327 @@
+/* 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
+
+/* Pullup selection DT settings */
+#define P3H2X4X_TP_PULLUP_250R 0x00
+#define P3H2X4X_TP_PULLUP_500R 0x01
+#define P3H2X4X_TP_PULLUP_1000R 0x02
+#define P3H2X4X_TP_PULLUP_2000R 0x03
+
+#define P3H2X4X_TP_PULLUP_DISABLED 0x00
+#define P3H2X4X_TP_PULLUP_ENABLED 0x01
+
+#define P3H2X4X_IO_STRENGTH_20_OHM 0x00
+#define P3H2X4X_IO_STRENGTH_30_OHM 0x01
+#define P3H2X4X_IO_STRENGTH_40_OHM 0x02
+#define P3H2X4X_IO_STRENGTH_50_OHM 0x03
+
+#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)
+
+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_tp_smbus_algo - add i2c adapter for target port configured as SMBus.
+ * @priv: p3h2x4x device structure.
+ * @tp: target port.
+ * Return: 0 in case of success, a negative EINVAL code if the error.
+ */
+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.
+ * @priv: p3h2x4x device structure.
+ * @tp: target port.
+ * Return: 0 in case of success, a negative EINVAL code if the error.
+ */
+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..f1a24a3d3ffa
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
@@ -0,0 +1,347 @@
+// 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->parent, 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, pullup, pullup);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(hub->regmap, P3H2X4X_TP_IO_MODE_CONF, mode, mode);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(hub->regmap, P3H2X4X_TP_SMBUS_AGNT_EN, smbus, 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, target_port, 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 = 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_TP_PULLUP_500R;
+ p3h2x4x_i3c_hub->hub_config.tp2367_pullup = P3H2X4X_TP_PULLUP_500R;
+ p3h2x4x_i3c_hub->hub_config.cp0_io_strength = P3H2X4X_IO_STRENGTH_20_OHM;
+ p3h2x4x_i3c_hub->hub_config.cp1_io_strength = P3H2X4X_IO_STRENGTH_20_OHM;
+ p3h2x4x_i3c_hub->hub_config.tp0145_io_strength = P3H2X4X_IO_STRENGTH_20_OHM;
+ p3h2x4x_i3c_hub->hub_config.tp2367_io_strength = P3H2X4X_IO_STRENGTH_20_OHM;
+
+ 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 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);
+
+ 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->parent->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);
+
+ /* 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");
+
+ /* 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 void p3h2x4x_i3c_hub_remove(struct platform_device *pdev)
+{
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = platform_get_drvdata(pdev);
+ struct p3h2x4x_dev *p3h2x4x = dev_get_drvdata(pdev->dev.parent);
+ u8 i;
+
+ for (i = 0; i < P3H2X4X_TP_MAX_COUNT; i++) {
+ if (!p3h2x4x_i3c_hub->tp_bus[i].is_registered)
+ continue;
+
+ if (p3h2x4x_i3c_hub->hub_config.tp_config[i].mode == P3H2X4X_TP_MODE_SMBUS)
+ i2c_del_adapter(p3h2x4x_i3c_hub->tp_bus[i].tp_smbus_adapter);
+ else if (p3h2x4x_i3c_hub->hub_config.tp_config[i].mode == P3H2X4X_TP_MODE_I3C)
+ i3c_master_unregister(&p3h2x4x_i3c_hub->tp_bus[i]
+ .hub_controller.controller);
+ }
+}
+
+static struct platform_driver p3h2x4x_i3c_hub_driver = {
+ .driver = {
+ .name = "p3h2x4x-i3c-hub",
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+ .probe = p3h2x4x_i3c_hub_probe,
+ .remove = p3h2x4x_i3c_hub_remove,
+};
+module_platform_driver(p3h2x4x_i3c_hub_driver);
+
+MODULE_AUTHOR("Aman Kumar Pandey <aman.kumarpandey@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..38505dda0e81
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
@@ -0,0 +1,124 @@
+// 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);
+
+ if (IS_ERR(p3h2x4x_hub->hub))
+ return PTR_ERR(p3h2x4x_hub->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;
+
+ 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..43639f04b77a
--- /dev/null
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
@@ -0,0 +1,269 @@
+// 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)) {
+ 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;
+
+ 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);
+ continue;
+ }
+
+ 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)) {
+ 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,
+};
+
+/**
+ * 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) {
+ devm_kfree(hub->dev, smbus_adapter);
+ 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)
+ return ret;
+
+ return regmap_write(hub->regmap, P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG, P3H2X4X_IBI_DISABLED);
+}
--
2.25.1
^ permalink raw reply related
* [PATCH v10 9/9] i3c: hub: p3h2x4x: Add SMBus slave mode support
From: Lakshay Piplani @ 2026-05-25 6:42 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
In-Reply-To: <20260525064209.2263045-1-lakshay.piplani@nxp.com>
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 v10:
- Split SMBus slave mode support into a separate patch
---
---
drivers/i3c/hub/p3h2840_i3c_hub.h | 10 ++
drivers/i3c/hub/p3h2840_i3c_hub_common.c | 6 +
drivers/i3c/hub/p3h2840_i3c_hub_i3c.c | 17 +++
drivers/i3c/hub/p3h2840_i3c_hub_smbus.c | 181 +++++++++++++++++++++++
4 files changed, 214 insertions(+)
diff --git a/drivers/i3c/hub/p3h2840_i3c_hub.h b/drivers/i3c/hub/p3h2840_i3c_hub.h
index d69fafbac584..84d9c66547c6 100644
--- a/drivers/i3c/hub/p3h2840_i3c_hub.h
+++ b/drivers/i3c/hub/p3h2840_i3c_hub.h
@@ -324,4 +324,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_common.c b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
index f1a24a3d3ffa..26d5e13455ca 100644
--- a/drivers/i3c/hub/p3h2840_i3c_hub_common.c
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
@@ -328,6 +328,12 @@ static void p3h2x4x_i3c_hub_remove(struct platform_device *pdev)
i3c_master_unregister(&p3h2x4x_i3c_hub->tp_bus[i]
.hub_controller.controller);
}
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ if (p3h2x4x->i3cdev) {
+ i3c_device_disable_ibi(p3h2x4x->i3cdev);
+ i3c_device_free_ibi(p3h2x4x->i3cdev);
+ }
+#endif
}
static struct platform_driver p3h2x4x_i3c_hub_driver = {
diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
index 38505dda0e81..544d961d0b8a 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)
{
@@ -120,5 +128,14 @@ 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;
}
+#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)
+ return ret;
+#endif
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
index 43639f04b77a..12fac276b041 100644
--- a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
+++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
@@ -15,6 +15,135 @@ 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, i;
+ u32 packet_len, slave_address, ret;
+
+ 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));
+
+ /* notify slave driver about received data */
+ if ((p3h2x4x_i3c_hub->tp_bus[tp].tp_smbus_client->addr & 0x7f) == (slave_address >> 1)) {
+ i2c_slave_event(p3h2x4x_i3c_hub->tp_bus[tp].tp_smbus_client,
+ I2C_SLAVE_WRITE_REQUESTED, (u8 *)&slave_address);
+ for (i = 0; i < packet_len; i++) {
+ temp = slave_rx_buffer[i];
+ i2c_slave_event(p3h2x4x_i3c_hub->tp_bus[tp].tp_smbus_client,
+ I2C_SLAVE_WRITE_RECEIVED, &temp);
+ }
+ i2c_slave_event(p3h2x4x_i3c_hub->tp_bus[tp].tp_smbus_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)
+{
+ u32 target_port_status, payload_byte_one, payload_byte_two;
+ struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub;
+ u32 ret, i;
+
+ payload_byte_one = (*(int *)payload->data);
+
+ 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 = (*(int *)(payload->data + 4));
+ 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);
+ return;
+ }
+
+ /* 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)
@@ -199,11 +328,63 @@ 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;
+
+ ret = regmap_clear_bits(hub->regmap,
+ P3H2X4X_TP_SMBUS_AGNT_IBI_CONFIG,
+ bus->tp_mask);
+ if (ret)
+ return ret;
+
+ bus->tp_smbus_client = NULL;
+ hub->hub_config.tp_config[bus->tp_port].ibi_en = false;
+
+ 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
* [PATCH v2 1/2] spi: tegra210-quad: Allocate DMA memory for DMA engine
From: Aaron Kling via B4 Relay @ 2026-05-25 6:47 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Laxman Dewangan, Mark Brown, Sumit Semwal, Christian König,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-tegra, linux-spi, linux-kernel, linux-media, dri-devel,
linaro-mm-sig, devicetree, Thierry Reding, Aaron Kling
In-Reply-To: <20260525-tegra194-qspi-iommu-v2-0-a11c53f804b2@gmail.com>
From: Aaron Kling <webgeek1234@gmail.com>
When the SPI controllers are running in DMA mode, it is the DMA engine
that performs the memory accesses rather than the SPI controller. Pass
the DMA engine's struct device pointer to the DMA API to make sure the
correct DMA operations are used.
Suggested-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
drivers/spi/spi-tegra210-quad.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index db28dd556484b2..588a929a97850a 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -226,11 +226,13 @@ struct tegra_qspi {
struct completion xfer_completion;
struct spi_transfer *curr_xfer;
+ struct device *rx_dma_dev;
struct dma_chan *rx_dma_chan;
u32 *rx_dma_buf;
dma_addr_t rx_dma_phys;
struct dma_async_tx_descriptor *rx_dma_desc;
+ struct device *tx_dma_dev;
struct dma_chan *tx_dma_chan;
u32 *tx_dma_buf;
dma_addr_t tx_dma_phys;
@@ -574,15 +576,15 @@ static int tegra_qspi_dma_map_xfer(struct tegra_qspi *tqspi, struct spi_transfer
len = DIV_ROUND_UP(tqspi->curr_dma_words * tqspi->bytes_per_word, 4) * 4;
if (t->tx_buf) {
- t->tx_dma = dma_map_single(tqspi->dev, (void *)tx_buf, len, DMA_TO_DEVICE);
- if (dma_mapping_error(tqspi->dev, t->tx_dma))
+ t->tx_dma = dma_map_single(tqspi->tx_dma_dev, (void *)tx_buf, len, DMA_TO_DEVICE);
+ if (dma_mapping_error(tqspi->tx_dma_dev, t->tx_dma))
return -ENOMEM;
}
if (t->rx_buf) {
- t->rx_dma = dma_map_single(tqspi->dev, (void *)rx_buf, len, DMA_FROM_DEVICE);
- if (dma_mapping_error(tqspi->dev, t->rx_dma)) {
- dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
+ t->rx_dma = dma_map_single(tqspi->rx_dma_dev, (void *)rx_buf, len, DMA_FROM_DEVICE);
+ if (dma_mapping_error(tqspi->rx_dma_dev, t->rx_dma)) {
+ dma_unmap_single(tqspi->tx_dma_dev, t->tx_dma, len, DMA_TO_DEVICE);
return -ENOMEM;
}
}
@@ -597,9 +599,9 @@ static void tegra_qspi_dma_unmap_xfer(struct tegra_qspi *tqspi, struct spi_trans
len = DIV_ROUND_UP(tqspi->curr_dma_words * tqspi->bytes_per_word, 4) * 4;
if (t->tx_buf)
- dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
+ dma_unmap_single(tqspi->tx_dma_dev, t->tx_dma, len, DMA_TO_DEVICE);
if (t->rx_buf)
- dma_unmap_single(tqspi->dev, t->rx_dma, len, DMA_FROM_DEVICE);
+ dma_unmap_single(tqspi->rx_dma_dev, t->rx_dma, len, DMA_FROM_DEVICE);
}
static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct spi_transfer *t)
@@ -745,7 +747,7 @@ static int tegra_qspi_start_cpu_based_transfer(struct tegra_qspi *qspi, struct s
static void tegra_qspi_deinit_dma(struct tegra_qspi *tqspi)
{
if (tqspi->tx_dma_buf) {
- dma_free_coherent(tqspi->dev, tqspi->dma_buf_size,
+ dma_free_coherent(tqspi->tx_dma_dev, tqspi->dma_buf_size,
tqspi->tx_dma_buf, tqspi->tx_dma_phys);
tqspi->tx_dma_buf = NULL;
}
@@ -756,7 +758,7 @@ static void tegra_qspi_deinit_dma(struct tegra_qspi *tqspi)
}
if (tqspi->rx_dma_buf) {
- dma_free_coherent(tqspi->dev, tqspi->dma_buf_size,
+ dma_free_coherent(tqspi->rx_dma_dev, tqspi->dma_buf_size,
tqspi->rx_dma_buf, tqspi->rx_dma_phys);
tqspi->rx_dma_buf = NULL;
}
@@ -782,6 +784,7 @@ static int tegra_qspi_init_dma(struct tegra_qspi *tqspi)
}
tqspi->rx_dma_chan = dma_chan;
+ tqspi->rx_dma_dev = dmaengine_get_dma_device(tqspi->rx_dma_chan);
dma_chan = dma_request_chan(tqspi->dev, "tx");
if (IS_ERR(dma_chan)) {
@@ -790,15 +793,19 @@ static int tegra_qspi_init_dma(struct tegra_qspi *tqspi)
}
tqspi->tx_dma_chan = dma_chan;
+ tqspi->tx_dma_dev = dmaengine_get_dma_device(tqspi->tx_dma_chan);
} else {
if (!device_iommu_mapped(tqspi->dev)) {
dev_warn(tqspi->dev,
"IOMMU not enabled in device-tree, falling back to PIO mode\n");
return 0;
}
+
+ tqspi->rx_dma_dev = tqspi->dev;
+ tqspi->tx_dma_dev = tqspi->dev;
}
- dma_buf = dma_alloc_coherent(tqspi->dev, tqspi->dma_buf_size, &dma_phys, GFP_KERNEL);
+ dma_buf = dma_alloc_coherent(tqspi->rx_dma_dev, tqspi->dma_buf_size, &dma_phys, GFP_KERNEL);
if (!dma_buf) {
err = -ENOMEM;
goto err_out;
@@ -807,7 +814,7 @@ static int tegra_qspi_init_dma(struct tegra_qspi *tqspi)
tqspi->rx_dma_buf = dma_buf;
tqspi->rx_dma_phys = dma_phys;
- dma_buf = dma_alloc_coherent(tqspi->dev, tqspi->dma_buf_size, &dma_phys, GFP_KERNEL);
+ dma_buf = dma_alloc_coherent(tqspi->tx_dma_dev, tqspi->dma_buf_size, &dma_phys, GFP_KERNEL);
if (!dma_buf) {
err = -ENOMEM;
goto err_out;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/2] arm64: tegra: Enable DMA Support on Tegra194 QSPI
From: Aaron Kling via B4 Relay @ 2026-05-25 6:47 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Laxman Dewangan, Mark Brown, Sumit Semwal, Christian König,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-tegra, linux-spi, linux-kernel, linux-media, dri-devel,
linaro-mm-sig, devicetree, Thierry Reding, Aaron Kling
The reason for this is to properly support the spi nor chip on the
Jetson Xavier NX module. Prior to this, it would time out on all
transfers and sometimes even trigger a cbb fault, locking up the entire
unit. With this, reading and writing to the flash memory works as
expected.
This also fixes the tegra210-quad spi driver to properly use the dma
memory space instead of the spi controllers. Without this, enabling dma
on the controllers results in mmu faults.
The driver change has only been tested on tegra210 / p3450 and tegra194
/ p3518 as that is the only available test platforms. Tegra234 and
Tegra241 should also be verified. I have p3766 for tegra234, but the
qspi flash memory is firewalled by mb1 on all publicly available
bootloaders, and no other spi devices are part of the devkit.
---
Changes in v2:
- Drop bindings patches
- Add patch to use dma memory space instead of the spi controllers when
dma is enabled.
- Drop iommu properties from final patch
- Link to v1: https://lore.kernel.org/r/20260515-tegra194-qspi-iommu-v1-0-57dfb63cd3d6@gmail.com
---
Aaron Kling (2):
spi: tegra210-quad: Allocate DMA memory for DMA engine
arm64: tegra: Enable DMA Support on Tegra194 QSPI
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 4 ++++
drivers/spi/spi-tegra210-quad.c | 29 ++++++++++++++++++-----------
2 files changed, 22 insertions(+), 11 deletions(-)
---
base-commit: c1ecb239fa3456529a32255359fc78b69eb9d847
change-id: 20260515-tegra194-qspi-iommu-e4e4644d5fdf
Best regards,
--
Aaron Kling <webgeek1234@gmail.com>
^ permalink raw reply
* [PATCH v2 2/2] arm64: tegra: Enable DMA Support on Tegra194 QSPI
From: Aaron Kling via B4 Relay @ 2026-05-25 6:47 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Laxman Dewangan, Mark Brown, Sumit Semwal, Christian König,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-tegra, linux-spi, linux-kernel, linux-media, dri-devel,
linaro-mm-sig, devicetree, Aaron Kling
In-Reply-To: <20260525-tegra194-qspi-iommu-v2-0-a11c53f804b2@gmail.com>
From: Aaron Kling <webgeek1234@gmail.com>
Without dma enabled pio mode is used and flash storage such as the one
on the p3668 module times out and cannot complete any transfers. In some
cases, these timeouts cause hangs and cbb faults.
Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 1d659454a6f9fe..e2ddbc6715d5e8 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -923,6 +923,8 @@ spi@3270000 {
<&bpmp TEGRA194_CLK_QSPI0_PM>;
clock-names = "qspi", "qspi_out";
resets = <&bpmp TEGRA194_RESET_QSPI0>;
+ dmas = <&gpcdma 5>, <&gpcdma 5>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -1013,6 +1015,8 @@ spi@3300000 {
<&bpmp TEGRA194_CLK_QSPI1_PM>;
clock-names = "qspi", "qspi_out";
resets = <&bpmp TEGRA194_RESET_QSPI1>;
+ dmas = <&gpcdma 6>, <&gpcdma 6>;
+ dma-names = "rx", "tx";
status = "disabled";
};
--
2.53.0
^ permalink raw reply related
* Re: [PATCH RFC v5 08/18] riscv_cbqri: Add capacity controller monitoring device ops
From: Drew Fustini @ 2026-05-25 6:58 UTC (permalink / raw)
To: sashiko-reviews; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260525002922.B257D1F000E9@smtp.kernel.org>
On Mon, May 25, 2026 at 12:29:22AM +0000, sashiko-bot@kernel.org wrote:
> > + /* Probe monitoring features */
> > + err = cbqri_probe_feature(ctrl, CBQRI_CC_MON_CTL_OFF,
> > + CBQRI_CC_MON_CTL_OP_READ_COUNTER, &status,
> > + &has_mon_at_code);
>
> [Severity: Medium]
> Is it safe to reuse cbqri_probe_feature() for the MON_CTL register here?
>
> cbqri_probe_feature() attempts to verify Access Type (AT) support by writing
> to the AT field bits. However, the MON_CTL register does not define an AT
> field, which means this implicitly writes to reserved (WPRI) bits. Could
> this helper be updated to conditionally skip the AT probe sequence for
> monitoring registers?
WPRI tolerates the write so this is not a correctness issue today. But
to avoid confusion, I will split cbqri_probe_feature() so the AT probe
sequence is only run for alloc_ctl registers.
> Will this probe sequence falsely disable monitoring on hardware?
>
> This invokes cbqri_probe_feature() using the READ_COUNTER operation, which
> zeroes out all other fields including EVT_ID. This sends a read command for
> the NONE event (EVT_ID=0) to an unconfigured slot, which the hardware may
> reject with an error status. If the hardware returns an error, this check
> will fail and disable monitoring. Could this probe instead use
> CONFIG_EVENT or handle expected failure codes?
The NONE event as is a valid configured state. But I will switch the
probe operation to CONFIG_EVENT with a probe-safe EVT_ID to remove the
ambiguity.
> > +/*
> > + * Pre-arm every MCID with the Occupancy event so a subsequent READ_COUNTER
> > + * just snapshots the live counter rather than re-configuring the slot.
> > + * Called once per CC during resctrl-side cpuhp online for the L3 monitoring
> > + * domain.
> > + */
> > +int cbqri_init_mon_counters(struct cbqri_controller *ctrl)
> > +{
> > + int i, err;
> > +
> > + for (i = 0; i < ctrl->mcid_count; i++) {
> > + mutex_lock(&ctrl->lock);
> > + err = cbqri_mon_op(ctrl, CBQRI_CC_MON_CTL_OFF,
> > + CBQRI_CC_MON_CTL_OP_CONFIG_EVENT,
> > + i, CBQRI_CC_EVT_ID_OCCUPANCY, NULL);
>
> [Severity: Medium]
> Does mcid_count need bounds checking against the physical hardware limit?
>
> The ctrl->mcid_count value is used as the loop boundary, but if firmware
> supplies an erroneously large value, this loop could block the CPU for an
> extended period during hotplug.
>
> Additionally, because the MCID field mask is 12 bits, any mcid_count larger
> than 4096 will be truncated in cbqri_mon_op(), causing logical MCIDs to
> incorrectly alias to the same physical hardware slot. Should an upper bound be
> enforced on mcid_count?
I'll add a check in v6.
> > +/* mon_ctl field masks (CC and BC share an identical OP/MCID/EVT_ID/STATUS layout) */
> > +#define CBQRI_MON_CTL_OP_MASK GENMASK(4, 0)
> > +#define CBQRI_MON_CTL_MCID_MASK GENMASK(19, 8)
> > +#define CBQRI_MON_CTL_EVT_ID_MASK GENMASK(27, 20)
>
> [Severity: High]
> Will using GENMASK() here inadvertently clear the upper 32 bits of the 64-bit
> control register on 32-bit architectures?
>
> When used with FIELD_MODIFY(), ~(_mask) will evaluate to a 32-bit unsigned
> value which gets zero-extended to 64 bits. When this is bitwise ANDed with
> the 64-bit register variable, it will wipe out the upper 32 bits, corrupting
> the STATUS field before the register is written back. Should these 64-bit
> field masks be defined with GENMASK_ULL() instead?
Yes, I will chnage to GENMASK_ULL.
-Drew
^ permalink raw reply
* Re: [PATCH v10 1/9] i3c: master: rename i3c_master_reattach_i3c_dev() to *_locked
From: sashiko-bot @ 2026-05-25 7:05 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260525064209.2263045-2-lakshay.piplani@nxp.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The return value of `i3c_master_reattach_i3c_dev_locked()` is ignored in `i3c_master_add_i3c_dev_locked()`, which can lead to an I3C address collision and bus address slot corruption on failure.
--
commit c8a363e4e146cbd0fba01ffdf2e47defa9bd5b84
Author: Lakshay Piplani <lakshay.piplani@nxp.com>
i3c: master: rename i3c_master_reattach_i3c_dev() to *_locked
This commit renames i3c_master_reattach_i3c_dev() to *_locked() to make the
locking requirement explicit and consistent with other I3C core helpers
that require the bus lock to be held by the caller.
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 6b8df8089a357..c45b450af319d 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> @@ -2358,7 +2358,7 @@ int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
> if (!ret) {
> old_dyn_addr = newdev->info.dyn_addr;
> newdev->info.dyn_addr = expected_dyn_addr;
> - i3c_master_reattach_i3c_dev(newdev, old_dyn_addr);
> + i3c_master_reattach_i3c_dev_locked(newdev, old_dyn_addr);
[Severity: High]
This is a pre-existing issue, but does this code ignore the return value
of the reattach function on failure?
If i3c_master_reattach_i3c_dev_locked() fails (for example, if the
controller's reattach_i3c_dev callback returns an error), it internally
calls i3c_master_put_i3c_addrs(). This frees the dynamic and static address
slots by marking them as I3C_ADDR_SLOT_FREE in the master's bus address map.
Because i3c_master_add_i3c_dev_locked() completely ignores the return value
and proceeds to return 0 (success), the device remains attached and active
on the bus tracking the newly assigned dynamic address, while the bus state
considers the address slot empty.
Could this structural inconsistency lead to the master assigning the exact
same dynamic address to a new hot-joining device or during subsequent
dynamic address assignment, resulting in an address collision and bus
communication corruption?
> } else {
> dev_err(&master->dev,
> "Failed to assign reserved/old address to device %d%llx",
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260525064209.2263045-1-lakshay.piplani@nxp.com?part=1
^ permalink raw reply
* Re: [PATCH v4] dt-bindings: iommu: riscv: Add bindings for Tenstorrent RISC-V IOMMU
From: Drew Fustini @ 2026-05-25 7:06 UTC (permalink / raw)
To: Tomasz Jeznach, Joerg Roedel, Will Deacon, Robin Murphy,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: iommu, linux-riscv, devicetree, linux-kernel, Joel Stanley,
Joerg Roedel, Nicholas Piggin
In-Reply-To: <20260521170652.1880662-2-fustini@kernel.org>
On Thu, May 21, 2026 at 10:06:33AM -0700, Drew Fustini wrote:
> From: Nicholas Piggin <npiggin@gmail.com>
>
> Extend the binding to cover details specific to the Tenstorrent RISC-V
> IOMMU. In particular, a second register range is added which contains
> M-privileged registers, e.g., PMAs and PMPs.
>
> The RISC-V spec S-privileged registers remain in the first register
> range and are compatible with "riscv,iommu" so the Linux driver does not
> notice any difference, but the binding will be used by OpenSBI and
> potentially other M-mode software.
>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Acked-by: Joerg Roedel <joerg.roedel@amd.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> [fustini: fix dt_binding_check errors]
> Signed-off-by: Drew Fustini <fustini@kernel.org>
This has been applied to tenstorrent-dt-for-next.
https://git.kernel.org/pub/scm/linux/kernel/git/tenstorrent/linux.git/commit/?id=33583baeb1ba7d328e6a9775d889036900b74cdb
Drew
^ permalink raw reply
* Re: [PATCH v4 2/3] clk: qcom: camcc-glymur: Add camera clock controller driver
From: Jagadeesh Kona @ 2026-05-25 7:06 UTC (permalink / raw)
To: Bryan O'Donoghue, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Brian Masney, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Konrad Dybcio
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, Taniya Das,
Konrad Dybcio
In-Reply-To: <0a197b43-a672-4849-91c7-6e5bfe3175f7@kernel.org>
On 5/18/2026 5:51 PM, Bryan O'Donoghue wrote:
> On 18/05/2026 11:23, Jagadeesh Kona wrote:
>>
>>
>> On 5/18/2026 1:05 PM, Bryan O'Donoghue wrote:
>>> On 17/05/2026 18:33, Jagadeesh Kona wrote:
>>>> +/* 1200.0 MHz Configuration */
>>>> +static const struct alpha_pll_config cam_cc_pll0_config = {
>>>> + .l = 0x3e,
>>>> + .alpha = 0x8000,
>>>> + .config_ctl_val = 0x25c400e7,
>>>> + .config_ctl_hi_val = 0x0a8060e0,
>>>> + .config_ctl_hi1_val = 0xf51dea20,
>>>> + .user_ctl_val = 0x00008408,
>>>> + .user_ctl_hi_val = 0x00000002,
>>>> +};
>>>
>>> I'll again push back on these magic numbers.
>>>
>>> At the very least you should be mentioning in the cover letter log why you _aren't_ making that change.
>>>
>>> Just reposting and hoping it slips by the person making the comment isn't too cool.
>>>
>>> Why can't qcom update the python? script that generates this code to enumerate fields instead of magic numbers here ?
>>>
>>> I get you don't want to do it but, just ignoring the review feedback is no OK.
>>>
>>> What gives ?
>>>
>>
>> Hi Bryan,
>>
>> I haven't ignored your comments & already responded to your earlier comment on why the bit fields are not
>> defined. Most of these values are static settings we get from PLL HW team and we program them only once
>> as is during bootup and are never reused again anywhere from PLL code, so these bits are not defined.
>>
>> Please find the earlier responses for your comments below:
>> https://lore.kernel.org/all/b92a2cbb-fe8d-4378-aa02-d91e2e4dfff4@oss.qualcomm.com/
>> https://lore.kernel.org/all/009ecdbb-2297-44eb-862d-233e3290691c@oss.qualcomm.com/
>>
>> Thanks,
>> Jagadeesh
>
> That's not in your overview letter so generally I'd advise to include things like "did X because Y" - "didn't do Q because Z" anyway, how does it make a difference if the values are static ?
>
> They are no less magic numbers that way.
>
> What exactly is the resistance to defining the bits ?
>
> I'll state again - when a vendor is submitting something upstream where that vendor 100% controls their own documentation - there's no reason at all to be presenting magic hex numbers - even more the case with generated code.
>
> Just update the script to enumerate the bit fields, I honestly don't get the aversion.
>
Hi Bryan,
There’s no standard interface for these bits, and bit definitions/fields vary across PLL types.
So, common macros aren’t feasible and would need redefinitions per controller. Since these bits
are not reused elsewhere, IMO directly using values from the hardware documentation keeps the
implementation simpler, avoids unnecessary abstraction, and makes debugging—through direct
comparison with the hardware spec easier.
Thanks,
Jagadeesh
^ permalink raw reply
* Re: [PATCH v9 0/5] Enable ICE clock scaling
From: Abhinaba Rakshit @ 2026-05-25 7:06 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
James E.J. Bottomley, Martin K. Petersen, Adrian Hunter,
Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Neeraj Soni, Harshal Dev, Kuldeep Singh
Cc: linux-arm-msm, linux-kernel, linux-scsi, linux-mmc, devicetree,
Konrad Dybcio
In-Reply-To: <20260525-enable-ice-clock-scaling-v9-0-c84613e9ce47@oss.qualcomm.com>
On Mon, May 25, 2026 at 12:55:47AM +0530, Abhinaba Rakshit wrote:
> Introduce support for dynamic clock scaling of the ICE (Inline Crypto Engine)
> using the OPP framework. During ICE device probe, the driver now attempts to
> parse an optional OPP table from the ICE-specific device tree node for
> DVFS-aware operations. API qcom_ice_scale_clk is exposed by ICE driver
> and is invoked by UFS host controller driver in response to clock scaling
> requests, ensuring coordination between ICE and host controller.
>
> For MMC controllers that do not support clock scaling, the ICE clock frequency
> is kept aligned with the MMC controller’s clock rate (TURBO) to ensure
> consistent operation.
>
> Dynamic clock scaling based on OPP tables enables better power-performance
> trade-offs. By adjusting ICE clock frequencies according to workload and power
> constraints, the system can achieve higher throughput when needed and
> reduce power consumption during idle or low-load conditions.
>
> The OPP table remains optional, absence of the table will not cause
> probe failure. However, in the absence of an OPP table, ICE clocks will
> remain at their default rates, which may limit performance under
> high-load scenarios or prevent performance optimizations during idle periods.
>
> Testing:
> * dtbs_check
> * Validated on Rb3Gen2 and qcs8300-ride-sx
>
> Merge Order and Dependencies
> ============================
>
> Patch 2 is dependent on patch 1 for the qcom_ice_scale_clk API to be available.
> Patch 3 is dependent on patch 1 for the qcom_ice_scale_clk API to be available.
>
> Due to dependency, all patches should go through Qcom SoC tree.
>
> This patchset supersedes earlier ICE clock scaling series (v1–v8) with updated dependencies.
> Hence, this patchset also *Depends-On* the following patchseries:
>
> [1] Add explicit clock vote and enable power-domain for QCOM-ICE
> https://lore.kernel.org/all/20260416-qcom_ice_power_and_clk_vote-v5-0-5ccf5d7e2846@oss.qualcomm.com/
>
> [2] Enable Inline crypto engine for kodiak and monaco
> https://lore.kernel.org/all/20260310113557.348502-1-neeraj.soni@oss.qualcomm.com/
>
> [3] Enable iface clock and power domain for kodiak and monaco ice sdhc
> https://lore.kernel.org/linux-arm-msm/20260409-ice_emmc_clock_addition-v2-0-90bbcc057361@oss.qualcomm.com/
>
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
> Changes in v9:
> - Kodiak ICE eMMC OPP-table entry corresponding to 300MHz is updated with SVS_L1.
> - Add 75MHz for Monaco ICE eMMC OPP-table.
> - Fix error handling and initialization of has_opp variable.
> - Pass ULONG_MAX as target freq instead of INT_MAX from sdhci_ice_init as it better adjusts the data-type of
> the function qcom_ice_scale_clk.
> - Link to v8: https://lore.kernel.org/r/20260409-enable-ice-clock-scaling-v8-0-ca1129798606@oss.qualcomm.com
Hello,
It appears that some of the dependencies for this patch series have already been picked,
and there have also been recent changes in the ICE driver that conflict with my patches.
Please avoid picking this patch series for now. I will post a new version based on the
tip of linux-next, with the ICE driver conflicts resolved.
Abhinaba Rakshit
^ permalink raw reply
* Re: [PATCH v7 3/4] soc: qcom: geni-se: Keep pinctrl active for multi-owner controllers
From: Mukesh Savaliya @ 2026-05-25 7:16 UTC (permalink / raw)
To: Bjorn Andersson
Cc: viken.dadhaniya, andi.shyti, robh, krzk+dt, conor+dt, vkoul,
Frank.Li, konradybcio, dmitry.baryshkov, linmq006, quic_jseerapu,
agross, linux-arm-msm, linux-i2c, devicetree, linux-kernel,
dmaengine, krzysztof.kozlowski, bartosz.golaszewski,
bjorn.andersson, konrad.dybcio
In-Reply-To: <ag_HGVQjIQuMoKO6@baldur>
Hi Bjorn, Thanks for the detailed review.
On 5/22/2026 8:36 AM, Bjorn Andersson wrote:
> On Thu, Apr 23, 2026 at 08:25:50PM +0530, Mukesh Kumar Savaliya wrote:
>> On platforms where a GENI Serial Engine is shared with another system
>> processor, selecting the "sleep" pinctrl state can disrupt ongoing
>> transfers initiated by the other processor.
>>
>
> Isn't it strange that the DeviceTree will define a sleep state for the
> OS to select, but when this other property is set the OS should never
> select this state?
>
The intent here is that for multi-owner configurations the
“sleep” pinctrl state is not safe to use, since the pins may
still be actively driven by another execution environment.
Selecting the sleep state in such cases can disrupt transfers
initiated by the other owner.
You're right that this constraint is currently not described
in the binding, which makes the behavior non-obvious.
shall i update the DT binding to clarify that when
"qcom,qup-multi-owner" is present ? The OS must not transition
the pins to the "sleep" state, as the hardware is shared and
may be active outside of Linux control.
Alternatively, we can also consider relaxing the requirement
to define a sleep state for such nodes if that aligns better
with DT expectations.
>> Teach geni_se_resources_off() to skip selecting the pinctrl sleep state
>> when the Serial Engine is marked as shared, while still allowing the
>> rest of the resource shutdown sequence to proceed.
>>
>> This is required for multi-owner configurations (described via DeviceTree
>> with qcom,qup-multi-owner on the protocol controller node).
>>
>
> The requirement as such is reasonable, but you don't define in the
> binding that when this property is set, the sleep state must not be
> selected by the OS...
>
Please let me know if you prefer second approach over the first, i shall
update accordingly.
> Regards,
> Bjorn
>
>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
>> ---
[...]
^ permalink raw reply
* Re: [PATCH v7 4/4] i2c: qcom-geni: Support multi-owner controllers in GPI mode
From: Mukesh Savaliya @ 2026-05-25 7:19 UTC (permalink / raw)
To: Bjorn Andersson
Cc: viken.dadhaniya, andi.shyti, robh, krzk+dt, conor+dt, vkoul,
Frank.Li, konradybcio, dmitry.baryshkov, linmq006, quic_jseerapu,
agross, linux-arm-msm, linux-i2c, devicetree, linux-kernel,
dmaengine, krzysztof.kozlowski, bartosz.golaszewski,
bjorn.andersson, konrad.dybcio
In-Reply-To: <ag_Ig7aQNNakiry_@baldur>
Thanks for the detailed review, Bjorn.
On 5/22/2026 8:45 AM, Bjorn Andersson wrote:
> On Thu, Apr 23, 2026 at 08:25:51PM +0530, Mukesh Kumar Savaliya wrote:
>> Some platforms use a QUP-based I2C controller in a configuration where the
>> controller is shared with another system processor. In this setup the
>> operating system must not assume exclusive ownership of the controller or
>> its associated pins.
>>
>> Add support for enabling multi-owner operation when DeviceTree specifies
>> qcom,qup-multi-owner. When enabled, mark the underlying serial engine as
>> shared so the common GENI resource handling avoids selecting the "sleep"
>> pinctrl state, which could disrupt transfers initiated by the other
>> processor.
>>
>> For GPI mode transfers, request lock/unlock TRE sequencing from the GPI
>
> "For GPI mode transfers" is there any other form?
>
Good point. The lock/unlock sequencing is only relevant when the
controller operates in GPI DMA mode. In FIFO mode the transfer path
does not go through the GPI engine, so this mechanism does not apply.
Let me clarify this wording in the commit message to make it explicit.
>> driver by setting a single lock_action selector per message, emitting lock
>> before the first message and unlock after the last message (handling the
>> single-message case as well). This serializes access to the shared
>> controller without requiring message-position flags to be passed into the
>> DMA engine layer.
>>
>> Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
>> ---
>> drivers/i2c/busses/i2c-qcom-geni.c | 22 +++++++++++++++++++++-
>> 1 file changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
>> index ae609bdd2ec4..a396ddc7d8f4 100644
>> --- a/drivers/i2c/busses/i2c-qcom-geni.c
>> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
>> @@ -815,6 +815,14 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>> if (i < num - 1)
>> peripheral.stretch = 1;
>>
>> + peripheral.lock_action = GPI_LOCK_NONE;
>> + if (gi2c->se.multi_owner) {
>> + if (i == 0)
>> + peripheral.lock_action = GPI_LOCK_ACQUIRE;
>> + else if (i == num - 1)
>> + peripheral.lock_action = GPI_LOCK_RELEASE;
>
> You say above that single-messages case is handled, but if num == 1 then
> we will hit i == 0, set the acquire, we will not hit else, and then we
> will exit the loop. What am I missing?
>
You are right, the current implementation does not handle the
single-message case correctly. In that case we need both acquire
and release for the same message.
I will fix this by explicitly handling the num == 1 case and setting
both lock and unlock for that transfer.
>> + }
>> +
>> peripheral.addr = msgs[i].addr;
>> if (i > 0 && (!(msgs[i].flags & I2C_M_RD)))
>> peripheral.multi_msg = false;
>> @@ -1014,6 +1022,11 @@ static int geni_i2c_probe(struct platform_device *pdev)
>> gi2c->clk_freq_out = I2C_MAX_STANDARD_MODE_FREQ;
>> }
>>
>> + if (of_property_read_bool(pdev->dev.of_node, "qcom,qup-multi-owner")) {
>> + gi2c->se.multi_owner = true;
>
> gi2c->se.multi_owner = of_property_read_bool(pdev->dev.of_node, "qcom,qup-multi-owner");
>
Agreed, this can be simplified. I will update accordingly.
>> + dev_dbg(&pdev->dev, "I2C controller is shared with another system processor\n");
>> + }
>> +
>> if (has_acpi_companion(dev))
>> ACPI_COMPANION_SET(&gi2c->adap.dev, ACPI_COMPANION(dev));
>>
>> @@ -1089,7 +1102,9 @@ static int geni_i2c_probe(struct platform_device *pdev)
>> }
>>
>> if (fifo_disable) {
>> - /* FIFO is disabled, so we can only use GPI DMA */
>> + /* FIFO is disabled, so we can only use GPI DMA.
>
> That's not how we format comments outside the network subsystem.
>
Ack, I will fix the comment formatting.
> Regards,
> Bjorn
>
>> + * SE can be shared in GSI mode between subsystems, each SS owns a GPII.
>> + */
>> gi2c->gpi_mode = true;
>> ret = setup_gpi_dma(gi2c);
>> if (ret)
>> @@ -1098,6 +1113,11 @@ static int geni_i2c_probe(struct platform_device *pdev)
>> dev_dbg(dev, "Using GPI DMA mode for I2C\n");
>> } else {
>> gi2c->gpi_mode = false;
>> +
>> + if (gi2c->se.multi_owner)
>> + return dev_err_probe(dev, -EINVAL,
>> + "I2C sharing not supported in non GSI mode\n");
>> +
>> tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
>>
>> /* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
>> --
>> 2.43.0
>>
^ permalink raw reply
* Re: [PATCH RFC v5 09/18] riscv_cbqri: Add bandwidth controller probe and allocation device ops
From: Drew Fustini @ 2026-05-25 7:21 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260525003040.9E4E41F000E9@smtp.kernel.org>
On Mon, May 25, 2026 at 12:30:40AM +0000, sashiko-bot@kernel.org wrote:
> > +static int cbqri_bc_alloc_op(struct cbqri_controller *ctrl, int operation, int rcid)
> > +{
> > + int reg_offset = CBQRI_BC_ALLOC_CTL_OFF;
> > + int status;
> > + u64 reg;
> > +
> > + lockdep_assert_held(&ctrl->lock);
> > +
> > + if (cbqri_wait_busy_flag(ctrl, reg_offset, ®) < 0) {
> > + pr_err_ratelimited("BUSY timeout before starting operation\n");
> > + return -EIO;
> > + }
> > + FIELD_MODIFY(CBQRI_CONTROL_REGISTERS_OP_MASK, ®, operation);
> > + FIELD_MODIFY(CBQRI_CONTROL_REGISTERS_RCID_MASK, ®, rcid);
> > + iowrite64(reg, ctrl->base + reg_offset);
>
> [Severity: High]
> Does this operation inadvertently preserve the Access Type (AT) field left over
> by firmware or prior probe operations?
I'll match cbqri_cc_alloc_op() and clear AT.
> > +static int cbqri_apply_bc_field(struct cbqri_controller *ctrl, u32 closid,
> > + enum cbqri_bc_field field, u64 val)
> > +{
> > + u64 rbwb = ctrl->rbwb_cache[closid];
> > + u64 mweight = ctrl->mweight_cache[closid];
> > + u64 readback;
> > + int ret;
> > +
> > + lockdep_assert_held(&ctrl->lock);
> > +
> > + if (field == CBQRI_BC_FIELD_RBWB)
> > + rbwb = val;
> > + else
> > + mweight = val;
> > +
> > + cbqri_set_bc_bw_alloc(ctrl, rbwb, mweight);
> > +
> > + ret = cbqri_bc_alloc_op(ctrl, CBQRI_BC_ALLOC_CTL_OP_CONFIG_LIMIT, closid);
>
> Could modifying the hardware staging register before verifying the controller
> is ready cause a silent corruption of the unmodified field?
Yes, I'll change it to wait for BUSY=0 before the staging write.
> > +/*
> > + * Read the Rbwb (reserved bandwidth blocks) for closid via READ_LIMIT.
> > + */
> > +int cbqri_read_rbwb(struct cbqri_controller *ctrl, u32 closid, u64 *rbwb_out)
> > +{
> > + int err;
> > +
> > + mutex_lock(&ctrl->lock);
> > + err = cbqri_bc_alloc_op(ctrl, CBQRI_BC_ALLOC_CTL_OP_READ_LIMIT, closid);
> > + if (err == 0)
> > + *rbwb_out = cbqri_get_rbwb(ctrl);
> > + mutex_unlock(&ctrl->lock);
> > + return err;
> > +}
> > +
> > +/*
> > + * Read the Mweight (opportunistic weight) for closid via READ_LIMIT.
> > + */
> > +int cbqri_read_mweight(struct cbqri_controller *ctrl, u32 closid, u64 *mweight_out)
> > +{
> > + int err;
> > +
> > + mutex_lock(&ctrl->lock);
> > + err = cbqri_bc_alloc_op(ctrl, CBQRI_BC_ALLOC_CTL_OP_READ_LIMIT, closid);
> > + if (err == 0)
> > + *mweight_out = cbqri_get_mweight(ctrl);
> > + mutex_unlock(&ctrl->lock);
> > + return err;
> > +}
>
> [Severity: High]
> Are these standalone read functions vulnerable to the same silent READ_LIMIT
> no-op hardware bug described in the commit message?
>
> Since they execute a bare READ_LIMIT without the pre-write sentinel validation
> used in cbqri_apply_bc_field(), could they return stale data from a prior
> operation if the hardware bug triggers?
I'll fix this in v6 with sentinel.
-Drew
^ permalink raw reply
* Re: [PATCH v2 1/2] spi: tegra210-quad: Allocate DMA memory for DMA engine
From: sashiko-bot @ 2026-05-25 7:27 UTC (permalink / raw)
To: Aaron Kling via B4 Relay; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260525-tegra194-qspi-iommu-v2-1-a11c53f804b2@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Unconditional DMA unmapping in tegra_qspi_dma_unmap_xfer() causes invalid unmaps for unpacked transfers.
- [High] The error path in tegra_qspi_dma_map_xfer() unmaps tx_dma without verifying if tx_buf was provided.
- [High] Aligning the DMA transfer length up to a multiple of 4 bytes can cause the DMA engine to write out of bounds.
- [High] DMA mappings are leaked when the transfer times out or when DMA setup fails.
--
commit 78250c2468331b7f824d5bbb2690d151a21edc13
Author: Aaron Kling <webgeek1234@gmail.com>
spi: tegra210-quad: Allocate DMA memory for DMA engine
When the SPI controllers are running in DMA mode, it is the DMA engine
that performs the memory accesses rather than the SPI controller. Pass
the DMA engine's struct device pointer to the DMA API to make sure the
correct DMA operations are used.
> diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
> index db28dd556484b..588a929a97850 100644
> --- a/drivers/spi/spi-tegra210-quad.c
> +++ b/drivers/spi/spi-tegra210-quad.c
[ ... ]
> @@ -574,15 +576,15 @@ static int tegra_qspi_dma_map_xfer(struct tegra_qspi *tqspi, struct spi_transfer
> len = DIV_ROUND_UP(tqspi->curr_dma_words * tqspi->bytes_per_word, 4) * 4;
[Severity: High]
This isn't a bug introduced by this patch, but could aligning this length up
to a multiple of 4 bytes cause the DMA engine to write out of bounds?
If the client requested a transfer length that is not a multiple of 4 bytes,
the DMA engine will write up to 3 bytes past the end of the client's buffer.
If the buffer lacks sufficient alignment padding, could this cause memory
corruption?
>
> if (t->tx_buf) {
> - t->tx_dma = dma_map_single(tqspi->dev, (void *)tx_buf, len, DMA_TO_DEVICE);
> - if (dma_mapping_error(tqspi->dev, t->tx_dma))
> + t->tx_dma = dma_map_single(tqspi->tx_dma_dev, (void *)tx_buf, len, DMA_TO_DEVICE);
> + if (dma_mapping_error(tqspi->tx_dma_dev, t->tx_dma))
> return -ENOMEM;
> }
>
> if (t->rx_buf) {
> - t->rx_dma = dma_map_single(tqspi->dev, (void *)rx_buf, len, DMA_FROM_DEVICE);
> - if (dma_mapping_error(tqspi->dev, t->rx_dma)) {
> - dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
> + t->rx_dma = dma_map_single(tqspi->rx_dma_dev, (void *)rx_buf, len, DMA_FROM_DEVICE);
> + if (dma_mapping_error(tqspi->rx_dma_dev, t->rx_dma)) {
> + dma_unmap_single(tqspi->tx_dma_dev, t->tx_dma, len, DMA_TO_DEVICE);
[Severity: High]
This isn't a bug introduced by this patch, but does this error path safely
unmap tx_dma?
If t->tx_buf is NULL during a half-duplex RX-only transfer, t->tx_dma is
never mapped. If the rx_dma mapping then fails, we will attempt to unmap an
uninitialized DMA address here.
Could this corrupt the IOMMU state and trigger faults?
> return -ENOMEM;
> }
> }
> @@ -597,9 +599,9 @@ static void tegra_qspi_dma_unmap_xfer(struct tegra_qspi *tqspi, struct spi_trans
> len = DIV_ROUND_UP(tqspi->curr_dma_words * tqspi->bytes_per_word, 4) * 4;
>
> if (t->tx_buf)
> - dma_unmap_single(tqspi->dev, t->tx_dma, len, DMA_TO_DEVICE);
> + dma_unmap_single(tqspi->tx_dma_dev, t->tx_dma, len, DMA_TO_DEVICE);
> if (t->rx_buf)
> - dma_unmap_single(tqspi->dev, t->rx_dma, len, DMA_FROM_DEVICE);
> + dma_unmap_single(tqspi->rx_dma_dev, t->rx_dma, len, DMA_FROM_DEVICE);
[Severity: High]
This isn't a bug introduced by this patch, but can this cause invalid DMA
unmaps for unpacked transfers?
For unpacked transfers, tegra_qspi_start_dma_based_transfer() skips mapping
client buffers and uses a pre-allocated bounce buffer instead.
Since this function is called unconditionally and only checks for the presence
of t->tx_buf and t->rx_buf, it will call dma_unmap_single() on the
uninitialized t->tx_dma and t->rx_dma fields when tqspi->is_packed is false.
> }
>
> static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct spi_transfer *t)
[Severity: High]
This isn't a bug introduced by this patch, but are DMA mappings leaked when
the transfer times out or when DMA setup fails?
When tegra_qspi_dma_map_xfer() dynamically maps client buffers, if subsequent
steps like dmaengine_slave_config() fail in this function, the driver returns
immediately without unmapping the buffers.
Additionally, if the DMA transfer times out in
tegra_qspi_transfer_one_message(), it jumps to the exit label and returns
without calling tegra_qspi_dma_unmap_xfer().
Could this permanently leak mapped DMA addresses?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260525-tegra194-qspi-iommu-v2-0-a11c53f804b2@gmail.com?part=1
^ permalink raw reply
* [PATCH] dt-bindings: cache: qcom,llcc: Document Maili SoC
From: Jingyi Wang @ 2026-05-25 7:27 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Conor Dooley, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski
Cc: aiqun.yu, tingwei.zhang, trilok.soni, yijie.yang, linux-arm-msm,
devicetree, linux-kernel, Jingyi Wang
Add documentation for the Last Level Cache Controller (LLCC) bindings
for Qualcomm Maili SoC which fallback to Hawi SoC with System Cache
Table (SCT) programmed by firmware outside of Linux.
Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
---
.../devicetree/bindings/cache/qcom,llcc.yaml | 63 ++++++++++++----------
1 file changed, 34 insertions(+), 29 deletions(-)
diff --git a/Documentation/devicetree/bindings/cache/qcom,llcc.yaml b/Documentation/devicetree/bindings/cache/qcom,llcc.yaml
index 34e3a2d78592..a1340226a0d0 100644
--- a/Documentation/devicetree/bindings/cache/qcom,llcc.yaml
+++ b/Documentation/devicetree/bindings/cache/qcom,llcc.yaml
@@ -19,35 +19,40 @@ description: |
properties:
compatible:
- enum:
- - qcom,eliza-llcc
- - qcom,glymur-llcc
- - qcom,hawi-llcc
- - qcom,ipq5424-llcc
- - qcom,kaanapali-llcc
- - qcom,qcs615-llcc
- - qcom,qcs8300-llcc
- - qcom,qdu1000-llcc
- - qcom,sa8775p-llcc
- - qcom,sar1130p-llcc
- - qcom,sar2130p-llcc
- - qcom,sc7180-llcc
- - qcom,sc7280-llcc
- - qcom,sc8180x-llcc
- - qcom,sc8280xp-llcc
- - qcom,sdm670-llcc
- - qcom,sdm845-llcc
- - qcom,shikra-llcc
- - qcom,sm6350-llcc
- - qcom,sm7150-llcc
- - qcom,sm8150-llcc
- - qcom,sm8250-llcc
- - qcom,sm8350-llcc
- - qcom,sm8450-llcc
- - qcom,sm8550-llcc
- - qcom,sm8650-llcc
- - qcom,sm8750-llcc
- - qcom,x1e80100-llcc
+ oneOf:
+ - enum:
+ - qcom,eliza-llcc
+ - qcom,glymur-llcc
+ - qcom,hawi-llcc
+ - qcom,ipq5424-llcc
+ - qcom,kaanapali-llcc
+ - qcom,qcs615-llcc
+ - qcom,qcs8300-llcc
+ - qcom,qdu1000-llcc
+ - qcom,sa8775p-llcc
+ - qcom,sar1130p-llcc
+ - qcom,sar2130p-llcc
+ - qcom,sc7180-llcc
+ - qcom,sc7280-llcc
+ - qcom,sc8180x-llcc
+ - qcom,sc8280xp-llcc
+ - qcom,sdm670-llcc
+ - qcom,sdm845-llcc
+ - qcom,shikra-llcc
+ - qcom,sm6350-llcc
+ - qcom,sm7150-llcc
+ - qcom,sm8150-llcc
+ - qcom,sm8250-llcc
+ - qcom,sm8350-llcc
+ - qcom,sm8450-llcc
+ - qcom,sm8550-llcc
+ - qcom,sm8650-llcc
+ - qcom,sm8750-llcc
+ - qcom,x1e80100-llcc
+ - items:
+ - enum:
+ - qcom,maili-llcc
+ - const: qcom,hawi-llcc
reg:
minItems: 1
---
base-commit: c1ecb239fa3456529a32255359fc78b69eb9d847
change-id: 20260525-maili-llcc-a4e215c75745
Best regards,
--
Jingyi Wang <jingyi.wang@oss.qualcomm.com>
^ permalink raw reply related
* Re: [PATCH v10 2/9] i3c: master: Expose the APIs to support I3C hub
From: sashiko-bot @ 2026-05-25 7:39 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260525064209.2263045-3-lakshay.piplani@nxp.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Medium] `i3c_master_reattach_i3c_dev_locked()` does not validate slot availability despite documentation claiming it returns `-EBUSY`.
Pre-existing issues:
- [High] `i3c_master_reattach_i3c_dev_locked` corrupts the bus address state if the reattach operation fails.
- [High] Missing synchronization and improper error handling in `i3c_dev_disable_ibi_locked` and `i3c_dev_free_ibi_locked` lead to Use-After-Free.
- [High] Missing memory barrier between `reinit_completion()` and `atomic_read()` can lead to a deadlock in `i3c_dev_disable_ibi_locked`.
--
commit 9189e89cd793462892bddcec9432ec1ed64052bf
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
i3c: master: Expose the APIs to support I3C hub
This commit exposes several internal static I3C functions as public APIs
to support a new I3C hub driver. It adds EXPORT_SYMBOL_GPL and kernel-doc
comments for functions handling IBI operations and device reattachment.
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index c45b450af319d..ec3547f75469e 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> -static int i3c_master_reattach_i3c_dev_locked(struct i3c_dev_desc *dev,
> - u8 old_dyn_addr)
> +/**
> + * i3c_master_reattach_i3c_dev_locked() - reattach an I3C device with a new address
> + * @dev: I3C device descriptor to reattach
> + * @old_dyn_addr: previous dynamic address of the device
> + *
> + * This function reattaches an existing I3C device to the bus when its dynamic
> + * address has changed. It updates the bus address slot status accordingly:
> + * - Marks the new dynamic address as occupied by an I3C device.
> + * - Frees the old dynamic address slot if applicable.
> + *
> + * This function must be called with the bus lock held in write mode.
> + *
> + * Return: 0 on success, or a negative error code if reattachment fails
> + * (e.g. -EBUSY if the new address slot is not free).
> + */
> +int i3c_master_reattach_i3c_dev_locked(struct i3c_dev_desc *dev,
> + u8 old_dyn_addr)
> {
> struct i3c_master_controller *master = i3c_dev_get_master(dev);
> int ret;
[Severity: Medium]
The kernel-doc mentions returning -EBUSY if the new address slot is not free.
Does the implementation actually check this?
It appears i3c_bus_set_addr_slot_status() is called unconditionally,
overwriting the slot status without validating if it was previously occupied.
Since external driver authors may rely on this documented contract, could this
lead to masking conflicts if they omit their own slot availability checks?
[ ... ]
> @@ -1677,6 +1692,7 @@ static int i3c_master_reattach_i3c_dev_locked(struct i3c_dev_desc *dev,
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(i3c_master_reattach_i3c_dev_locked);
[Severity: High]
This is a pre-existing issue, but exporting this function exposes an error
handling path that can corrupt the bus address state. If the hardware reattach
callback fails:
i3c_master_reattach_i3c_dev_locked() {
...
if (master->ops->reattach_i3c_dev) {
ret = master->ops->reattach_i3c_dev(dev, old_dyn_addr);
if (ret) {
i3c_master_put_i3c_addrs(dev);
return ret;
}
...
}
Calling i3c_master_put_i3c_addrs() marks the device's dynamic and static
address slots as I3C_ADDR_SLOT_FREE. However, the device is not logically
detached, remains in the bus list, and retains its dyn_addr.
Since the bus allocator now views the slot as free, can subsequent dynamic
address assignments or hot-joins assign this address to another device, leading
to address collisions on the bus?
[ ... ]
> +/**
> + * i3c_dev_disable_ibi_locked() - Disable IBIs coming from a specific device
> + * @dev: device on which IBIs should be disabled
> + *
> + * This function disable IBIs coming from a specific device and wait for
> + * all pending IBIs to be processed.
> + *
> + * Context: Must be called with mutex_lock(&dev->desc->ibi_lock) held.
> + * Return: 0 in case of success, a negative error core otherwise.
> + */
> int i3c_dev_disable_ibi_locked(struct i3c_dev_desc *dev)
> {
> struct i3c_master_controller *master;
> @@ -3216,7 +3242,22 @@ int i3c_dev_disable_ibi_locked(struct i3c_dev_desc *dev)
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(i3c_dev_disable_ibi_locked);
[Severity: High]
This isn't a bug introduced by this patch, but does the lack of a memory
barrier here risk a deadlock?
i3c_dev_disable_ibi_locked() {
...
reinit_completion(&dev->ibi->all_ibis_handled);
if (atomic_read(&dev->ibi->pending_ibis))
wait_for_completion(&dev->ibi->all_ibis_handled);
...
}
Because there is no full memory barrier between writing the completion state in
reinit_completion() and reading the atomic counter, could store-load reordering
occur?
If a concurrent IBI workqueue handler decrements pending_ibis to 0 and calls
complete() before the completion reset is globally visible, the
wait_for_completion() call would block indefinitely.
[Severity: High]
Also a pre-existing issue, but does this lack synchronization with in-flight
interrupts?
If an IRQ handler is executing on another CPU right before
atomic_inc(&dev->ibi->pending_ibis) is called, i3c_dev_disable_ibi_locked()
could read pending_ibis as 0 and return early.
If i3c_dev_free_ibi_locked() is subsequently called to free dev->ibi, would the
delayed IRQ handler then access freed memory?
[ ... ]
> +/**
> + * i3c_dev_free_ibi_locked() - Free all resources needed for IBI handling
> + * @dev: device on which you want to release IBI resources
> + *
> + * This function is responsible for de-allocating resources previously
> + * allocated by i3c_device_request_ibi(). It should be called after disabling
> + * IBIs with i3c_device_disable_ibi().
> + *
> + * Context: Must be called with mutex_lock(&dev->desc->ibi_lock) held.
> + */
> void i3c_dev_free_ibi_locked(struct i3c_dev_desc *dev)
> {
> struct i3c_master_controller *master = i3c_dev_get_master(dev);
> @@ -3301,6 +3366,7 @@ void i3c_dev_free_ibi_locked(struct i3c_dev_desc *dev)
> kfree(dev->ibi);
> dev->ibi = NULL;
> }
> +EXPORT_SYMBOL_GPL(i3c_dev_free_ibi_locked);
[Severity: High]
This isn't a bug introduced by this patch, but exporting these APIs highlights
a potential use-after-free vector.
i3c_dev_free_ibi_locked() {
...
if (dev->ibi->enabled) {
...
ret = i3c_dev_disable_ibi_locked(dev);
...
if (ret)
dev_err(&master->dev, "Failed to disable IBI before freeing\n");
}
...
kfree(dev->ibi);
}
If i3c_dev_disable_ibi_locked() fails to disable hardware IBIs, this function
logs an error but proceeds to call kfree(dev->ibi) and destroy the workqueue.
Since the hardware might still be enabled and firing interrupts, won't the
master's IRQ handler attempt to access the freed dev->ibi when it receives the
next interrupt?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260525064209.2263045-1-lakshay.piplani@nxp.com?part=2
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: display: bridge: Document Renesas RZ/G3L LVDS encoder
From: Biju Das @ 2026-05-25 7:45 UTC (permalink / raw)
To: sashiko-reviews
Cc: robh, conor+dt, devicetree, wsa+renesas, linux-renesas-soc
In-Reply-To: <20260524195829.960401F000E9@smtp.kernel.org>
Hi Bot,
On Sun, May 24, 2026 at 8:58 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] The compatible string 'renesas,r9a08g046-lvds-cmn' is used in the dt-binding example but is not documented.
> - [Low] The parent node 'lvds-cmn' in the example lacks a 'ranges' property, breaking address translation for its child nodes.
> --
>
> commit be595c0db2d85d9ea1396f0f30ac7ad75f4101fe
> Author: Biju Das <biju.das.jz@bp.renesas.com>
>
> dt-bindings: display: bridge: Document Renesas RZ/G3L LVDS encoder
>
> Documents the LVDS encoder IP found on the RZ/G3L SoC. It supports
> single-link mode and shares a peripheral clock and reset signal with the
> MIPI-DSI interface.
>
> > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml
> > new file mode 100644
> > index 0000000000000..b1f6d020ae7b1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml
>
> [ ... ]
>
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/renesas,r9a08g046-cpg.h>
> > +
> > + lvds-cmn@108a0000 {
> > + compatible = "renesas,r9a08g046-lvds-cmn",
> > + "simple-mfd", "syscon";
>
> [Severity: Low]
> Is this compatible string documented somewhere else? It looks like
> renesas,r9a08g046-lvds-cmn isn't present in the tree, which might cause
> dt_binding_check to fail when it validates the example.
This compatible is documented and is in next [1].
Q1) Can you please reun the AI review with [1] ?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20260522&id=51284d8b1dbcd7fa0220c49eeab29b14617e0d88
>
> > + reg = <0x108a0000 0x10000>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
>
> [Severity: Low]
> Does this parent node need a ranges property? Without it, address translation
> for the child node's reg = <0x10 0x8> might fail, and dtc often warns about
> missing ranges in simple-mfd nodes with children.
I don't get any error or dtc warnings. Have you ran dt-binding check
before providing
such comment?
Cheers,
Biju
^ permalink raw reply
* Re: [PATCH 1/2] crypto: Delete Qualcomm crypto engine driver
From: Demi Marie Obenour @ 2026-05-25 7:49 UTC (permalink / raw)
To: Kuldeep Singh, Eric Biggers, Krzysztof Kozlowski
Cc: Dmitry Baryshkov, Herbert Xu, David S. Miller, Thara Gopinath,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Russell King, linux-kernel, linux-crypto,
linux-arm-msm, Ard Biesheuvel, devicetree, linux-arm-kernel
In-Reply-To: <e36172c6-4424-4b77-9b3c-47dcdbdff05b@oss.qualcomm.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 1441 bytes --]
On 5/25/26 02:29, Kuldeep Singh wrote:
>> This driver is more than an order of magnitude slower than the CPU for
>> both encryption and hashing. See:
>>
>> https://lore.kernel.org/r/20250704070322.20692-1-ebiggers@kernel.org/
>> https://lore.kernel.org/r/20250615031807.GA81869@sol/
>>
>> There are many examples of it having bugs as well, for example see the
>> second link above.
>>
>> That's why it had to be disabled via the cra_priority system. This
>> driver was actively making Linux worse.
>>
>> This isn't particularly unique to drivers/crypto/, of course. This one
>> we just have data on, so it's a bit clearer.
>>
>> I've yet to see any real reason to keep this driver.
>
> https://lore.kernel.org/all/c1697372-54ec-4f57-85d9-ad375ff1a44d@oss.qualcomm.com/
> Kindly check my latest reply to the thread. There are numerous usecases
> like DRM(Digital rights management) coming up and qce driver is required
> for secure content.
1. This content is restricted. It's no more secure.
2. General purpose (desktop, laptop) systems won't be able to display
restricted content. To the best of my knowledge, the only Wayland
compositors that support restricted content are Weston and Chromium
OS's compositor.
3. Will this code be useful without out of tree drivers?
4. Does this use the existing crypto APIs or a separate API?
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4 2/3] clk: qcom: camcc-glymur: Add camera clock controller driver
From: Bryan O'Donoghue @ 2026-05-25 7:49 UTC (permalink / raw)
To: Jagadeesh Kona, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Brian Masney, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel, Taniya Das,
Konrad Dybcio
In-Reply-To: <66335474-d600-45ab-9ac6-e946f24142c8@oss.qualcomm.com>
On 25/05/2026 08:06, Jagadeesh Kona wrote:
>> That's not in your overview letter so generally I'd advise to include things like "did X because Y" - "didn't do Q because Z" anyway, how does it make a difference if the values are static ?
>>
>> They are no less magic numbers that way.
>>
>> What exactly is the resistance to defining the bits ?
>>
>> I'll state again - when a vendor is submitting something upstream where that vendor 100% controls their own documentation - there's no reason at all to be presenting magic hex numbers - even more the case with generated code.
>>
>> Just update the script to enumerate the bit fields, I honestly don't get the aversion.
>>
> Hi Bryan,
>
> There’s no standard interface for these bits, and bit definitions/fields vary across PLL types.
> So, common macros aren’t feasible and would need redefinitions per controller. Since these bits
> are not reused elsewhere
- Asking for named bits not common macros
- Reuse isn't why you name a bit
, IMO directly using values from the hardware documentation keeps the
> implementation simpler, avoids unnecessary abstraction, and makes debugging—through direct
> comparison with the hardware spec easier.
How are hex values in upstream code easier to debug ?
Without the spec you can't change or understand hex values in upstream
code, which is the whole point I'm making here.
---
bod
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: tegra: Enable DMA Support on Tegra194 QSPI
From: sashiko-bot @ 2026-05-25 7:53 UTC (permalink / raw)
To: Aaron Kling via B4 Relay; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260525-tegra194-qspi-iommu-v2-2-a11c53f804b2@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Calling dma_unmap_single on an unmapped DMA address during the DMA mapping error path.
- [High] DMA buffer overflow causing memory corruption when reading unaligned sizes in packed mode.
--
commit ab48224cb433a33d32049c6cfc39eea2bdf25194
Author: Aaron Kling <webgeek1234@gmail.com>
arm64: tegra: Enable DMA Support on Tegra194 QSPI
Without dma enabled pio mode is used and flash storage such as the one
on the p3668 module times out and cannot complete any transfers. In some
cases, these timeouts cause hangs and cbb faults.
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index 1d659454a6f9f..e2ddbc6715d5e 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -923,6 +923,8 @@ spi@3270000 {
> <&bpmp TEGRA194_CLK_QSPI0_PM>;
> clock-names = "qspi", "qspi_out";
> resets = <&bpmp TEGRA194_RESET_QSPI0>;
> + dmas = <&gpcdma 5>, <&gpcdma 5>;
> + dma-names = "rx", "tx";
[Severity: High]
This isn't a bug introduced by this patch, but enabling DMA mode exposes a
pre-existing issue in the driver.
In tegra_qspi_dma_map_xfer(), t->tx_dma is mapped conditionally only if
t->tx_buf is non-NULL.
However, if mapping t->rx_buf subsequently fails, the error path
unconditionally calls dma_unmap_single(..., t->tx_dma, ...).
If t->tx_buf was NULL, t->tx_dma is zero or uninitialized.
Could this lead to unmapping an invalid DMA address, potentially corrupting
IOMMU page tables or triggering a panic?
[Severity: High]
This is a pre-existing issue, but directly switching the controller to
DMA mode makes a buffer overflow vulnerability triggerable.
In tegra_qspi_start_dma_based_transfer(), the driver calculates the mapped
and transferred DMA length by rounding up to the nearest multiple of 4:
len = DIV_ROUND_UP(..., 4) * 4.
For an unaligned SPI read (e.g., 5 bytes), len becomes 8. The driver maps 8
bytes and programs the DMA engine to write exactly 8 bytes into the client's
t->rx_buf via dmaengine_prep_slave_single().
Because the client's t->rx_buf is only guaranteed to be the requested 5 bytes
long, does this cause the DMA hardware to blindly overwrite adjacent memory
by 3 bytes?
Could this out-of-bounds write corrupt adjacent heap objects or KASAN
redzones?
> status = "disabled";
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260525-tegra194-qspi-iommu-v2-0-a11c53f804b2@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v10 3/9] i3c: master: Add APIs for I3C hub support
From: sashiko-bot @ 2026-05-25 8:00 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260525064209.2263045-4-lakshay.piplani@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incorrect logic and API contract violation in `i3c_master_supports_ccc_cmd()`.
- [Medium] `i3c_master_direct_attach_i3c_dev_locked()` incorrectly returns `-EINVAL` for devices without addresses.
--
commit e1382a24562500ebcc8529e9a5fff3016b719a36
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
i3c: master: Add APIs for I3C hub support
This commit adds helpers for attaching and detaching I3C devices, as well as
helpers to check CCC support and send CCC commands. These additions prepare the
subsystem for I3C hub support.
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index ec3547f75469e..4f0565faa182f 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
[ ... ]
> +/**
> + * 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 -EINVAL;
[Severity: Medium]
The documentation for this function states that "Devices without an assigned
address are ignored." However, this code returns -EINVAL instead of 0.
Looking at the internal i3c_master_attach_i3c_dev(), it returns 0 to safely
ignore unaddressed devices until they acquire an address. Should this new
helper also return 0 to match both its documentation and the subsystem's
existing convention?
> +
> + /* 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_supports_ccc_cmd() - check CCC command support
> + * @master: I3C master controller
> + * @cmd: CCC command to verify
> + *
> + * This function verifies whether the given I3C master controller supports
> + * the specified Common Command Code (CCC).
> + *
> + * Return: 0 if the CCC command is supported and executed successfully,
> + * -EINVAL if arguments are invalid,
> + * -EOPNOTSUPP if the master does not support CCC commands,
> + * or another negative error code from the master's operation.
> + */
> +int i3c_master_supports_ccc_cmd(struct i3c_master_controller *master,
> + const struct i3c_ccc_cmd *cmd)
> +{
> + if (!cmd || !master)
> + return -EINVAL;
> +
> + if (!master->ops->supports_ccc_cmd)
> + return -EOPNOTSUPP;
> +
> + return master->ops->supports_ccc_cmd(master, cmd);
> +}
[Severity: Medium]
Does this function's logic and return value match its documentation and the I3C
subsystem semantics?
The docstring states it returns 0 if a command is supported, but it actually
returns the boolean result of the supports_ccc_cmd callback (1 for supported,
0 for unsupported). A caller checking for 0 as success would incorrectly treat
supported commands as unsupported.
Additionally, if the callback is absent, this returns -EOPNOTSUPP. However, in
other places like i3c_master_send_ccc_cmd_locked(), the core I3C logic assumes
all commands are supported if the callback is NULL. Could returning -EOPNOTSUPP
here falsely indicate no commands are supported?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260525064209.2263045-1-lakshay.piplani@nxp.com?part=3
^ permalink raw reply
* Re: [PATCH v5 1/3] dt-bindings: mfd: aspeed,ast2x00-scu: Support AST2700 SoC1 pinctrl
From: Linus Walleij @ 2026-05-25 8:03 UTC (permalink / raw)
To: Billy Tsai
Cc: Tony Lindgren, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Joel Stanley, Andrew Jeffery, Bartosz Golaszewski, Lee Jones,
Ryan Chen, patrickw3, linux-gpio, devicetree, linux-kernel,
linux-arm-kernel, linux-aspeed, BMC-SW, openbmc, Andrew Jeffery,
linux-clk, Conor Dooley
In-Reply-To: <20260521-pinctrl-single-bit-v5-1-308be2c160fc@aspeedtech.com>
On Thu, May 21, 2026 at 11:17 AM Billy Tsai <billy_tsai@aspeedtech.com> wrote:
> The AST2700 SoC integrates two interconnected SoC instances, each
> managed by its own System Control Unit (SCU).
>
> Allow the AST2700 SoC1 pin controller to be described as a child
> node of the SCU by extending the compatible strings accepted by
> the SCU binding.
>
> There is no functional change to the SCU binding beyond permitting
> the aspeed,ast2700-soc1-pinctrl compatible string.
>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Lee can you pick this up at your convenience?
I'll apply patches 2 & 3.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v5 0/3] pinctrl: aspeed: Add AST2700 SoC1 support
From: Linus Walleij @ 2026-05-25 8:06 UTC (permalink / raw)
To: Billy Tsai
Cc: Tony Lindgren, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Joel Stanley, Andrew Jeffery, Bartosz Golaszewski, Lee Jones,
Ryan Chen, patrickw3, linux-gpio, devicetree, linux-kernel,
linux-arm-kernel, linux-aspeed, BMC-SW, openbmc, Andrew Jeffery,
linux-clk, Conor Dooley
In-Reply-To: <20260521-pinctrl-single-bit-v5-0-308be2c160fc@aspeedtech.com>
On Thu, May 21, 2026 at 11:17 AM Billy Tsai <billy_tsai@aspeedtech.com> wrote:
> Legacy ASPEED pin controllers have historically not had a coherent
> register interface. Control fields often had no consistent mapping to
> individual pins, and configuring a function frequently required
> coordinating multiple control bits across several registers. As a
> result, the existing ASPEED pinctrl drivers rely on complex macro
> infrastructure to describe the dependencies between pins, functions,
> and register fields.
>
> The pin controller for SoC1 in the AST2700 breaks from this legacy
> design.
>
> For SoC1, each pin maps directly to a dedicated function field in the
> SCU register space that determines the active mux function for that
> pin. This results in a much more regular register layout compared to
> previous generations.
>
> While the behaviour is conceptually similar to pinctrl-single, the
> register layout and configuration model differ enough that reusing
> pinctrl-single directly is not practical. Therefore this driver is
> implemented as a SoC-specific pinctrl driver using static data tables
> to describe the register layout.
>
> The binding reuses the standard pinmux and generic pin configuration
> schemas and does not introduce any custom Devicetree properties.
>
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Patches 2 & 3 applied to the pinctrl tree, thanks Billy!
Sashiko is moaning about something, look into it but my confidence
is low since it is using weasel words like "critical" for things that are
certainly not critical, if there is some validty to these comments it
can certainly be fixed up in-tree.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v10 4/9] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
From: sashiko-bot @ 2026-05-25 8:08 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260525064209.2263045-5-lakshay.piplani@nxp.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The binding lacks the required standard properties and node patterns for the GPIO controller capability described in its documentation.
- [Medium] The schema claims to support the entire P3H2x4x family but only implements the compatible string and port constraints for the 8-port `nxp,p3h2840` variant.
- [Medium] The MAINTAINERS entry incorrectly routes patches to the list administrator instead of the community mailing list.
- [Low] The `compatible` property is omitted from the top-level `required` list.
--
commit 046e7a899894c4ab96ab9fb9da8523aa5d9646fd
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 multiport I3C hub
family. These devices support upstream I3C/I2C/SMBus connections and allow
communication with multiple downstream peripheral devices.
> 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
[ ... ]
> +description: |
> + P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841) is a family of multiport I3C
> + hub devices that connect to:-
[ ... ]
> + 4. P3H2440/ P3H2441 are 4 port I3C hub with four I3C/I2C Target Port.
> + Target ports can be configured as I2C/SMBus, I3C or GPIO and connect to
> + peripherals.
[Severity: Medium]
The description states that target ports can be configured as GPIO, but are
the standard gpio-controller and #gpio-cells properties missing from this
schema? When hardware includes GPIO controller capabilities, subsystem
guidelines require documenting these properties and adding them to the
required list.
> +
> +properties:
> + compatible:
> + const: nxp,p3h2840
[Severity: Medium]
Does the schema fully support the P3H2440, P3H2441, and P3H2841 variants
mentioned in the description? The compatible property strictly enforces
nxp,p3h2840, which will cause validation to fail for the other models.
Additionally, wouldn't the 4-port variants need conditional constraints to
limit the target port reg maximum to 3 instead of 7?
[ ... ]
> +required:
> + - reg
[Severity: Low]
Is the compatible property intentionally missing from the top-level required
list? While the select logic might still evaluate nodes correctly, omitting
it is a deviation from standard device tree bindings style.
[ ... ]
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2fb1c75afd163..c820bc5a55f57 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
[ ... ]
> +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-owner@lists.infradead.org
[Severity: Medium]
Is the routing for the I3C mailing list correct here? The -owner suffix
routes emails to the list administrators rather than the community mailing
list at linux-i3c@lists.infradead.org. This configuration will cause
developers using get_maintainer.pl to unintentionally spam the list owners
instead of sending their patches for community review.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260525064209.2263045-1-lakshay.piplani@nxp.com?part=4
^ permalink raw reply
* [PATCH v8 0/2] media: nxp: Add CSI Pixel Formatter support
From: Guoniu Zhou @ 2026-05-25 8:12 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Laurent Pinchart, Frank Li
Cc: imx, linux-media, devicetree, linux-arm-kernel, linux-kernel,
Guoniu Zhou, Frank Li, Krzysztof Kozlowski
CSI Pixel Formatter is a module found on i.MX95. It could unpack the
pixels received by the formatter and reformat them to meet the pixel
link format requirement.
This patch series adds a new V4L2 driver for CSI Pixel Formatter.
v4l2-compliance 1.28.1-5233, 64 bits, 64-bit time_t
v4l2-compliance SHA: fc15e229d9d3 2024-07-23 19:22:15
Compliance test for device /dev/v4l-subdev9:
Driver Info:
Driver version : 7.1.0
Capabilities : 0x00000002
Streams Support
Client Capabilities: 0x0000000000000003
streams interval-uses-which
Required ioctls:
test VIDIOC_SUDBEV_QUERYCAP: OK
test invalid ioctls: OK
Allow for multiple opens:
test second /dev/v4l-subdev9 open: OK
test VIDIOC_SUBDEV_QUERYCAP: OK
test for unlimited opens: OK
Debug ioctls:
test VIDIOC_LOG_STATUS: OK (Not Supported)
Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0
Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0
Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)
Sub-Device routing ioctls:
test Try VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: OK
test Active VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: OK
Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
test VIDIOC_QUERYCTRL: OK (Not Supported)
test VIDIOC_G/S_CTRL: OK (Not Supported)
test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 0 Private Controls: 0
Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported)
test VIDIOC_G/S_PARM: OK (Not Supported)
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK (Not Supported)
test VIDIOC_TRY_FMT: OK (Not Supported)
test VIDIOC_S_FMT: OK (Not Supported)
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK (Not Supported)
Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported)
test CREATE_BUFS maximum buffers: OK
test VIDIOC_REMOVE_BUFS: OK
test VIDIOC_EXPBUF: OK (Not Supported)
test Requests: OK (Not Supported)
Total for device /dev/v4l-subdev9: 47, Succeeded: 47, Failed: 0, Warnings: 0
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
Changes in v8:
- Rebase to latest media/next
- Use standard port reference instead of video-interfaces.yaml
- Add parent syscon node in example to show device integration
- Remove fmt field and look up format from subdev state instead
- Unify function and structure naming to use csi_formatter_ prefix
- Remove misleading alignment comment from set_fmt function
- Optimize get_frame_desc to call once per start_stream
- Replace V4L2_FRAME_DESC_ENTRY_MAX with CSI_FORMATTER_VC_NUM in loops
- Remove redundant debug message in enable_streams
- Use MEDIA_PAD_FL_MUST_CONNECT flag instead of manual link check
- Link to v7: https://lore.kernel.org/r/20260518-csi_formatter-v7-0-562b750557e3@oss.nxp.com
Changes in v7:
- Change compatible to imx95-csi-formatter as IP is i.MX95 specific per Marco's suggestion
Link: https://lore.kernel.org/linux-media/20260511-csi_formatter-v6-0-01028e312e2b@oss.nxp.com/T/#mcd135b3de179b3cb69daa1fd6e0e8e27c85b3332
- Update references from imx9 to imx95 for consistency with dt-bindings
- Enable PM runtime before async registration
- Link to v6: https://lore.kernel.org/r/20260511-csi_formatter-v6-0-01028e312e2b@oss.nxp.com
Changes in v6:
- Rebase to latest media/next
- Update v4l2-compliace test
- Remove unused header includes
- Unify macro naming: VCx/VCX -> VC and parameter x -> vc
- Remove unused format field from csi_formatter struct
- Use compact initialization for formats array
- Make find_csi_format() return NULL instead of default format
- Use unsigned int for array index in find_csi_format()
- Add err_ prefix to error handling labels
- Add v4l2_subdev_cleanup() and reorder cleanup sequence
- Update enable_streams debug output format
- Rename VC_MAX to VC_NUM and fix boundary check
- Update CSI formatter Kconfig description
- Use v4l2_subdev_get_frame_desc_passthrough() helper
- Fix error paths in async registration and probe
- Add mutex to protect enabled_streams
- Switch to devm_pm_runtime_enable()
- Remove redundant num_routes check in set_routing
- Optimize get_index_by_dt() and add warning for unsupported type
- csi_formatter_start/stop_stream: Process all streams in mask
- Link to v5: https://lore.kernel.org/r/20260123-csi_formatter-v5-0-d5b803f867bf@nxp.com
Changes in v5:
- Remove CSI_FORMATTER_DRV_NAME macro since only use once.
- Remove sd->owner = THIS_MODULE;
- Simplify code by using DEFINE_RUNTIME_DEV_PM_OPS macro.
- Link to v4: https://lore.kernel.org/r/20260122-csi_formatter-v4-0-6f6fcad1c33a@nxp.com
Changes in v4:
- Rebase to latest media/next.
- Add comments to describe the index field in formatter_dt_to_index_map array.
- Link to v3: https://lore.kernel.org/r/20251219-csi_formatter-v3-0-8680d6d87091@nxp.com
Changes in v3:
- Rename nxp,imx9-csi-formatter.yaml to fsl,imx9-csi-formatter.yaml.
- Drop clock-names property.
- Drop macro IMX95_PD_CAMERA definition and use a constant directly.
[PATCH 1/2] media: dt-bindings: Add CSI Pixel Formatter DT bindings
- Remove the assignment driver.owner = THIS_MODULE.
- Assign struct fwnode_handle *ep __free(fwnode_handle) when definition.
- Update yaml file name for csi formatter in MAINTAINERS.
[PATCH 2/2] media: nxp: Add i.MX9 CSI pixel formatter v4l2 driver
- Link to v2: https://lore.kernel.org/r/20251217-csi_formatter-v2-0-62168af80210@nxp.com
Changes in v2:
- Delete "|" for description key.
- Add empty line between child node and property.
- Delete labels for endpoint of child nodes.
[PATCH 1/2] media: dt-bindings: Add CSI Pixel Formatter DT bindings
- Update commit message.
- Use the value defined by bellow macros directly since they are used only once.
#define CSI_FORMATTER_DEF_MBUS_CODE MEDIA_BUS_FMT_UYVY8_1X16
#define CSI_FORMATTER_DEF_PIX_WIDTH 1920U
#define CSI_FORMATTER_DEF_PIX_HEIGHT 1080U
#define CSI_FORMATTER_MAX_PIX_WIDTH 0xffff
#define CSI_FORMATTER_MAX_PIX_HEIGHT 0xffff
- Use macro pm_ptr() to fix build warning when CONFIG_PM is disabled.
- Finish route loop by break statement, instead of goto.
- Return dev_err_probe() when meet errors in probe() function instead of dev_err().
- Remove MODULE_ALIAS().
- Refine .enable(.dsable)_stream callback implementation, include bellow changes:
Add stream checking.
Fix potential pm runtime count unbalance issue.
Add stop stream error handling when enabling remote subdev stream.
- Use __free(fwnode_handle) to drop reference to a device node automatically.
[PATCH 2/2] media: nxp: Add i.MX9 CSI pixel formatter v4l2 driver
- Link to v1: https://lore.kernel.org/r/20251203-csi_formatter-v1-0-eb9e1147b49e@nxp.com
---
Guoniu Zhou (2):
media: dt-bindings: Add CSI Pixel Formatter DT bindings
media: nxp: Add i.MX95 CSI pixel formatter v4l2 driver
.../bindings/media/fsl,imx95-csi-formatter.yaml | 92 +++
MAINTAINERS | 8 +
drivers/media/platform/nxp/Kconfig | 14 +
drivers/media/platform/nxp/Makefile | 1 +
drivers/media/platform/nxp/imx95-csi-formatter.c | 757 +++++++++++++++++++++
5 files changed, 872 insertions(+)
---
base-commit: a3d78e74dd3ed04797ea351edb7f0a19b961c063
change-id: 20251125-csi_formatter-e6d29316dce6
Best regards,
--
Guoniu Zhou <guoniu.zhou@oss.nxp.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox