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