Re: [PATCH V3 06/11] i3c: mipi-i3c-hci-pci: Assign unique device names and IDs for Intel LPSS I3C

Adrian Hunter
2025-12-16T18:56:24Z

On 16/12/2025 19:14, Adrian Hunter wrote:
> On 16/12/2025 19:08, Frank Li wrote:
>> On Tue, Dec 16, 2025 at 06:56:37PM +0200, Adrian Hunter wrote:
>>> Simplify the code and ensure names and IDs align with device documentation.
>>> Use explicit device names and IDs for Intel LPSS I3C controllers instead of
>>> dynamically allocated values.
>>>
>>> Add "intel-lpss-i3c" to the platform_device_id table in the mipi-i3c-hci
>>> driver and use the same name for Intel I3C controllers in the
>>> mipi_i3c_hci_pci driver. Assign hard-coded IDs to reflect the hardware
>>> layout.
>>>
>>> Intel SoCs include two I3C PCI devices in the Low Power Subsystem (LPSS),
>>> each supporting up to two I3C buses. The second PCI device is assigned ID 2
>>> (not 1) to match this topology. Additional IDs will be introduced when
>>> Multi-Bus Instance support is implemented.
>>>
>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>> ---
>>>
>>>
>>> Change in V3:
>>>
>>> New patch
>>>
>>>
>>> drivers/i3c/master/mipi-i3c-hci/core.c | 7 +++
>>> .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 43 ++++++++++---------
>>> 2 files changed, 30 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
>>> index 07fb91a12593..3d6544a64188 100644
>>> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
>>> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
>>> @@ -790,9 +790,16 @@ static const struct acpi_device_id i3c_hci_acpi_match[] = {
>>> };
>>> MODULE_DEVICE_TABLE(acpi, i3c_hci_acpi_match);
>>>
>>> +static const struct platform_device_id i3c_hci_driver_ids[] = {
>>> + { .name = "intel-lpss-i3c" },
>>> + { /* sentinel */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(platform, i3c_hci_driver_ids);
>>> +
>>> static struct platform_driver i3c_hci_driver = {
>>> .probe = i3c_hci_probe,
>>> .remove = i3c_hci_remove,
>>> + .id_table = i3c_hci_driver_ids,
>>> .driver = {
>>> .name = "mipi-i3c-hci",
>>> .of_match_table = of_match_ptr(i3c_hci_of_match),
>>> 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 0fd3587671e1..3b319fbf18ce 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
>>> @@ -27,10 +27,10 @@ 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);
>>> + const char *name;
>>> + int id;
>>> };
>>>
>>> -static DEFINE_IDA(mipi_i3c_hci_pci_ida);
>>> -
>>> #define INTEL_PRIV_OFFSET 0x2b0
>>> #define INTEL_PRIV_SIZE 0x28
>>> #define INTEL_RESETS 0x04
>>> @@ -179,9 +179,18 @@ static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci)
>>> intel_ltr_hide(&hci->pci->dev);
>>> }
>>>
>>> -static const struct mipi_i3c_hci_pci_info intel_info = {
>>> +static const struct mipi_i3c_hci_pci_info intel_1_info = {
>>> .init = intel_i3c_init,
>>> .exit = intel_i3c_exit,
>>> + .name = "intel-lpss-i3c",
>>> + .id = 0,
>>> +};
>>> +
>>> +static const struct mipi_i3c_hci_pci_info intel_2_info = {
>>> + .init = intel_i3c_init,
>>> + .exit = intel_i3c_exit,
>>> + .name = "intel-lpss-i3c",
>>> + .id = 2,
>>> };
>>>
>>> static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>> @@ -189,7 +198,7 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>> {
>>> 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)
>>> @@ -217,11 +226,9 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>> res[1].start = pci_irq_vector(hci->pci, 0);
>>> res[1].end = res[1].start;
>>>
>>> - dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
>>> - if (dev_id < 0)
>>> - return dev_id;
>>> + hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
>>>
>>> - hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
>>> + hci->pdev = platform_device_alloc(hci->info->name, hci->info->id);
>>> if (!hci->pdev)
>>> return -ENOMEM;
>>>
>>> @@ -232,7 +239,6 @@ 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->init) {
>>> ret = hci->info->init(hci);
>>> if (ret)
>>> @@ -252,7 +258,6 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
>>> hci->info->exit(hci);
>>> err:
>>> platform_device_put(hci->pdev);
>>> - ida_free(&mipi_i3c_hci_pci_ida, dev_id);
>>> return ret;
>>> }
>>>
>>> @@ -260,28 +265,26 @@ 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);
>>> }
>>>
>>> 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},
>>> + { PCI_VDEVICE(INTEL, 0x4d7c), (kernel_ulong_t)&intel_1_info},
>>
>> If there there are two 0x4d7c pcie cards in system, id will be the same.
>> Is it okay?
>
> These are not cards. These PCI device IDs are unique to each device
> in the SoC, so duplicate IDs would never happen.

Note, that an id value can also be PLATFORM_DEVID_AUTO or
PLATFORM_DEVID_NONE, so there is still flexibility.

>
>>
>> Frank
>>
>>> + { PCI_VDEVICE(INTEL, 0x4d6f), (kernel_ulong_t)&intel_2_info},
>>> /* Panther Lake-H */
>>> - { PCI_VDEVICE(INTEL, 0xe37c), (kernel_ulong_t)&intel_info},
>>> - { PCI_VDEVICE(INTEL, 0xe36f), (kernel_ulong_t)&intel_info},
>>> + { PCI_VDEVICE(INTEL, 0xe37c), (kernel_ulong_t)&intel_1_info},
>>> + { PCI_VDEVICE(INTEL, 0xe36f), (kernel_ulong_t)&intel_2_info},
>>> /* Panther Lake-P */
>>> - { PCI_VDEVICE(INTEL, 0xe47c), (kernel_ulong_t)&intel_info},
>>> - { PCI_VDEVICE(INTEL, 0xe46f), (kernel_ulong_t)&intel_info},
>>> + { PCI_VDEVICE(INTEL, 0xe47c), (kernel_ulong_t)&intel_1_info},
>>> + { PCI_VDEVICE(INTEL, 0xe46f), (kernel_ulong_t)&intel_2_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_1_info},
>>> + { PCI_VDEVICE(INTEL, 0x6e2d), (kernel_ulong_t)&intel_2_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
>