[PATCH] char: xillybus: Replace deprecated MSI API

[PATCH] char: xillybus: Replace deprecated MSI API

[Salah Triki]

Replace deprecated pci_enable_msi() with pci_alloc_irq_vectors(). And
add pci_free_irq_vectors() to free the allocated vectors before removing
the device.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
 drivers/char/xillybus/xillybus_pcie.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/char/xillybus/xillybus_pcie.c b/drivers/char/xillybus/xillybus_pcie.c
index 9858711e3e79..fc7bffbd36ed 100644
--- a/drivers/char/xillybus/xillybus_pcie.c
+++ b/drivers/char/xillybus/xillybus_pcie.c
@@ -76,7 +76,8 @@ static int xilly_probe(struct pci_dev *pdev,
 	pci_set_master(pdev);
 
 	/* Set up a single MSI interrupt */
-	if (pci_enable_msi(pdev)) {
+	rc = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (rc < 0) {
 		dev_err(endpoint->dev,
 			"Failed to enable MSI interrupts. Aborting.\n");
 		return -ENODEV;
@@ -112,6 +113,8 @@ static void xilly_remove(struct pci_dev *pdev)
 {
 	struct xilly_endpoint *endpoint = pci_get_drvdata(pdev);
 
+	pci_free_irq_vectors(pdev);
+
 	xillybus_endpoint_remove(endpoint);
 }
 
-- 
2.43.0

Re: [PATCH] char: xillybus: Replace deprecated MSI API

[Eli Billauer]

Hello,

Thanks for your patch.

I can see that you've chosen PCI_IRQ_ALL_TYPES as the selector of IRQ 
types. As far as I understand, this changes the behavior of the driver, 
allowing interrupt types that it shouldn't...?

Another thing: I have to admit that I missed the need for a call to 
pci_disable_msi() when the device shuts down (which is what 
pci_free_irq_vectors() effectively does). Thanks for bringing that to my 
attention.

However, as this is needed, it must be assured that this call is made in 
any case that the device's initialization fails. This driver is using 
the managed device API, so I suppose the correct way is to add an devm 
action for pci_free_irq_vectors(). It appears to me that just adding it 
to the remove method won't be sufficient.

Regards,
    Eli

On 16/07/2025 21:39, Salah Triki wrote:
> Replace deprecated pci_enable_msi() with pci_alloc_irq_vectors(). And
> add pci_free_irq_vectors() to free the allocated vectors before removing
> the device.
> 
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
>   drivers/char/xillybus/xillybus_pcie.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/xillybus/xillybus_pcie.c b/drivers/char/xillybus/xillybus_pcie.c
> index 9858711e3e79..fc7bffbd36ed 100644
> --- a/drivers/char/xillybus/xillybus_pcie.c
> +++ b/drivers/char/xillybus/xillybus_pcie.c
> @@ -76,7 +76,8 @@ static int xilly_probe(struct pci_dev *pdev,
>   	pci_set_master(pdev);
>   
>   	/* Set up a single MSI interrupt */
> -	if (pci_enable_msi(pdev)) {
> +	rc = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
> +	if (rc < 0) {
>   		dev_err(endpoint->dev,
>   			"Failed to enable MSI interrupts. Aborting.\n");
>   		return -ENODEV;
> @@ -112,6 +113,8 @@ static void xilly_remove(struct pci_dev *pdev)
>   {
>   	struct xilly_endpoint *endpoint = pci_get_drvdata(pdev);
>   
> +	pci_free_irq_vectors(pdev);
> +
>   	xillybus_endpoint_remove(endpoint);
>   }
>   

[PATCH v2] char: xillybus: Replace deprecated MSI API

[Salah Triki]

Replace deprecated pci_enable_msi() with pci_alloc_irq_vectors(). And
add devm action to free irq vectors.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
Changes in v2:
    - Replace PCI_IRQ_ALL_TYPES with PCI_IRQ_MSI
    - Delete pci_free_irq_vectors(pdev) in remove function
    - Add devm action that calls pci_free_irq_vectors(pdev)

 drivers/char/xillybus/xillybus_pcie.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/char/xillybus/xillybus_pcie.c b/drivers/char/xillybus/xillybus_pcie.c
index 9858711e3e79..ed3b77cb8127 100644
--- a/drivers/char/xillybus/xillybus_pcie.c
+++ b/drivers/char/xillybus/xillybus_pcie.c
@@ -32,6 +32,11 @@ static const struct pci_device_id xillyids[] = {
 	{ /* End: all zeroes */ }
 };
 
+static void xilly_pci_free_irq_vectors(void *data)
+{
+	pci_free_irq_vectors(data);
+}
+
 static int xilly_probe(struct pci_dev *pdev,
 		       const struct pci_device_id *ent)
 {
@@ -76,11 +81,21 @@ static int xilly_probe(struct pci_dev *pdev,
 	pci_set_master(pdev);
 
 	/* Set up a single MSI interrupt */
-	if (pci_enable_msi(pdev)) {
+	rc = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
+	if (rc < 0) {
 		dev_err(endpoint->dev,
 			"Failed to enable MSI interrupts. Aborting.\n");
 		return -ENODEV;
 	}
+	
+	rc = devm_add_action(&pdev->dev, xilly_pci_free_irq_vectors, pdev);
+       	if (rc) {
+		dev_err(endpoint->dev,
+			"Failed to add devm action. Aborting.\n");
+		pci_free_irq_vectors(pdev);
+		return -ENODEV;
+	}
+
 	rc = devm_request_irq(&pdev->dev, pdev->irq, xillybus_isr, 0,
 			      xillyname, endpoint);
 	if (rc) {
-- 
2.43.0

[PATCH v3] char: xillybus: Replace deprecated MSI API

[Salah Triki]

Replace deprecated pci_enable_msi() with pci_alloc_irq_vectors(). And
add devm action to free irq vectors.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
Changes in v3:
    - Some checkpatch cleanups

Changes in v2:
    - Replace PCI_IRQ_ALL_TYPES with PCI_IRQ_MSI
    - Delete pci_free_irq_vectors(pdev) in remove function
    - Add devm action that calls pci_free_irq_vectors(pdev)

 drivers/char/xillybus/xillybus_pcie.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/char/xillybus/xillybus_pcie.c b/drivers/char/xillybus/xillybus_pcie.c
index 9858711e3e79..373b3ccd2e8f 100644
--- a/drivers/char/xillybus/xillybus_pcie.c
+++ b/drivers/char/xillybus/xillybus_pcie.c
@@ -32,6 +32,11 @@ static const struct pci_device_id xillyids[] = {
 	{ /* End: all zeroes */ }
 };
 
+static void xilly_pci_free_irq_vectors(void *data)
+{
+	pci_free_irq_vectors(data);
+}
+
 static int xilly_probe(struct pci_dev *pdev,
 		       const struct pci_device_id *ent)
 {
@@ -76,11 +81,21 @@ static int xilly_probe(struct pci_dev *pdev,
 	pci_set_master(pdev);
 
 	/* Set up a single MSI interrupt */
-	if (pci_enable_msi(pdev)) {
+	rc = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
+	if (rc < 0) {
 		dev_err(endpoint->dev,
 			"Failed to enable MSI interrupts. Aborting.\n");
 		return -ENODEV;
 	}
+
+	rc = devm_add_action(&pdev->dev, xilly_pci_free_irq_vectors, pdev);
+	if (rc) {
+		dev_err(endpoint->dev,
+			"Failed to add devm action. Aborting.\n");
+		pci_free_irq_vectors(pdev);
+		return -ENODEV;
+	}
+
 	rc = devm_request_irq(&pdev->dev, pdev->irq, xillybus_isr, 0,
 			      xillyname, endpoint);
 	if (rc) {
-- 
2.43.0

Re: [PATCH v3] char: xillybus: Replace deprecated MSI API

[Eli Billauer]

Hello,

On 18/07/2025 2:55, Salah Triki wrote:

> +static void xilly_pci_free_irq_vectors(void *data)
> +{
> +	pci_free_irq_vectors(data);
> +}
> +

I'm sorry, but I have mislead you: The call to pci_free_irq_vectors() is 
done automatically by the kernel's own pcim_msi_release() function if 
the device is managed, which it is. So the driver is functionally 
correct in this matter as is. The only remaining issue is the deprecated 
API.

I discovered this using ftrace on the relevant functions in the kernel, 
as I was preparing for testing your patch against hardware.

Regards,
   Eli

[PATCH v4] char: xillybus: Replace deprecated MSI API

[Salah Triki]

Replace deprecated pci_enable_msi() with pci_alloc_irq_vectors().

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
Changes in v4:
   - Drop devm_add_action() since it is useless

Changes in v3:
    - Some checkpatch cleanups

Changes in v2:
    - Replace PCI_IRQ_ALL_TYPES with PCI_IRQ_MSI
    - Delete pci_free_irq_vectors(pdev) in remove function
    - Add devm action that calls pci_free_irq_vectors(pdev)

 drivers/char/xillybus/xillybus_pcie.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/char/xillybus/xillybus_pcie.c b/drivers/char/xillybus/xillybus_pcie.c
index 9858711e3e79..c8b4cdfe695a 100644
--- a/drivers/char/xillybus/xillybus_pcie.c
+++ b/drivers/char/xillybus/xillybus_pcie.c
@@ -76,7 +76,8 @@ static int xilly_probe(struct pci_dev *pdev,
 	pci_set_master(pdev);
 
 	/* Set up a single MSI interrupt */
-	if (pci_enable_msi(pdev)) {
+	rc = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
+	if (rc < 0) {
 		dev_err(endpoint->dev,
 			"Failed to enable MSI interrupts. Aborting.\n");
 		return -ENODEV;
-- 
2.43.0

Re: [PATCH v4] char: xillybus: Replace deprecated MSI API

[Eli Billauer]

Thanks, and sorry for the back-and-forth.

Acked-by: Eli Billauer <eli.billauer@gmail.com>

On 19/07/2025 6:51, Salah Triki wrote:
> Replace deprecated pci_enable_msi() with pci_alloc_irq_vectors().
> 
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
> Changes in v4:
>     - Drop devm_add_action() since it is useless
> 
> Changes in v3:
>      - Some checkpatch cleanups
> 
> Changes in v2:
>      - Replace PCI_IRQ_ALL_TYPES with PCI_IRQ_MSI
>      - Delete pci_free_irq_vectors(pdev) in remove function
>      - Add devm action that calls pci_free_irq_vectors(pdev)
> 
>   drivers/char/xillybus/xillybus_pcie.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/xillybus/xillybus_pcie.c b/drivers/char/xillybus/xillybus_pcie.c
> index 9858711e3e79..c8b4cdfe695a 100644
> --- a/drivers/char/xillybus/xillybus_pcie.c
> +++ b/drivers/char/xillybus/xillybus_pcie.c
> @@ -76,7 +76,8 @@ static int xilly_probe(struct pci_dev *pdev,
>   	pci_set_master(pdev);
>   
>   	/* Set up a single MSI interrupt */
> -	if (pci_enable_msi(pdev)) {
> +	rc = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
> +	if (rc < 0) {
>   		dev_err(endpoint->dev,
>   			"Failed to enable MSI interrupts. Aborting.\n");
>   		return -ENODEV;

Re: [PATCH v4] char: xillybus: Replace deprecated MSI API

[Salah Triki]

You are welcome

Best regards,
Salah Triki

Re: [PATCH v4] char: xillybus: Replace deprecated MSI API

[Greg Kroah-Hartman]

On Sat, Jul 19, 2025 at 05:51:36AM +0100, Salah Triki wrote:
> Replace deprecated pci_enable_msi() with pci_alloc_irq_vectors().

This says what you are doing, but not _WHY_ you are doing it.

If this was a simple search/replace, why would it not have been done
already?  Are you _SURE_ this is correct, and if so, why?  You need to
prove it here...

thanks,

greg k-h

Re: [PATCH v4] char: xillybus: Replace deprecated MSI API

[Salah Triki]

On Sun, Jul 20, 2025 at 10:33:47AM +0200, Greg Kroah-Hartman wrote:
> 
> This says what you are doing, but not _WHY_ you are doing it.
> 

I did the replacement because pci_enable_msi() is deprecated, isn't that
enough ?

Best regards,
Salah Triki

Re: [PATCH v4] char: xillybus: Replace deprecated MSI API

[Greg Kroah-Hartman]

On Sun, Jul 20, 2025 at 09:56:29AM +0100, Salah Triki wrote:
> On Sun, Jul 20, 2025 at 10:33:47AM +0200, Greg Kroah-Hartman wrote:
> > 
> > This says what you are doing, but not _WHY_ you are doing it.
> > 
> 
> I did the replacement because pci_enable_msi() is deprecated, isn't that
> enough ?

If it was a simple search/replace, why wouldn't have been done already?

Again, you need to prove why this is ok at all.  pci_enable_msi()
shouldn't be used in new drivers, but what's wrong with it being in
existing drivers?  Especially in ones that you can't test to verify it
still works after changing the code?

thanks,

greg k-h

Re: [PATCH v4] char: xillybus: Replace deprecated MSI API

[Salah Triki]

Do you agree that for someone who wants to contribute to the kernel by doing
cleanups, it is better to do these cleanups on a part that is not directly
related to the drivers, because it is necessary to have hardware to test the
changes.

Best regards,
Salah Triki

Re: [PATCH v4] char: xillybus: Replace deprecated MSI API

[Greg Kroah-Hartman]

On Sun, Jul 20, 2025 at 06:03:50PM +0100, Salah Triki wrote:
> Do you agree that for someone who wants to contribute to the kernel by doing
> cleanups, it is better to do these cleanups on a part that is not directly
> related to the drivers, because it is necessary to have hardware to test the
> changes.

I'm sorry, but I have no context here for what you are asking about.

Please always quote emails properly, and realize that some of us get
hundreds, if not thousands, of emails a day to deal with.

thanks,

greg k-h