diff mbox series

[3/7] thermal: int340x: processor_thermal: Use non MSI interrupts

Message ID 20230620230150.3068704-4-srinivas.pandruvada@linux.intel.com
State Superseded
Headers show
Series thermal: processor_thermal: Suport workload hint | expand

Commit Message

Srinivas Pandruvada June 20, 2023, 11:01 p.m. UTC
There are issues in using MSI interrupts for processor thermal device.
The support is not consistent, across generations. Even in the same
generation, there are issue in getting interrupts via MSI.

Hence always use legacy PCI interrupts by default, instead of MSI.
Add a module param to use of MSI, so that MSI can be still used.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 .../processor_thermal_device_pci.c            | 33 ++++++++++++-------
 1 file changed, 22 insertions(+), 11 deletions(-)

Comments

Zhang Rui June 21, 2023, 2:53 p.m. UTC | #1
On Tue, 2023-06-20 at 16:01 -0700, Srinivas Pandruvada wrote:
> There are issues in using MSI interrupts for processor thermal
> device.
> The support is not consistent, across generations. Even in the same
> generation, there are issue in getting interrupts via MSI.
> 
> Hence always use legacy PCI interrupts by default, instead of MSI.
> Add a module param to use of MSI, so that MSI can be still used.
> 
> Signed-off-by: Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com>
> ---
>  .../processor_thermal_device_pci.c            | 33 ++++++++++++-----
> --
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git
> a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.
> c
> b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.
> c
> index 5a2bcfff0a68..057778f7bece 100644
> ---
> a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.
> c
> +++
> b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.
> c
> @@ -15,6 +15,11 @@
>  
>  #define DRV_NAME "proc_thermal_pci"
>  
> +static int msi_enabled;
> +module_param(msi_enabled, int, 0644);

why not use

static bool msi_enabled;
module_params(msi_enabled, bool, 0644);

thanks,
rui

