i3c: mipi-i3c-hci-pci: Define Multi-Bus Instances for Intel controllers

All entries in the id_table have driver_data. Do not repeatedly check for NULL driver_data.

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:

Drop default driver data

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +3 -4

index dc8ede0f8ad8..8ade911e3835 100644
@@ -229,7 +229,7 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 		goto err;
 
 	hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
-	if (hci->info && hci->info->init) {
+	if (hci->info->init) {
 		ret = hci->info->init(hci);
 		if (ret)
 			goto err;

@@ -244,7 +244,7 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	return 0;
 
 err_exit:
-	if (hci->info && hci->info->exit)
+	if (hci->info->exit)
 		hci->info->exit(hci);
 err:
 	platform_device_put(hci->pdev);

@@ -258,7 +258,7 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
 	struct platform_device *pdev = hci->pdev;
 	int dev_id = pdev->id;
 
-	if (hci->info && hci->info->exit)
+	if (hci->info->exit)
 		hci->info->exit(hci);
 
 	platform_device_unregister(pdev);
--

2.51.0

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Use pci_alloc_irq_vectors() to support all IRQ types. Do not call pci_free_irq_vectors() because it is unnecessary when the device is managed due to the use of pcim_enable_device().

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:
New patch split from "i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances"

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +6 -3

index 8ade911e3835..0fd3587671e1 100644
@@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 
 	pci_set_master(pci);
 
+	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (ret < 0)
+		return ret;
+
 	memset(&res, 0, sizeof(res));
 
 	res[0].flags = IORESOURCE_MEM;

@@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	res[0].end = pci_resource_end(pci, 0);
 
 	res[1].flags = IORESOURCE_IRQ;
-	res[1].start = pci->irq;
-	res[1].end = pci->irq;
+	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)
--

2.51.0

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Quoted 3 lines · Adrian Hunter

Use pci_alloc_irq_vectors() to support all IRQ types. Do not call pci_free_irq_vectors() because it is unnecessary when the device is managed due to the use of pcim_enable_device().

pcim_enable_device() add action pcim_disable_device(), I have not see pci_free_irq_vectors() in pcim_disable_device().

Can you tell me where call pci_free_irq_vectors?

And move these small patch ahead in serise.

Frank

Quoted 47 lines

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:
New patch split from "i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances"

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +6 -3

index 8ade911e3835..0fd3587671e1 100644
@@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 
 	pci_set_master(pci);
 
+	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (ret < 0)
+		return ret;
+
 	memset(&res, 0, sizeof(res));
 
 	res[0].flags = IORESOURCE_MEM;

@@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	res[0].end = pci_resource_end(pci, 0);
 
 	res[1].flags = IORESOURCE_IRQ;
-	res[1].start = pci->irq;
-	res[1].end = pci->irq;
+	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)
--

2.51.0

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Quoted 9 lines · Frank Li

Quoted 3 lines · Adrian Hunter

Use pci_alloc_irq_vectors() to support all IRQ types. Do not call pci_free_irq_vectors() because it is unnecessary when the device is managed due to the use of pcim_enable_device().

pcim_enable_device() add action pcim_disable_device(), I have not see pci_free_irq_vectors() in pcim_disable_device().

Can you tell me where call pci_free_irq_vectors?

pci_alloc_irq_vectors()

	pci_alloc_irq_vectors_affinity()
		__pci_enable_msix_range()
			pci_setup_msi_context()
				pcim_setup_msi_release()
					devm_add_action(pcim_msi_release)
							:
							pci_free_irq_vectors()
		__pci_enable_msi_range()
			pci_setup_msi_context()

Quoted 48 lines

And move these small patch ahead in serise.

Frank

Quoted 43 lines

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:
New patch split from "i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances"

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +6 -3

index 8ade911e3835..0fd3587671e1 100644
@@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 
 	pci_set_master(pci);
 
+	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (ret < 0)
+		return ret;
+
 	memset(&res, 0, sizeof(res));
 
 	res[0].flags = IORESOURCE_MEM;

@@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	res[0].end = pci_resource_end(pci, 0);
 
 	res[1].flags = IORESOURCE_IRQ;
-	res[1].start = pci->irq;
-	res[1].end = pci->irq;
+	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)
--

2.51.0

Quoted 4 lines

Quoted 4 lines

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Quoted 12 lines · Adrian Hunter

Quoted 9 lines · Frank Li

Quoted 3 lines · Adrian Hunter

Use pci_alloc_irq_vectors() to support all IRQ types. Do not call pci_free_irq_vectors() because it is unnecessary when the device is managed due to the use of pcim_enable_device().

pcim_enable_device() add action pcim_disable_device(), I have not see pci_free_irq_vectors() in pcim_disable_device().

Can you tell me where call pci_free_irq_vectors?

pci_alloc_irq_vectors()

Quoted 9 lines

	pci_alloc_irq_vectors_affinity()
		__pci_enable_msix_range()
			pci_setup_msi_context()
				pcim_setup_msi_release()
					devm_add_action(pcim_msi_release)
							:
							pci_free_irq_vectors()
		__pci_enable_msi_range()
			pci_setup_msi_context()

Okay, it should be that pci_alloc_irq_vectors() provide auto free. not because pcim_enable_device()?

The below statement should be wrong.

"Do not call pci_free_irq_vectors() because it is unnecessary when the device is managed due to the use of pcim_enable_device()."

Frank

Quoted 58 lines

Quoted 48 lines

And move these small patch ahead in serise.

Frank

Quoted 43 lines

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:
New patch split from "i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances"

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +6 -3

index 8ade911e3835..0fd3587671e1 100644
@@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 
 	pci_set_master(pci);
 
+	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (ret < 0)
+		return ret;
+
 	memset(&res, 0, sizeof(res));
 
 	res[0].flags = IORESOURCE_MEM;

@@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	res[0].end = pci_resource_end(pci, 0);
 
 	res[1].flags = IORESOURCE_IRQ;
-	res[1].start = pci->irq;
-	res[1].end = pci->irq;
+	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)
--

2.51.0

Quoted 4 lines

Quoted 4 lines

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Quoted 23 lines · Frank Li

Quoted 21 lines · Adrian Hunter

Quoted 9 lines · Frank Li

Quoted 3 lines · Adrian Hunter

Use pci_alloc_irq_vectors() to support all IRQ types. Do not call pci_free_irq_vectors() because it is unnecessary when the device is managed due to the use of pcim_enable_device().

pcim_enable_device() add action pcim_disable_device(), I have not see pci_free_irq_vectors() in pcim_disable_device().

Can you tell me where call pci_free_irq_vectors?

pci_alloc_irq_vectors()

	pci_alloc_irq_vectors_affinity()
		__pci_enable_msix_range()
			pci_setup_msi_context()
				pcim_setup_msi_release()
					devm_add_action(pcim_msi_release)
							:
							pci_free_irq_vectors()
		__pci_enable_msi_range()
			pci_setup_msi_context()

Quoted 2 lines

Okay, it should be that pci_alloc_irq_vectors() provide auto free. not because pcim_enable_device()?

No, it is conditional on pci_is_managed(dev) which is true if pcim_enable_device() has been used.

Quoted 66 lines

The below statement should be wrong.

"Do not call pci_free_irq_vectors() because it is unnecessary when the device is managed due to the use of pcim_enable_device()."

Frank

Quoted 58 lines

Quoted 48 lines

And move these small patch ahead in serise.

Frank

Quoted 43 lines

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:
New patch split from "i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances"

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +6 -3

index 8ade911e3835..0fd3587671e1 100644
@@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 
 	pci_set_master(pci);
 
+	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (ret < 0)
+		return ret;
+
 	memset(&res, 0, sizeof(res));
 
 	res[0].flags = IORESOURCE_MEM;

@@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	res[0].end = pci_resource_end(pci, 0);
 
 	res[1].flags = IORESOURCE_IRQ;
-	res[1].start = pci->irq;
-	res[1].end = pci->irq;
+	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)
--

2.51.0

Quoted 4 lines

Quoted 4 lines

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Quoted 24 lines · Adrian Hunter

Quoted 23 lines · Frank Li

Quoted 21 lines · Adrian Hunter

Quoted 9 lines · Frank Li

Quoted 3 lines · Adrian Hunter

Use pci_alloc_irq_vectors() to support all IRQ types. Do not call pci_free_irq_vectors() because it is unnecessary when the device is managed due to the use of pcim_enable_device().

pcim_enable_device() add action pcim_disable_device(), I have not see pci_free_irq_vectors() in pcim_disable_device().

