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

[Frank Li <Frank.li@nxp.com>]

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

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

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

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

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

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

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

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

It is not good to hide hci in macro.

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

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

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