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

Frank Li
2025-12-17T19:25:12Z

On Tue, Dec 16, 2025 at 08:56:05PM +0200, Adrian Hunter wrote:
> 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.

Thanks

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> >
> >>
> >> 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
> >
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c