Can you tell me where call pci_free_irq_vectors?

pci_alloc_irq_vectors()

	pci_alloc_irq_vectors_affinity()
		__pci_enable_msix_range()
			pci_setup_msi_context()
				pcim_setup_msi_release()
					devm_add_action(pcim_msi_release)
							:
							pci_free_irq_vectors()
		__pci_enable_msi_range()
			pci_setup_msi_context()

Quoted 5 lines

Quoted 2 lines

Okay, it should be that pci_alloc_irq_vectors() provide auto free. not because pcim_enable_device()?

No, it is conditional on pci_is_managed(dev) which is true if pcim_enable_device() has been used.

I have not found any document said that,

* Managed pci_enable_device(). Device will automatically be disabled on
* driver detach.

which have not mentioned that it also controller free irqs.

I suggest commit message descript whole story or change PCI's document to descript it clearly. I think other person should have similar question if he have not closely follow PCI's change and go through PCI codes.

Frank

Quoted 72 lines

Quoted 66 lines

The below statement should be wrong.

"Do not call pci_free_irq_vectors() because it is unnecessary when the device is managed due to the use of pcim_enable_device()."

Frank

Quoted 58 lines

Quoted 48 lines

And move these small patch ahead in serise.

Frank

Quoted 43 lines

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:
New patch split from "i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances"

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +6 -3

index 8ade911e3835..0fd3587671e1 100644
@@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 
 	pci_set_master(pci);
 
+	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (ret < 0)
+		return ret;
+
 	memset(&res, 0, sizeof(res));
 
 	res[0].flags = IORESOURCE_MEM;

@@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	res[0].end = pci_resource_end(pci, 0);
 
 	res[1].flags = IORESOURCE_IRQ;
-	res[1].start = pci->irq;
-	res[1].end = pci->irq;
+	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)
--

2.51.0

Quoted 4 lines

Quoted 4 lines

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Quoted 31 lines · Frank Li

Quoted 29 lines · Adrian Hunter

Quoted 23 lines · Frank Li

Quoted 12 lines · Adrian Hunter

Quoted 9 lines · Frank Li

Quoted 3 lines · Adrian Hunter

Use pci_alloc_irq_vectors() to support all IRQ types. Do not call pci_free_irq_vectors() because it is unnecessary when the device is managed due to the use of pcim_enable_device().

pcim_enable_device() add action pcim_disable_device(), I have not see pci_free_irq_vectors() in pcim_disable_device().

Can you tell me where call pci_free_irq_vectors?

pci_alloc_irq_vectors()

Quoted 9 lines

	pci_alloc_irq_vectors_affinity()
		__pci_enable_msix_range()
			pci_setup_msi_context()
				pcim_setup_msi_release()
					devm_add_action(pcim_msi_release)
							:
							pci_free_irq_vectors()
		__pci_enable_msi_range()
			pci_setup_msi_context()

Quoted 2 lines

Okay, it should be that pci_alloc_irq_vectors() provide auto free. not because pcim_enable_device()?

No, it is conditional on pci_is_managed(dev) which is true if pcim_enable_device() has been used.

Quoted 6 lines

I have not found any document said that,

* Managed pci_enable_device(). Device will automatically be disabled on
* driver detach.

which have not mentioned that it also controller free irqs.

It is not a secret:

commit 5982a539cdcedcf1a87709c01125e1596eee2c9a

Author: Salah Triki <[email protected]>
Date:   Sat Jul 19 07:33:36 2025 +0100

accel/amdxdna: Delete pci_free_irq_vectors()

The device is managed so pci_free_irq_vectors() is called automatically no need to do it manually.

Reviewed-by Jacek Lawrynowicz <[email protected]>

Signed-off-by Salah Triki <[email protected]>

Signed-off-by Lizhi Hou <[email protected]>

Link https://lore.kernel.org/r/aHs8QAfUlFeNp7qL@pc open here

commit 604f7e7777d663033063886b6a5362d0e6092e3a

Author: Michał Winiarski <[email protected]>
Date:   Wed Nov 29 22:44:59 2023 +0100

drm/xe/irq: Don't call pci_free_irq_vectors

For devres managed devices, pci_alloc_irq_vectors is also managed (see pci_setup_msi_context for reference).

PCI device used by Xe is devres managed (it was enabled with pcim_enable_device), which means that calls to pci_free_irq_vectors are redundant and can be safely removed.

Signed-off-by Michał Winiarski <[email protected]>

Reviewed-by Matt Roper <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

Signed-off-by Rodrigo Vivi <[email protected]>

commit a264cee31f13ae3b8d32b3e53774759afa55361e

Author: Srinivas Pandruvada <[email protected]>
Date:   Wed Jun 19 10:21:07 2024 -0700

thermal: intel: int340x: Remove unnecessary calls to free irq

Remove calls to devm_free_irq() and pci_free_irq_vectors(). They will be called on driver release anyway.

Signed-off-by Srinivas Pandruvada <[email protected]>

Link https://patch.msgid.link/[email protected] open here

Signed-off-by Rafael J. Wysocki <[email protected]>

commit 5e74a4b3ec1816e3bbfd715d46ae29d2508079cb

Author: Christophe JAILLET <[email protected]>
Date:   Sun Jun 5 22:50:48 2022 +0200

stmmac: intel: Fix an error handling path in intel_eth_pci_probe()

When the managed API is used, there is no need to explicitly call pci_free_irq_vectors().

This looks to be a left-over from the commit in the Fixes tag. Only the .remove() function had been updated.

So remove this unused function call and update goto label accordingly.

Fixes 8accc467758e ("stmmac: intel: use managed PCI function on probe and resume")

Signed-off-by Christophe JAILLET <[email protected]>

Reviewed-by Wong Vee Khee <[email protected]>

Link https://lore.kernel.org/r/1ac9b6787b0db83b0095711882c55c77c8ea8da0.1654462241.git.christophe.jaillet@wanadoo.fr open here

Signed-off-by Paolo Abeni <[email protected]>

Quoted 79 lines

I suggest commit message descript whole story or change PCI's document to descript it clearly. I think other person should have similar question if he have not closely follow PCI's change and go through PCI codes.

Frank

Quoted 72 lines

Quoted 66 lines

The below statement should be wrong.

"Do not call pci_free_irq_vectors() because it is unnecessary when the device is managed due to the use of pcim_enable_device()."

Frank

Quoted 58 lines

Quoted 48 lines

And move these small patch ahead in serise.

Frank

Quoted 43 lines

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:
New patch split from "i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances"

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +6 -3

index 8ade911e3835..0fd3587671e1 100644
@@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 
 	pci_set_master(pci);
 
+	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (ret < 0)
+		return ret;
+
 	memset(&res, 0, sizeof(res));
 
 	res[0].flags = IORESOURCE_MEM;

@@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	res[0].end = pci_resource_end(pci, 0);
 
 	res[1].flags = IORESOURCE_IRQ;
-	res[1].start = pci->irq;
-	res[1].end = pci->irq;
+	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)
--

2.51.0

Quoted 4 lines

Quoted 4 lines

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Prepare for Multi-Bus instance support. Pass base_regs to child device.

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:
New patch split from "i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances"

 .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 39 ++++++++++---------
 1 file changed, 20 insertions(+), 19 deletions(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +20 -20

index 0fd3587671e1..0180d99a37e4 100644
@@ -14,12 +14,14 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/platform_data/mipi-i3c-hci.h>
 #include <linux/platform_device.h>
 #include <linux/pm_qos.h>
 
 struct mipi_i3c_hci_pci {
 	struct pci_dev *pci;
 	struct platform_device *pdev;
+	void __iomem *base;
 	const struct mipi_i3c_hci_pci_info *info;
 	void *private;
 };

@@ -32,7 +34,6 @@ struct mipi_i3c_hci_pci_info {
 static DEFINE_IDA(mipi_i3c_hci_pci_ida);
 
 #define INTEL_PRIV_OFFSET		0x2b0
-#define INTEL_PRIV_SIZE			0x28
 #define INTEL_RESETS			0x04
 #define INTEL_RESETS_RESET		BIT(0)
 #define INTEL_RESETS_RESET_DONE		BIT(1)

@@ -143,19 +144,12 @@ static void intel_reset(void __iomem *priv)
 	writel(INTEL_RESETS_RESET, priv + INTEL_RESETS);
 }
 
-static void __iomem *intel_priv(struct pci_dev *pci)
-{
-	resource_size_t base = pci_resource_start(pci, 0);
-
-	return devm_ioremap(&pci->dev, base + INTEL_PRIV_OFFSET, INTEL_PRIV_SIZE);
-}
-
 static int intel_i3c_init(struct mipi_i3c_hci_pci *hci)
 {
 	struct intel_host *host = devm_kzalloc(&hci->pci->dev, sizeof(*host), GFP_KERNEL);
-	void __iomem *priv = intel_priv(hci->pci);
+	void __iomem *priv = hci->base + INTEL_PRIV_OFFSET;
 
-	if (!host || !priv)
+	if (!host)
 		return -ENOMEM;
 
 	dma_set_mask_and_coherent(&hci->pci->dev, DMA_BIT_MASK(64));

@@ -187,8 +181,9 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
 static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 				  const struct pci_device_id *id)
 {
+	struct mipi_i3c_hci_platform_data pdata = {};
 	struct mipi_i3c_hci_pci *hci;
-	struct resource res[2];
+	struct resource res;
 	int dev_id, ret;
 
 	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);

@@ -203,19 +198,19 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 
 	pci_set_master(pci);
 
+	hci->base = pcim_iomap_region(pci, 0, pci_name(pci));
+	if (IS_ERR(hci->base))
+		return PTR_ERR(hci->base);
+
 	ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
 	if (ret < 0)
 		return ret;
 
 	memset(&res, 0, sizeof(res));
 
-	res[0].flags = IORESOURCE_MEM;
-	res[0].start = pci_resource_start(pci, 0);
-	res[0].end = pci_resource_end(pci, 0);
-
-	res[1].flags = IORESOURCE_IRQ;
-	res[1].start = pci_irq_vector(hci->pci, 0);
-	res[1].end = res[1].start;
+	res.flags = IORESOURCE_IRQ;
+	res.start = pci_irq_vector(hci->pci, 0);
+	res.end = res.start;
 
 	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
 	if (dev_id < 0)

@@ -228,7 +223,13 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	hci->pdev->dev.parent = &pci->dev;
 	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
 
-	ret = platform_device_add_resources(hci->pdev, res, ARRAY_SIZE(res));
+	ret = platform_device_add_resources(hci->pdev, &res, 1);
+	if (ret)
+		goto err;
+
+	pdata.base_regs = hci->base;
+
+	ret = platform_device_add_data(hci->pdev, &pdata, sizeof(pdata));
 	if (ret)
 		goto err;
 
--

2.51.0

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Prepare for Multi-Bus instance support. Convert to MFD driver but still support only 1 instance.

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:
New patch split from "i3c: mipi-i3c-hci-pci: Add support for Multi-Bus Instances"

 drivers/i3c/master/Kconfig                    |   1 +
 .../master/mipi-i3c-hci/mipi-i3c-hci-pci.c    | 100 ++++++++++--------
 2 files changed, 58 insertions(+), 43 deletions(-)

drivers/i3c/master/Kconfig +1 -0

index 82cf330778d5..2609f2b18e0a 100644
@@ -69,6 +69,7 @@ config MIPI_I3C_HCI_PCI
 	tristate "MIPI I3C Host Controller Interface PCI support"
 	depends on MIPI_I3C_HCI
 	depends on PCI
+	select MFD_CORE
 	help
 	  Support for MIPI I3C Host Controller Interface compatible hardware
 	  on the PCI bus.

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +57 -44

index 0180d99a37e4..68088967942b 100644
@@ -12,6 +12,7 @@
 #include <linux/idr.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
+#include <linux/mfd/core.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/platform_data/mipi-i3c-hci.h>

@@ -20,8 +21,8 @@
 
 struct mipi_i3c_hci_pci {
 	struct pci_dev *pci;
-	struct platform_device *pdev;
 	void __iomem *base;
+	int dev_id;
 	const struct mipi_i3c_hci_pci_info *info;
 	void *private;
 };

@@ -178,13 +179,59 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
 	.exit = intel_i3c_exit,
 };
 
+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,
+					struct mipi_i3c_hci_pci_cell_data *data,
+					struct mfd_cell *cell)
+{
+	data->pdata.base_regs = hci->base;
+
+	data->res = DEFINE_RES_IRQ(0);
+
+	cell->name = "mipi-i3c-hci";
+	cell->id = hci->dev_id;
+	cell->platform_data = &data->pdata;
+	cell->pdata_size = sizeof(data->pdata);
+	cell->num_resources = 1;
+	cell->resources = &data->res;
+}
+
+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);
+	int irq = pci_irq_vector(hci->pci, 0);
+	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;
+
+	mipi_i3c_hci_pci_setup_cell(hci, data, cells);
+
+	ret = mfd_add_devices(&hci->pci->dev, 0, cells, 1, NULL, irq, NULL);
+	if (ret)
+		goto err_free_ids;
+
+	return 0;
+
+err_free_ids:
+	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
+	return ret;
+}
+
 static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 				  const struct pci_device_id *id)
 {
-	struct mipi_i3c_hci_platform_data pdata = {};
 	struct mipi_i3c_hci_pci *hci;
-	struct resource res;
-	int dev_id, ret;
+	int ret;
 
 	hci = devm_kzalloc(&pci->dev, sizeof(*hci), GFP_KERNEL);
 	if (!hci)

@@ -206,41 +253,13 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 	if (ret < 0)
 		return ret;
 
-	memset(&res, 0, sizeof(res));
-
-	res.flags = IORESOURCE_IRQ;
-	res.start = pci_irq_vector(hci->pci, 0);
-	res.end = res.start;
-
-	dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
-	if (dev_id < 0)
-		return dev_id;
-
-	hci->pdev = platform_device_alloc("mipi-i3c-hci", dev_id);
-	if (!hci->pdev)
-		return -ENOMEM;
-
-	hci->pdev->dev.parent = &pci->dev;
-	device_set_node(&hci->pdev->dev, dev_fwnode(&pci->dev));
-
-	ret = platform_device_add_resources(hci->pdev, &res, 1);
-	if (ret)
-		goto err;
-
-	pdata.base_regs = hci->base;
+	hci->info = (const struct mipi_i3c_hci_pci_info *)id->driver_data;
 
-	ret = platform_device_add_data(hci->pdev, &pdata, sizeof(pdata));
+	ret = hci->info->init ? hci->info->init(hci) : 0;
 	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)
-			goto err;
-	}
+		return ret;
 
-	ret = platform_device_add(hci->pdev);
+	ret = mipi_i3c_hci_pci_add_instances(hci);
 	if (ret)
 		goto err_exit;
 

@@ -251,23 +270,18 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
 err_exit:
 	if (hci->info->exit)
 		hci->info->exit(hci);
-err:
-	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 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);
+	mfd_remove_devices(&pci->dev);
+	ida_free(&mipi_i3c_hci_pci_ida, hci->dev_id);
 }
 
 static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
--

2.51.0

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

A MIPI I3C Host Controller with the Multi-Bus Instance capability supports multiple I3C Buses (up to 15).

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).

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 <[email protected]>

---

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(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10 -1

index 68088967942b..de1f71763786 100644
@@ -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);

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10 -0

 };
 
+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);

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +4 -2

+}
+
 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";

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +11 -10

 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
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Quoted 2 lines · Adrian Hunter

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).

...

Quoted 3 lines

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.

Quoted 54 lines

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 <[email protected]>

---

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(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10 -1

index 68088967942b..de1f71763786 100644
@@ -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;
 };
 

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 190

Quoted 7 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +2 -0

 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.

Quoted 18 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +15 -0

 };
 
+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;
+	}
+

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 212

Quoted 5 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +5 -0

+	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.

Quoted 19 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +4 -2

+}
+
 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";

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 237

Quoted 10 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +3 -1

-	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)

Quoted 54 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +11 -10

 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
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Quoted 66 lines · Frank Li

Quoted 2 lines · Adrian Hunter

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).

...

Quoted 3 lines

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.

Quoted 54 lines

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 <[email protected]>

---

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(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10 -1

index 68088967942b..de1f71763786 100644
@@ -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;
 };
 

Quoted 9 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 190

Quoted 7 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +2 -0

 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

Quoted 19 lines

Quoted 19 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +15 -0

 };
 
+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;
+	}
+

Quoted 8 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 212

Quoted 5 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +5 -0

+	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()

Quoted 86 lines

Quoted 20 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +4 -2

+}
+
 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";

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 237

Quoted 10 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +3 -1

-	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)

Quoted 50 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +11 -10

 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

Quoted 4 lines

Quoted 4 lines

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Quoted 77 lines · Adrian Hunter

Quoted 66 lines · Frank Li

Quoted 2 lines · Adrian Hunter

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).

...

Quoted 3 lines

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.

Quoted 54 lines

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 <[email protected]>

---

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(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10 -1

index 68088967942b..de1f71763786 100644
@@ -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;
 };
 

Quoted 9 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 190

Quoted 7 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +2 -0

 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.

Quoted 1 line

Adding multiple instances is in a separate patch

Okay, commit message should mention it.

Quoted 28 lines

Quoted 27 lines

Quoted 14 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10 -0

 };
 
+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);

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 212

Quoted 10 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10 -0

+		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.

Quoted 6 lines

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.

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.

It should be better than total 15 instances doesn't.

Frank

Quoted 96 lines

Quoted 86 lines

Quoted 20 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +4 -2

+}
+
 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";

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 237

Quoted 10 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +3 -1

-	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)

Quoted 50 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +11 -10

 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

Quoted 4 lines

Quoted 4 lines

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Quoted 110 lines · Frank Li

Quoted 77 lines · Adrian Hunter

Quoted 66 lines · Frank Li

Quoted 2 lines · Adrian Hunter

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).

...

Quoted 3 lines

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.

Quoted 54 lines

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 <[email protected]>

---

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(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10 -1

index 68088967942b..de1f71763786 100644
@@ -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;
 };
 

Quoted 9 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 190

Quoted 7 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +2 -0

 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.

Quoted 1 line

Adding multiple instances is in a separate patch

Okay, commit message should mention it.

Quoted 28 lines

Quoted 27 lines

Quoted 14 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10 -0

 };
 
+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);

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 212

Quoted 10 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +10 -0

+		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.

Quoted 8 lines

Quoted 6 lines

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.

Quoted 1 line

Quoted 4 lines

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.

Quoted 3 lines

It should be better than total 15 instances doesn't.

Not necessarily. If whole probe fails, the user might try again and succeed.

Quoted 100 lines

Frank

Quoted 96 lines

Quoted 86 lines

Quoted 20 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +4 -2

+}
+
 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";

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c static void intel_i3c_exit(struct mipi_i3c_hci_pci *hci) · line 237

Quoted 10 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +3 -1

-	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)

Quoted 50 lines

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +11 -10

 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

Quoted 4 lines

Quoted 4 lines

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Define Multi-Bus Instances at offset 0x400 for some Intel controllers.

Signed-off-by Adrian Hunter <[email protected]>

---

Changes in V2:
Also define instance 0 in driver_data

 .../i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c  | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +12 -6

index de1f71763786..9937718dc069 100644
@@ -190,6 +190,13 @@ static const struct mipi_i3c_hci_pci_info intel_info = {
 	.instance_count = 1,
 };
 
+static const struct mipi_i3c_hci_pci_info intel_mi_info = {
+	.init = intel_i3c_init,
+	.exit = intel_i3c_exit,
+	.instance_offset = {0, 0x400},
+	.instance_count = 2,
+};
+
 static void mipi_i3c_hci_pci_free_ids(struct mipi_i3c_hci_pci *hci)
 {
 	for (int i = 0; i < hci->dev_id_cnt; i++)

@@ -323,17 +330,17 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
 
 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, 0x4d7c), (kernel_ulong_t)&intel_mi_info},
 	{ PCI_VDEVICE(INTEL, 0x4d6f), (kernel_ulong_t)&intel_info},
 	/* Panther Lake-H */
-	{ PCI_VDEVICE(INTEL, 0xe37c), (kernel_ulong_t)&intel_info},
+	{ PCI_VDEVICE(INTEL, 0xe37c), (kernel_ulong_t)&intel_mi_info},
 	{ PCI_VDEVICE(INTEL, 0xe36f), (kernel_ulong_t)&intel_info},
 	/* Panther Lake-P */
-	{ PCI_VDEVICE(INTEL, 0xe47c), (kernel_ulong_t)&intel_info},
+	{ PCI_VDEVICE(INTEL, 0xe47c), (kernel_ulong_t)&intel_mi_info},
 	{ PCI_VDEVICE(INTEL, 0xe46f), (kernel_ulong_t)&intel_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_mi_info},
+	{ PCI_VDEVICE(INTEL, 0x6e2d), (kernel_ulong_t)&intel_mi_info},
 	{ },
 };
 MODULE_DEVICE_TABLE(pci, mipi_i3c_hci_pci_devices);
--

2.51.0

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c