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

[Zhang, Rui]

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@xxxxxxxxxxxxxxx>
> ---
>  .../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: