[PATCH V2 00/10] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers

[PATCH V2 00/10] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers

[Adrian Hunter]

Hi


Changes in V2:
	Add Frank's Rev'd-by for patches 1 - 3
	Drop default driver data
	New patches split from "i3c: mipi-i3c-hci-pci: Add support for
	Multi-Bus Instances"
	Conversion to MFD split into separate patch
	Simplify ID allocation / free
	Correct use of __free()
	Also define instance 0 in driver_data


A MIPI I3C Host Controller with the Multi-Bus Instance capability supports
multiple I3C Buses (up to 15), with one instance of the HCI Register Set
and one instance of I3C Bus Controller Logic for each I3C Bus, in a single
hardware function (e.g. PCIe B/D/F).

Add support for that to mipi-i3c-hci-pci and mipi-i3c-hci and define
Multi-Bus Instances for Intel controllers.

There are also 3 minor tidy-up patches to start with.


Adrian Hunter (10):
      i3c: mipi-i3c-hci: Remove duplicate blank lines
      i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0
      i3c: mipi-i3c-hci: Quieten initialization messages
      i3c: mipi-i3c-hci: Allow for Multi-Bus Instances
      i3c: mipi-i3c-hci-pci: Do not repeatedly check for NULL driver_data
      i3c: mipi-i3c-hci-pci: Enable MSI support
      i3c: mipi-i3c-hci-pci: Use parent MMIO mapping
      i3c: mipi-i3c-hci-pci: Convert to MFD driver
      i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances
      i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers

 drivers/i3c/master/Kconfig                         |   1 +
 drivers/i3c/master/mipi-i3c-hci/cmd_v1.c           |   2 -
 drivers/i3c/master/mipi-i3c-hci/cmd_v2.c           |   2 -
 drivers/i3c/master/mipi-i3c-hci/core.c             |  41 +++--
 drivers/i3c/master/mipi-i3c-hci/dat_v1.c           |   1 -
 drivers/i3c/master/mipi-i3c-hci/dma.c              |   6 +-
 drivers/i3c/master/mipi-i3c-hci/ext_caps.c         |  42 +++--
 drivers/i3c/master/mipi-i3c-hci/ext_caps.h         |   1 -
 drivers/i3c/master/mipi-i3c-hci/hci.h              |   5 -
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 173 ++++++++++++++-------
 drivers/i3c/master/mipi-i3c-hci/pio.c              |   9 +-
 include/linux/platform_data/mipi-i3c-hci.h         |  15 ++
 12 files changed, 186 insertions(+), 112 deletions(-)
 create mode 100644 include/linux/platform_data/mipi-i3c-hci.h


Regards
Adrian

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V2 01/10] i3c: mipi-i3c-hci: Remove duplicate blank lines

[Adrian Hunter]

Remove duplicate blank lines from mipi-i3c-hci code.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---


Changes in V2:

	Add Frank's Rev'd-by


 drivers/i3c/master/mipi-i3c-hci/cmd_v1.c   | 2 --
 drivers/i3c/master/mipi-i3c-hci/cmd_v2.c   | 2 --
 drivers/i3c/master/mipi-i3c-hci/core.c     | 2 --
 drivers/i3c/master/mipi-i3c-hci/dat_v1.c   | 1 -
 drivers/i3c/master/mipi-i3c-hci/dma.c      | 2 --
 drivers/i3c/master/mipi-i3c-hci/ext_caps.c | 1 -
 drivers/i3c/master/mipi-i3c-hci/ext_caps.h | 1 -
 drivers/i3c/master/mipi-i3c-hci/hci.h      | 5 -----
 drivers/i3c/master/mipi-i3c-hci/pio.c      | 1 -
 9 files changed, 17 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c b/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c
index eb8a3ae2990d..fe260461e7e6 100644
--- a/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c
+++ b/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c
@@ -15,7 +15,6 @@
 #include "dat.h"
 #include "dct.h"
 
-
 /*
  * Address Assignment Command
  */
@@ -100,7 +99,6 @@
 #define CMD_M0_VENDOR_INFO_PRESENT		   W0_BIT_( 7)
 #define CMD_M0_TID(v)			FIELD_PREP(W0_MASK( 6,  3), v)
 
-
 /* Data Transfer Speed and Mode */
 enum hci_cmd_mode {
 	MODE_I3C_SDR0		= 0x0,
diff --git a/drivers/i3c/master/mipi-i3c-hci/cmd_v2.c b/drivers/i3c/master/mipi-i3c-hci/cmd_v2.c
index efb4326a25b7..3729e6419581 100644
--- a/drivers/i3c/master/mipi-i3c-hci/cmd_v2.c
+++ b/drivers/i3c/master/mipi-i3c-hci/cmd_v2.c
@@ -16,7 +16,6 @@
 #include "cmd.h"
 #include "xfer_mode_rate.h"
 
-
 /*
  * Unified Data Transfer Command
  */
@@ -62,7 +61,6 @@
 #define CMD_A0_ASSIGN_ADDRESS(v)	FIELD_PREP(W0_MASK( 14,   8), v)
 #define CMD_A0_TID(v)			FIELD_PREP(W0_MASK(  6,   3), v)
 
-
 static unsigned int get_i3c_rate_idx(struct i3c_hci *hci)
 {
 	struct i3c_bus *bus = i3c_master_get_bus(&hci->master);
diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index 607d77ab0e54..211321f73e02 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -21,7 +21,6 @@
 #include "cmd.h"
 #include "dat.h"
 
-
 /*
  * Host Controller Capabilities and Operation Registers
  */
@@ -109,7 +108,6 @@
 #define DEV_CTX_BASE_LO			0x60
 #define DEV_CTX_BASE_HI			0x64
 
-
 static inline struct i3c_hci *to_i3c_hci(struct i3c_master_controller *m)
 {
 	return container_of(m, struct i3c_hci, master);
diff --git a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
index 85c4916972e4..cc5d2deb23ab 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
@@ -15,7 +15,6 @@
 #include "hci.h"
 #include "dat.h"
 
-
 /*
  * Device Address Table Structure
  */
diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
index c401a9425cdc..f20db2899989 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dma.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
@@ -20,7 +20,6 @@
 #include "cmd.h"
 #include "ibi.h"
 
-
 /*
  * Software Parameter Values (somewhat arb itrary for now).
  * Some of them could be determined at run time eventually.
@@ -124,7 +123,6 @@
 #define DATA_BUF_IOC			BIT(30)	/* Interrupt on Completion */
 #define DATA_BUF_BLOCK_SIZE		GENMASK(15, 0)
 
-
 struct hci_rh_data {
 	void __iomem *regs;
 	void *xfer, *resp, *ibi_status, *ibi_data;
diff --git a/drivers/i3c/master/mipi-i3c-hci/ext_caps.c b/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
index 7714f00ea9cc..40939af0b0e3 100644
--- a/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
+++ b/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
@@ -16,7 +16,6 @@
 #include "ext_caps.h"
 #include "xfer_mode_rate.h"
 
-
 /* Extended Capability Header */
 #define CAP_HEADER_LENGTH		GENMASK(23, 8)
 #define CAP_HEADER_ID			GENMASK(7, 0)
diff --git a/drivers/i3c/master/mipi-i3c-hci/ext_caps.h b/drivers/i3c/master/mipi-i3c-hci/ext_caps.h
index 9df17822fdb4..b15e629951f0 100644
--- a/drivers/i3c/master/mipi-i3c-hci/ext_caps.h
+++ b/drivers/i3c/master/mipi-i3c-hci/ext_caps.h
@@ -13,7 +13,6 @@
 /* MIPI vendor IDs */
 #define MIPI_VENDOR_NXP			0x11b
 
-
 int i3c_hci_parse_ext_caps(struct i3c_hci *hci);
 
 #endif
diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
index 249ccb13c909..3f88b67bc5cc 100644
--- a/drivers/i3c/master/mipi-i3c-hci/hci.h
+++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
@@ -62,7 +62,6 @@ struct i3c_hci {
 	void *vendor_data;
 };
 
-
 /*
  * Structure to represent a master initiated transfer.
  * The rnw, data and data_len fields must be initialized before calling any
@@ -108,7 +107,6 @@ static inline void hci_free_xfer(struct hci_xfer *xfer, unsigned int n)
 	kfree(xfer);
 }
 
-
 /* This abstracts PIO vs DMA operations */
 struct hci_io_ops {
 	bool (*irq_handler)(struct i3c_hci *hci);
@@ -126,21 +124,18 @@ struct hci_io_ops {
 extern const struct hci_io_ops mipi_i3c_hci_pio;
 extern const struct hci_io_ops mipi_i3c_hci_dma;
 
-
 /* Our per device master private data */
 struct i3c_hci_dev_data {
 	int dat_idx;
 	void *ibi_data;
 };
 
-
 /* list of quirks */
 #define HCI_QUIRK_RAW_CCC	BIT(1)	/* CCC framing must be explicit */
 #define HCI_QUIRK_PIO_MODE	BIT(2)  /* Set PIO mode for AMD platforms */
 #define HCI_QUIRK_OD_PP_TIMING		BIT(3)  /* Set OD and PP timings for AMD platforms */
 #define HCI_QUIRK_RESP_BUF_THLD		BIT(4)  /* Set resp buf thld to 0 for AMD platforms */
 
-
 /* global functions */
 void mipi_i3c_hci_resume(struct i3c_hci *hci);
 void mipi_i3c_hci_pio_reset(struct i3c_hci *hci);
diff --git a/drivers/i3c/master/mipi-i3c-hci/pio.c b/drivers/i3c/master/mipi-i3c-hci/pio.c
index 710faa46a00f..142f3f79415b 100644
--- a/drivers/i3c/master/mipi-i3c-hci/pio.c
+++ b/drivers/i3c/master/mipi-i3c-hci/pio.c
@@ -15,7 +15,6 @@
 #include "cmd.h"
 #include "ibi.h"
 
-
 /*
  * PIO Access Area
  */
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V2 02/10] i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0

[Adrian Hunter]

Extended Capability ID value 0 is special.  It signifies the end of the
list.  Stop reading Extended Capabilities if capability ID is 0.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---


Changes in V2:

	Add Frank's Rev'd-by


 drivers/i3c/master/mipi-i3c-hci/ext_caps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/ext_caps.c b/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
index 40939af0b0e3..024bccf23fd0 100644
--- a/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
+++ b/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
@@ -271,7 +271,7 @@ int i3c_hci_parse_ext_caps(struct i3c_hci *hci)
 		cap_length = FIELD_GET(CAP_HEADER_LENGTH, cap_header);
 		dev_dbg(&hci->master.dev, "id=0x%02x length=%d",
 			cap_id, cap_length);
-		if (!cap_length)
+		if (!cap_id || !cap_length)
 			break;
 		if (curr_cap + cap_length * 4 >= end) {
 			dev_err(&hci->master.dev,
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V2 03/10] i3c: mipi-i3c-hci: Quieten initialization messages

[Adrian Hunter]

The copious initialization messages are at most useful only for debugging.
Change them from dev_info() or dev_notice() to dev_dbg().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---


Changes in V2:

	Add Frank's Rev'd-by


 drivers/i3c/master/mipi-i3c-hci/core.c     | 18 +++++-----
 drivers/i3c/master/mipi-i3c-hci/dma.c      |  4 +--
 drivers/i3c/master/mipi-i3c-hci/ext_caps.c | 39 +++++++++++-----------
 drivers/i3c/master/mipi-i3c-hci/pio.c      |  8 ++---
 4 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index 211321f73e02..07fb91a12593 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -597,8 +597,8 @@ static int i3c_hci_init(struct i3c_hci *hci)
 	hci->DAT_entry_size = FIELD_GET(DAT_ENTRY_SIZE, regval) ? 0 : 8;
 	if (size_in_dwords)
 		hci->DAT_entries = 4 * hci->DAT_entries / hci->DAT_entry_size;
-	dev_info(&hci->master.dev, "DAT: %u %u-bytes entries at offset %#x\n",
-		 hci->DAT_entries, hci->DAT_entry_size, offset);
+	dev_dbg(&hci->master.dev, "DAT: %u %u-bytes entries at offset %#x\n",
+		hci->DAT_entries, hci->DAT_entry_size, offset);
 
 	regval = reg_read(DCT_SECTION);
 	offset = FIELD_GET(DCT_TABLE_OFFSET, regval);
@@ -607,23 +607,23 @@ static int i3c_hci_init(struct i3c_hci *hci)
 	hci->DCT_entry_size = FIELD_GET(DCT_ENTRY_SIZE, regval) ? 0 : 16;
 	if (size_in_dwords)
 		hci->DCT_entries = 4 * hci->DCT_entries / hci->DCT_entry_size;
-	dev_info(&hci->master.dev, "DCT: %u %u-bytes entries at offset %#x\n",
-		 hci->DCT_entries, hci->DCT_entry_size, offset);
+	dev_dbg(&hci->master.dev, "DCT: %u %u-bytes entries at offset %#x\n",
+		hci->DCT_entries, hci->DCT_entry_size, offset);
 
 	regval = reg_read(RING_HEADERS_SECTION);
 	offset = FIELD_GET(RING_HEADERS_OFFSET, regval);
 	hci->RHS_regs = offset ? hci->base_regs + offset : NULL;
-	dev_info(&hci->master.dev, "Ring Headers at offset %#x\n", offset);
+	dev_dbg(&hci->master.dev, "Ring Headers at offset %#x\n", offset);
 
 	regval = reg_read(PIO_SECTION);
 	offset = FIELD_GET(PIO_REGS_OFFSET, regval);
 	hci->PIO_regs = offset ? hci->base_regs + offset : NULL;
-	dev_info(&hci->master.dev, "PIO section at offset %#x\n", offset);
+	dev_dbg(&hci->master.dev, "PIO section at offset %#x\n", offset);
 
 	regval = reg_read(EXT_CAPS_SECTION);
 	offset = FIELD_GET(EXT_CAPS_OFFSET, regval);
 	hci->EXTCAPS_regs = offset ? hci->base_regs + offset : NULL;
-	dev_info(&hci->master.dev, "Extended Caps at offset %#x\n", offset);
+	dev_dbg(&hci->master.dev, "Extended Caps at offset %#x\n", offset);
 
 	ret = i3c_hci_parse_ext_caps(hci);
 	if (ret)
@@ -705,7 +705,7 @@ static int i3c_hci_init(struct i3c_hci *hci)
 			ret = -EIO;
 		} else {
 			hci->io = &mipi_i3c_hci_dma;
-			dev_info(&hci->master.dev, "Using DMA\n");
+			dev_dbg(&hci->master.dev, "Using DMA\n");
 		}
 	}
 
@@ -717,7 +717,7 @@ static int i3c_hci_init(struct i3c_hci *hci)
 			ret = -EIO;
 		} else {
 			hci->io = &mipi_i3c_hci_pio;
-			dev_info(&hci->master.dev, "Using PIO\n");
+			dev_dbg(&hci->master.dev, "Using PIO\n");
 		}
 	}
 
diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
index f20db2899989..0f6bbe184e85 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dma.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
@@ -212,7 +212,7 @@ static int hci_dma_init(struct i3c_hci *hci)
 
 	regval = rhs_reg_read(CONTROL);
 	nr_rings = FIELD_GET(MAX_HEADER_COUNT_CAP, regval);
-	dev_info(&hci->master.dev, "%d DMA rings available\n", nr_rings);
+	dev_dbg(&hci->master.dev, "%d DMA rings available\n", nr_rings);
 	if (unlikely(nr_rings > 8)) {
 		dev_err(&hci->master.dev, "number of rings should be <= 8\n");
 		nr_rings = 8;
@@ -232,7 +232,7 @@ static int hci_dma_init(struct i3c_hci *hci)
 	for (i = 0; i < rings->total; i++) {
 		u32 offset = rhs_reg_read(RHn_OFFSET(i));
 
-		dev_info(&hci->master.dev, "Ring %d at offset %#x\n", i, offset);
+		dev_dbg(&hci->master.dev, "Ring %d at offset %#x\n", i, offset);
 		ret = -EINVAL;
 		if (!offset)
 			goto err_out;
diff --git a/drivers/i3c/master/mipi-i3c-hci/ext_caps.c b/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
index 024bccf23fd0..79c6b52df6e7 100644
--- a/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
+++ b/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
@@ -26,9 +26,9 @@ static int hci_extcap_hardware_id(struct i3c_hci *hci, void __iomem *base)
 	hci->vendor_version_id	= readl(base + 0x08);
 	hci->vendor_product_id	= readl(base + 0x0c);
 
-	dev_info(&hci->master.dev, "vendor MIPI ID: %#x\n", hci->vendor_mipi_id);
-	dev_info(&hci->master.dev, "vendor version ID: %#x\n", hci->vendor_version_id);
-	dev_info(&hci->master.dev, "vendor product ID: %#x\n", hci->vendor_product_id);
+	dev_dbg(&hci->master.dev, "vendor MIPI ID: %#x\n", hci->vendor_mipi_id);
+	dev_dbg(&hci->master.dev, "vendor version ID: %#x\n", hci->vendor_version_id);
+	dev_dbg(&hci->master.dev, "vendor product ID: %#x\n", hci->vendor_product_id);
 
 	/* ought to go in a table if this grows too much */
 	switch (hci->vendor_mipi_id) {
@@ -48,7 +48,7 @@ static int hci_extcap_master_config(struct i3c_hci *hci, void __iomem *base)
 	static const char * const functionality[] = {
 		"(unknown)", "master only", "target only",
 		"primary/secondary master" };
-	dev_info(&hci->master.dev, "operation mode: %s\n", functionality[operation_mode]);
+	dev_dbg(&hci->master.dev, "operation mode: %s\n", functionality[operation_mode]);
 	if (operation_mode & 0x1)
 		return 0;
 	dev_err(&hci->master.dev, "only master mode is currently supported\n");
@@ -60,7 +60,7 @@ static int hci_extcap_multi_bus(struct i3c_hci *hci, void __iomem *base)
 	u32 bus_instance = readl(base + 0x04);
 	unsigned int count = FIELD_GET(GENMASK(3, 0), bus_instance);
 
-	dev_info(&hci->master.dev, "%d bus instances\n", count);
+	dev_dbg(&hci->master.dev, "%d bus instances\n", count);
 	return 0;
 }
 
@@ -70,7 +70,7 @@ static int hci_extcap_xfer_modes(struct i3c_hci *hci, void __iomem *base)
 	u32 entries = FIELD_GET(CAP_HEADER_LENGTH, header) - 1;
 	unsigned int index;
 
-	dev_info(&hci->master.dev, "transfer mode table has %d entries\n",
+	dev_dbg(&hci->master.dev, "transfer mode table has %d entries\n",
 		 entries);
 	base += 4;  /* skip header */
 	for (index = 0; index < entries; index++) {
@@ -94,7 +94,7 @@ static int hci_extcap_xfer_rates(struct i3c_hci *hci, void __iomem *base)
 
 	base += 4;  /* skip header */
 
-	dev_info(&hci->master.dev, "available data rates:\n");
+	dev_dbg(&hci->master.dev, "available data rates:\n");
 	for (index = 0; index < entries; index++) {
 		rate_entry = readl(base);
 		dev_dbg(&hci->master.dev, "entry %d: 0x%08x",
@@ -102,7 +102,7 @@ static int hci_extcap_xfer_rates(struct i3c_hci *hci, void __iomem *base)
 		rate = FIELD_GET(XFERRATE_ACTUAL_RATE_KHZ, rate_entry);
 		rate_id = FIELD_GET(XFERRATE_RATE_ID, rate_entry);
 		mode_id = FIELD_GET(XFERRATE_MODE_ID, rate_entry);
-		dev_info(&hci->master.dev, "rate %d for %s = %d kHz\n",
+		dev_dbg(&hci->master.dev, "rate %d for %s = %d kHz\n",
 			 rate_id,
 			 mode_id == XFERRATE_MODE_I3C ? "I3C" :
 			 mode_id == XFERRATE_MODE_I2C ? "I2C" :
@@ -121,7 +121,7 @@ static int hci_extcap_auto_command(struct i3c_hci *hci, void __iomem *base)
 	u32 autocmd_ext_config = readl(base + 0x08);
 	unsigned int count = FIELD_GET(GENMASK(3, 0), autocmd_ext_config);
 
-	dev_info(&hci->master.dev, "%d/%d active auto-command entries\n",
+	dev_dbg(&hci->master.dev, "%d/%d active auto-command entries\n",
 		 count, max_count);
 	/* remember auto-command register location for later use */
 	hci->AUTOCMD_regs = base;
@@ -130,46 +130,46 @@ static int hci_extcap_auto_command(struct i3c_hci *hci, void __iomem *base)
 
 static int hci_extcap_debug(struct i3c_hci *hci, void __iomem *base)
 {
-	dev_info(&hci->master.dev, "debug registers present\n");
+	dev_dbg(&hci->master.dev, "debug registers present\n");
 	hci->DEBUG_regs = base;
 	return 0;
 }
 
 static int hci_extcap_scheduled_cmd(struct i3c_hci *hci, void __iomem *base)
 {
-	dev_info(&hci->master.dev, "scheduled commands available\n");
+	dev_dbg(&hci->master.dev, "scheduled commands available\n");
 	/* hci->schedcmd_regs = base; */
 	return 0;
 }
 
 static int hci_extcap_non_curr_master(struct i3c_hci *hci, void __iomem *base)
 {
-	dev_info(&hci->master.dev, "Non-Current Master support available\n");
+	dev_dbg(&hci->master.dev, "Non-Current Master support available\n");
 	/* hci->NCM_regs = base; */
 	return 0;
 }
 
 static int hci_extcap_ccc_resp_conf(struct i3c_hci *hci, void __iomem *base)
 {
-	dev_info(&hci->master.dev, "CCC Response Configuration available\n");
+	dev_dbg(&hci->master.dev, "CCC Response Configuration available\n");
 	return 0;
 }
 
 static int hci_extcap_global_DAT(struct i3c_hci *hci, void __iomem *base)
 {
-	dev_info(&hci->master.dev, "Global DAT available\n");
+	dev_dbg(&hci->master.dev, "Global DAT available\n");
 	return 0;
 }
 
 static int hci_extcap_multilane(struct i3c_hci *hci, void __iomem *base)
 {
-	dev_info(&hci->master.dev, "Master Multi-Lane support available\n");
+	dev_dbg(&hci->master.dev, "Master Multi-Lane support available\n");
 	return 0;
 }
 
 static int hci_extcap_ncm_multilane(struct i3c_hci *hci, void __iomem *base)
 {
-	dev_info(&hci->master.dev, "NCM Multi-Lane support available\n");
+	dev_dbg(&hci->master.dev, "NCM Multi-Lane support available\n");
 	return 0;
 }
 
@@ -202,7 +202,7 @@ static const struct hci_ext_caps ext_capabilities[] = {
 static int hci_extcap_vendor_NXP(struct i3c_hci *hci, void __iomem *base)
 {
 	hci->vendor_data = (__force void *)base;
-	dev_info(&hci->master.dev, "Build Date Info = %#x\n", readl(base + 1*4));
+	dev_dbg(&hci->master.dev, "Build Date Info = %#x\n", readl(base + 1*4));
 	/* reset the FPGA */
 	writel(0xdeadbeef, base + 1*4);
 	return 0;
@@ -240,7 +240,7 @@ static int hci_extcap_vendor_specific(struct i3c_hci *hci, void __iomem *base,
 	}
 
 	if (!vendor_cap_entry) {
-		dev_notice(&hci->master.dev,
+		dev_dbg(&hci->master.dev,
 			   "unknown ext_cap 0x%02x for vendor 0x%02x\n",
 			   cap_id, hci->vendor_mipi_id);
 		return 0;
@@ -295,8 +295,7 @@ int i3c_hci_parse_ext_caps(struct i3c_hci *hci)
 			}
 		}
 		if (!cap_entry) {
-			dev_notice(&hci->master.dev,
-				   "unknown ext_cap 0x%02x\n", cap_id);
+			dev_dbg(&hci->master.dev, "unknown ext_cap 0x%02x\n", cap_id);
 		} else if (cap_length < cap_entry->min_length) {
 			dev_err(&hci->master.dev,
 				"ext_cap 0x%02x has size %d (expecting >= %d)\n",
diff --git a/drivers/i3c/master/mipi-i3c-hci/pio.c b/drivers/i3c/master/mipi-i3c-hci/pio.c
index 142f3f79415b..5020c84c323f 100644
--- a/drivers/i3c/master/mipi-i3c-hci/pio.c
+++ b/drivers/i3c/master/mipi-i3c-hci/pio.c
@@ -148,13 +148,13 @@ static int hci_pio_init(struct i3c_hci *hci)
 	spin_lock_init(&pio->lock);
 
 	size_val = pio_reg_read(QUEUE_SIZE);
-	dev_info(&hci->master.dev, "CMD/RESP FIFO = %ld entries\n",
+	dev_dbg(&hci->master.dev, "CMD/RESP FIFO = %ld entries\n",
 		 FIELD_GET(CR_QUEUE_SIZE, size_val));
-	dev_info(&hci->master.dev, "IBI FIFO = %ld bytes\n",
+	dev_dbg(&hci->master.dev, "IBI FIFO = %ld bytes\n",
 		 4 * FIELD_GET(IBI_STATUS_SIZE, size_val));
-	dev_info(&hci->master.dev, "RX data FIFO = %d bytes\n",
+	dev_dbg(&hci->master.dev, "RX data FIFO = %d bytes\n",
 		 4 * (2 << FIELD_GET(RX_DATA_BUFFER_SIZE, size_val)));
-	dev_info(&hci->master.dev, "TX data FIFO = %d bytes\n",
+	dev_dbg(&hci->master.dev, "TX data FIFO = %d bytes\n",
 		 4 * (2 << FIELD_GET(TX_DATA_BUFFER_SIZE, size_val)));
 
 	/*
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V2 04/10] i3c: mipi-i3c-hci: Allow for Multi-Bus Instances

[Adrian Hunter]

A MIPI I3C Host Controller with the Multi-Bus Instance capability supports
multiple I3C Buses (up to 15), with one instance of the HCI Register Set
and one instance of I3C Bus Controller Logic for each I3C Bus, in a single
hardware function (e.g. PCIe B/D/F).

A separate platform device will represent each instance, but it is
necessary to allow for shared resources.

Multi-bus instances share the same MMIO address range, but not necessarily
in separate contiguous sub-ranges. To avoid overlapping mappings, provide
base_regs from the parent mapping.

Allow the IRQ to be shared.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2:

	None


 drivers/i3c/master/mipi-i3c-hci/core.c     | 21 +++++++++++++++++----
 include/linux/platform_data/mipi-i3c-hci.h | 15 +++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/platform_data/mipi-i3c-hci.h

diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index 07fb91a12593..05b8df1cf86d 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -14,6 +14,7 @@
 #include <linux/interrupt.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
+#include <linux/platform_data/mipi-i3c-hci.h>
 #include <linux/platform_device.h>
 
 #include "hci.h"
@@ -737,15 +738,27 @@ static int i3c_hci_init(struct i3c_hci *hci)
 
 static int i3c_hci_probe(struct platform_device *pdev)
 {
+	const struct mipi_i3c_hci_platform_data *pdata = pdev->dev.platform_data;
 	struct i3c_hci *hci;
 	int irq, ret;
 
 	hci = devm_kzalloc(&pdev->dev, sizeof(*hci), GFP_KERNEL);
 	if (!hci)
 		return -ENOMEM;
-	hci->base_regs = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(hci->base_regs))
-		return PTR_ERR(hci->base_regs);
+
+	/*
+	 * Multi-bus instances share the same MMIO address range, but not
+	 * necessarily in separate contiguous sub-ranges. To avoid overlapping
+	 * mappings, provide base_regs from the parent mapping.
+	 */
+	if (pdata)
+		hci->base_regs = pdata->base_regs;
+
+	if (!hci->base_regs) {
+		hci->base_regs = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(hci->base_regs))
+			return PTR_ERR(hci->base_regs);
+	}
 
 	platform_set_drvdata(pdev, hci);
 	/* temporary for dev_printk's, to be replaced in i3c_master_register */
@@ -759,7 +772,7 @@ static int i3c_hci_probe(struct platform_device *pdev)
 
 	irq = platform_get_irq(pdev, 0);
 	ret = devm_request_irq(&pdev->dev, irq, i3c_hci_irq_handler,
-			       0, NULL, hci);
+			       IRQF_SHARED, NULL, hci);
 	if (ret)
 		return ret;
 
diff --git a/include/linux/platform_data/mipi-i3c-hci.h b/include/linux/platform_data/mipi-i3c-hci.h
new file mode 100644
index 000000000000..ab7395f455f9
--- /dev/null
+++ b/include/linux/platform_data/mipi-i3c-hci.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef INCLUDE_PLATFORM_DATA_MIPI_I3C_HCI_H
+#define INCLUDE_PLATFORM_DATA_MIPI_I3C_HCI_H
+
+#include <linux/compiler_types.h>
+
+/**
+ * struct mipi_i3c_hci_platform_data - Platform-dependent data for mipi_i3c_hci
+ * @base_regs: Register set base address (to support multi-bus instances)
+ */
+struct mipi_i3c_hci_platform_data {
+	void __iomem *base_regs;
+};
+
+#endif
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V2 05/10] i3c: mipi-i3c-hci-pci: Do not repeatedly check for NULL driver_data

[Adrian Hunter]

All entries in the id_table have driver_data. Do not repeatedly check for
NULL driver_data.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2:

	Drop default driver data


 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index dc8ede0f8ad8..8ade911e3835 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -229,7 +229,7 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 		goto err;
 
 	hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
-	if (hci->info && hci->info->init) {
+	if (hci->info->init) {
 		ret = hci->info->init(hci);
 		if (ret)
 			goto err;
@@ -244,7 +244,7 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	return 0;
 
 err_exit:
-	if (hci->info && hci->info->exit)
+	if (hci->info->exit)
 		hci->info->exit(hci);
 err:
 	platform_device_put(hci->pdev);
@@ -258,7 +258,7 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
 	struct platform_device *pdev = hci->pdev;
 	int dev_id = pdev->id;
 
-	if (hci->info && hci->info->exit)
+	if (hci->info->exit)
 		hci->info->exit(hci);
 
 	platform_device_unregister(pdev);
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 05/10] i3c: mipi-i3c-hci-pci: Do not repeatedly check for NULL driver_data

[Frank Li]

On Thu, Dec 11, 2025 at 03:48:04PM +0200, Adrian Hunter wrote:
> All entries in the id_table have driver_data. Do not repeatedly check for
> NULL driver_data.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V2:
>
> 	Drop default driver data
>
>
>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index dc8ede0f8ad8..8ade911e3835 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -229,7 +229,7 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  		goto err;
>
>  	hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
> -	if (hci->info && hci->info->init) {
> +	if (hci->info->init) {
>  		ret = hci->info->init(hci);
>  		if (ret)
>  			goto err;
> @@ -244,7 +244,7 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  	return 0;
>
>  err_exit:
> -	if (hci->info && hci->info->exit)
> +	if (hci->info->exit)
>  		hci->info->exit(hci);
>  err:
>  	platform_device_put(hci->pdev);
> @@ -258,7 +258,7 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
>  	struct platform_device *pdev = hci->pdev;
>  	int dev_id = pdev->id;
>
> -	if (hci->info && hci->info->exit)
> +	if (hci->info->exit)
>  		hci->info->exit(hci);
>
>  	platform_device_unregister(pdev);
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V2 06/10] i3c: mipi-i3c-hci-pci: Enable MSI support

[Adrian Hunter]

Use pci_alloc_irq_vectors() to support all IRQ types.  Do not call
pci_free_irq_vectors() because it is unnecessary when the device is
managed due to the use of pcim_enable_device().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2:
	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
	Multi-Bus Instances"


 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 8ade911e3835..0fd3587671e1 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 
 	pci_set_master(pci);
 
+	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (ret < 0)
+		return ret;
+
 	memset(&res, 0, sizeof(res));
 
 	res[0].flags = IORESOURCE_MEM;
@@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	res[0].end = pci_resource_end(pci, 0);
 
 	res[1].flags = IORESOURCE_IRQ;
-	res[1].start = pci->irq;
-	res[1].end = pci->irq;
+	res[1].start = pci_irq_vector(hci->pci, 0);
+	res[1].end = res[1].start;
 
 	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
 	if (dev_id < 0)
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 06/10] i3c: mipi-i3c-hci-pci: Enable MSI support

[Frank Li]

On Thu, Dec 11, 2025 at 03:48:05PM +0200, Adrian Hunter wrote:
> Use pci_alloc_irq_vectors() to support all IRQ types.  Do not call
> pci_free_irq_vectors() because it is unnecessary when the device is
> managed due to the use of pcim_enable_device().

pcim_enable_device() add action pcim_disable_device(), I have not see
pci_free_irq_vectors() in pcim_disable_device().

Can you tell me where call pci_free_irq_vectors?

And move these small patch ahead in serise.

Frank
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>
>
> Changes in V2:
> 	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
> 	Multi-Bus Instances"
>
>
>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 8ade911e3835..0fd3587671e1 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>
>  	pci_set_master(pci);
>
> +	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
> +	if (ret < 0)
> +		return ret;
> +
>  	memset(&res, 0, sizeof(res));
>
>  	res[0].flags = IORESOURCE_MEM;
> @@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  	res[0].end = pci_resource_end(pci, 0);
>
>  	res[1].flags = IORESOURCE_IRQ;
> -	res[1].start = pci->irq;
> -	res[1].end = pci->irq;
> +	res[1].start = pci_irq_vector(hci->pci, 0);
> +	res[1].end = res[1].start;
>
>  	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>  	if (dev_id < 0)
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 06/10] i3c: mipi-i3c-hci-pci: Enable MSI support

[Adrian Hunter]

On 11/12/2025 17:40, Frank Li wrote:
> On Thu, Dec 11, 2025 at 03:48:05PM +0200, Adrian Hunter wrote:
>> Use pci_alloc_irq_vectors() to support all IRQ types.  Do not call
>> pci_free_irq_vectors() because it is unnecessary when the device is
>> managed due to the use of pcim_enable_device().
> 
> pcim_enable_device() add action pcim_disable_device(), I have not see
> pci_free_irq_vectors() in pcim_disable_device().
> 
> Can you tell me where call pci_free_irq_vectors?

pci_alloc_irq_vectors()
	pci_alloc_irq_vectors_affinity()
		__pci_enable_msix_range()
			pci_setup_msi_context()
				pcim_setup_msi_release()
					devm_add_action(pcim_msi_release)
							:
							pci_free_irq_vectors()
		__pci_enable_msi_range()
			pci_setup_msi_context()

> 
> And move these small patch ahead in serise.
> 
> Frank
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>
>>
>> Changes in V2:
>> 	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
>> 	Multi-Bus Instances"
>>
>>
>>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> index 8ade911e3835..0fd3587671e1 100644
>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> @@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>
>>  	pci_set_master(pci);
>>
>> +	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
>> +	if (ret < 0)
>> +		return ret;
>> +
>>  	memset(&res, 0, sizeof(res));
>>
>>  	res[0].flags = IORESOURCE_MEM;
>> @@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>  	res[0].end = pci_resource_end(pci, 0);
>>
>>  	res[1].flags = IORESOURCE_IRQ;
>> -	res[1].start = pci->irq;
>> -	res[1].end = pci->irq;
>> +	res[1].start = pci_irq_vector(hci->pci, 0);
>> +	res[1].end = res[1].start;
>>
>>  	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>>  	if (dev_id < 0)
>> --
>> 2.51.0
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 06/10] i3c: mipi-i3c-hci-pci: Enable MSI support

[Frank Li]

On Thu, Dec 11, 2025 at 06:19:56PM +0200, Adrian Hunter wrote:
> On 11/12/2025 17:40, Frank Li wrote:
> > On Thu, Dec 11, 2025 at 03:48:05PM +0200, Adrian Hunter wrote:
> >> Use pci_alloc_irq_vectors() to support all IRQ types.  Do not call
> >> pci_free_irq_vectors() because it is unnecessary when the device is
> >> managed due to the use of pcim_enable_device().
> >
> > pcim_enable_device() add action pcim_disable_device(), I have not see
> > pci_free_irq_vectors() in pcim_disable_device().
> >
> > Can you tell me where call pci_free_irq_vectors?
>
> pci_alloc_irq_vectors()
> 	pci_alloc_irq_vectors_affinity()
> 		__pci_enable_msix_range()
> 			pci_setup_msi_context()
> 				pcim_setup_msi_release()
> 					devm_add_action(pcim_msi_release)
> 							:
> 							pci_free_irq_vectors()
> 		__pci_enable_msi_range()
> 			pci_setup_msi_context()

Okay, it should be that pci_alloc_irq_vectors() provide auto free. not
because pcim_enable_device()?

The below statement should be wrong.

"Do not call pci_free_irq_vectors() because it is unnecessary when the device is
managed due to the use of pcim_enable_device()."

Frank
>
> >
> > And move these small patch ahead in serise.
> >
> > Frank
> >>
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >> ---
> >>
> >>
> >> Changes in V2:
> >> 	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
> >> 	Multi-Bus Instances"
> >>
> >>
> >>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> index 8ade911e3835..0fd3587671e1 100644
> >> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> @@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>
> >>  	pci_set_master(pci);
> >>
> >> +	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
> >> +	if (ret < 0)
> >> +		return ret;
> >> +
> >>  	memset(&res, 0, sizeof(res));
> >>
> >>  	res[0].flags = IORESOURCE_MEM;
> >> @@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>  	res[0].end = pci_resource_end(pci, 0);
> >>
> >>  	res[1].flags = IORESOURCE_IRQ;
> >> -	res[1].start = pci->irq;
> >> -	res[1].end = pci->irq;
> >> +	res[1].start = pci_irq_vector(hci->pci, 0);
> >> +	res[1].end = res[1].start;
> >>
> >>  	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> >>  	if (dev_id < 0)
> >> --
> >> 2.51.0
> >>
> >>
> >> --
> >> linux-i3c mailing list
> >> linux-i3c@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 06/10] i3c: mipi-i3c-hci-pci: Enable MSI support

[Adrian Hunter]

On 12/12/2025 19:38, Frank Li wrote:
> On Thu, Dec 11, 2025 at 06:19:56PM +0200, Adrian Hunter wrote:
>> On 11/12/2025 17:40, Frank Li wrote:
>>> On Thu, Dec 11, 2025 at 03:48:05PM +0200, Adrian Hunter wrote:
>>>> Use pci_alloc_irq_vectors() to support all IRQ types.  Do not call
>>>> pci_free_irq_vectors() because it is unnecessary when the device is
>>>> managed due to the use of pcim_enable_device().
>>>
>>> pcim_enable_device() add action pcim_disable_device(), I have not see
>>> pci_free_irq_vectors() in pcim_disable_device().
>>>
>>> Can you tell me where call pci_free_irq_vectors?
>>
>> pci_alloc_irq_vectors()
>> 	pci_alloc_irq_vectors_affinity()
>> 		__pci_enable_msix_range()
>> 			pci_setup_msi_context()
>> 				pcim_setup_msi_release()
>> 					devm_add_action(pcim_msi_release)
>> 							:
>> 							pci_free_irq_vectors()
>> 		__pci_enable_msi_range()
>> 			pci_setup_msi_context()
> 
> Okay, it should be that pci_alloc_irq_vectors() provide auto free. not
> because pcim_enable_device()?

No, it is conditional on pci_is_managed(dev) which is true if
pcim_enable_device() has been used.

> 
> The below statement should be wrong.
> 
> "Do not call pci_free_irq_vectors() because it is unnecessary when the device is
> managed due to the use of pcim_enable_device()."
> 
> Frank
>>
>>>
>>> And move these small patch ahead in serise.
>>>
>>> Frank
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>> ---
>>>>
>>>>
>>>> Changes in V2:
>>>> 	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
>>>> 	Multi-Bus Instances"
>>>>
>>>>
>>>>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
>>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> index 8ade911e3835..0fd3587671e1 100644
>>>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> @@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>>>
>>>>  	pci_set_master(pci);
>>>>
>>>> +	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
>>>> +	if (ret < 0)
>>>> +		return ret;
>>>> +
>>>>  	memset(&res, 0, sizeof(res));
>>>>
>>>>  	res[0].flags = IORESOURCE_MEM;
>>>> @@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>>>  	res[0].end = pci_resource_end(pci, 0);
>>>>
>>>>  	res[1].flags = IORESOURCE_IRQ;
>>>> -	res[1].start = pci->irq;
>>>> -	res[1].end = pci->irq;
>>>> +	res[1].start = pci_irq_vector(hci->pci, 0);
>>>> +	res[1].end = res[1].start;
>>>>
>>>>  	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>>>>  	if (dev_id < 0)
>>>> --
>>>> 2.51.0
>>>>
>>>>
>>>> --
>>>> linux-i3c mailing list
>>>> linux-i3c@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 06/10] i3c: mipi-i3c-hci-pci: Enable MSI support

[Frank Li]

On Mon, Dec 15, 2025 at 07:26:55PM +0200, Adrian Hunter wrote:
> On 12/12/2025 19:38, Frank Li wrote:
> > On Thu, Dec 11, 2025 at 06:19:56PM +0200, Adrian Hunter wrote:
> >> On 11/12/2025 17:40, Frank Li wrote:
> >>> On Thu, Dec 11, 2025 at 03:48:05PM +0200, Adrian Hunter wrote:
> >>>> Use pci_alloc_irq_vectors() to support all IRQ types.  Do not call
> >>>> pci_free_irq_vectors() because it is unnecessary when the device is
> >>>> managed due to the use of pcim_enable_device().
> >>>
> >>> pcim_enable_device() add action pcim_disable_device(), I have not see
> >>> pci_free_irq_vectors() in pcim_disable_device().
> >>>
> >>> Can you tell me where call pci_free_irq_vectors?
> >>
> >> pci_alloc_irq_vectors()
> >> 	pci_alloc_irq_vectors_affinity()
> >> 		__pci_enable_msix_range()
> >> 			pci_setup_msi_context()
> >> 				pcim_setup_msi_release()
> >> 					devm_add_action(pcim_msi_release)
> >> 							:
> >> 							pci_free_irq_vectors()
> >> 		__pci_enable_msi_range()
> >> 			pci_setup_msi_context()
> >
> > Okay, it should be that pci_alloc_irq_vectors() provide auto free. not
> > because pcim_enable_device()?
>
> No, it is conditional on pci_is_managed(dev) which is true if
> pcim_enable_device() has been used.

I have not found any document said that,

 * Managed pci_enable_device(). Device will automatically be disabled on
 * driver detach.

which have not mentioned that it also controller free irqs.

I suggest commit message descript whole story or change PCI's document
to descript it clearly. I think other person should have similar question
if he have not closely follow PCI's change and go through PCI codes.

Frank

>
> >
> > The below statement should be wrong.
> >
> > "Do not call pci_free_irq_vectors() because it is unnecessary when the device is
> > managed due to the use of pcim_enable_device()."
> >
> > Frank
> >>
> >>>
> >>> And move these small patch ahead in serise.
> >>>
> >>> Frank
> >>>>
> >>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >>>> ---
> >>>>
> >>>>
> >>>> Changes in V2:
> >>>> 	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
> >>>> 	Multi-Bus Instances"
> >>>>
> >>>>
> >>>>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
> >>>>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> index 8ade911e3835..0fd3587671e1 100644
> >>>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> @@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>>>
> >>>>  	pci_set_master(pci);
> >>>>
> >>>> +	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
> >>>> +	if (ret < 0)
> >>>> +		return ret;
> >>>> +
> >>>>  	memset(&res, 0, sizeof(res));
> >>>>
> >>>>  	res[0].flags = IORESOURCE_MEM;
> >>>> @@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>>>  	res[0].end = pci_resource_end(pci, 0);
> >>>>
> >>>>  	res[1].flags = IORESOURCE_IRQ;
> >>>> -	res[1].start = pci->irq;
> >>>> -	res[1].end = pci->irq;
> >>>> +	res[1].start = pci_irq_vector(hci->pci, 0);
> >>>> +	res[1].end = res[1].start;
> >>>>
> >>>>  	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> >>>>  	if (dev_id < 0)
> >>>> --
> >>>> 2.51.0
> >>>>
> >>>>
> >>>> --
> >>>> linux-i3c mailing list
> >>>> linux-i3c@lists.infradead.org
> >>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
> >>
> >>
> >> --
> >> linux-i3c mailing list
> >> linux-i3c@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 06/10] i3c: mipi-i3c-hci-pci: Enable MSI support

[Adrian Hunter]

On 15/12/2025 19:44, Frank Li wrote:
> On Mon, Dec 15, 2025 at 07:26:55PM +0200, Adrian Hunter wrote:
>> On 12/12/2025 19:38, Frank Li wrote:
>>> On Thu, Dec 11, 2025 at 06:19:56PM +0200, Adrian Hunter wrote:
>>>> On 11/12/2025 17:40, Frank Li wrote:
>>>>> On Thu, Dec 11, 2025 at 03:48:05PM +0200, Adrian Hunter wrote:
>>>>>> Use pci_alloc_irq_vectors() to support all IRQ types.  Do not call
>>>>>> pci_free_irq_vectors() because it is unnecessary when the device is
>>>>>> managed due to the use of pcim_enable_device().
>>>>>
>>>>> pcim_enable_device() add action pcim_disable_device(), I have not see
>>>>> pci_free_irq_vectors() in pcim_disable_device().
>>>>>
>>>>> Can you tell me where call pci_free_irq_vectors?
>>>>
>>>> pci_alloc_irq_vectors()
>>>> 	pci_alloc_irq_vectors_affinity()
>>>> 		__pci_enable_msix_range()
>>>> 			pci_setup_msi_context()
>>>> 				pcim_setup_msi_release()
>>>> 					devm_add_action(pcim_msi_release)
>>>> 							:
>>>> 							pci_free_irq_vectors()
>>>> 		__pci_enable_msi_range()
>>>> 			pci_setup_msi_context()
>>>
>>> Okay, it should be that pci_alloc_irq_vectors() provide auto free. not
>>> because pcim_enable_device()?
>>
>> No, it is conditional on pci_is_managed(dev) which is true if
>> pcim_enable_device() has been used.
> 
> I have not found any document said that,
> 
>  * Managed pci_enable_device(). Device will automatically be disabled on
>  * driver detach.
> 
> which have not mentioned that it also controller free irqs.

It is not a secret:

commit 5982a539cdcedcf1a87709c01125e1596eee2c9a
Author: Salah Triki <salah.triki@gmail.com>
Date:   Sat Jul 19 07:33:36 2025 +0100

    accel/amdxdna: Delete pci_free_irq_vectors()
    
    The device is managed so pci_free_irq_vectors() is called automatically
    no need to do it manually.
    
    Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
    Signed-off-by: Salah Triki <salah.triki@gmail.com>
    Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
    Link: https://lore.kernel.org/r/aHs8QAfUlFeNp7qL@pc

commit 604f7e7777d663033063886b6a5362d0e6092e3a
Author: Michał Winiarski <michal.winiarski@intel.com>
Date:   Wed Nov 29 22:44:59 2023 +0100

    drm/xe/irq: Don't call pci_free_irq_vectors
    
    For devres managed devices, pci_alloc_irq_vectors is also managed (see
    pci_setup_msi_context for reference).
    PCI device used by Xe is devres managed (it was enabled with
    pcim_enable_device), which means that calls to pci_free_irq_vectors are
    redundant and can be safely removed.
    
    Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
    Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
    Link: https://lore.kernel.org/r/20231129214509.1174116-4-michal.winiarski@intel.com
    Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

commit a264cee31f13ae3b8d32b3e53774759afa55361e
Author: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Date:   Wed Jun 19 10:21:07 2024 -0700

    thermal: intel: int340x: Remove unnecessary calls to free irq
    
    Remove calls to devm_free_irq() and pci_free_irq_vectors().
    They will be called on driver release anyway.
    
    Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
    Link: https://patch.msgid.link/20240619172109.497639-2-srinivas.pandruvada@linux.intel.com
    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

commit 5e74a4b3ec1816e3bbfd715d46ae29d2508079cb
Author: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date:   Sun Jun 5 22:50:48 2022 +0200

    stmmac: intel: Fix an error handling path in intel_eth_pci_probe()
    
    When the managed API is used, there is no need to explicitly call
    pci_free_irq_vectors().
    
    This looks to be a left-over from the commit in the Fixes tag. Only the
    .remove() function had been updated.
    
    So remove this unused function call and update goto label accordingly.
    
    Fixes: 8accc467758e ("stmmac: intel: use managed PCI function on probe and resume")
    Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
    Reviewed-by: Wong Vee Khee <vee.khee.wong@linux.intel.com>
    Link: https://lore.kernel.org/r/1ac9b6787b0db83b0095711882c55c77c8ea8da0.1654462241.git.christophe.jaillet@wanadoo.fr
    Signed-off-by: Paolo Abeni <pabeni@redhat.com>


> 
> I suggest commit message descript whole story or change PCI's document
> to descript it clearly. I think other person should have similar question
> if he have not closely follow PCI's change and go through PCI codes.
> 
> Frank
> 
>>
>>>
>>> The below statement should be wrong.
>>>
>>> "Do not call pci_free_irq_vectors() because it is unnecessary when the device is
>>> managed due to the use of pcim_enable_device()."
>>>
>>> Frank
>>>>
>>>>>
>>>>> And move these small patch ahead in serise.
>>>>>
>>>>> Frank
>>>>>>
>>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>>>> ---
>>>>>>
>>>>>>
>>>>>> Changes in V2:
>>>>>> 	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
>>>>>> 	Multi-Bus Instances"
>>>>>>
>>>>>>
>>>>>>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
>>>>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>>>> index 8ade911e3835..0fd3587671e1 100644
>>>>>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>>>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>>>> @@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>>>>>
>>>>>>  	pci_set_master(pci);
>>>>>>
>>>>>> +	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
>>>>>> +	if (ret < 0)
>>>>>> +		return ret;
>>>>>> +
>>>>>>  	memset(&res, 0, sizeof(res));
>>>>>>
>>>>>>  	res[0].flags = IORESOURCE_MEM;
>>>>>> @@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>>>>>  	res[0].end = pci_resource_end(pci, 0);
>>>>>>
>>>>>>  	res[1].flags = IORESOURCE_IRQ;
>>>>>> -	res[1].start = pci->irq;
>>>>>> -	res[1].end = pci->irq;
>>>>>> +	res[1].start = pci_irq_vector(hci->pci, 0);
>>>>>> +	res[1].end = res[1].start;
>>>>>>
>>>>>>  	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>>>>>>  	if (dev_id < 0)
>>>>>> --
>>>>>> 2.51.0
>>>>>>
>>>>>>
>>>>>> --
>>>>>> linux-i3c mailing list
>>>>>> linux-i3c@lists.infradead.org
>>>>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
>>>>
>>>>
>>>> --
>>>> linux-i3c mailing list
>>>> linux-i3c@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 06/10] i3c: mipi-i3c-hci-pci: Enable MSI support

[Frank Li]

On Mon, Dec 15, 2025 at 08:19:10PM +0200, Adrian Hunter wrote:
> On 15/12/2025 19:44, Frank Li wrote:
> > On Mon, Dec 15, 2025 at 07:26:55PM +0200, Adrian Hunter wrote:
> >> On 12/12/2025 19:38, Frank Li wrote:
> >>> On Thu, Dec 11, 2025 at 06:19:56PM +0200, Adrian Hunter wrote:
> >>>> On 11/12/2025 17:40, Frank Li wrote:
> >>>>> On Thu, Dec 11, 2025 at 03:48:05PM +0200, Adrian Hunter wrote:
> >>>>>> Use pci_alloc_irq_vectors() to support all IRQ types.  Do not call
> >>>>>> pci_free_irq_vectors() because it is unnecessary when the device is
> >>>>>> managed due to the use of pcim_enable_device().
> >>>>>
> >>>>> pcim_enable_device() add action pcim_disable_device(), I have not see
> >>>>> pci_free_irq_vectors() in pcim_disable_device().
> >>>>>
> >>>>> Can you tell me where call pci_free_irq_vectors?
> >>>>
> >>>> pci_alloc_irq_vectors()
> >>>> 	pci_alloc_irq_vectors_affinity()
> >>>> 		__pci_enable_msix_range()
> >>>> 			pci_setup_msi_context()
> >>>> 				pcim_setup_msi_release()
> >>>> 					devm_add_action(pcim_msi_release)
> >>>> 							:
> >>>> 							pci_free_irq_vectors()
> >>>> 		__pci_enable_msi_range()
> >>>> 			pci_setup_msi_context()
> >>>
> >>> Okay, it should be that pci_alloc_irq_vectors() provide auto free. not
> >>> because pcim_enable_device()?
> >>
> >> No, it is conditional on pci_is_managed(dev) which is true if
> >> pcim_enable_device() has been used.
> >
> > I have not found any document said that,
> >
> >  * Managed pci_enable_device(). Device will automatically be disabled on
> >  * driver detach.
> >
> > which have not mentioned that it also controller free irqs.
>
> It is not a secret:

Agree, it may well known by someones. But it is not obvious from
description and function name.

Ideally create patch to update document, of course, it should separate
works.

>
> commit 5982a539cdcedcf1a87709c01125e1596eee2c9a
> Author: Salah Triki <salah.triki@gmail.com>
> Date:   Sat Jul 19 07:33:36 2025 +0100
>
>     accel/amdxdna: Delete pci_free_irq_vectors()
>
>     The device is managed so pci_free_irq_vectors() is called automatically
>     no need to do it manually.
>
>     Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
>     Signed-off-by: Salah Triki <salah.triki@gmail.com>
>     Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>     Link: https://lore.kernel.org/r/aHs8QAfUlFeNp7qL@pc
>
> commit 604f7e7777d663033063886b6a5362d0e6092e3a
> Author: Michał Winiarski <michal.winiarski@intel.com>
> Date:   Wed Nov 29 22:44:59 2023 +0100
>
>     drm/xe/irq: Don't call pci_free_irq_vectors
>
>     For devres managed devices, pci_alloc_irq_vectors is also managed (see
>     pci_setup_msi_context for reference).

I prefer use this sentence at your commit message.

Frank

>     PCI device used by Xe is devres managed (it was enabled with
>     pcim_enable_device), which means that calls to pci_free_irq_vectors are
>     redundant and can be safely removed.
>
>     Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
>     Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>     Link: https://lore.kernel.org/r/20231129214509.1174116-4-michal.winiarski@intel.com
>     Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> commit a264cee31f13ae3b8d32b3e53774759afa55361e
> Author: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Date:   Wed Jun 19 10:21:07 2024 -0700
>
>     thermal: intel: int340x: Remove unnecessary calls to free irq
>
>     Remove calls to devm_free_irq() and pci_free_irq_vectors().
>     They will be called on driver release anyway.
>
>     Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>     Link: https://patch.msgid.link/20240619172109.497639-2-srinivas.pandruvada@linux.intel.com
>     Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> commit 5e74a4b3ec1816e3bbfd715d46ae29d2508079cb
> Author: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Date:   Sun Jun 5 22:50:48 2022 +0200
>
>     stmmac: intel: Fix an error handling path in intel_eth_pci_probe()
>
>     When the managed API is used, there is no need to explicitly call
>     pci_free_irq_vectors().
>
>     This looks to be a left-over from the commit in the Fixes tag. Only the
>     .remove() function had been updated.
>
>     So remove this unused function call and update goto label accordingly.
>
>     Fixes: 8accc467758e ("stmmac: intel: use managed PCI function on probe and resume")
>     Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>     Reviewed-by: Wong Vee Khee <vee.khee.wong@linux.intel.com>
>     Link: https://lore.kernel.org/r/1ac9b6787b0db83b0095711882c55c77c8ea8da0.1654462241.git.christophe.jaillet@wanadoo.fr
>     Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>
>
> >
> > I suggest commit message descript whole story or change PCI's document
> > to descript it clearly. I think other person should have similar question
> > if he have not closely follow PCI's change and go through PCI codes.
> >
> > Frank
> >
> >>
> >>>
> >>> The below statement should be wrong.
> >>>
> >>> "Do not call pci_free_irq_vectors() because it is unnecessary when the device is
> >>> managed due to the use of pcim_enable_device()."
> >>>
> >>> Frank
> >>>>
> >>>>>
> >>>>> And move these small patch ahead in serise.
> >>>>>
> >>>>> Frank
> >>>>>>
> >>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >>>>>> ---
> >>>>>>
> >>>>>>
> >>>>>> Changes in V2:
> >>>>>> 	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
> >>>>>> 	Multi-Bus Instances"
> >>>>>>
> >>>>>>
> >>>>>>  drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
> >>>>>>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>>>> index 8ade911e3835..0fd3587671e1 100644
> >>>>>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>>>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>>>> @@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>>>>>
> >>>>>>  	pci_set_master(pci);
> >>>>>>
> >>>>>> +	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
> >>>>>> +	if (ret < 0)
> >>>>>> +		return ret;
> >>>>>> +
> >>>>>>  	memset(&res, 0, sizeof(res));
> >>>>>>
> >>>>>>  	res[0].flags = IORESOURCE_MEM;
> >>>>>> @@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>>>>>  	res[0].end = pci_resource_end(pci, 0);
> >>>>>>
> >>>>>>  	res[1].flags = IORESOURCE_IRQ;
> >>>>>> -	res[1].start = pci->irq;
> >>>>>> -	res[1].end = pci->irq;
> >>>>>> +	res[1].start = pci_irq_vector(hci->pci, 0);
> >>>>>> +	res[1].end = res[1].start;
> >>>>>>
> >>>>>>  	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> >>>>>>  	if (dev_id < 0)
> >>>>>> --
> >>>>>> 2.51.0
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> linux-i3c mailing list
> >>>>>> linux-i3c@lists.infradead.org
> >>>>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
> >>>>
> >>>>
> >>>> --
> >>>> linux-i3c mailing list
> >>>> linux-i3c@lists.infradead.org
> >>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
> >>
> >>
> >> --
> >> linux-i3c mailing list
> >> linux-i3c@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V2 07/10] i3c: mipi-i3c-hci-pci: Use parent MMIO mapping

[Adrian Hunter]

Prepare for Multi-Bus instance support.  Pass base_regs to child device.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2:
	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
	Multi-Bus Instances"


 .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 39 ++++++++++---------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 0fd3587671e1..0180d99a37e4 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -14,12 +14,14 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/platform_data/mipi-i3c-hci.h>
 #include <linux/platform_device.h>
 #include <linux/pm_qos.h>
 
 struct mipi_i3c_hci_pci {
 	struct pci_dev *pci;
 	struct platform_device *pdev;
+	void __iomem *base;
 	const struct mipi_i3c_hci_pci_info *info;
 	void *private;
 };
@@ -32,7 +34,6 @@ struct mipi_i3c_hci_pci_info {
 static DEFINE_IDA(mipi_i3c_hci_pci_ida);
 
 #define INTEL_PRIV_OFFSET		0x2b0
-#define INTEL_PRIV_SIZE			0x28
 #define INTEL_RESETS			0x04
 #define INTEL_RESETS_RESET		BIT(0)
 #define INTEL_RESETS_RESET_DONE		BIT(1)
@@ -143,19 +144,12 @@ static void intel_reset(void __iomem *priv)
 	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
 }
 
-static void __iomem *intel_priv(struct pci_dev *pci)
-{
-	resource_size_t base = pci_resource_start(pci, 0);
-
-	return devm_ioremap(&pci->dev, base + INTEL_PRIV_OFFSET, INTEL_PRIV_SIZE);
-}
-
 static int intel_i3c_init(struct mipi_i3c_hci_pci *hci)
 {
 	struct intel_host *host = devm_kzalloc(&hci->pci->dev, sizeof(*host), GFP_KERNEL);
-	void __iomem *priv = intel_priv(hci->pci);
+	void __iomem *priv = hci->base + INTEL_PRIV_OFFSET;
 
-	if (!host || !priv)
+	if (!host)
 		return -ENOMEM;
 
 	dma_set_mask_and_coherent(&hci->pci->dev, DMA_BIT_MASK(64));
@@ -187,8 +181,9 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
 static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 				  const struct pci_device_id *id)
 {
+	struct mipi_i3c_hci_platform_data pdata = {};
 	struct mipi_i3c_hci_pci *hci;
-	struct resource res[2];
+	struct resource res;
 	int dev_id, ret;
 
 	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
@@ -203,19 +198,19 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 
 	pci_set_master(pci);
 
+	hci->base = pcim_iomap_region(pci, 0, pci_name(pci));
+	if (IS_ERR(hci->base))
+		return PTR_ERR(hci->base);
+
 	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
 	if (ret < 0)
 		return ret;
 
 	memset(&res, 0, sizeof(res));
 
-	res[0].flags = IORESOURCE_MEM;
-	res[0].start = pci_resource_start(pci, 0);
-	res[0].end = pci_resource_end(pci, 0);
-
-	res[1].flags = IORESOURCE_IRQ;
-	res[1].start = pci_irq_vector(hci->pci, 0);
-	res[1].end = res[1].start;
+	res.flags = IORESOURCE_IRQ;
+	res.start = pci_irq_vector(hci->pci, 0);
+	res.end = res.start;
 
 	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
 	if (dev_id < 0)
@@ -228,7 +223,13 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	hci->pdev->dev.parent = &pci->dev;
 	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
 
-	ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
+	ret = platform_device_add_resources(hci->pdev, &res, 1);
+	if (ret)
+		goto err;
+
+	pdata.base_regs = hci->base;
+
+	ret = platform_device_add_data(hci->pdev, &pdata, sizeof(pdata));
 	if (ret)
 		goto err;
 
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 07/10] i3c: mipi-i3c-hci-pci: Use parent MMIO mapping

[Frank Li]

On Thu, Dec 11, 2025 at 03:48:06PM +0200, Adrian Hunter wrote:
> Prepare for Multi-Bus instance support.  Pass base_regs to child device.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

subjest: Pass base regs as platform data to i3c core device

Frank
>
>
> Changes in V2:
> 	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
> 	Multi-Bus Instances"
>
>
>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 39 ++++++++++---------
>  1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 0fd3587671e1..0180d99a37e4 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -14,12 +14,14 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
> +#include <linux/platform_data/mipi-i3c-hci.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_qos.h>
>
>  struct mipi_i3c_hci_pci {
>  	struct pci_dev *pci;
>  	struct platform_device *pdev;
> +	void __iomem *base;
>  	const struct mipi_i3c_hci_pci_info *info;
>  	void *private;
>  };
> @@ -32,7 +34,6 @@ struct mipi_i3c_hci_pci_info {
>  static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>
>  #define INTEL_PRIV_OFFSET		0x2b0
> -#define INTEL_PRIV_SIZE			0x28
>  #define INTEL_RESETS			0x04
>  #define INTEL_RESETS_RESET		BIT(0)
>  #define INTEL_RESETS_RESET_DONE		BIT(1)
> @@ -143,19 +144,12 @@ static void intel_reset(void __iomem *priv)
>  	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
>  }
>
> -static void __iomem *intel_priv(struct pci_dev *pci)
> -{
> -	resource_size_t base = pci_resource_start(pci, 0);
> -
> -	return devm_ioremap(&pci->dev, base + INTEL_PRIV_OFFSET, INTEL_PRIV_SIZE);
> -}
> -
>  static int intel_i3c_init(struct mipi_i3c_hci_pci *hci)
>  {
>  	struct intel_host *host = devm_kzalloc(&hci->pci->dev, sizeof(*host), GFP_KERNEL);
> -	void __iomem *priv = intel_priv(hci->pci);
> +	void __iomem *priv = hci->base + INTEL_PRIV_OFFSET;
>
> -	if (!host || !priv)
> +	if (!host)
>  		return -ENOMEM;
>
>  	dma_set_mask_and_coherent(&hci->pci->dev, DMA_BIT_MASK(64));
> @@ -187,8 +181,9 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
>  static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  				  const struct pci_device_id *id)
>  {
> +	struct mipi_i3c_hci_platform_data pdata = {};
>  	struct mipi_i3c_hci_pci *hci;
> -	struct resource res[2];
> +	struct resource res;
>  	int dev_id, ret;
>
>  	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
> @@ -203,19 +198,19 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>
>  	pci_set_master(pci);
>
> +	hci->base = pcim_iomap_region(pci, 0, pci_name(pci));
> +	if (IS_ERR(hci->base))
> +		return PTR_ERR(hci->base);
> +
>  	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
>  	if (ret < 0)
>  		return ret;
>
>  	memset(&res, 0, sizeof(res));
>
> -	res[0].flags = IORESOURCE_MEM;
> -	res[0].start = pci_resource_start(pci, 0);
> -	res[0].end = pci_resource_end(pci, 0);
> -
> -	res[1].flags = IORESOURCE_IRQ;
> -	res[1].start = pci_irq_vector(hci->pci, 0);
> -	res[1].end = res[1].start;
> +	res.flags = IORESOURCE_IRQ;
> +	res.start = pci_irq_vector(hci->pci, 0);
> +	res.end = res.start;
>
>  	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>  	if (dev_id < 0)
> @@ -228,7 +223,13 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  	hci->pdev->dev.parent = &pci->dev;
>  	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
>
> -	ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
> +	ret = platform_device_add_resources(hci->pdev, &res, 1);
> +	if (ret)
> +		goto err;
> +
> +	pdata.base_regs = hci->base;
> +
> +	ret = platform_device_add_data(hci->pdev, &pdata, sizeof(pdata));
>  	if (ret)
>  		goto err;
>
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V2 08/10] i3c: mipi-i3c-hci-pci: Convert to MFD driver

[Adrian Hunter]

Prepare for Multi-Bus instance support.  Convert to MFD driver but still
support only 1 instance.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2:
	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
	Multi-Bus Instances"


 drivers/i3c/master/Kconfig                    |   1 +
 .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 100 ++++++++++--------
 2 files changed, 58 insertions(+), 43 deletions(-)

diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
index 82cf330778d5..2609f2b18e0a 100644
--- a/drivers/i3c/master/Kconfig
+++ b/drivers/i3c/master/Kconfig
@@ -69,6 +69,7 @@ config MIPI_I3C_HCI_PCI
 	tristate "MIPI I3C Host Controller Interface PCI support"
 	depends on MIPI_I3C_HCI
 	depends on PCI
+	select MFD_CORE
 	help
 	  Support for MIPI I3C Host Controller Interface compatible hardware
 	  on the PCI bus.
diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 0180d99a37e4..68088967942b 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -12,6 +12,7 @@
 #include <linux/idr.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
+#include <linux/mfd/core.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/platform_data/mipi-i3c-hci.h>
@@ -20,8 +21,8 @@
 
 struct mipi_i3c_hci_pci {
 	struct pci_dev *pci;
-	struct platform_device *pdev;
 	void __iomem *base;
+	int dev_id;
 	const struct mipi_i3c_hci_pci_info *info;
 	void *private;
 };
@@ -178,13 +179,59 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
 	.exit = intel_i3c_exit,
 };
 
+struct mipi_i3c_hci_pci_cell_data {
+	struct mipi_i3c_hci_platform_data pdata;
+	struct resource res;
+};
+
+static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci,
+					struct mipi_i3c_hci_pci_cell_data *data,
+					struct mfd_cell *cell)
+{
+	data->pdata.base_regs = hci->base;
+
+	data->res = DEFINE_RES_IRQ(0);
+
+	cell->name = "mipi-i3c-hci";
+	cell->id = hci->dev_id;
+	cell->platform_data = &data->pdata;
+	cell->pdata_size = sizeof(data->pdata);
+	cell->num_resources = 1;
+	cell->resources = &data->res;
+}
+
+static int mipi_i3c_hci_pci_add_instances(struct mipi_i3c_hci_pci *hci)
+{
+	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = kzalloc(sizeof(*data), GFP_KERNEL);
+	struct mfd_cell *cells __free(kfree) = kzalloc(sizeof(*cells), GFP_KERNEL);
+	int irq = pci_irq_vector(hci->pci, 0);
+	int ret;
+
+	if (!cells || !data)
+		return -ENOMEM;
+
+	hci->dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
+	if (hci->dev_id < 0)
+		return hci->dev_id;
+
+	mipi_i3c_hci_pci_setup_cell(hci, data, cells);
+
+	ret = mfd_add_devices(&hci->pci->dev, 0, cells, 1, NULL, irq, NULL);
+	if (ret)
+		goto err_free_ids;
+
+	return 0;
+
+err_free_ids:
+	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
+	return ret;
+}
+
 static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 				  const struct pci_device_id *id)
 {
-	struct mipi_i3c_hci_platform_data pdata = {};
 	struct mipi_i3c_hci_pci *hci;
-	struct resource res;
-	int dev_id, ret;
+	int ret;
 
 	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
 	if (!hci)
@@ -206,41 +253,13 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	if (ret < 0)
 		return ret;
 
-	memset(&res, 0, sizeof(res));
-
-	res.flags = IORESOURCE_IRQ;
-	res.start = pci_irq_vector(hci->pci, 0);
-	res.end = res.start;
-
-	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
-	if (dev_id < 0)
-		return dev_id;
-
-	hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
-	if (!hci->pdev)
-		return -ENOMEM;
-
-	hci->pdev->dev.parent = &pci->dev;
-	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
-
-	ret = platform_device_add_resources(hci->pdev, &res, 1);
-	if (ret)
-		goto err;
-
-	pdata.base_regs = hci->base;
+	hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
 
-	ret = platform_device_add_data(hci->pdev, &pdata, sizeof(pdata));
+	ret = hci->info->init ? hci->info->init(hci) : 0;
 	if (ret)
-		goto err;
-
-	hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
-	if (hci->info->init) {
-		ret = hci->info->init(hci);
-		if (ret)
-			goto err;
-	}
+		return ret;
 
-	ret = platform_device_add(hci->pdev);
+	ret = mipi_i3c_hci_pci_add_instances(hci);
 	if (ret)
 		goto err_exit;
 
@@ -251,23 +270,18 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 err_exit:
 	if (hci->info->exit)
 		hci->info->exit(hci);
-err:
-	platform_device_put(hci->pdev);
-	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
 	return ret;
 }
 
 static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
 {
 	struct mipi_i3c_hci_pci *hci = pci_get_drvdata(pci);
-	struct platform_device *pdev = hci->pdev;
-	int dev_id = pdev->id;
 
 	if (hci->info->exit)
 		hci->info->exit(hci);
 
-	platform_device_unregister(pdev);
-	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
+	mfd_remove_devices(&pci->dev);
+	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
 }
 
 static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 08/10] i3c: mipi-i3c-hci-pci: Convert to MFD driver

[Frank Li]

On Thu, Dec 11, 2025 at 03:48:07PM +0200, Adrian Hunter wrote:
> Prepare for Multi-Bus instance support.  Convert to MFD driver but still
> support only 1 instance.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V2:
> 	New patch split from "i3c: mipi-i3c-hci-pci: Add support for
> 	Multi-Bus Instances"
>
>
>  drivers/i3c/master/Kconfig                    |   1 +
>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 100 ++++++++++--------
>  2 files changed, 58 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
> index 82cf330778d5..2609f2b18e0a 100644
> --- a/drivers/i3c/master/Kconfig
> +++ b/drivers/i3c/master/Kconfig
> @@ -69,6 +69,7 @@ config MIPI_I3C_HCI_PCI
>  	tristate "MIPI I3C Host Controller Interface PCI support"
>  	depends on MIPI_I3C_HCI
>  	depends on PCI
> +	select MFD_CORE
>  	help
>  	  Support for MIPI I3C Host Controller Interface compatible hardware
>  	  on the PCI bus.
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 0180d99a37e4..68088967942b 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -12,6 +12,7 @@
>  #include <linux/idr.h>
>  #include <linux/iopoll.h>
>  #include <linux/kernel.h>
> +#include <linux/mfd/core.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/platform_data/mipi-i3c-hci.h>
> @@ -20,8 +21,8 @@
>
>  struct mipi_i3c_hci_pci {
>  	struct pci_dev *pci;
> -	struct platform_device *pdev;
>  	void __iomem *base;
> +	int dev_id;
>  	const struct mipi_i3c_hci_pci_info *info;
>  	void *private;
>  };
> @@ -178,13 +179,59 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
>  	.exit = intel_i3c_exit,
>  };
>
> +struct mipi_i3c_hci_pci_cell_data {
> +	struct mipi_i3c_hci_platform_data pdata;
> +	struct resource res;
> +};
> +
> +static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci,
> +					struct mipi_i3c_hci_pci_cell_data *data,
> +					struct mfd_cell *cell)
> +{
> +	data->pdata.base_regs = hci->base;
> +
> +	data->res = DEFINE_RES_IRQ(0);
> +
> +	cell->name = "mipi-i3c-hci";
> +	cell->id = hci->dev_id;
> +	cell->platform_data = &data->pdata;
> +	cell->pdata_size = sizeof(data->pdata);
> +	cell->num_resources = 1;
> +	cell->resources = &data->res;
> +}
> +
> +static int mipi_i3c_hci_pci_add_instances(struct mipi_i3c_hci_pci *hci)
> +{
> +	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = kzalloc(sizeof(*data), GFP_KERNEL);
> +	struct mfd_cell *cells __free(kfree) = kzalloc(sizeof(*cells), GFP_KERNEL);
> +	int irq = pci_irq_vector(hci->pci, 0);
> +	int ret;
> +
> +	if (!cells || !data)
> +		return -ENOMEM;
> +
> +	hci->dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> +	if (hci->dev_id < 0)
> +		return hci->dev_id;
> +
> +	mipi_i3c_hci_pci_setup_cell(hci, data, cells);
> +
> +	ret = mfd_add_devices(&hci->pci->dev, 0, cells, 1, NULL, irq, NULL);
> +	if (ret)
> +		goto err_free_ids;
> +
> +	return 0;
> +
> +err_free_ids:
> +	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
> +	return ret;
> +}
> +
>  static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  				  const struct pci_device_id *id)
>  {
> -	struct mipi_i3c_hci_platform_data pdata = {};
>  	struct mipi_i3c_hci_pci *hci;
> -	struct resource res;
> -	int dev_id, ret;
> +	int ret;
>
>  	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
>  	if (!hci)
> @@ -206,41 +253,13 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  	if (ret < 0)
>  		return ret;
>
> -	memset(&res, 0, sizeof(res));
> -
> -	res.flags = IORESOURCE_IRQ;
> -	res.start = pci_irq_vector(hci->pci, 0);
> -	res.end = res.start;
> -
> -	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> -	if (dev_id < 0)
> -		return dev_id;
> -
> -	hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
> -	if (!hci->pdev)
> -		return -ENOMEM;
> -
> -	hci->pdev->dev.parent = &pci->dev;
> -	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
> -
> -	ret = platform_device_add_resources(hci->pdev, &res, 1);
> -	if (ret)
> -		goto err;
> -
> -	pdata.base_regs = hci->base;
> +	hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
>
> -	ret = platform_device_add_data(hci->pdev, &pdata, sizeof(pdata));
> +	ret = hci->info->init ? hci->info->init(hci) : 0;
>  	if (ret)
> -		goto err;
> -
> -	hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
> -	if (hci->info->init) {
> -		ret = hci->info->init(hci);
> -		if (ret)
> -			goto err;
> -	}
> +		return ret;
>
> -	ret = platform_device_add(hci->pdev);
> +	ret = mipi_i3c_hci_pci_add_instances(hci);
>  	if (ret)
>  		goto err_exit;
>
> @@ -251,23 +270,18 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>  err_exit:
>  	if (hci->info->exit)
>  		hci->info->exit(hci);
> -err:
> -	platform_device_put(hci->pdev);
> -	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
>  	return ret;
>  }
>
>  static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
>  {
>  	struct mipi_i3c_hci_pci *hci = pci_get_drvdata(pci);
> -	struct platform_device *pdev = hci->pdev;
> -	int dev_id = pdev->id;
>
>  	if (hci->info->exit)
>  		hci->info->exit(hci);
>
> -	platform_device_unregister(pdev);
> -	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
> +	mfd_remove_devices(&pci->dev);
> +	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
>  }
>
>  static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V2 09/10] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances

[Adrian Hunter]

A MIPI I3C Host Controller with the Multi-Bus Instance capability supports
multiple I3C Buses (up to 15), with one instance of the HCI Register Set
and one instance of I3C Bus Controller Logic for each I3C Bus, in a single
hardware function (e.g. PCIe B/D/F).

Create an MFD cell for each instance.  Use platform_data to pass the
instance's register set start address.

MIPI I3C specification defines an Extended Capability to hold the offset
of each instance register set.  However parsing to find that information is
relatively complicated compared with just including it in the driver data.
Do that for now.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2:
	Conversion to MFD split into separate patch
	Simplify ID allocation / free
	Correct use of __free()
	Also define instance 0 in driver_data


 .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 63 +++++++++++++++----
 1 file changed, 50 insertions(+), 13 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index 68088967942b..de1f71763786 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -19,10 +19,17 @@
 #include <linux/platform_device.h>
 #include <linux/pm_qos.h>
 
+/*
+ * There can up to 15 instances, but implementations have at most 2 at this
+ * time.
+ */
+#define INST_MAX 2
+
 struct mipi_i3c_hci_pci {
 	struct pci_dev *pci;
 	void __iomem *base;
-	int dev_id;
+	int dev_id[INST_MAX];
+	int dev_id_cnt;
 	const struct mipi_i3c_hci_pci_info *info;
 	void *private;
 };
@@ -30,6 +37,8 @@ struct mipi_i3c_hci_pci {
 struct mipi_i3c_hci_pci_info {
 	int (*init)(struct mipi_i3c_hci_pci *hci);
 	void (*exit)(struct mipi_i3c_hci_pci *hci);
+	u32 instance_offset[INST_MAX];
+	int instance_count;
 };
 
 static DEFINE_IDA(mipi_i3c_hci_pci_ida);
@@ -177,53 +186,81 @@ static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci)
 static const struct mipi_i3c_hci_pci_info intel_info = {
 	.init = intel_i3c_init,
 	.exit = intel_i3c_exit,
+	.instance_offset = {0},
+	.instance_count = 1,
 };
 
+static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
+{
+	for (int i = 0; i < hci->dev_id_cnt; i++)
+		ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id[i]);
+}
+
+static int mipi_i3c_hci_pci_alloc_ids(struct mipi_i3c_hci_pci *hci, int nr)
+{
+	for (int i = 0; i < nr; i++) {
+		hci->dev_id[i] = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
+		if (hci->dev_id[i]  < 0)
+			goto err_free_ids;
+		hci->dev_id_cnt = i + 1;
+	}
+
+	return 0;
+
+err_free_ids:
+	mipi_i3c_hci_pci_free_ids(hci);
+	return -ENOMEM;
+}
+
 struct mipi_i3c_hci_pci_cell_data {
 	struct mipi_i3c_hci_platform_data pdata;
 	struct resource res;
 };
 
-static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci,
+static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci, int idx,
 					struct mipi_i3c_hci_pci_cell_data *data,
 					struct mfd_cell *cell)
 {
-	data->pdata.base_regs = hci->base;
+	data->pdata.base_regs = hci->base + hci->info->instance_offset[idx];
 
 	data->res = DEFINE_RES_IRQ(0);
 
 	cell->name = "mipi-i3c-hci";
-	cell->id = hci->dev_id;
+	cell->id = hci->dev_id[idx];
 	cell->platform_data = &data->pdata;
 	cell->pdata_size = sizeof(data->pdata);
 	cell->num_resources = 1;
 	cell->resources = &data->res;
 }
 
