| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0jPJqpX5xJX0j5OC21sp093O_B+jsL1mSBztR1wORV0Fg@mail.gmail.com>
Date: Thu, 23 Oct 2025 16:59:59 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Sławomir Rosek <srosek@...gle.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Alex Hung <alexhung@...il.com>,
Hans de Goede <hansg@...nel.org>, Ilpo Jarvinen <ilpo.jarvinen@...ux.intel.com>,
AceLan Kao <acelan.kao@...onical.com>, Daniel Lezcano <daniel.lezcano@...aro.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Zhang Rui <rui.zhang@...el.com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>, Tomasz Nowicki <tnowicki@...gle.com>,
Stanislaw Kardach <skardach@...gle.com>, Michal Krawczyk <mikrawczyk@...gle.com>,
linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
platform-driver-x86@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v3 1/6] ACPI: DPTF: Ignore SoC DTS thermal while scanning
On Thu, Oct 23, 2025 at 4:36 PM Sławomir Rosek <srosek@...gle.com> wrote:
>
> On Wed, Oct 22, 2025 at 8:33 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
> >
> > On Thu, Oct 2, 2025 at 1:34 PM Slawomir Rosek <srosek@...gle.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@...gle.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
> > > --
>
Powered by blists - more mailing lists