Re: [PATCH 08/13] i3c: mipi-i3c-hci-pci: Allocate a structure for mipi_i3c_hci_pci device information

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

On Wed, Nov 12, 2025 at 09:25:14PM +0200, Adrian Hunter wrote:
> On 12/11/2025 18:41, Frank Li wrote:
> > On Wed, Nov 12, 2025 at 12:03:34PM +0200, Adrian Hunter wrote:
> >> Allocate a structure for mipi_i3c_hci_pci device information, in
> >> preparation for additional changes that need to store mipi_i3c_hci_pci
> >> device-specific information.
> >>
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >> ---
> >>  .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 29 ++++++++++++-------
> >>  1 file changed, 19 insertions(+), 10 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 b3b6b6f43af2..9fa89af7479f 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
> >> @@ -14,6 +14,10 @@
> >>  #include <linux/pci.h>
> >>  #include <linux/platform_device.h>
> >>
> >> +struct mipi_i3c_hci_pci {
> >> +	struct platform_device *pdev;
> >> +};
> >> +
> >
> > Is it simpler by using platform_device_register_data(), pass down platform
> > related information by void *data?
>
> No, this is just a normal allocated structure for the PCI driver
> (mipi_i3c_hci_pci) to put information related to the PCI device.
> It is later used to store the current Latency Tolerance Reporting (LTR)
> register values.

Is it static value, or does it change after boot in other words?

>
> So it is not the same thing as platform_data.

Base on your code to create platform data is duplicate what's did by
platform_device_register_data().

The pci device create platform devices,  platform driver should not use
pci's information directly. platform driver should be reused if it probe
from acpi or dt-tree.

I think it'd better pass down platform data information, which can include
callback to fetch needed information for difference's parent devices.

Frank

>
> >
> > Frank
> >>  struct mipi_i3c_hci_pci_info {
> >>  	int (*init)(struct pci_dev *pci);
> >>  };
> >> @@ -71,10 +75,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>  				  const struct pci_device_id *id)
> >>  {
> >>  	const struct mipi_i3c_hci_pci_info *info;
> >> -	struct platform_device *pdev;
> >> +	struct mipi_i3c_hci_pci *hci;
> >>  	struct resource res[2];
> >>  	int dev_id, ret;
> >>
> >> +	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
> >> +	if (!hci)
> >> +		return -ENOMEM;
> >> +
> >>  	ret = pcim_enable_device(pci);
> >>  	if (ret)
> >>  		return ret;
> >> @@ -95,14 +103,14 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>  	if (dev_id < 0)
> >>  		return dev_id;
> >>
> >> -	pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
> >> -	if (!pdev)
> >> +	hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
> >> +	if (!hci->pdev)
> >>  		return -ENOMEM;
> >>
> >> -	pdev->dev.parent = &pci->dev;
> >> -	device_set_node(&pdev->dev, dev_fwnode(&pci->dev));
> >> +	hci->pdev->dev.parent = &pci->dev;
> >> +	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
> >>
> >> -	ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
> >> +	ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
> >>  	if (ret)
> >>  		goto err;
> >>
> >> @@ -113,23 +121,24 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>  			goto err;
> >>  	}
> >>
> >> -	ret = platform_device_add(pdev);
> >> +	ret = platform_device_add(hci->pdev);
> >>  	if (ret)
> >>  		goto err;
> >>
> >> -	pci_set_drvdata(pci, pdev);
> >> +	pci_set_drvdata(pci, hci);
> >>
> >>  	return 0;
> >>
> >>  err:
> >> -	platform_device_put(pdev);
> >> +	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 platform_device *pdev = pci_get_drvdata(pci);
> >> +	struct mipi_i3c_hci_pci *hci = pci_get_drvdata(pci);
> >> +	struct platform_device *pdev = hci->pdev;
> >>  	int dev_id = pdev->id;
> >>
> >>  	platform_device_unregister(pdev);
> >> --
> >> 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

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