+#define mipi_i3c_hci_pci_alloc(x) kcalloc(hci->info->instance_count, sizeof(*(x)), GFP_KERNEL)
+
 static int mipi_i3c_hci_pci_add_instances(struct mipi_i3c_hci_pci *hci)
 {
-	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = kzalloc(sizeof(*data), GFP_KERNEL);
-	struct mfd_cell *cells __free(kfree) = kzalloc(sizeof(*cells), GFP_KERNEL);
+	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = mipi_i3c_hci_pci_alloc(data);
+	struct mfd_cell *cells __free(kfree) = mipi_i3c_hci_pci_alloc(cells);
 	int irq = pci_irq_vector(hci->pci, 0);
+	int nr = hci->info->instance_count;
 	int ret;
 
 	if (!cells || !data)
 		return -ENOMEM;
 
-	hci->dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
-	if (hci->dev_id < 0)
-		return hci->dev_id;
+	ret = mipi_i3c_hci_pci_alloc_ids(hci, nr);
+	if (ret)
+		return ret;
 
-	mipi_i3c_hci_pci_setup_cell(hci, data, cells);
+	for (int i = 0; i < nr; i++)
+		mipi_i3c_hci_pci_setup_cell(hci, i, data + i, cells + i);
 
-	ret = mfd_add_devices(&hci->pci->dev, 0, cells, 1, NULL, irq, NULL);
+	ret = mfd_add_devices(&hci->pci->dev, 0, cells, nr, NULL, irq, NULL);
 	if (ret)
 		goto err_free_ids;
 
 	return 0;
 
 err_free_ids:
-	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
+	mipi_i3c_hci_pci_free_ids(hci);
 	return ret;
 }
 
@@ -281,7 +318,7 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
 		hci->info->exit(hci);
 
 	mfd_remove_devices(&pci->dev);
-	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
+	mipi_i3c_hci_pci_free_ids(hci);
 }
 
 static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 09/10] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances

[Frank Li]

On Thu, Dec 11, 2025 at 03:48:08PM +0200, Adrian Hunter wrote:
> A MIPI I3C Host Controller with the Multi-Bus Instance capability supports
> multiple I3C Buses (up to 15).
with single hardware function (e.g. PCIe B/D/F).

...
> with one instance of the HCI Register Set
> and one instance of I3C Bus Controller Logic for each I3C Bus, in a single
> hardware function (e.g. PCIe B/D/F).

Each I3C bus have indepedent HCI register space and I3C Bus controller logic.

>
> Create an MFD cell for each instance.  Use platform_data to pass the
> instance's register set start address.
>
> MIPI I3C specification defines an Extended Capability to hold the offset
> of each instance register set.  However parsing to find that information is
> relatively complicated compared with just including it in the driver data.
> Do that for now.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>
>
> Changes in V2:
> 	Conversion to MFD split into separate patch
> 	Simplify ID allocation / free
> 	Correct use of __free()
> 	Also define instance 0 in driver_data
>
>
>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 63 +++++++++++++++----
>  1 file changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index 68088967942b..de1f71763786 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -19,10 +19,17 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_qos.h>
>
> +/*
> + * There can up to 15 instances, but implementations have at most 2 at this
> + * time.
> + */
> +#define INST_MAX 2
> +
>  struct mipi_i3c_hci_pci {
>  	struct pci_dev *pci;
>  	void __iomem *base;
> -	int dev_id;
> +	int dev_id[INST_MAX];
> +	int dev_id_cnt;
>  	const struct mipi_i3c_hci_pci_info *info;
>  	void *private;
>  };
> @@ -30,6 +37,8 @@ struct mipi_i3c_hci_pci {
>  struct mipi_i3c_hci_pci_info {
>  	int (*init)(struct mipi_i3c_hci_pci *hci);
>  	void (*exit)(struct mipi_i3c_hci_pci *hci);
> +	u32 instance_offset[INST_MAX];
> +	int instance_count;
>  };
>
>  static DEFINE_IDA(mipi_i3c_hci_pci_ida);
> @@ -177,53 +186,81 @@ static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci)
>  static const struct mipi_i3c_hci_pci_info intel_info = {
>  	.init = intel_i3c_init,
>  	.exit = intel_i3c_exit,
> +	.instance_offset = {0},
> +	.instance_count = 1,

just one instance? suppose at least 2, otherwise, not need this patch.

>  };
>
> +static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
> +{
> +	for (int i = 0; i < hci->dev_id_cnt; i++)
> +		ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id[i]);
> +}
> +
> +static int mipi_i3c_hci_pci_alloc_ids(struct mipi_i3c_hci_pci *hci, int nr)
> +{
> +	for (int i = 0; i < nr; i++) {
> +		hci->dev_id[i] = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> +		if (hci->dev_id[i]  < 0)
> +			goto err_free_ids;
> +		hci->dev_id_cnt = i + 1;
> +	}
> +
> +	return 0;
> +
> +err_free_ids:
> +	mipi_i3c_hci_pci_free_ids(hci);
> +	return -ENOMEM;

You have to handle ida error, actually, it makes not big difference with
mfd and add multi platform devices.

> +}
> +
>  struct mipi_i3c_hci_pci_cell_data {
>  	struct mipi_i3c_hci_platform_data pdata;
>  	struct resource res;
>  };
>
> -static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci,
> +static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci, int idx,
>  					struct mipi_i3c_hci_pci_cell_data *data,
>  					struct mfd_cell *cell)
>  {
> -	data->pdata.base_regs = hci->base;
> +	data->pdata.base_regs = hci->base + hci->info->instance_offset[idx];
>
>  	data->res = DEFINE_RES_IRQ(0);
>
>  	cell->name = "mipi-i3c-hci";
> -	cell->id = hci->dev_id;
> +	cell->id = hci->dev_id[idx];
>  	cell->platform_data = &data->pdata;
>  	cell->pdata_size = sizeof(data->pdata);
>  	cell->num_resources = 1;
>  	cell->resources = &data->res;
>  }
>
> +#define mipi_i3c_hci_pci_alloc(x) kcalloc(hci->info->instance_count, sizeof(*(x)), GFP_KERNEL)
> +

It is not good to hide hci in macro.

mipi_i3c_hci_pci_alloc(hci, x); or
mipi_i3c_hci_pci_alloc(nr, x)

>  static int mipi_i3c_hci_pci_add_instances(struct mipi_i3c_hci_pci *hci)
>  {
> -	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = kzalloc(sizeof(*data), GFP_KERNEL);
> -	struct mfd_cell *cells __free(kfree) = kzalloc(sizeof(*cells), GFP_KERNEL);
> +	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = mipi_i3c_hci_pci_alloc(data);
> +	struct mfd_cell *cells __free(kfree) = mipi_i3c_hci_pci_alloc(cells);
>  	int irq = pci_irq_vector(hci->pci, 0);
> +	int nr = hci->info->instance_count;
>  	int ret;
>
>  	if (!cells || !data)
>  		return -ENOMEM;
>
> -	hci->dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> -	if (hci->dev_id < 0)
> -		return hci->dev_id;
> +	ret = mipi_i3c_hci_pci_alloc_ids(hci, nr);
> +	if (ret)
> +		return ret;
>
> -	mipi_i3c_hci_pci_setup_cell(hci, data, cells);
> +	for (int i = 0; i < nr; i++)
> +		mipi_i3c_hci_pci_setup_cell(hci, i, data + i, cells + i);
>
> -	ret = mfd_add_devices(&hci->pci->dev, 0, cells, 1, NULL, irq, NULL);
> +	ret = mfd_add_devices(&hci->pci->dev, 0, cells, nr, NULL, irq, NULL);
>  	if (ret)
>  		goto err_free_ids;
>
>  	return 0;
>
>  err_free_ids:
> -	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
> +	mipi_i3c_hci_pci_free_ids(hci);
>  	return ret;
>  }
>
> @@ -281,7 +318,7 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
>  		hci->info->exit(hci);
>
>  	mfd_remove_devices(&pci->dev);
> -	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
> +	mipi_i3c_hci_pci_free_ids(hci);
>  }
>
>  static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 09/10] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances

[Adrian Hunter]

On 11/12/2025 18:44, Frank Li wrote:
> On Thu, Dec 11, 2025 at 03:48:08PM +0200, Adrian Hunter wrote:
>> A MIPI I3C Host Controller with the Multi-Bus Instance capability supports
>> multiple I3C Buses (up to 15).
> with single hardware function (e.g. PCIe B/D/F).
> 
> ...
>> with one instance of the HCI Register Set
>> and one instance of I3C Bus Controller Logic for each I3C Bus, in a single
>> hardware function (e.g. PCIe B/D/F).
> 
> Each I3C bus have indepedent HCI register space and I3C Bus controller logic.
> 
>>
>> Create an MFD cell for each instance.  Use platform_data to pass the
>> instance's register set start address.
>>
>> MIPI I3C specification defines an Extended Capability to hold the offset
>> of each instance register set.  However parsing to find that information is
>> relatively complicated compared with just including it in the driver data.
>> Do that for now.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>
>>
>> Changes in V2:
>> 	Conversion to MFD split into separate patch
>> 	Simplify ID allocation / free
>> 	Correct use of __free()
>> 	Also define instance 0 in driver_data
>>
>>
>>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 63 +++++++++++++++----
>>  1 file changed, 50 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> index 68088967942b..de1f71763786 100644
>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>> @@ -19,10 +19,17 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_qos.h>
>>
>> +/*
>> + * There can up to 15 instances, but implementations have at most 2 at this
>> + * time.
>> + */
>> +#define INST_MAX 2
>> +
>>  struct mipi_i3c_hci_pci {
>>  	struct pci_dev *pci;
>>  	void __iomem *base;
>> -	int dev_id;
>> +	int dev_id[INST_MAX];
>> +	int dev_id_cnt;
>>  	const struct mipi_i3c_hci_pci_info *info;
>>  	void *private;
>>  };
>> @@ -30,6 +37,8 @@ struct mipi_i3c_hci_pci {
>>  struct mipi_i3c_hci_pci_info {
>>  	int (*init)(struct mipi_i3c_hci_pci *hci);
>>  	void (*exit)(struct mipi_i3c_hci_pci *hci);
>> +	u32 instance_offset[INST_MAX];
>> +	int instance_count;
>>  };
>>
>>  static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>> @@ -177,53 +186,81 @@ static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci)
>>  static const struct mipi_i3c_hci_pci_info intel_info = {
>>  	.init = intel_i3c_init,
>>  	.exit = intel_i3c_exit,
>> +	.instance_offset = {0},
>> +	.instance_count = 1,
> 
> just one instance? suppose at least 2, otherwise, not need this patch.

Adding multiple instances is in a separate patch

> 
>>  };
>>
>> +static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
>> +{
>> +	for (int i = 0; i < hci->dev_id_cnt; i++)
>> +		ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id[i]);
>> +}
>> +
>> +static int mipi_i3c_hci_pci_alloc_ids(struct mipi_i3c_hci_pci *hci, int nr)
>> +{
>> +	for (int i = 0; i < nr; i++) {
>> +		hci->dev_id[i] = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>> +		if (hci->dev_id[i]  < 0)
>> +			goto err_free_ids;
>> +		hci->dev_id_cnt = i + 1;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_free_ids:
>> +	mipi_i3c_hci_pci_free_ids(hci);
>> +	return -ENOMEM;
> 
> You have to handle ida error, actually, it makes not big difference with
> mfd and add multi platform devices.

The ids have to be defined in any case.

MFD still handles:
	Setting the firmware node
	Deleting devices if 1 fails to create
	Removing all devices i.e. mfd_remove_devices()

> 
>> +}
>> +
>>  struct mipi_i3c_hci_pci_cell_data {
>>  	struct mipi_i3c_hci_platform_data pdata;
>>  	struct resource res;
>>  };
>>
>> -static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci,
>> +static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci, int idx,
>>  					struct mipi_i3c_hci_pci_cell_data *data,
>>  					struct mfd_cell *cell)
>>  {
>> -	data->pdata.base_regs = hci->base;
>> +	data->pdata.base_regs = hci->base + hci->info->instance_offset[idx];
>>
>>  	data->res = DEFINE_RES_IRQ(0);
>>
>>  	cell->name = "mipi-i3c-hci";
>> -	cell->id = hci->dev_id;
>> +	cell->id = hci->dev_id[idx];
>>  	cell->platform_data = &data->pdata;
>>  	cell->pdata_size = sizeof(data->pdata);
>>  	cell->num_resources = 1;
>>  	cell->resources = &data->res;
>>  }
>>
>> +#define mipi_i3c_hci_pci_alloc(x) kcalloc(hci->info->instance_count, sizeof(*(x)), GFP_KERNEL)
>> +
> 
> It is not good to hide hci in macro.
> 
> mipi_i3c_hci_pci_alloc(hci, x); or
> mipi_i3c_hci_pci_alloc(nr, x)
> 
>>  static int mipi_i3c_hci_pci_add_instances(struct mipi_i3c_hci_pci *hci)
>>  {
>> -	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = kzalloc(sizeof(*data), GFP_KERNEL);
>> -	struct mfd_cell *cells __free(kfree) = kzalloc(sizeof(*cells), GFP_KERNEL);
>> +	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = mipi_i3c_hci_pci_alloc(data);
>> +	struct mfd_cell *cells __free(kfree) = mipi_i3c_hci_pci_alloc(cells);
>>  	int irq = pci_irq_vector(hci->pci, 0);
>> +	int nr = hci->info->instance_count;
>>  	int ret;
>>
>>  	if (!cells || !data)
>>  		return -ENOMEM;
>>
>> -	hci->dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>> -	if (hci->dev_id < 0)
>> -		return hci->dev_id;
>> +	ret = mipi_i3c_hci_pci_alloc_ids(hci, nr);
>> +	if (ret)
>> +		return ret;
>>
>> -	mipi_i3c_hci_pci_setup_cell(hci, data, cells);
>> +	for (int i = 0; i < nr; i++)
>> +		mipi_i3c_hci_pci_setup_cell(hci, i, data + i, cells + i);
>>
>> -	ret = mfd_add_devices(&hci->pci->dev, 0, cells, 1, NULL, irq, NULL);
>> +	ret = mfd_add_devices(&hci->pci->dev, 0, cells, nr, NULL, irq, NULL);
>>  	if (ret)
>>  		goto err_free_ids;
>>
>>  	return 0;
>>
>>  err_free_ids:
>> -	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
>> +	mipi_i3c_hci_pci_free_ids(hci);
>>  	return ret;
>>  }
>>
>> @@ -281,7 +318,7 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
>>  		hci->info->exit(hci);
>>
>>  	mfd_remove_devices(&pci->dev);
>> -	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
>> +	mipi_i3c_hci_pci_free_ids(hci);
>>  }
>>
>>  static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
>> --
>> 2.51.0
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 09/10] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances

[Frank Li]

On Fri, Dec 12, 2025 at 04:08:24PM +0200, Adrian Hunter wrote:
> On 11/12/2025 18:44, Frank Li wrote:
> > On Thu, Dec 11, 2025 at 03:48:08PM +0200, Adrian Hunter wrote:
> >> A MIPI I3C Host Controller with the Multi-Bus Instance capability supports
> >> multiple I3C Buses (up to 15).
> > with single hardware function (e.g. PCIe B/D/F).
> >
> > ...
> >> with one instance of the HCI Register Set
> >> and one instance of I3C Bus Controller Logic for each I3C Bus, in a single
> >> hardware function (e.g. PCIe B/D/F).
> >
> > Each I3C bus have indepedent HCI register space and I3C Bus controller logic.
> >
> >>
> >> Create an MFD cell for each instance.  Use platform_data to pass the
> >> instance's register set start address.
> >>
> >> MIPI I3C specification defines an Extended Capability to hold the offset
> >> of each instance register set.  However parsing to find that information is
> >> relatively complicated compared with just including it in the driver data.
> >> Do that for now.
> >>
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >> ---
> >>
> >>
> >> Changes in V2:
> >> 	Conversion to MFD split into separate patch
> >> 	Simplify ID allocation / free
> >> 	Correct use of __free()
> >> 	Also define instance 0 in driver_data
> >>
> >>
> >>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 63 +++++++++++++++----
> >>  1 file changed, 50 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> index 68088967942b..de1f71763786 100644
> >> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >> @@ -19,10 +19,17 @@
> >>  #include <linux/platform_device.h>
> >>  #include <linux/pm_qos.h>
> >>
> >> +/*
> >> + * There can up to 15 instances, but implementations have at most 2 at this
> >> + * time.
> >> + */
> >> +#define INST_MAX 2
> >> +
> >>  struct mipi_i3c_hci_pci {
> >>  	struct pci_dev *pci;
> >>  	void __iomem *base;
> >> -	int dev_id;
> >> +	int dev_id[INST_MAX];
> >> +	int dev_id_cnt;
> >>  	const struct mipi_i3c_hci_pci_info *info;
> >>  	void *private;
> >>  };
> >> @@ -30,6 +37,8 @@ struct mipi_i3c_hci_pci {
> >>  struct mipi_i3c_hci_pci_info {
> >>  	int (*init)(struct mipi_i3c_hci_pci *hci);
> >>  	void (*exit)(struct mipi_i3c_hci_pci *hci);
> >> +	u32 instance_offset[INST_MAX];
> >> +	int instance_count;
> >>  };
> >>
> >>  static DEFINE_IDA(mipi_i3c_hci_pci_ida);
> >> @@ -177,53 +186,81 @@ static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci)
> >>  static const struct mipi_i3c_hci_pci_info intel_info = {
> >>  	.init = intel_i3c_init,
> >>  	.exit = intel_i3c_exit,
> >> +	.instance_offset = {0},
> >> +	.instance_count = 1,
> >
> > just one instance? suppose at least 2, otherwise, not need this patch.
>
> Adding multiple instances is in a separate patch

Okay, commit message should mention it.

>
> >
> >>  };
> >>
> >> +static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
> >> +{
> >> +	for (int i = 0; i < hci->dev_id_cnt; i++)
> >> +		ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id[i]);
> >> +}
> >> +
> >> +static int mipi_i3c_hci_pci_alloc_ids(struct mipi_i3c_hci_pci *hci, int nr)
> >> +{
> >> +	for (int i = 0; i < nr; i++) {
> >> +		hci->dev_id[i] = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> >> +		if (hci->dev_id[i]  < 0)
> >> +			goto err_free_ids;
> >> +		hci->dev_id_cnt = i + 1;
> >> +	}
> >> +
> >> +	return 0;
> >> +
> >> +err_free_ids:
> >> +	mipi_i3c_hci_pci_free_ids(hci);
> >> +	return -ENOMEM;
> >
> > You have to handle ida error, actually, it makes not big difference with
> > mfd and add multi platform devices.
>
> The ids have to be defined in any case.
>
> MFD still handles:
> 	Setting the firmware node
> 	Deleting devices if 1 fails to create
> 	Removing all devices i.e. mfd_remove_devices()

Actaully, why need remove previous failure device. assume support 15 device.

7 success.  failure at 8 instance.

I think it'd better keep 7 already success devices.  So 1-7 can work.
8-15 can't work.

It should be better than total 15 instances doesn't.

Frank


>
> >
> >> +}
> >> +
> >>  struct mipi_i3c_hci_pci_cell_data {
> >>  	struct mipi_i3c_hci_platform_data pdata;
> >>  	struct resource res;
> >>  };
> >>
> >> -static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci,
> >> +static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci, int idx,
> >>  					struct mipi_i3c_hci_pci_cell_data *data,
> >>  					struct mfd_cell *cell)
> >>  {
> >> -	data->pdata.base_regs = hci->base;
> >> +	data->pdata.base_regs = hci->base + hci->info->instance_offset[idx];
> >>
> >>  	data->res = DEFINE_RES_IRQ(0);
> >>
> >>  	cell->name = "mipi-i3c-hci";
> >> -	cell->id = hci->dev_id;
> >> +	cell->id = hci->dev_id[idx];
> >>  	cell->platform_data = &data->pdata;
> >>  	cell->pdata_size = sizeof(data->pdata);
> >>  	cell->num_resources = 1;
> >>  	cell->resources = &data->res;
> >>  }
> >>
> >> +#define mipi_i3c_hci_pci_alloc(x) kcalloc(hci->info->instance_count, sizeof(*(x)), GFP_KERNEL)
> >> +
> >
> > It is not good to hide hci in macro.
> >
> > mipi_i3c_hci_pci_alloc(hci, x); or
> > mipi_i3c_hci_pci_alloc(nr, x)
> >
> >>  static int mipi_i3c_hci_pci_add_instances(struct mipi_i3c_hci_pci *hci)
> >>  {
> >> -	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = kzalloc(sizeof(*data), GFP_KERNEL);
> >> -	struct mfd_cell *cells __free(kfree) = kzalloc(sizeof(*cells), GFP_KERNEL);
> >> +	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = mipi_i3c_hci_pci_alloc(data);
> >> +	struct mfd_cell *cells __free(kfree) = mipi_i3c_hci_pci_alloc(cells);
> >>  	int irq = pci_irq_vector(hci->pci, 0);
> >> +	int nr = hci->info->instance_count;
> >>  	int ret;
> >>
> >>  	if (!cells || !data)
> >>  		return -ENOMEM;
> >>
> >> -	hci->dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> >> -	if (hci->dev_id < 0)
> >> -		return hci->dev_id;
> >> +	ret = mipi_i3c_hci_pci_alloc_ids(hci, nr);
> >> +	if (ret)
> >> +		return ret;
> >>
> >> -	mipi_i3c_hci_pci_setup_cell(hci, data, cells);
> >> +	for (int i = 0; i < nr; i++)
> >> +		mipi_i3c_hci_pci_setup_cell(hci, i, data + i, cells + i);
> >>
> >> -	ret = mfd_add_devices(&hci->pci->dev, 0, cells, 1, NULL, irq, NULL);
> >> +	ret = mfd_add_devices(&hci->pci->dev, 0, cells, nr, NULL, irq, NULL);
> >>  	if (ret)
> >>  		goto err_free_ids;
> >>
> >>  	return 0;
> >>
> >>  err_free_ids:
> >> -	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
> >> +	mipi_i3c_hci_pci_free_ids(hci);
> >>  	return ret;
> >>  }
> >>
> >> @@ -281,7 +318,7 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
> >>  		hci->info->exit(hci);
> >>
> >>  	mfd_remove_devices(&pci->dev);
> >> -	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
> >> +	mipi_i3c_hci_pci_free_ids(hci);
> >>  }
> >>
> >>  static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
> >> --
> >> 2.51.0
> >>
> >>
> >> --
> >> linux-i3c mailing list
> >> linux-i3c@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 09/10] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances

[Adrian Hunter]

On 12/12/2025 19:46, Frank Li wrote:
> On Fri, Dec 12, 2025 at 04:08:24PM +0200, Adrian Hunter wrote:
>> On 11/12/2025 18:44, Frank Li wrote:
>>> On Thu, Dec 11, 2025 at 03:48:08PM +0200, Adrian Hunter wrote:
>>>> A MIPI I3C Host Controller with the Multi-Bus Instance capability supports
>>>> multiple I3C Buses (up to 15).
>>> with single hardware function (e.g. PCIe B/D/F).
>>>
>>> ...
>>>> with one instance of the HCI Register Set
>>>> and one instance of I3C Bus Controller Logic for each I3C Bus, in a single
>>>> hardware function (e.g. PCIe B/D/F).
>>>
>>> Each I3C bus have indepedent HCI register space and I3C Bus controller logic.
>>>
>>>>
>>>> Create an MFD cell for each instance.  Use platform_data to pass the
>>>> instance's register set start address.
>>>>
>>>> MIPI I3C specification defines an Extended Capability to hold the offset
>>>> of each instance register set.  However parsing to find that information is
>>>> relatively complicated compared with just including it in the driver data.
>>>> Do that for now.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>> ---
>>>>
>>>>
>>>> Changes in V2:
>>>> 	Conversion to MFD split into separate patch
>>>> 	Simplify ID allocation / free
>>>> 	Correct use of __free()
>>>> 	Also define instance 0 in driver_data
>>>>
>>>>
>>>>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 63 +++++++++++++++----
>>>>  1 file changed, 50 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> index 68088967942b..de1f71763786 100644
>>>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
>>>> @@ -19,10 +19,17 @@
>>>>  #include <linux/platform_device.h>
>>>>  #include <linux/pm_qos.h>
>>>>
>>>> +/*
>>>> + * There can up to 15 instances, but implementations have at most 2 at this
>>>> + * time.
>>>> + */
>>>> +#define INST_MAX 2
>>>> +
>>>>  struct mipi_i3c_hci_pci {
>>>>  	struct pci_dev *pci;
>>>>  	void __iomem *base;
>>>> -	int dev_id;
>>>> +	int dev_id[INST_MAX];
>>>> +	int dev_id_cnt;
>>>>  	const struct mipi_i3c_hci_pci_info *info;
>>>>  	void *private;
>>>>  };
>>>> @@ -30,6 +37,8 @@ struct mipi_i3c_hci_pci {
>>>>  struct mipi_i3c_hci_pci_info {
>>>>  	int (*init)(struct mipi_i3c_hci_pci *hci);
>>>>  	void (*exit)(struct mipi_i3c_hci_pci *hci);
>>>> +	u32 instance_offset[INST_MAX];
>>>> +	int instance_count;
>>>>  };
>>>>
>>>>  static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>>>> @@ -177,53 +186,81 @@ static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci)
>>>>  static const struct mipi_i3c_hci_pci_info intel_info = {
>>>>  	.init = intel_i3c_init,
>>>>  	.exit = intel_i3c_exit,
>>>> +	.instance_offset = {0},
>>>> +	.instance_count = 1,
>>>
>>> just one instance? suppose at least 2, otherwise, not need this patch.
>>
>> Adding multiple instances is in a separate patch
> 
> Okay, commit message should mention it.
> 
>>
>>>
>>>>  };
>>>>
>>>> +static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
>>>> +{
>>>> +	for (int i = 0; i < hci->dev_id_cnt; i++)
>>>> +		ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id[i]);
>>>> +}
>>>> +
>>>> +static int mipi_i3c_hci_pci_alloc_ids(struct mipi_i3c_hci_pci *hci, int nr)
>>>> +{
>>>> +	for (int i = 0; i < nr; i++) {
>>>> +		hci->dev_id[i] = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>>>> +		if (hci->dev_id[i]  < 0)
>>>> +			goto err_free_ids;
>>>> +		hci->dev_id_cnt = i + 1;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +
>>>> +err_free_ids:
>>>> +	mipi_i3c_hci_pci_free_ids(hci);
>>>> +	return -ENOMEM;
>>>
>>> You have to handle ida error, actually, it makes not big difference with
>>> mfd and add multi platform devices.
>>
>> The ids have to be defined in any case.
>>
>> MFD still handles:
>> 	Setting the firmware node
>> 	Deleting devices if 1 fails to create
>> 	Removing all devices i.e. mfd_remove_devices()
> 
> Actaully, why need remove previous failure device. assume support 15 device.

Keeps things tidy.

> 
> 7 success.  failure at 8 instance.
> 
> I think it'd better keep 7 already success devices.  So 1-7 can work.
> 8-15 can't work.

If adding devices fails, the system is out of memory
or in some catastrophic state, so there is no need to try to
cater for that case.

> 
> It should be better than total 15 instances doesn't.

Not necessarily.  If whole probe fails, the user might try again
and succeed.

> 
> Frank
> 
> 
>>
>>>
>>>> +}
>>>> +
>>>>  struct mipi_i3c_hci_pci_cell_data {
>>>>  	struct mipi_i3c_hci_platform_data pdata;
>>>>  	struct resource res;
>>>>  };
>>>>
>>>> -static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci,
>>>> +static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci, int idx,
>>>>  					struct mipi_i3c_hci_pci_cell_data *data,
>>>>  					struct mfd_cell *cell)
>>>>  {
>>>> -	data->pdata.base_regs = hci->base;
>>>> +	data->pdata.base_regs = hci->base + hci->info->instance_offset[idx];
>>>>
>>>>  	data->res = DEFINE_RES_IRQ(0);
>>>>
>>>>  	cell->name = "mipi-i3c-hci";
>>>> -	cell->id = hci->dev_id;
>>>> +	cell->id = hci->dev_id[idx];
>>>>  	cell->platform_data = &data->pdata;
>>>>  	cell->pdata_size = sizeof(data->pdata);
>>>>  	cell->num_resources = 1;
>>>>  	cell->resources = &data->res;
>>>>  }
>>>>
>>>> +#define mipi_i3c_hci_pci_alloc(x) kcalloc(hci->info->instance_count, sizeof(*(x)), GFP_KERNEL)
>>>> +
>>>
>>> It is not good to hide hci in macro.
>>>
>>> mipi_i3c_hci_pci_alloc(hci, x); or
>>> mipi_i3c_hci_pci_alloc(nr, x)
>>>
>>>>  static int mipi_i3c_hci_pci_add_instances(struct mipi_i3c_hci_pci *hci)
>>>>  {
>>>> -	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = kzalloc(sizeof(*data), GFP_KERNEL);
>>>> -	struct mfd_cell *cells __free(kfree) = kzalloc(sizeof(*cells), GFP_KERNEL);
>>>> +	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = mipi_i3c_hci_pci_alloc(data);
>>>> +	struct mfd_cell *cells __free(kfree) = mipi_i3c_hci_pci_alloc(cells);
>>>>  	int irq = pci_irq_vector(hci->pci, 0);
>>>> +	int nr = hci->info->instance_count;
>>>>  	int ret;
>>>>
>>>>  	if (!cells || !data)
>>>>  		return -ENOMEM;
>>>>
>>>> -	hci->dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>>>> -	if (hci->dev_id < 0)
>>>> -		return hci->dev_id;
>>>> +	ret = mipi_i3c_hci_pci_alloc_ids(hci, nr);
>>>> +	if (ret)
>>>> +		return ret;
>>>>
>>>> -	mipi_i3c_hci_pci_setup_cell(hci, data, cells);
>>>> +	for (int i = 0; i < nr; i++)
>>>> +		mipi_i3c_hci_pci_setup_cell(hci, i, data + i, cells + i);
>>>>
>>>> -	ret = mfd_add_devices(&hci->pci->dev, 0, cells, 1, NULL, irq, NULL);
>>>> +	ret = mfd_add_devices(&hci->pci->dev, 0, cells, nr, NULL, irq, NULL);
>>>>  	if (ret)
>>>>  		goto err_free_ids;
>>>>
>>>>  	return 0;
>>>>
>>>>  err_free_ids:
>>>> -	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
>>>> +	mipi_i3c_hci_pci_free_ids(hci);
>>>>  	return ret;
>>>>  }
>>>>
>>>> @@ -281,7 +318,7 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
>>>>  		hci->info->exit(hci);
>>>>
>>>>  	mfd_remove_devices(&pci->dev);
>>>> -	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
>>>> +	mipi_i3c_hci_pci_free_ids(hci);
>>>>  }
>>>>
>>>>  static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
>>>> --
>>>> 2.51.0
>>>>
>>>>
>>>> --
>>>> linux-i3c mailing list
>>>> linux-i3c@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 09/10] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances

[Frank Li]

On Mon, Dec 15, 2025 at 07:37:39PM +0200, Adrian Hunter wrote:
> On 12/12/2025 19:46, Frank Li wrote:
> > On Fri, Dec 12, 2025 at 04:08:24PM +0200, Adrian Hunter wrote:
> >> On 11/12/2025 18:44, Frank Li wrote:
> >>> On Thu, Dec 11, 2025 at 03:48:08PM +0200, Adrian Hunter wrote:
> >>>> A MIPI I3C Host Controller with the Multi-Bus Instance capability supports
> >>>> multiple I3C Buses (up to 15).
> >>> with single hardware function (e.g. PCIe B/D/F).
> >>>
> >>> ...
> >>>> with one instance of the HCI Register Set
> >>>> and one instance of I3C Bus Controller Logic for each I3C Bus, in a single
> >>>> hardware function (e.g. PCIe B/D/F).
> >>>
> >>> Each I3C bus have indepedent HCI register space and I3C Bus controller logic.
> >>>
> >>>>
> >>>> Create an MFD cell for each instance.  Use platform_data to pass the
> >>>> instance's register set start address.
> >>>>
> >>>> MIPI I3C specification defines an Extended Capability to hold the offset
> >>>> of each instance register set.  However parsing to find that information is
> >>>> relatively complicated compared with just including it in the driver data.
> >>>> Do that for now.
> >>>>
> >>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >>>> ---
> >>>>
> >>>>
> >>>> Changes in V2:
> >>>> 	Conversion to MFD split into separate patch
> >>>> 	Simplify ID allocation / free
> >>>> 	Correct use of __free()
> >>>> 	Also define instance 0 in driver_data
> >>>>
> >>>>
> >>>>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 63 +++++++++++++++----
> >>>>  1 file changed, 50 insertions(+), 13 deletions(-)
> >>>>
> >>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> index 68088967942b..de1f71763786 100644
> >>>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>> @@ -19,10 +19,17 @@
> >>>>  #include <linux/platform_device.h>
> >>>>  #include <linux/pm_qos.h>
> >>>>
> >>>> +/*
> >>>> + * There can up to 15 instances, but implementations have at most 2 at this
> >>>> + * time.
> >>>> + */
> >>>> +#define INST_MAX 2
> >>>> +
> >>>>  struct mipi_i3c_hci_pci {
> >>>>  	struct pci_dev *pci;
> >>>>  	void __iomem *base;
> >>>> -	int dev_id;
> >>>> +	int dev_id[INST_MAX];
> >>>> +	int dev_id_cnt;
> >>>>  	const struct mipi_i3c_hci_pci_info *info;
> >>>>  	void *private;
> >>>>  };
> >>>> @@ -30,6 +37,8 @@ struct mipi_i3c_hci_pci {
> >>>>  struct mipi_i3c_hci_pci_info {
> >>>>  	int (*init)(struct mipi_i3c_hci_pci *hci);
> >>>>  	void (*exit)(struct mipi_i3c_hci_pci *hci);
> >>>> +	u32 instance_offset[INST_MAX];
> >>>> +	int instance_count;
> >>>>  };
> >>>>
> >>>>  static DEFINE_IDA(mipi_i3c_hci_pci_ida);
> >>>> @@ -177,53 +186,81 @@ static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci)
> >>>>  static const struct mipi_i3c_hci_pci_info intel_info = {
> >>>>  	.init = intel_i3c_init,
> >>>>  	.exit = intel_i3c_exit,
> >>>> +	.instance_offset = {0},
> >>>> +	.instance_count = 1,
> >>>
> >>> just one instance? suppose at least 2, otherwise, not need this patch.
> >>
> >> Adding multiple instances is in a separate patch
> >
> > Okay, commit message should mention it.
> >
> >>
> >>>
> >>>>  };
> >>>>
> >>>> +static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
> >>>> +{
> >>>> +	for (int i = 0; i < hci->dev_id_cnt; i++)
> >>>> +		ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id[i]);
> >>>> +}
> >>>> +
> >>>> +static int mipi_i3c_hci_pci_alloc_ids(struct mipi_i3c_hci_pci *hci, int nr)
> >>>> +{
> >>>> +	for (int i = 0; i < nr; i++) {
> >>>> +		hci->dev_id[i] = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> >>>> +		if (hci->dev_id[i]  < 0)
> >>>> +			goto err_free_ids;
> >>>> +		hci->dev_id_cnt = i + 1;
> >>>> +	}
> >>>> +
> >>>> +	return 0;
> >>>> +
> >>>> +err_free_ids:
> >>>> +	mipi_i3c_hci_pci_free_ids(hci);
> >>>> +	return -ENOMEM;
> >>>
> >>> You have to handle ida error, actually, it makes not big difference with
> >>> mfd and add multi platform devices.
> >>
> >> The ids have to be defined in any case.
> >>
> >> MFD still handles:
> >> 	Setting the firmware node
> >> 	Deleting devices if 1 fails to create
> >> 	Removing all devices i.e. mfd_remove_devices()
> >
> > Actaully, why need remove previous failure device. assume support 15 device.
>
> Keeps things tidy.
>
> >
> > 7 success.  failure at 8 instance.
> >
> > I think it'd better keep 7 already success devices.  So 1-7 can work.
> > 8-15 can't work.
>
> If adding devices fails, the system is out of memory
> or in some catastrophic state, so there is no need to try to
> cater for that case.

Okay, I remembered system use defer probe to resolve dependence problem.
It is most likely all success if dependent device probed.

Go ahead, sorry for annoise.

Frnak
>
> >
> > It should be better than total 15 instances doesn't.
>
> Not necessarily.  If whole probe fails, the user might try again
> and succeed.
>
> >
> > Frank
> >
> >
> >>
> >>>
> >>>> +}
> >>>> +
> >>>>  struct mipi_i3c_hci_pci_cell_data {
> >>>>  	struct mipi_i3c_hci_platform_data pdata;
> >>>>  	struct resource res;
> >>>>  };
> >>>>
> >>>> -static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci,
> >>>> +static void mipi_i3c_hci_pci_setup_cell(struct mipi_i3c_hci_pci *hci, int idx,
> >>>>  					struct mipi_i3c_hci_pci_cell_data *data,
> >>>>  					struct mfd_cell *cell)
> >>>>  {
> >>>> -	data->pdata.base_regs = hci->base;
> >>>> +	data->pdata.base_regs = hci->base + hci->info->instance_offset[idx];
> >>>>
> >>>>  	data->res = DEFINE_RES_IRQ(0);
> >>>>
> >>>>  	cell->name = "mipi-i3c-hci";
> >>>> -	cell->id = hci->dev_id;
> >>>> +	cell->id = hci->dev_id[idx];
> >>>>  	cell->platform_data = &data->pdata;
> >>>>  	cell->pdata_size = sizeof(data->pdata);
> >>>>  	cell->num_resources = 1;
> >>>>  	cell->resources = &data->res;
> >>>>  }
> >>>>
> >>>> +#define mipi_i3c_hci_pci_alloc(x) kcalloc(hci->info->instance_count, sizeof(*(x)), GFP_KERNEL)
> >>>> +
> >>>
> >>> It is not good to hide hci in macro.
> >>>
> >>> mipi_i3c_hci_pci_alloc(hci, x); or
> >>> mipi_i3c_hci_pci_alloc(nr, x)
> >>>
> >>>>  static int mipi_i3c_hci_pci_add_instances(struct mipi_i3c_hci_pci *hci)
> >>>>  {
> >>>> -	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = kzalloc(sizeof(*data), GFP_KERNEL);
> >>>> -	struct mfd_cell *cells __free(kfree) = kzalloc(sizeof(*cells), GFP_KERNEL);
> >>>> +	struct mipi_i3c_hci_pci_cell_data *data __free(kfree) = mipi_i3c_hci_pci_alloc(data);
> >>>> +	struct mfd_cell *cells __free(kfree) = mipi_i3c_hci_pci_alloc(cells);
> >>>>  	int irq = pci_irq_vector(hci->pci, 0);
> >>>> +	int nr = hci->info->instance_count;
> >>>>  	int ret;
> >>>>
> >>>>  	if (!cells || !data)
> >>>>  		return -ENOMEM;
> >>>>
> >>>> -	hci->dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> >>>> -	if (hci->dev_id < 0)
> >>>> -		return hci->dev_id;
> >>>> +	ret = mipi_i3c_hci_pci_alloc_ids(hci, nr);
> >>>> +	if (ret)
> >>>> +		return ret;
> >>>>
> >>>> -	mipi_i3c_hci_pci_setup_cell(hci, data, cells);
> >>>> +	for (int i = 0; i < nr; i++)
> >>>> +		mipi_i3c_hci_pci_setup_cell(hci, i, data + i, cells + i);
> >>>>
> >>>> -	ret = mfd_add_devices(&hci->pci->dev, 0, cells, 1, NULL, irq, NULL);
> >>>> +	ret = mfd_add_devices(&hci->pci->dev, 0, cells, nr, NULL, irq, NULL);
> >>>>  	if (ret)
> >>>>  		goto err_free_ids;
> >>>>
> >>>>  	return 0;
> >>>>
> >>>>  err_free_ids:
> >>>> -	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
> >>>> +	mipi_i3c_hci_pci_free_ids(hci);
> >>>>  	return ret;
> >>>>  }
> >>>>
> >>>> @@ -281,7 +318,7 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
> >>>>  		hci->info->exit(hci);
> >>>>
> >>>>  	mfd_remove_devices(&pci->dev);
> >>>> -	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
> >>>> +	mipi_i3c_hci_pci_free_ids(hci);
> >>>>  }
> >>>>
> >>>>  static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
> >>>> --
> >>>> 2.51.0
> >>>>
> >>>>
> >>>> --
> >>>> linux-i3c mailing list
> >>>> linux-i3c@lists.infradead.org
> >>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
> >>
> >>
> >> --
> >> linux-i3c mailing list
> >> linux-i3c@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH V2 10/10] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers

[Adrian Hunter]

Define Multi-Bus Instances at offset 0x400 for some Intel controllers.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2:
	Also define instance 0 in driver_data


 .../i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c  | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index de1f71763786..9937718dc069 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -190,6 +190,13 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
 	.instance_count = 1,
 };
 
+static const struct mipi_i3c_hci_pci_info intel_mi_info = {
+	.init = intel_i3c_init,
+	.exit = intel_i3c_exit,
+	.instance_offset = {0, 0x400},
+	.instance_count = 2,
+};
+
 static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
 {
 	for (int i = 0; i < hci->dev_id_cnt; i++)
@@ -323,17 +330,17 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
 
 static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
 	/* Wildcat Lake-U */
-	{ PCI_VDEVICE(INTEL, 0x4d7c), (kernel_ulong_t)&intel_info},
+	{ PCI_VDEVICE(INTEL, 0x4d7c), (kernel_ulong_t)&intel_mi_info},
 	{ PCI_VDEVICE(INTEL, 0x4d6f), (kernel_ulong_t)&intel_info},
 	/* Panther Lake-H */
-	{ PCI_VDEVICE(INTEL, 0xe37c), (kernel_ulong_t)&intel_info},
+	{ PCI_VDEVICE(INTEL, 0xe37c), (kernel_ulong_t)&intel_mi_info},
 	{ PCI_VDEVICE(INTEL, 0xe36f), (kernel_ulong_t)&intel_info},
 	/* Panther Lake-P */
-	{ PCI_VDEVICE(INTEL, 0xe47c), (kernel_ulong_t)&intel_info},
+	{ PCI_VDEVICE(INTEL, 0xe47c), (kernel_ulong_t)&intel_mi_info},
 	{ PCI_VDEVICE(INTEL, 0xe46f), (kernel_ulong_t)&intel_info},
 	/* Nova Lake-S */
-	{ PCI_VDEVICE(INTEL, 0x6e2c), (kernel_ulong_t)&intel_info},
-	{ PCI_VDEVICE(INTEL, 0x6e2d), (kernel_ulong_t)&intel_info},
+	{ PCI_VDEVICE(INTEL, 0x6e2c), (kernel_ulong_t)&intel_mi_info},
+	{ PCI_VDEVICE(INTEL, 0x6e2d), (kernel_ulong_t)&intel_mi_info},
 	{ },
 };
 MODULE_DEVICE_TABLE(pci, mipi_i3c_hci_pci_devices);
-- 
2.51.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH V2 10/10] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers

[Frank Li]

On Thu, Dec 11, 2025 at 03:48:09PM +0200, Adrian Hunter wrote:
> Define Multi-Bus Instances at offset 0x400 for some Intel controllers.
>

subject: Define Multi-Bus instances for supported controllers.

> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>
>
> Changes in V2:
> 	Also define instance 0 in driver_data
>
>
>  .../i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c  | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index de1f71763786..9937718dc069 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -190,6 +190,13 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
>  	.instance_count = 1,
>  };
>
> +static const struct mipi_i3c_hci_pci_info intel_mi_info = {
> +	.init = intel_i3c_init,
> +	.exit = intel_i3c_exit,
> +	.instance_offset = {0, 0x400},
> +	.instance_count = 2,
> +};
> +
>  static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
>  {
>  	for (int i = 0; i < hci->dev_id_cnt; i++)
> @@ -323,17 +330,17 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
>
>  static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
>  	/* Wildcat Lake-U */
> -	{ PCI_VDEVICE(INTEL, 0x4d7c), (kernel_ulong_t)&intel_info},
> +	{ PCI_VDEVICE(INTEL, 0x4d7c), (kernel_ulong_t)&intel_mi_info},
>  	{ PCI_VDEVICE(INTEL, 0x4d6f), (kernel_ulong_t)&intel_info},
>  	/* Panther Lake-H */
> -	{ PCI_VDEVICE(INTEL, 0xe37c), (kernel_ulong_t)&intel_info},
> +	{ PCI_VDEVICE(INTEL, 0xe37c), (kernel_ulong_t)&intel_mi_info},
>  	{ PCI_VDEVICE(INTEL, 0xe36f), (kernel_ulong_t)&intel_info},
>  	/* Panther Lake-P */
> -	{ PCI_VDEVICE(INTEL, 0xe47c), (kernel_ulong_t)&intel_info},
> +	{ PCI_VDEVICE(INTEL, 0xe47c), (kernel_ulong_t)&intel_mi_info},
>  	{ PCI_VDEVICE(INTEL, 0xe46f), (kernel_ulong_t)&intel_info},
>  	/* Nova Lake-S */
> -	{ PCI_VDEVICE(INTEL, 0x6e2c), (kernel_ulong_t)&intel_info},
> -	{ PCI_VDEVICE(INTEL, 0x6e2d), (kernel_ulong_t)&intel_info},
> +	{ PCI_VDEVICE(INTEL, 0x6e2c), (kernel_ulong_t)&intel_mi_info},
> +	{ PCI_VDEVICE(INTEL, 0x6e2d), (kernel_ulong_t)&intel_mi_info},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(pci, mipi_i3c_hci_pci_devices);
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c