[PATCH 6/7] i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances
Frank Li
Frank.li at nxp.com
Wed Dec 10 07:13:09 PST 2025
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 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.
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 at lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> 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