[PATCH v2 07/11] drivers: thermal: Hide PCI driver when CONFIG_PCI is unset

[PATCH v2 07/11] drivers: thermal: Hide PCI driver when CONFIG_PCI is unset

[Sinan Kaya]

This driver is both a platform and PCI driver. Hide PCI specific pieces
when CONFIG_PCI is unset.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 .../intel/int340x_thermal/processor_thermal_device.c      | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
index 284cf2c5a8fd..b84a475a1162 100644
--- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
+++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
@@ -374,6 +374,7 @@ static int int3401_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PCI
 static irqreturn_t proc_thermal_pci_msi_irq(int irq, void *devid)
 {
 	struct proc_thermal_device *proc_priv;
@@ -482,6 +483,7 @@ static struct pci_driver proc_thermal_pci_driver = {
 	.remove		= proc_thermal_pci_remove,
 	.id_table	= proc_thermal_pci_ids,
 };
+#endif
 
 static const struct acpi_device_id int3401_device_ids[] = {
 	{"INT3401", 0},
@@ -505,16 +507,18 @@ static int __init proc_thermal_init(void)
 	ret = platform_driver_register(&int3401_driver);
 	if (ret)
 		return ret;
-
+#ifdef CONFIG_PCI
 	ret = pci_register_driver(&proc_thermal_pci_driver);
-
+#endif
 	return ret;
 }
 
 static void __exit proc_thermal_exit(void)
 {
 	platform_driver_unregister(&int3401_driver);
+#ifdef CONFIG_PCI
 	pci_unregister_driver(&proc_thermal_pci_driver);
+#endif
 }
 
 module_init(proc_thermal_init);
-- 
2.19.0

Re: [PATCH v2 07/11] drivers: thermal: Hide PCI driver when CONFIG_PCI is unset

[Rafael J. Wysocki]

On Sat, Dec 22, 2018 at 6:49 PM Sinan Kaya <okaya@kernel.org> wrote:
>
> This driver is both a platform and PCI driver. Hide PCI specific pieces
> when CONFIG_PCI is unset.
>
> Signed-off-by: Sinan Kaya <okaya@kernel.org>
> ---
>  .../intel/int340x_thermal/processor_thermal_device.c      | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> index 284cf2c5a8fd..b84a475a1162 100644
> --- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> +++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> @@ -374,6 +374,7 @@ static int int3401_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> +#ifdef CONFIG_PCI
>  static irqreturn_t proc_thermal_pci_msi_irq(int irq, void *devid)
>  {
>         struct proc_thermal_device *proc_priv;
> @@ -482,6 +483,7 @@ static struct pci_driver proc_thermal_pci_driver = {
>         .remove         = proc_thermal_pci_remove,
>         .id_table       = proc_thermal_pci_ids,
>  };
> +#endif
>
>  static const struct acpi_device_id int3401_device_ids[] = {
>         {"INT3401", 0},
> @@ -505,16 +507,18 @@ static int __init proc_thermal_init(void)
>         ret = platform_driver_register(&int3401_driver);
>         if (ret)
>                 return ret;
> -
> +#ifdef CONFIG_PCI
>         ret = pci_register_driver(&proc_thermal_pci_driver);
> -
> +#endif
>         return ret;
>  }
>
>  static void __exit proc_thermal_exit(void)
>  {
>         platform_driver_unregister(&int3401_driver);
> +#ifdef CONFIG_PCI
>         pci_unregister_driver(&proc_thermal_pci_driver);
> +#endif
>  }
>
>  module_init(proc_thermal_init);

Can you actually test this driver with your new #ifdef stuff and with
CONFIG_PCI unset?

Have you really test it in that configuration?

If not, you shouldn't even post this patch.

Please make the driver depend on PCI.

Re: [PATCH v2 07/11] drivers: thermal: Hide PCI driver when CONFIG_PCI is unset

[Sinan Kaya]

On 12/23/2018 10:48 PM, Rafael J. Wysocki wrote:
> Can you actually test this driver with your new #ifdef stuff and with
> CONFIG_PCI unset?
> 
> Have you really test it in that configuration?
> 

No, I haven't. Code was compile tested only.

> If not, you shouldn't even post this patch.
> 

I was hoping the maintainer to chime in. By looking at the code, it was
registering a platform and PCI driver. I assumed that it was generic code
that can be used with and without PCI.

> Please make the driver depend on PCI.

I can do that.

[PATCH v2 11/11] drivers: thermal: Make PCI dependency explicit

[Sinan Kaya]

IOSF_CORE depends on PCI. This was never mentioned.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 drivers/thermal/intel/int340x_thermal/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/intel/int340x_thermal/Kconfig b/drivers/thermal/intel/int340x_thermal/Kconfig
index 0582bd12a239..0ca908d12750 100644
--- a/drivers/thermal/intel/int340x_thermal/Kconfig
+++ b/drivers/thermal/intel/int340x_thermal/Kconfig
@@ -4,7 +4,7 @@
 
 config INT340X_THERMAL
 	tristate "ACPI INT340X thermal drivers"
-	depends on X86 && ACPI
+	depends on X86 && ACPI && PCI
 	select THERMAL_GOV_USER_SPACE
 	select ACPI_THERMAL_REL
 	select ACPI_FAN
-- 
2.19.0