> +MODULE_PARM_DESC(msi_enabled,
> +       "Use PCI MSI based interrupts for processor thermal
> device.");
> +
>  struct proc_thermal_pci {
>         struct pci_dev *pdev;
>         struct proc_thermal_device *proc_priv;
> @@ -219,8 +224,6 @@ static int proc_thermal_pci_probe(struct pci_dev
> *pdev, const struct pci_device_
>                 return ret;
>         }
>  
> -       pci_set_master(pdev);
> -
>         INIT_DELAYED_WORK(&pci_info->work,
> proc_thermal_threshold_work_fn);
>  
>         ret = proc_thermal_add(&pdev->dev, proc_priv);
> @@ -248,16 +251,23 @@ static int proc_thermal_pci_probe(struct
> pci_dev *pdev, const struct pci_device_
>                 goto err_ret_mmio;
>         }
>  
> -       /* request and enable interrupt */
> -       ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
> -       if (ret < 0) {
> -               dev_err(&pdev->dev, "Failed to allocate vectors!\n");
> -               goto err_ret_tzone;
> -       }
> -       if (!pdev->msi_enabled && !pdev->msix_enabled)
> +       if (msi_enabled) {
> +               pci_set_master(pdev);
> +               /* request and enable interrupt */
> +               ret = pci_alloc_irq_vectors(pdev, 1, 1,
> PCI_IRQ_ALL_TYPES);
> +               if (ret < 0) {
> +                       dev_err(&pdev->dev, "Failed to allocate
> vectors!\n");
> +                       goto err_ret_tzone;
> +               }
> +               if (!pdev->msi_enabled && !pdev->msix_enabled)
> +                       irq_flag = IRQF_SHARED;
> +
> +               irq =  pci_irq_vector(pdev, 0);
> +       } else {
>                 irq_flag = IRQF_SHARED;
> +               irq = pdev->irq;
> +       }
>  
> -       irq =  pci_irq_vector(pdev, 0);
>         ret = devm_request_threaded_irq(&pdev->dev, irq,
>                                         proc_thermal_irq_handler,
> NULL,
>                                         irq_flag, KBUILD_MODNAME,
> pci_info);
> @@ -273,7 +283,8 @@ static int proc_thermal_pci_probe(struct pci_dev
> *pdev, const struct pci_device_
>         return 0;
>  
>  err_free_vectors:
> -       pci_free_irq_vectors(pdev);
> +       if (msi_enabled)
> +               pci_free_irq_vectors(pdev);
>  err_ret_tzone:
>         thermal_zone_device_unregister(pci_info->tzone);
>  err_ret_mmio:
Srinivas Pandruvada June 21, 2023, 3:02 p.m. UTC | #2
On Wed, 2023-06-21 at 14:53 +0000, Zhang, Rui wrote:
> On Tue, 2023-06-20 at 16:01 -0700, Srinivas Pandruvada wrote:
> > There are issues in using MSI interrupts for processor thermal
> > device.
> > The support is not consistent, across generations. Even in the same
> > generation, there are issue in getting interrupts via MSI.
> > 
> > Hence always use legacy PCI interrupts by default, instead of MSI.
> > Add a module param to use of MSI, so that MSI can be still used.
> > 
> > Signed-off-by: Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com>
> > ---
> >  .../processor_thermal_device_pci.c            | 33 ++++++++++++---
> > --
> > --
> >  1 file changed, 22 insertions(+), 11 deletions(-)
> > 
> > diff --git
> > a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pc
> > i.
> > c
> > b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pc
> > i.
> > c
> > index 5a2bcfff0a68..057778f7bece 100644
> > ---
> > a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pc
> > i.
> > c
> > +++
> > b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pc
> > i.
> > c
> > @@ -15,6 +15,11 @@
> >  
> >  #define DRV_NAME "proc_thermal_pci"
> >  
> > +static int msi_enabled;
> > +module_param(msi_enabled, int, 0644);
> 
> why not use
> 
> static bool msi_enabled;
> module_params(msi_enabled, bool, 0644);
> 
Sure.

Thanks,
Srinivas

> thanks,
> rui
> 
> > +MODULE_PARM_DESC(msi_enabled,
> > +       "Use PCI MSI based interrupts for processor thermal
> > device.");
> > +
> >  struct proc_thermal_pci {
> >         struct pci_dev *pdev;
> >         struct proc_thermal_device *proc_priv;
> > @@ -219,8 +224,6 @@ static int proc_thermal_pci_probe(struct
> > pci_dev
> > *pdev, const struct pci_device_
> >                 return ret;
> >         }
> >  
> > -       pci_set_master(pdev);
> > -
> >         INIT_DELAYED_WORK(&pci_info->work,
> > proc_thermal_threshold_work_fn);
> >  
> >         ret = proc_thermal_add(&pdev->dev, proc_priv);
> > @@ -248,16 +251,23 @@ static int proc_thermal_pci_probe(struct
> > pci_dev *pdev, const struct pci_device_
> >                 goto err_ret_mmio;
> >         }
> >  
> > -       /* request and enable interrupt */
> > -       ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
> > -       if (ret < 0) {
> > -               dev_err(&pdev->dev, "Failed to allocate
> > vectors!\n");
> > -               goto err_ret_tzone;
> > -       }
> > -       if (!pdev->msi_enabled && !pdev->msix_enabled)
> > +       if (msi_enabled) {
> > +               pci_set_master(pdev);
> > +               /* request and enable interrupt */
> > +               ret = pci_alloc_irq_vectors(pdev, 1, 1,
> > PCI_IRQ_ALL_TYPES);
> > +               if (ret < 0) {
> > +                       dev_err(&pdev->dev, "Failed to allocate
> > vectors!\n");
> > +                       goto err_ret_tzone;
> > +               }
> > +               if (!pdev->msi_enabled && !pdev->msix_enabled)
> > +                       irq_flag = IRQF_SHARED;
> > +
> > +               irq =  pci_irq_vector(pdev, 0);
> > +       } else {
> >                 irq_flag = IRQF_SHARED;
> > +               irq = pdev->irq;
> > +       }
> >  
> > -       irq =  pci_irq_vector(pdev, 0);
> >         ret = devm_request_threaded_irq(&pdev->dev, irq,
> >                                         proc_thermal_irq_handler,
> > NULL,
> >                                         irq_flag, KBUILD_MODNAME,
> > pci_info);
> > @@ -273,7 +283,8 @@ static int proc_thermal_pci_probe(struct
> > pci_dev
> > *pdev, const struct pci_device_
> >         return 0;
> >  
> >  err_free_vectors:
> > -       pci_free_irq_vectors(pdev);
> > +       if (msi_enabled)
> > +               pci_free_irq_vectors(pdev);
> >  err_ret_tzone:
> >         thermal_zone_device_unregister(pci_info->tzone);
> >  err_ret_mmio:
>
diff mbox series

Patch

diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
index 5a2bcfff0a68..057778f7bece 100644
--- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
+++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
@@ -15,6 +15,11 @@ 
 
 #define DRV_NAME "proc_thermal_pci"
 
+static int msi_enabled;
+module_param(msi_enabled, int, 0644);
+MODULE_PARM_DESC(msi_enabled,
+	"Use PCI MSI based interrupts for processor thermal device.");
+
 struct proc_thermal_pci {
 	struct pci_dev *pdev;
 	struct proc_thermal_device *proc_priv;
@@ -219,8 +224,6 @@  static int proc_thermal_pci_probe(struct pci_dev *pdev, const struct pci_device_
 		return ret;
 	}
 
-	pci_set_master(pdev);
-
 	INIT_DELAYED_WORK(&pci_info->work, proc_thermal_threshold_work_fn);
 
 	ret = proc_thermal_add(&pdev->dev, proc_priv);
@@ -248,16 +251,23 @@  static int proc_thermal_pci_probe(struct pci_dev *pdev, const struct pci_device_
 		goto err_ret_mmio;
 	}
 
-	/* request and enable interrupt */
-	ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to allocate vectors!\n");
-		goto err_ret_tzone;
-	}
-	if (!pdev->msi_enabled && !pdev->msix_enabled)
+	if (msi_enabled) {
+		pci_set_master(pdev);
+		/* request and enable interrupt */
+		ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "Failed to allocate vectors!\n");
+			goto err_ret_tzone;
+		}
+		if (!pdev->msi_enabled && !pdev->msix_enabled)
+			irq_flag = IRQF_SHARED;
+
+		irq =  pci_irq_vector(pdev, 0);
+	} else {
 		irq_flag = IRQF_SHARED;
+		irq = pdev->irq;
+	}
 
-	irq =  pci_irq_vector(pdev, 0);
 	ret = devm_request_threaded_irq(&pdev->dev, irq,
 					proc_thermal_irq_handler, NULL,
 					irq_flag, KBUILD_MODNAME, pci_info);
@@ -273,7 +283,8 @@  static int proc_thermal_pci_probe(struct pci_dev *pdev, const struct pci_device_
 	return 0;
 
 err_free_vectors:
-	pci_free_irq_vectors(pdev);
+	if (msi_enabled)
+		pci_free_irq_vectors(pdev);
 err_ret_tzone:
 	thermal_zone_device_unregister(pci_info->tzone);
 err_ret_mmio: