* [PATCH 0/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers
@ 2025-12-09 11:50 Adrian Hunter
2025-12-09 11:50 ` [PATCH 1/7] i3c: mipi-i3c-hci: Remove duplicate blank lines Adrian Hunter
` (6 more replies)
0 siblings, 7 replies; 18+ messages in thread
From: Adrian Hunter @ 2025-12-09 11:50 UTC (permalink / raw)
To: alexandre.belloni; +Cc: Frank.Li, linux-i3c
Hi
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 (7):
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: Define default driver data
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 | 204 +++++++++++++++------
drivers/i3c/master/mipi-i3c-hci/pio.c | 9 +-
include/linux/platform_data/mipi-i3c-hci.h | 15 ++
12 files changed, 215 insertions(+), 114 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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/7] i3c: mipi-i3c-hci: Remove duplicate blank lines
2025-12-09 11:50 [PATCH 0/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers Adrian Hunter
@ 2025-12-09 11:50 ` Adrian Hunter
2025-12-09 16:21 ` Frank Li
2025-12-09 11:50 ` [PATCH 2/7] i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0 Adrian Hunter
` (5 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Adrian Hunter @ 2025-12-09 11:50 UTC (permalink / raw)
To: alexandre.belloni; +Cc: Frank.Li, linux-i3c
Remove duplicate blank lines from mipi-i3c-hci code.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/7] i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0
2025-12-09 11:50 [PATCH 0/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers Adrian Hunter
2025-12-09 11:50 ` [PATCH 1/7] i3c: mipi-i3c-hci: Remove duplicate blank lines Adrian Hunter
@ 2025-12-09 11:50 ` Adrian Hunter
2025-12-09 16:22 ` Frank Li
2025-12-09 11:51 ` [PATCH 3/7] i3c: mipi-i3c-hci: Quieten initialization messages Adrian Hunter
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Adrian Hunter @ 2025-12-09 11:50 UTC (permalink / raw)
To: alexandre.belloni; +Cc: Frank.Li, linux-i3c
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>
---
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/7] i3c: mipi-i3c-hci: Quieten initialization messages
2025-12-09 11:50 [PATCH 0/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers Adrian Hunter
2025-12-09 11:50 ` [PATCH 1/7] i3c: mipi-i3c-hci: Remove duplicate blank lines Adrian Hunter
2025-12-09 11:50 ` [PATCH 2/7] i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0 Adrian Hunter
@ 2025-12-09 11:51 ` Adrian Hunter
2025-12-09 16:25 ` Frank Li
2025-12-09 11:51 ` [PATCH 4/7] i3c: mipi-i3c-hci: Allow for Multi-Bus Instances Adrian Hunter
` (3 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Adrian Hunter @ 2025-12-09 11:51 UTC (permalink / raw)
To: alexandre.belloni; +Cc: Frank.Li, linux-i3c
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>
---
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/7] i3c: mipi-i3c-hci: Allow for Multi-Bus Instances
2025-12-09 11:50 [PATCH 0/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers Adrian Hunter
` (2 preceding siblings ...)
2025-12-09 11:51 ` [PATCH 3/7] i3c: mipi-i3c-hci: Quieten initialization messages Adrian Hunter
@ 2025-12-09 11:51 ` Adrian Hunter
2025-12-09 11:51 ` [PATCH 5/7] i3c: mipi-i3c-hci-pci: Define default driver data Adrian Hunter
` (2 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2025-12-09 11:51 UTC (permalink / raw)
To: alexandre.belloni; +Cc: Frank.Li, linux-i3c
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>
---
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/7] i3c: mipi-i3c-hci-pci: Define default driver data
2025-12-09 11:50 [PATCH 0/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers Adrian Hunter
` (3 preceding siblings ...)
2025-12-09 11:51 ` [PATCH 4/7] i3c: mipi-i3c-hci: Allow for Multi-Bus Instances Adrian Hunter
@ 2025-12-09 11:51 ` Adrian Hunter
2025-12-09 16:36 ` Frank Li
2025-12-09 11:51 ` [PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances Adrian Hunter
2025-12-09 11:51 ` [PATCH 7/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers Adrian Hunter
6 siblings, 1 reply; 18+ messages in thread
From: Adrian Hunter @ 2025-12-09 11:51 UTC (permalink / raw)
To: alexandre.belloni; +Cc: Frank.Li, linux-i3c
Define default driver data, to save having to repeatedly check for NULL.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 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..ccaec5d3d248 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
@@ -184,6 +184,9 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
.exit = intel_i3c_exit,
};
+static const struct mipi_i3c_hci_pci_info dflt_info = {
+};
+
static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
const struct pci_device_id *id)
{
@@ -228,8 +231,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
if (ret)
goto err;
- hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
- if (hci->info && hci->info->init) {
+ hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data ?: &dflt_info;
+ if (hci->info->init) {
ret = hci->info->init(hci);
if (ret)
goto err;
@@ -244,7 +247,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 +261,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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances
2025-12-09 11:50 [PATCH 0/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers Adrian Hunter
` (4 preceding siblings ...)
2025-12-09 11:51 ` [PATCH 5/7] i3c: mipi-i3c-hci-pci: Define default driver data Adrian Hunter
@ 2025-12-09 11:51 ` Adrian Hunter
2025-12-09 17:10 ` Frank Li
2025-12-10 6:01 ` Krzysztof Kozlowski
2025-12-09 11:51 ` [PATCH 7/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers Adrian Hunter
6 siblings, 2 replies; 18+ messages in thread
From: Adrian Hunter @ 2025-12-09 11:51 UTC (permalink / raw)
To: alexandre.belloni; +Cc: Frank.Li, linux-i3c
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).
Convert to a Multifunction driver and create an MFD cell for each instance.
Create a separate platform device 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>
---
drivers/i3c/master/Kconfig | 1 +
.../master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 180 +++++++++++++-----
2 files changed, 131 insertions(+), 50 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 ccaec5d3d248..145f9adadf75 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,27 +12,43 @@
#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>
#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_instance {
+ int dev_id;
+ u32 offset;
+};
+
struct mipi_i3c_hci_pci {
struct pci_dev *pci;
- struct platform_device *pdev;
+ void __iomem *base;
const struct mipi_i3c_hci_pci_info *info;
+ struct mipi_i3c_hci_pci_instance instances[INST_MAX];
void *private;
};
struct mipi_i3c_hci_pci_info {
+ struct mipi_i3c_hci_platform_data pdata;
int (*init)(struct mipi_i3c_hci_pci *hci);
void (*exit)(struct mipi_i3c_hci_pci *hci);
+ u32 instance_offset[INST_MAX - 1]; /* Excludes instance at offset 0 */
+ int instance_count;
};
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 +159,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,12 +196,106 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
static const struct mipi_i3c_hci_pci_info dflt_info = {
};
+static void mipi_i3c_hci_pci_init_ids(struct mipi_i3c_hci_pci *hci)
+{
+ /* 0 is a valid id, so set unallocated ids to -1 */
+ for (int i = 0; i < INST_MAX; i++)
+ hci->instances[i].dev_id = -1;
+}
+
+static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
+{
+ /* ida_free() ignores negative ids */
+ for (int i = 0; i < INST_MAX; i++)
+ ida_free(&mipi_i3c_hci_pci_ida, hci->instances[i].dev_id);
+
+ mipi_i3c_hci_pci_init_ids(hci);
+}
+
+static int mipi_i3c_hci_pci_alloc_ids(struct mipi_i3c_hci_pci *hci, int nr)
+{
+ int dev_id;
+
+ mipi_i3c_hci_pci_init_ids(hci);
+
+ for (int i = 0; i < nr; i++) {
+ dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
+ if (dev_id < 0)
+ goto err_free_ids;
+ hci->instances[i].dev_id = dev_id;
+ }
+
+ 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, int idx,
+ struct mipi_i3c_hci_pci_cell_data *data,
+ struct mfd_cell *cell)
+{
+ u32 offset = idx ? hci->info->instance_offset[idx - 1] : 0;
+
+ hci->instances[idx].offset = offset;
+
+ data->pdata = hci->info->pdata;
+ data->pdata.base_regs = hci->base + offset;
+
+ data->res = DEFINE_RES_IRQ(0);
+
+ cell->name = "mipi-i3c-hci";
+ cell->id = hci->instances[idx].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)
+{
+ int instance_count = hci->info->instance_count + 1; /* Include instance at offset 0 */
+ struct mipi_i3c_hci_pci_cell_data *data __free(kfree);
+ struct mfd_cell *cells __free(kfree);
+ int irq;
+ int ret;
+
+ cells = kcalloc(instance_count, sizeof(*cells), GFP_KERNEL);
+ data = kcalloc(instance_count, sizeof(*data), GFP_KERNEL);
+ if (!cells || !data)
+ return -ENOMEM;
+
+ ret = mipi_i3c_hci_pci_alloc_ids(hci, instance_count);
+ if (ret)
+ return ret;
+
+ for (int i = 0; i < instance_count; i++)
+ mipi_i3c_hci_pci_setup_cell(hci, i, data + i, cells + i);
+
+ irq = pci_irq_vector(hci->pci, 0);
+
+ ret = mfd_add_devices(&hci->pci->dev, 0, cells, instance_count, NULL, irq, NULL);
+ if (ret)
+ goto err_free_ids;
+
+ return 0;
+
+err_free_ids:
+ mipi_i3c_hci_pci_free_ids(hci);
+ return ret;
+}
+
static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
const struct pci_device_id *id)
{
struct mipi_i3c_hci_pci *hci;
- struct resource res[2];
- int dev_id, ret;
+ int ret;
hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
if (!hci)
@@ -204,68 +307,45 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
if (ret)
return ret;
- pci_set_master(pci);
-
- 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;
- res[1].end = pci->irq;
+ ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
+ if (ret < 0)
+ return ret;
- dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
- if (dev_id < 0)
- return dev_id;
+ pci_set_master(pci);
- hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
- if (!hci->pdev)
- return -ENOMEM;
+ hci->base = pcim_iomap_region(pci, 0, pci_name(pci));
+ if (IS_ERR(hci->base))
+ return PTR_ERR(hci->base);
- hci->pdev->dev.parent = &pci->dev;
- device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
+ hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data ?: &dflt_info;
- ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
+ ret = hci->info->init ? hci->info->init(hci) : 0;
if (ret)
- goto err;
+ return ret;
- hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data ?: &dflt_info;
- if (hci->info->init) {
- ret = hci->info->init(hci);
- if (ret)
- goto err;
- }
+ pci_set_drvdata(pci, hci);
- ret = platform_device_add(hci->pdev);
+ ret = mipi_i3c_hci_pci_add_instances(hci);
if (ret)
goto err_exit;
- pci_set_drvdata(pci, hci);
-
return 0;
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);
+ 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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 7/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers
2025-12-09 11:50 [PATCH 0/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers Adrian Hunter
` (5 preceding siblings ...)
2025-12-09 11:51 ` [PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances Adrian Hunter
@ 2025-12-09 11:51 ` Adrian Hunter
6 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2025-12-09 11:51 UTC (permalink / raw)
To: alexandre.belloni; +Cc: Frank.Li, linux-i3c
Define Multi-Bus Instances at offset 0x400 for some Intel controllers.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
.../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 145f9adadf75..d62c7b6bcc58 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
@@ -193,6 +193,13 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
.exit = intel_i3c_exit,
};
+static const struct mipi_i3c_hci_pci_info intel_mi_info = {
+ .init = intel_i3c_init,
+ .exit = intel_i3c_exit,
+ .instance_offset = {0x400},
+ .instance_count = 1,
+};
+
static const struct mipi_i3c_hci_pci_info dflt_info = {
};
@@ -350,17 +357,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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/7] i3c: mipi-i3c-hci: Remove duplicate blank lines
2025-12-09 11:50 ` [PATCH 1/7] i3c: mipi-i3c-hci: Remove duplicate blank lines Adrian Hunter
@ 2025-12-09 16:21 ` Frank Li
0 siblings, 0 replies; 18+ messages in thread
From: Frank Li @ 2025-12-09 16:21 UTC (permalink / raw)
To: Adrian Hunter; +Cc: alexandre.belloni, linux-i3c
On Tue, Dec 09, 2025 at 01:50:58PM +0200, Adrian Hunter wrote:
> 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>
> 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
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/7] i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0
2025-12-09 11:50 ` [PATCH 2/7] i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0 Adrian Hunter
@ 2025-12-09 16:22 ` Frank Li
0 siblings, 0 replies; 18+ messages in thread
From: Frank Li @ 2025-12-09 16:22 UTC (permalink / raw)
To: Adrian Hunter; +Cc: alexandre.belloni, linux-i3c
On Tue, Dec 09, 2025 at 01:50:59PM +0200, Adrian Hunter wrote:
> 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>
> 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
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/7] i3c: mipi-i3c-hci: Quieten initialization messages
2025-12-09 11:51 ` [PATCH 3/7] i3c: mipi-i3c-hci: Quieten initialization messages Adrian Hunter
@ 2025-12-09 16:25 ` Frank Li
0 siblings, 0 replies; 18+ messages in thread
From: Frank Li @ 2025-12-09 16:25 UTC (permalink / raw)
To: Adrian Hunter; +Cc: alexandre.belloni, linux-i3c
On Tue, Dec 09, 2025 at 01:51:00PM +0200, Adrian Hunter wrote:
> 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>
> ---
If have chance, move this to debug fs.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> 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
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/7] i3c: mipi-i3c-hci-pci: Define default driver data
2025-12-09 11:51 ` [PATCH 5/7] i3c: mipi-i3c-hci-pci: Define default driver data Adrian Hunter
@ 2025-12-09 16:36 ` Frank Li
2025-12-10 8:27 ` Adrian Hunter
0 siblings, 1 reply; 18+ messages in thread
From: Frank Li @ 2025-12-09 16:36 UTC (permalink / raw)
To: Adrian Hunter; +Cc: alexandre.belloni, linux-i3c
On Tue, Dec 09, 2025 at 01:51:02PM +0200, Adrian Hunter wrote:
> Define default driver data, to save having to repeatedly check for NULL.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 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..ccaec5d3d248 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
> @@ -184,6 +184,9 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
> .exit = intel_i3c_exit,
> };
>
> +static const struct mipi_i3c_hci_pci_info dflt_info = {
> +};
> +
> static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> const struct pci_device_id *id)
> {
> @@ -228,8 +231,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> if (ret)
> goto err;
>
> - hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
I see all devices have driver_data in mipi_i3c_hci_pci_devices()
"
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, 0x4d6f), (kernel_ulong_t)&intel_info},
/* Panther Lake-H */
{ PCI_VDEVICE(INTEL, 0xe37c), (kernel_ulong_t)&intel_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, 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},
{ },
};
"
Frank
> - if (hci->info && hci->info->init) {
> + hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data ?: &dflt_info;
> + if (hci->info->init) {
> ret = hci->info->init(hci);
> if (ret)
> goto err;
> @@ -244,7 +247,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 +261,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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances
2025-12-09 11:51 ` [PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances Adrian Hunter
@ 2025-12-09 17:10 ` Frank Li
2025-12-10 9:17 ` Adrian Hunter
2025-12-10 6:01 ` Krzysztof Kozlowski
1 sibling, 1 reply; 18+ messages in thread
From: Frank Li @ 2025-12-09 17:10 UTC (permalink / raw)
To: Adrian Hunter; +Cc: alexandre.belloni, linux-i3c
On Tue, Dec 09, 2025 at 01:51:03PM +0200, Adrian Hunter wrote:
> 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).
>
> Convert to a Multifunction driver and create an MFD cell for each instance.
> Create a separate platform device 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>
> ---
> drivers/i3c/master/Kconfig | 1 +
> .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 180 +++++++++++++-----
> 2 files changed, 131 insertions(+), 50 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 ccaec5d3d248..145f9adadf75 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,27 +12,43 @@
> #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>
> #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_instance {
> + int dev_id;
> + u32 offset;
> +};
> +
> struct mipi_i3c_hci_pci {
> struct pci_dev *pci;
> - struct platform_device *pdev;
> + void __iomem *base;
> const struct mipi_i3c_hci_pci_info *info;
> + struct mipi_i3c_hci_pci_instance instances[INST_MAX];
> void *private;
> };
>
> struct mipi_i3c_hci_pci_info {
> + struct mipi_i3c_hci_platform_data pdata;
> int (*init)(struct mipi_i3c_hci_pci *hci);
> void (*exit)(struct mipi_i3c_hci_pci *hci);
> + u32 instance_offset[INST_MAX - 1]; /* Excludes instance at offset 0 */
what difference with mipi_i3c_hci_pci_instance: offset?
> + int instance_count;
> };
>
> 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 +159,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;
Does need lock if mult instance to access this MMIO space? Especial enable
disable irq, which generally use read modify write pattern.
>
> - if (!host || !priv)
> + if (!host)
> return -ENOMEM;
>
> dma_set_mask_and_coherent(&hci->pci->dev, DMA_BIT_MASK(64));
> @@ -187,12 +196,106 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
> static const struct mipi_i3c_hci_pci_info dflt_info = {
> };
>
> +static void mipi_i3c_hci_pci_init_ids(struct mipi_i3c_hci_pci *hci)
> +{
> + /* 0 is a valid id, so set unallocated ids to -1 */
> + for (int i = 0; i < INST_MAX; i++)
> + hci->instances[i].dev_id = -1;
> +}
> +
> +static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
> +{
> + /* ida_free() ignores negative ids */
> + for (int i = 0; i < INST_MAX; i++)
> + ida_free(&mipi_i3c_hci_pci_ida, hci->instances[i].dev_id);
> +
> + mipi_i3c_hci_pci_init_ids(hci);
> +}
> +
> +static int mipi_i3c_hci_pci_alloc_ids(struct mipi_i3c_hci_pci *hci, int nr)
> +{
> + int dev_id;
> +
> + mipi_i3c_hci_pci_init_ids(hci);
> +
> + for (int i = 0; i < nr; i++) {
> + dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> + if (dev_id < 0)
> + goto err_free_ids;
> + hci->instances[i].dev_id = dev_id;
> + }
> +
> + return 0;
> +
> +err_free_ids:
> + mipi_i3c_hci_pci_free_ids(hci);
> + return -ENOMEM;
> +}
Feel like above logic have some complex, add nr in mipi_i3c_hci_pci.
INST_MAX can be 15.
mipi_i3c_hci_pci_free_ids(hci)
{
for (int i = 0; i < hci->nr; i++)
ida_free(&mipi_i3c_hci_pci_ida, hci->instances[i].dev_id);
}
mipi_i3c_hci_pci_alloc_ids(hci, nr)
{
for (int i = 0; i < nr; i++) {
dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
if (dev_id) < 0)
goto err_free_ids;
hci->nr = i;
}
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, int idx,
> + struct mipi_i3c_hci_pci_cell_data *data,
> + struct mfd_cell *cell)
> +{
> + u32 offset = idx ? hci->info->instance_offset[idx - 1] : 0;
When idx is 1, copy from instance_offset[0] ?
when idx is 0, offset is 0, so hci->instances[0].offset = offset;
so hci->instances[0].offset is 0.
then hci->instances[1].offset will be 0?
> +
> + hci->instances[idx].offset = offset;
> +
> + data->pdata = hci->info->pdata;
> + data->pdata.base_regs = hci->base + offset;
> +
> + data->res = DEFINE_RES_IRQ(0);
> +
> + cell->name = "mipi-i3c-hci";
> + cell->id = hci->instances[idx].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)
> +{
> + int instance_count = hci->info->instance_count + 1; /* Include instance at offset 0 */
> + struct mipi_i3c_hci_pci_cell_data *data __free(kfree);
now prefer assign value when declear, see cleanup.h
struct mipi_i3c_hci_pci_cell_data *data __free(kfree) =
kcalloc(instance_count, sizeof(*data), GFP_KERNEL);
> + struct mfd_cell *cells __free(kfree);
> + int irq;
> + int ret;
> +
> + cells = kcalloc(instance_count, sizeof(*cells), GFP_KERNEL);
> + data = kcalloc(instance_count, sizeof(*data), GFP_KERNEL);
> + if (!cells || !data)
> + return -ENOMEM;
> +
> + ret = mipi_i3c_hci_pci_alloc_ids(hci, instance_count);
> + if (ret)
> + return ret;
> +
> + for (int i = 0; i < instance_count; i++)
> + mipi_i3c_hci_pci_setup_cell(hci, i, data + i, cells + i);
> +
> + irq = pci_irq_vector(hci->pci, 0);
> +
> + ret = mfd_add_devices(&hci->pci->dev, 0, cells, instance_count, NULL, irq, NULL);
> + if (ret)
> + goto err_free_ids;
> +
> + return 0;
> +
> +err_free_ids:
> + mipi_i3c_hci_pci_free_ids(hci);
> + return ret;
> +}
> +
> static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> const struct pci_device_id *id)
> {
> struct mipi_i3c_hci_pci *hci;
> - struct resource res[2];
> - int dev_id, ret;
> + int ret;
>
> hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
> if (!hci)
> @@ -204,68 +307,45 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> if (ret)
> return ret;
>
> - pci_set_master(pci);
> -
> - 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;
> - res[1].end = pci->irq;
> + ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
> + if (ret < 0)
> + return ret;
>
> - dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> - if (dev_id < 0)
> - return dev_id;
> + pci_set_master(pci);
>
> - hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
> - if (!hci->pdev)
> - return -ENOMEM;
> + hci->base = pcim_iomap_region(pci, 0, pci_name(pci));
> + if (IS_ERR(hci->base))
> + return PTR_ERR(hci->base);
>
> - hci->pdev->dev.parent = &pci->dev;
> - device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
> + hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data ?: &dflt_info;
>
> - ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
> + ret = hci->info->init ? hci->info->init(hci) : 0;
> if (ret)
> - goto err;
> + return ret;
>
> - hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data ?: &dflt_info;
> - if (hci->info->init) {
> - ret = hci->info->init(hci);
> - if (ret)
> - goto err;
> - }
> + pci_set_drvdata(pci, hci);
>
> - ret = platform_device_add(hci->pdev);
> + ret = mipi_i3c_hci_pci_add_instances(hci);
> if (ret)
> goto err_exit;
>
> - pci_set_drvdata(pci, hci);
> -
Is it simple if
for (int i = 0; i < nr; i++)
platform_device_register_data(hci->pdev, data, ..., i);
Generally, mfd is used for difference kind devices. Do you know any exiting
code use for create multi instance?
Frank
> return 0;
>
> 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);
> + 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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances
2025-12-09 11:51 ` [PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances Adrian Hunter
2025-12-09 17:10 ` Frank Li
@ 2025-12-10 6:01 ` Krzysztof Kozlowski
1 sibling, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-10 6:01 UTC (permalink / raw)
To: Adrian Hunter, alexandre.belloni; +Cc: Frank.Li, linux-i3c
On 09/12/2025 12:51, Adrian Hunter wrote:
> +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)
> +{
> + u32 offset = idx ? hci->info->instance_offset[idx - 1] : 0;
> +
> + hci->instances[idx].offset = offset;
> +
> + data->pdata = hci->info->pdata;
> + data->pdata.base_regs = hci->base + offset;
> +
> + data->res = DEFINE_RES_IRQ(0);
> +
> + cell->name = "mipi-i3c-hci";
> + cell->id = hci->instances[idx].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)
> +{
> + int instance_count = hci->info->instance_count + 1; /* Include instance at offset 0 */
> + struct mipi_i3c_hci_pci_cell_data *data __free(kfree);
No, don't do that. Please read cleanup.h.
> + struct mfd_cell *cells __free(kfree);
> + int irq;
> + int ret;
> +
Best regards,
Krzysztof
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/7] i3c: mipi-i3c-hci-pci: Define default driver data
2025-12-09 16:36 ` Frank Li
@ 2025-12-10 8:27 ` Adrian Hunter
2025-12-10 15:14 ` Frank Li
0 siblings, 1 reply; 18+ messages in thread
From: Adrian Hunter @ 2025-12-10 8:27 UTC (permalink / raw)
To: Frank Li; +Cc: alexandre.belloni, linux-i3c
On 09/12/2025 18:36, Frank Li wrote:
> On Tue, Dec 09, 2025 at 01:51:02PM +0200, Adrian Hunter wrote:
>> Define default driver data, to save having to repeatedly check for NULL.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>> drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 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..ccaec5d3d248 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
>> @@ -184,6 +184,9 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
>> .exit = intel_i3c_exit,
>> };
>>
>> +static const struct mipi_i3c_hci_pci_info dflt_info = {
>> +};
>> +
>> static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>> const struct pci_device_id *id)
>> {
>> @@ -228,8 +231,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>> if (ret)
>> goto err;
>>
>> - hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
>
> I see all devices have driver_data in mipi_i3c_hci_pci_devices()
Seems safer not to assume that. Alternative is to return an error.
>
> "
> 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, 0x4d6f), (kernel_ulong_t)&intel_info},
> /* Panther Lake-H */
> { PCI_VDEVICE(INTEL, 0xe37c), (kernel_ulong_t)&intel_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, 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},
> { },
> };
> "
>
> Frank
>
>
>> - if (hci->info && hci->info->init) {
>> + hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data ?: &dflt_info;
>> + if (hci->info->init) {
>> ret = hci->info->init(hci);
>> if (ret)
>> goto err;
>> @@ -244,7 +247,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 +261,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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances
2025-12-09 17:10 ` Frank Li
@ 2025-12-10 9:17 ` Adrian Hunter
2025-12-10 15:13 ` Frank Li
0 siblings, 1 reply; 18+ messages in thread
From: Adrian Hunter @ 2025-12-10 9:17 UTC (permalink / raw)
To: Frank Li; +Cc: alexandre.belloni, linux-i3c
On 09/12/2025 19:10, Frank Li wrote:
> On Tue, Dec 09, 2025 at 01:51:03PM +0200, Adrian Hunter wrote:
>> 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).
>>
>> Convert to a Multifunction driver and create an MFD cell for each instance.
>> Create a separate platform device 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>
>> ---
>> drivers/i3c/master/Kconfig | 1 +
>> .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 180 +++++++++++++-----
>> 2 files changed, 131 insertions(+), 50 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 ccaec5d3d248..145f9adadf75 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,27 +12,43 @@
>> #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>
>> #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_instance {
>> + int dev_id;
>> + u32 offset;
>> +};
>> +
>> struct mipi_i3c_hci_pci {
>> struct pci_dev *pci;
>> - struct platform_device *pdev;
>> + void __iomem *base;
>> const struct mipi_i3c_hci_pci_info *info;
>> + struct mipi_i3c_hci_pci_instance instances[INST_MAX];
>> void *private;
>> };
>>
>> struct mipi_i3c_hci_pci_info {
>> + struct mipi_i3c_hci_platform_data pdata;
>> int (*init)(struct mipi_i3c_hci_pci *hci);
>> void (*exit)(struct mipi_i3c_hci_pci *hci);
>> + u32 instance_offset[INST_MAX - 1]; /* Excludes instance at offset 0 */
>
> what difference with mipi_i3c_hci_pci_instance: offset?
The instance at offset 0 is mandatory and so is not listed in
driver data.
>
>> + int instance_count;
>> };
>>
>> 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 +159,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;
>
> Does need lock if mult instance to access this MMIO space? Especial enable
> disable irq, which generally use read modify write pattern.
No, the MIPI I3C register sets are separate.
>
>>
>> - if (!host || !priv)
>> + if (!host)
>> return -ENOMEM;
>>
>> dma_set_mask_and_coherent(&hci->pci->dev, DMA_BIT_MASK(64));
>> @@ -187,12 +196,106 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
>> static const struct mipi_i3c_hci_pci_info dflt_info = {
>> };
>>
>> +static void mipi_i3c_hci_pci_init_ids(struct mipi_i3c_hci_pci *hci)
>> +{
>> + /* 0 is a valid id, so set unallocated ids to -1 */
>> + for (int i = 0; i < INST_MAX; i++)
>> + hci->instances[i].dev_id = -1;
>> +}
>> +
>> +static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
>> +{
>> + /* ida_free() ignores negative ids */
>> + for (int i = 0; i < INST_MAX; i++)
>> + ida_free(&mipi_i3c_hci_pci_ida, hci->instances[i].dev_id);
>> +
>> + mipi_i3c_hci_pci_init_ids(hci);
>> +}
>> +
>> +static int mipi_i3c_hci_pci_alloc_ids(struct mipi_i3c_hci_pci *hci, int nr)
>> +{
>> + int dev_id;
>> +
>> + mipi_i3c_hci_pci_init_ids(hci);
>> +
>> + for (int i = 0; i < nr; i++) {
>> + dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>> + if (dev_id < 0)
>> + goto err_free_ids;
>> + hci->instances[i].dev_id = dev_id;
>> + }
>> +
>> + return 0;
>> +
>> +err_free_ids:
>> + mipi_i3c_hci_pci_free_ids(hci);
>> + return -ENOMEM;
>> +}
>
> Feel like above logic have some complex, add nr in mipi_i3c_hci_pci.
> INST_MAX can be 15.
>
> mipi_i3c_hci_pci_free_ids(hci)
> {
> for (int i = 0; i < hci->nr; i++)
> ida_free(&mipi_i3c_hci_pci_ida, hci->instances[i].dev_id);
> }
>
> mipi_i3c_hci_pci_alloc_ids(hci, nr)
> {
> for (int i = 0; i < nr; i++) {
> dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> if (dev_id) < 0)
> goto err_free_ids;
>
> hci->nr = i;
> }
>
> 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, int idx,
>> + struct mipi_i3c_hci_pci_cell_data *data,
>> + struct mfd_cell *cell)
>> +{
>> + u32 offset = idx ? hci->info->instance_offset[idx - 1] : 0;
>
> When idx is 1, copy from instance_offset[0] ?
>
> when idx is 0, offset is 0, so hci->instances[0].offset = offset;
> so hci->instances[0].offset is 0.
>
> then hci->instances[1].offset will be 0?
There is 1 mandatory instance at offset 0. Other instances, if
there are any, can be listed in driver data instance_offset[].
>
>> +
>> + hci->instances[idx].offset = offset;
>> +
>> + data->pdata = hci->info->pdata;
>> + data->pdata.base_regs = hci->base + offset;
>> +
>> + data->res = DEFINE_RES_IRQ(0);
>> +
>> + cell->name = "mipi-i3c-hci";
>> + cell->id = hci->instances[idx].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)
>> +{
>> + int instance_count = hci->info->instance_count + 1; /* Include instance at offset 0 */
>> + struct mipi_i3c_hci_pci_cell_data *data __free(kfree);
>
> now prefer assign value when declear, see cleanup.h
>
> struct mipi_i3c_hci_pci_cell_data *data __free(kfree) =
> kcalloc(instance_count, sizeof(*data), GFP_KERNEL);
>
>> + struct mfd_cell *cells __free(kfree);
>> + int irq;
>> + int ret;
>> +
>> + cells = kcalloc(instance_count, sizeof(*cells), GFP_KERNEL);
>> + data = kcalloc(instance_count, sizeof(*data), GFP_KERNEL);
>> + if (!cells || !data)
>> + return -ENOMEM;
>> +
>> + ret = mipi_i3c_hci_pci_alloc_ids(hci, instance_count);
>> + if (ret)
>> + return ret;
>> +
>> + for (int i = 0; i < instance_count; i++)
>> + mipi_i3c_hci_pci_setup_cell(hci, i, data + i, cells + i);
>> +
>> + irq = pci_irq_vector(hci->pci, 0);
>> +
>> + ret = mfd_add_devices(&hci->pci->dev, 0, cells, instance_count, NULL, irq, NULL);
>> + if (ret)
>> + goto err_free_ids;
>> +
>> + return 0;
>> +
>> +err_free_ids:
>> + mipi_i3c_hci_pci_free_ids(hci);
>> + return ret;
>> +}
>> +
>> static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>> const struct pci_device_id *id)
>> {
>> struct mipi_i3c_hci_pci *hci;
>> - struct resource res[2];
>> - int dev_id, ret;
>> + int ret;
>>
>> hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
>> if (!hci)
>> @@ -204,68 +307,45 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>> if (ret)
>> return ret;
>>
>> - pci_set_master(pci);
>> -
>> - 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;
>> - res[1].end = pci->irq;
>> + ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
>> + if (ret < 0)
>> + return ret;
>>
>> - dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>> - if (dev_id < 0)
>> - return dev_id;
>> + pci_set_master(pci);
>>
>> - hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
>> - if (!hci->pdev)
>> - return -ENOMEM;
>> + hci->base = pcim_iomap_region(pci, 0, pci_name(pci));
>> + if (IS_ERR(hci->base))
>> + return PTR_ERR(hci->base);
>>
>> - hci->pdev->dev.parent = &pci->dev;
>> - device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
>> + hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data ?: &dflt_info;
>>
>> - ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
>> + ret = hci->info->init ? hci->info->init(hci) : 0;
>> if (ret)
>> - goto err;
>> + return ret;
>>
>> - hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data ?: &dflt_info;
>> - if (hci->info->init) {
>> - ret = hci->info->init(hci);
>> - if (ret)
>> - goto err;
>> - }
>> + pci_set_drvdata(pci, hci);
>>
>> - ret = platform_device_add(hci->pdev);
>> + ret = mipi_i3c_hci_pci_add_instances(hci);
>> if (ret)
>> goto err_exit;
>>
>> - pci_set_drvdata(pci, hci);
>> -
>
> Is it simple if
>
> for (int i = 0; i < nr; i++)
> platform_device_register_data(hci->pdev, data, ..., i);
mfd is simpler because the cell contains all needed information and
mfd handles removing devices if one device creation fails.
>
> Generally, mfd is used for difference kind devices. Do you know any exiting
> code use for create multi instance?
There are examples of mfds with multiple instances of the same
kind of device:
drivers/mfd/88pm860x-core.c bk_devs[]
drivers/mfd/da9052-core.c da9052_subdev_info[]
Don't see any reason why there have to be different kinds of
devices.
>
> Frank
>
>
>> return 0;
>>
>> 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);
>> + 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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances
2025-12-10 9:17 ` Adrian Hunter
@ 2025-12-10 15:13 ` Frank Li
0 siblings, 0 replies; 18+ messages in thread
From: Frank Li @ 2025-12-10 15:13 UTC (permalink / raw)
To: Adrian Hunter; +Cc: alexandre.belloni, linux-i3c
On Wed, Dec 10, 2025 at 11:17:43AM +0200, Adrian Hunter wrote:
> On 09/12/2025 19:10, Frank Li wrote:
> > On Tue, Dec 09, 2025 at 01:51:03PM +0200, Adrian Hunter wrote:
> >> 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).
> >>
> >> Convert to a Multifunction driver and create an MFD cell for each instance.
> >> Create a separate platform device 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>
> >> ---
> >> drivers/i3c/master/Kconfig | 1 +
> >> .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 180 +++++++++++++-----
> >> 2 files changed, 131 insertions(+), 50 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 ccaec5d3d248..145f9adadf75 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,27 +12,43 @@
> >> #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>
> >> #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_instance {
> >> + int dev_id;
> >> + u32 offset;
> >> +};
> >> +
> >> struct mipi_i3c_hci_pci {
> >> struct pci_dev *pci;
> >> - struct platform_device *pdev;
> >> + void __iomem *base;
> >> const struct mipi_i3c_hci_pci_info *info;
> >> + struct mipi_i3c_hci_pci_instance instances[INST_MAX];
> >> void *private;
> >> };
> >>
> >> struct mipi_i3c_hci_pci_info {
> >> + struct mipi_i3c_hci_platform_data pdata;
> >> int (*init)(struct mipi_i3c_hci_pci *hci);
> >> void (*exit)(struct mipi_i3c_hci_pci *hci);
> >> + u32 instance_offset[INST_MAX - 1]; /* Excludes instance at offset 0 */
> >
> > what difference with mipi_i3c_hci_pci_instance: offset?
>
> The instance at offset 0 is mandatory and so is not listed in
> driver data.
I think unified it should be simple.
>
> >
> >> + int instance_count;
> >> };
> >>
> >> 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 +159,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;
> >
> > Does need lock if mult instance to access this MMIO space? Especial enable
> > disable irq, which generally use read modify write pattern.
>
> No, the MIPI I3C register sets are separate.
>
> >
> >>
> >> - if (!host || !priv)
> >> + if (!host)
> >> return -ENOMEM;
> >>
> >> dma_set_mask_and_coherent(&hci->pci->dev, DMA_BIT_MASK(64));
> >> @@ -187,12 +196,106 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
> >> static const struct mipi_i3c_hci_pci_info dflt_info = {
> >> };
> >>
> >> +static void mipi_i3c_hci_pci_init_ids(struct mipi_i3c_hci_pci *hci)
> >> +{
> >> + /* 0 is a valid id, so set unallocated ids to -1 */
> >> + for (int i = 0; i < INST_MAX; i++)
> >> + hci->instances[i].dev_id = -1;
> >> +}
> >> +
> >> +static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
> >> +{
> >> + /* ida_free() ignores negative ids */
> >> + for (int i = 0; i < INST_MAX; i++)
> >> + ida_free(&mipi_i3c_hci_pci_ida, hci->instances[i].dev_id);
> >> +
> >> + mipi_i3c_hci_pci_init_ids(hci);
> >> +}
> >> +
> >> +static int mipi_i3c_hci_pci_alloc_ids(struct mipi_i3c_hci_pci *hci, int nr)
> >> +{
> >> + int dev_id;
> >> +
> >> + mipi_i3c_hci_pci_init_ids(hci);
> >> +
> >> + for (int i = 0; i < nr; i++) {
> >> + dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> >> + if (dev_id < 0)
> >> + goto err_free_ids;
> >> + hci->instances[i].dev_id = dev_id;
> >> + }
> >> +
> >> + return 0;
> >> +
> >> +err_free_ids:
> >> + mipi_i3c_hci_pci_free_ids(hci);
> >> + return -ENOMEM;
> >> +}
> >
> > Feel like above logic have some complex, add nr in mipi_i3c_hci_pci.
> > INST_MAX can be 15.
> >
> > mipi_i3c_hci_pci_free_ids(hci)
> > {
> > for (int i = 0; i < hci->nr; i++)
> > ida_free(&mipi_i3c_hci_pci_ida, hci->instances[i].dev_id);
> > }
> >
> > mipi_i3c_hci_pci_alloc_ids(hci, nr)
> > {
> > for (int i = 0; i < nr; i++) {
> > dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> > if (dev_id) < 0)
> > goto err_free_ids;
> >
> > hci->nr = i;
> > }
> >
> > 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, int idx,
> >> + struct mipi_i3c_hci_pci_cell_data *data,
> >> + struct mfd_cell *cell)
> >> +{
> >> + u32 offset = idx ? hci->info->instance_offset[idx - 1] : 0;
> >
> > When idx is 1, copy from instance_offset[0] ?
> >
> > when idx is 0, offset is 0, so hci->instances[0].offset = offset;
> > so hci->instances[0].offset is 0.
> >
> > then hci->instances[1].offset will be 0?
>
> There is 1 mandatory instance at offset 0. Other instances, if
> there are any, can be listed in driver data instance_offset[].
If put instance 0 into instances[], code logic should be simple.
>
> >
> >> +
> >> + hci->instances[idx].offset = offset;
> >> +
> >> + data->pdata = hci->info->pdata;
> >> + data->pdata.base_regs = hci->base + offset;
> >> +
> >> + data->res = DEFINE_RES_IRQ(0);
> >> +
> >> + cell->name = "mipi-i3c-hci";
> >> + cell->id = hci->instances[idx].dev_id;
> >> + cell->platform_data = &data->pdata;
> >> + cell->pdata_size = sizeof(data->pdata);
> >> + cell->num_resources = 1;
> >> + cell->resources = &data->res;
> >> +}
> >> +
...
> >>
> >> - pci_set_drvdata(pci, hci);
> >> -
> >
> > Is it simple if
> >
> > for (int i = 0; i < nr; i++)
> > platform_device_register_data(hci->pdev, data, ..., i);
>
> mfd is simpler because the cell contains all needed information and
> mfd handles removing devices if one device creation fails.
>
> >
> > Generally, mfd is used for difference kind devices. Do you know any exiting
> > code use for create multi instance?
>
> There are examples of mfds with multiple instances of the same
> kind of device:
> drivers/mfd/88pm860x-core.c bk_devs[]
> drivers/mfd/da9052-core.c da9052_subdev_info[]
>
> Don't see any reason why there have to be different kinds of
> devices.
Okay, can you create a patch first to convert to mfd from
platform_device_add() even just one devices.
Than add more instance for it.
>
> >
> > Frank
> >
> >
> >> return 0;
> >>
> >> 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);
> >> + 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
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/7] i3c: mipi-i3c-hci-pci: Define default driver data
2025-12-10 8:27 ` Adrian Hunter
@ 2025-12-10 15:14 ` Frank Li
0 siblings, 0 replies; 18+ messages in thread
From: Frank Li @ 2025-12-10 15:14 UTC (permalink / raw)
To: Adrian Hunter; +Cc: alexandre.belloni, linux-i3c
On Wed, Dec 10, 2025 at 10:27:05AM +0200, Adrian Hunter wrote:
> On 09/12/2025 18:36, Frank Li wrote:
> > On Tue, Dec 09, 2025 at 01:51:02PM +0200, Adrian Hunter wrote:
> >> Define default driver data, to save having to repeatedly check for NULL.
> >>
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >> ---
> >> drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 11 +++++++----
> >> 1 file changed, 7 insertions(+), 4 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..ccaec5d3d248 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
> >> @@ -184,6 +184,9 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
> >> .exit = intel_i3c_exit,
> >> };
> >>
> >> +static const struct mipi_i3c_hci_pci_info dflt_info = {
> >> +};
> >> +
> >> static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >> const struct pci_device_id *id)
> >> {
> >> @@ -228,8 +231,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >> if (ret)
> >> goto err;
> >>
> >> - hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
> >
> > I see all devices have driver_data in mipi_i3c_hci_pci_devices()
>
> Seems safer not to assume that. Alternative is to return an error.
If we can guaratee, needn't reduntant check.
Frank
>
> >
> > "
> > 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, 0x4d6f), (kernel_ulong_t)&intel_info},
> > /* Panther Lake-H */
> > { PCI_VDEVICE(INTEL, 0xe37c), (kernel_ulong_t)&intel_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, 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},
> > { },
> > };
> > "
> >
> > Frank
> >
> >
> >> - if (hci->info && hci->info->init) {
> >> + hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data ?: &dflt_info;
> >> + if (hci->info->init) {
> >> ret = hci->info->init(hci);
> >> if (ret)
> >> goto err;
> >> @@ -244,7 +247,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 +261,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
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2025-12-10 15:15 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-09 11:50 [PATCH 0/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers Adrian Hunter
2025-12-09 11:50 ` [PATCH 1/7] i3c: mipi-i3c-hci: Remove duplicate blank lines Adrian Hunter
2025-12-09 16:21 ` Frank Li
2025-12-09 11:50 ` [PATCH 2/7] i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0 Adrian Hunter
2025-12-09 16:22 ` Frank Li
2025-12-09 11:51 ` [PATCH 3/7] i3c: mipi-i3c-hci: Quieten initialization messages Adrian Hunter
2025-12-09 16:25 ` Frank Li
2025-12-09 11:51 ` [PATCH 4/7] i3c: mipi-i3c-hci: Allow for Multi-Bus Instances Adrian Hunter
2025-12-09 11:51 ` [PATCH 5/7] i3c: mipi-i3c-hci-pci: Define default driver data Adrian Hunter
2025-12-09 16:36 ` Frank Li
2025-12-10 8:27 ` Adrian Hunter
2025-12-10 15:14 ` Frank Li
2025-12-09 11:51 ` [PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances Adrian Hunter
2025-12-09 17:10 ` Frank Li
2025-12-10 9:17 ` Adrian Hunter
2025-12-10 15:13 ` Frank Li
2025-12-10 6:01 ` Krzysztof Kozlowski
2025-12-09 11:51 ` [PATCH 7/7] i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers Adrian Hunter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox