[PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances
Adrian Hunter
adrian.hunter at intel.com
Wed Dec 10 01:17:43 PST 2025
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 at 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 at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c
More information about the linux-i3c
mailing list