[PATCH v3 1/6] ACPI: DPTF: Ignore SoC DTS thermal while scanning

Slawomir Rosek posted 6 patches 4 months, 1 week ago
[PATCH v3 1/6] ACPI: DPTF: Ignore SoC DTS thermal while scanning
Posted by Slawomir Rosek 4 months, 1 week ago
The Intel SoC DTS thermal driver on Baytrail platform uses IRQ 86 for
critical overheating notification. The IRQ 86 is described in the _CRS
control method of INT3401 device, thus Intel SoC DTS thermal driver
requires INT3401 device to be enumerated.

Since dependency on INT3401 device is unrelated to DPTF the IS_ENABLE()
macro is removed from ACPI DPTF INT340X scan handler, instead Kconfig
is updated to ensure proper enumeration of INT3401 device.

Fixes: 014d9d5d0cc1 ("ACPI/int340x_thermal: enumerate INT3401 for Intel SoC DTS thermal driver")
Signed-off-by: Slawomir Rosek <srosek@google.com>
---
 drivers/acpi/dptf/int340x_thermal.c | 7 +------
 drivers/thermal/intel/Kconfig       | 3 ++-
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/dptf/int340x_thermal.c b/drivers/acpi/dptf/int340x_thermal.c
index a222df059a16..947fe50c2ef6 100644
--- a/drivers/acpi/dptf/int340x_thermal.c
+++ b/drivers/acpi/dptf/int340x_thermal.c
@@ -11,10 +11,9 @@
 
 #include "../internal.h"
 
-#define INT3401_DEVICE 0X01
 static const struct acpi_device_id int340x_thermal_device_ids[] = {
 	{"INT3400"},
-	{"INT3401", INT3401_DEVICE},
+	{"INT3401"},
 	{"INT3402"},
 	{"INT3403"},
 	{"INT3404"},
@@ -76,10 +75,6 @@ static int int340x_thermal_handler_attach(struct acpi_device *adev,
 {
 	if (IS_ENABLED(CONFIG_INT340X_THERMAL))
 		acpi_create_platform_device(adev, NULL);
-	/* Intel SoC DTS thermal driver needs INT3401 to set IRQ descriptor */
-	else if (IS_ENABLED(CONFIG_INTEL_SOC_DTS_THERMAL) &&
-		 id->driver_data == INT3401_DEVICE)
-		acpi_create_platform_device(adev, NULL);
 	return 1;
 }
 
diff --git a/drivers/thermal/intel/Kconfig b/drivers/thermal/intel/Kconfig
index e0268fac7093..f9e275538e29 100644
--- a/drivers/thermal/intel/Kconfig
+++ b/drivers/thermal/intel/Kconfig
@@ -44,7 +44,8 @@ config INTEL_SOC_DTS_IOSF_CORE
 
 config INTEL_SOC_DTS_THERMAL
 	tristate "Intel SoCs DTS thermal driver"
-	depends on X86 && PCI && ACPI
+	depends on X86_64 && PCI && ACPI
+	select INT340X_THERMAL
 	select INTEL_SOC_DTS_IOSF_CORE
 	help
 	  Enable this to register Intel SoCs (e.g. Bay Trail) platform digital
-- 
2.51.0.618.g983fd99d29-goog
Re: [PATCH v3 1/6] ACPI: DPTF: Ignore SoC DTS thermal while scanning
Posted by Rafael J. Wysocki 3 months, 2 weeks ago
On Thu, Oct 2, 2025 at 1:34 PM Slawomir Rosek <srosek@google.com> wrote:
>
> The Intel SoC DTS thermal driver on Baytrail platform uses IRQ 86 for
> critical overheating notification. The IRQ 86 is described in the _CRS
> control method of INT3401 device, thus Intel SoC DTS thermal driver
> requires INT3401 device to be enumerated.

I don't think that the specific interrupt number is relevant here.  It
would be sufficient to say something like "The IRQ used by the Intel
SoC DTS thermal device for critical overheating notification is listed
in _CRS of device INT3401 which therefore needs to be enumerated for
Intel SoC DTS thermal to work."

> Since dependency on INT3401 device is unrelated to DPTF the IS_ENABLE()
> macro is removed from ACPI DPTF INT340X scan handler, instead Kconfig
> is updated to ensure proper enumeration of INT3401 device.

It is not entirely clear what happens in this patch after reading the
above paragraph.

I would rather continue the previous thought by saying that the
enumeration happens by binding the int3401_thermal driver to the
INT3401 platform device.  Thus CONFIG_INT340X_THERMAL is in fact
necessary for enumerating it, so checking CONFIG_INTEL_SOC_DTS_THERMAL
in int340x_thermal_handler_attach() is pointless and INT340X_THERMAL
may as well be selected by INTEL_SOC_DTS_THERMAL.

> Fixes: 014d9d5d0cc1 ("ACPI/int340x_thermal: enumerate INT3401 for Intel SoC DTS thermal driver")

Why do you want this tag to be added?

> Signed-off-by: Slawomir Rosek <srosek@google.com>
> ---
>  drivers/acpi/dptf/int340x_thermal.c | 7 +------
>  drivers/thermal/intel/Kconfig       | 3 ++-
>  2 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/dptf/int340x_thermal.c b/drivers/acpi/dptf/int340x_thermal.c
> index a222df059a16..947fe50c2ef6 100644
> --- a/drivers/acpi/dptf/int340x_thermal.c
> +++ b/drivers/acpi/dptf/int340x_thermal.c
> @@ -11,10 +11,9 @@
>
>  #include "../internal.h"
>
> -#define INT3401_DEVICE 0X01
>  static const struct acpi_device_id int340x_thermal_device_ids[] = {
>         {"INT3400"},
> -       {"INT3401", INT3401_DEVICE},
> +       {"INT3401"},
>         {"INT3402"},
>         {"INT3403"},
>         {"INT3404"},
> @@ -76,10 +75,6 @@ static int int340x_thermal_handler_attach(struct acpi_device *adev,
>  {
>         if (IS_ENABLED(CONFIG_INT340X_THERMAL))
>                 acpi_create_platform_device(adev, NULL);
> -       /* Intel SoC DTS thermal driver needs INT3401 to set IRQ descriptor */
> -       else if (IS_ENABLED(CONFIG_INTEL_SOC_DTS_THERMAL) &&
> -                id->driver_data == INT3401_DEVICE)
> -               acpi_create_platform_device(adev, NULL);
>         return 1;
>  }
>
> diff --git a/drivers/thermal/intel/Kconfig b/drivers/thermal/intel/Kconfig
> index e0268fac7093..f9e275538e29 100644
> --- a/drivers/thermal/intel/Kconfig
> +++ b/drivers/thermal/intel/Kconfig
> @@ -44,7 +44,8 @@ config INTEL_SOC_DTS_IOSF_CORE
>
>  config INTEL_SOC_DTS_THERMAL
>         tristate "Intel SoCs DTS thermal driver"
> -       depends on X86 && PCI && ACPI
> +       depends on X86_64 && PCI && ACPI

AFAICS NET needs to be added to the dependency list above or selecting
INT340X_THERMAL below may not actually cause it to be built.

> +       select INT340X_THERMAL
>         select INTEL_SOC_DTS_IOSF_CORE
>         help
>           Enable this to register Intel SoCs (e.g. Bay Trail) platform digital
> --
Re: [PATCH v3 1/6] ACPI: DPTF: Ignore SoC DTS thermal while scanning
Posted by Sławomir Rosek 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 8:33 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Oct 2, 2025 at 1:34 PM Slawomir Rosek <srosek@google.com> wrote:
> >
> > The Intel SoC DTS thermal driver on Baytrail platform uses IRQ 86 for
> > critical overheating notification. The IRQ 86 is described in the _CRS
> > control method of INT3401 device, thus Intel SoC DTS thermal driver
> > requires INT3401 device to be enumerated.
>
> I don't think that the specific interrupt number is relevant here.  It
> would be sufficient to say something like "The IRQ used by the Intel
> SoC DTS thermal device for critical overheating notification is listed
> in _CRS of device INT3401 which therefore needs to be enumerated for
> Intel SoC DTS thermal to work."

Above text is copied from the original change which is linked below.
I will rephrase it as you suggested.

>
> > Since dependency on INT3401 device is unrelated to DPTF the IS_ENABLE()
> > macro is removed from ACPI DPTF INT340X scan handler, instead Kconfig
> > is updated to ensure proper enumeration of INT3401 device.
>
> It is not entirely clear what happens in this patch after reading the
> above paragraph.
>
> I would rather continue the previous thought by saying that the
> enumeration happens by binding the int3401_thermal driver to the
> INT3401 platform device.  Thus CONFIG_INT340X_THERMAL is in fact
> necessary for enumerating it, so checking CONFIG_INTEL_SOC_DTS_THERMAL
> in int340x_thermal_handler_attach() is pointless and INT340X_THERMAL
> may as well be selected by INTEL_SOC_DTS_THERMAL.

Sure, I will rephrase it to clearly describe what happens here.

>
> > Fixes: 014d9d5d0cc1 ("ACPI/int340x_thermal: enumerate INT3401 for Intel SoC DTS thermal driver")
>
> Why do you want this tag to be added?

Just for context. The IS_ENABLE is added to the scan handler in 014d9d5d0cc1
and this change is about to fix that. I can remove the tag if it's not needed.

>
> > Signed-off-by: Slawomir Rosek <srosek@google.com>
> > ---
> >  drivers/acpi/dptf/int340x_thermal.c | 7 +------
> >  drivers/thermal/intel/Kconfig       | 3 ++-
> >  2 files changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/acpi/dptf/int340x_thermal.c b/drivers/acpi/dptf/int340x_thermal.c
> > index a222df059a16..947fe50c2ef6 100644
> > --- a/drivers/acpi/dptf/int340x_thermal.c
> > +++ b/drivers/acpi/dptf/int340x_thermal.c
> > @@ -11,10 +11,9 @@
> >
> >  #include "../internal.h"
> >
> > -#define INT3401_DEVICE 0X01
> >  static const struct acpi_device_id int340x_thermal_device_ids[] = {
> >         {"INT3400"},
> > -       {"INT3401", INT3401_DEVICE},
> > +       {"INT3401"},
> >         {"INT3402"},
> >         {"INT3403"},
> >         {"INT3404"},
> > @@ -76,10 +75,6 @@ static int int340x_thermal_handler_attach(struct acpi_device *adev,
> >  {
> >         if (IS_ENABLED(CONFIG_INT340X_THERMAL))
> >                 acpi_create_platform_device(adev, NULL);
> > -       /* Intel SoC DTS thermal driver needs INT3401 to set IRQ descriptor */
> > -       else if (IS_ENABLED(CONFIG_INTEL_SOC_DTS_THERMAL) &&
> > -                id->driver_data == INT3401_DEVICE)
> > -               acpi_create_platform_device(adev, NULL);
> >         return 1;
> >  }
> >
> > diff --git a/drivers/thermal/intel/Kconfig b/drivers/thermal/intel/Kconfig
> > index e0268fac7093..f9e275538e29 100644
> > --- a/drivers/thermal/intel/Kconfig
> > +++ b/drivers/thermal/intel/Kconfig
> > @@ -44,7 +44,8 @@ config INTEL_SOC_DTS_IOSF_CORE
> >
> >  config INTEL_SOC_DTS_THERMAL
> >         tristate "Intel SoCs DTS thermal driver"
> > -       depends on X86 && PCI && ACPI
> > +       depends on X86_64 && PCI && ACPI
>
> AFAICS NET needs to be added to the dependency list above or selecting
> INT340X_THERMAL below may not actually cause it to be built.

Right. Previously I tried to "select NET" (INTEL_SOC_DTS_THERMAL does not
depend on NET directly so it seemed to be more appropriate to me) but got
circular dependency error so I assumed it is ensured somehow. Now I checked
again and you are right, I was able to disable NET and get unmet dependencies.
I will add dependency on NET. Thanks

>
> > +       select INT340X_THERMAL
> >         select INTEL_SOC_DTS_IOSF_CORE
> >         help
> >           Enable this to register Intel SoCs (e.g. Bay Trail) platform digital
> > --
Re: [PATCH v3 1/6] ACPI: DPTF: Ignore SoC DTS thermal while scanning
Posted by Rafael J. Wysocki 3 months, 2 weeks ago
On Thu, Oct 23, 2025 at 4:36 PM Sławomir Rosek <srosek@google.com> wrote:
>
> On Wed, Oct 22, 2025 at 8:33 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Thu, Oct 2, 2025 at 1:34 PM Slawomir Rosek <srosek@google.com> wrote:
> > >
> > > The Intel SoC DTS thermal driver on Baytrail platform uses IRQ 86 for
> > > critical overheating notification. The IRQ 86 is described in the _CRS
> > > control method of INT3401 device, thus Intel SoC DTS thermal driver
> > > requires INT3401 device to be enumerated.
> >
> > I don't think that the specific interrupt number is relevant here.  It
> > would be sufficient to say something like "The IRQ used by the Intel
> > SoC DTS thermal device for critical overheating notification is listed
> > in _CRS of device INT3401 which therefore needs to be enumerated for
> > Intel SoC DTS thermal to work."
>
> Above text is copied from the original change which is linked below.
> I will rephrase it as you suggested.
>
> >
> > > Since dependency on INT3401 device is unrelated to DPTF the IS_ENABLE()
> > > macro is removed from ACPI DPTF INT340X scan handler, instead Kconfig
> > > is updated to ensure proper enumeration of INT3401 device.
> >
> > It is not entirely clear what happens in this patch after reading the
> > above paragraph.
> >
> > I would rather continue the previous thought by saying that the
> > enumeration happens by binding the int3401_thermal driver to the
> > INT3401 platform device.  Thus CONFIG_INT340X_THERMAL is in fact
> > necessary for enumerating it, so checking CONFIG_INTEL_SOC_DTS_THERMAL
> > in int340x_thermal_handler_attach() is pointless and INT340X_THERMAL
> > may as well be selected by INTEL_SOC_DTS_THERMAL.
>
> Sure, I will rephrase it to clearly describe what happens here.
>
> >
> > > Fixes: 014d9d5d0cc1 ("ACPI/int340x_thermal: enumerate INT3401 for Intel SoC DTS thermal driver")
> >
> > Why do you want this tag to be added?
>
> Just for context. The IS_ENABLE is added to the scan handler in 014d9d5d0cc1
> and this change is about to fix that. I can remove the tag if it's not needed.

I don't see a functional issue with this, it is just dead code AFAICS.
As a rule, Fixes: tags are not added to patches removing dead code.

> >
> > > Signed-off-by: Slawomir Rosek <srosek@google.com>
> > > ---
> > >  drivers/acpi/dptf/int340x_thermal.c | 7 +------
> > >  drivers/thermal/intel/Kconfig       | 3 ++-
> > >  2 files changed, 3 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/acpi/dptf/int340x_thermal.c b/drivers/acpi/dptf/int340x_thermal.c
> > > index a222df059a16..947fe50c2ef6 100644
> > > --- a/drivers/acpi/dptf/int340x_thermal.c
> > > +++ b/drivers/acpi/dptf/int340x_thermal.c
> > > @@ -11,10 +11,9 @@
> > >
> > >  #include "../internal.h"
> > >
> > > -#define INT3401_DEVICE 0X01
> > >  static const struct acpi_device_id int340x_thermal_device_ids[] = {
> > >         {"INT3400"},
> > > -       {"INT3401", INT3401_DEVICE},
> > > +       {"INT3401"},
> > >         {"INT3402"},
> > >         {"INT3403"},
> > >         {"INT3404"},
> > > @@ -76,10 +75,6 @@ static int int340x_thermal_handler_attach(struct acpi_device *adev,
> > >  {
> > >         if (IS_ENABLED(CONFIG_INT340X_THERMAL))
> > >                 acpi_create_platform_device(adev, NULL);
> > > -       /* Intel SoC DTS thermal driver needs INT3401 to set IRQ descriptor */
> > > -       else if (IS_ENABLED(CONFIG_INTEL_SOC_DTS_THERMAL) &&
> > > -                id->driver_data == INT3401_DEVICE)
> > > -               acpi_create_platform_device(adev, NULL);
> > >         return 1;
> > >  }
> > >
> > > diff --git a/drivers/thermal/intel/Kconfig b/drivers/thermal/intel/Kconfig
> > > index e0268fac7093..f9e275538e29 100644
> > > --- a/drivers/thermal/intel/Kconfig
> > > +++ b/drivers/thermal/intel/Kconfig
> > > @@ -44,7 +44,8 @@ config INTEL_SOC_DTS_IOSF_CORE
> > >
> > >  config INTEL_SOC_DTS_THERMAL
> > >         tristate "Intel SoCs DTS thermal driver"
> > > -       depends on X86 && PCI && ACPI
> > > +       depends on X86_64 && PCI && ACPI
> >
> > AFAICS NET needs to be added to the dependency list above or selecting
> > INT340X_THERMAL below may not actually cause it to be built.
>
> Right. Previously I tried to "select NET" (INTEL_SOC_DTS_THERMAL does not
> depend on NET directly so it seemed to be more appropriate to me) but got
> circular dependency error so I assumed it is ensured somehow. Now I checked
> again and you are right, I was able to disable NET and get unmet dependencies.
> I will add dependency on NET. Thanks

The rule of thumb is that if you want A to select B, then they both
need to depend on the same things.

> >
> > > +       select INT340X_THERMAL
> > >         select INTEL_SOC_DTS_IOSF_CORE
> > >         help
> > >           Enable this to register Intel SoCs (e.g. Bay Trail) platform digital
> > > --
>