* Re: [PATCH] thermal: core: fix use-after-free due to init/cancel delayed_work race
From: Mauricio Faria de Oliveira @ 2026-03-25 14:28 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Daniel Lezcano, Zhang Rui, Lukasz Luba, linux-pm, linux-kernel,
kernel-dev, syzbot+3b3852c6031d0f30dfaf
In-Reply-To: <772a77c80b6ad216dec4cc10d3fbb133@igalia.com>
On 2026-03-25 11:17, Mauricio Faria de Oliveira wrote:
> Thanks for looking into this.
>
> On 2026-03-25 09:47, Rafael J. Wysocki wrote:
>> I can see the one between thermal_zone_device_unregister() and
>> thermal_zone_device_resume(), but that can be addressed by adding a
>> TZ_STATE_FLAG_EXIT check to the latter AFAICS.
>
Please disregard this paragraph; I incorrectly read/wrote _resume()
as thermal_zone_pm_complete() discussed above. The rest should be
right. I'll review this and get back shortly.
> In the example describe above and detailed below, apparently that
> is not sufficient, if I'm not missing anything. See, if _resume()
> is reached with thermal_list_lock held, thermal_zone_device_exit()
> is waiting for thermal_list_lock before setting TZ_STATE_FLAG_EXIT,
> thus a check for it in _resume() would find it clear yet.
--
Mauricio
^ permalink raw reply
* Re: [PATCH v4 0/5] mm: zone lock tracepoint instrumentation
From: Steven Rostedt @ 2026-03-25 14:19 UTC (permalink / raw)
To: Dmitry Ilvokhin
Cc: Andrew Morton, Matthew Wilcox, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Masami Hiramatsu, Mathieu Desnoyers, Rafael J. Wysocki,
Pavel Machek, Len Brown, Brendan Jackman, Johannes Weiner, Zi Yan,
Oscar Salvador, Qi Zheng, Shakeel Butt, linux-kernel, linux-mm,
linux-trace-kernel, linux-pm
In-Reply-To: <acPRq1YPeGR8EqMB@shell.ilvokhin.com>
On Wed, 25 Mar 2026 12:14:35 +0000
Dmitry Ilvokhin <d@ilvokhin.com> wrote:
> > Please send that v2 sometime and hopefully Steven can help push it along?
>
> I'll send the next version of the generic locking series soon. Any help
> in pushing it along would be appreciated.
I'll see what I can do when I see v2!
-- Steve
^ permalink raw reply
* Re: [PATCH] thermal: core: fix use-after-free due to init/cancel delayed_work race
From: Mauricio Faria de Oliveira @ 2026-03-25 14:17 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Daniel Lezcano, Zhang Rui, Lukasz Luba, linux-pm, linux-kernel,
kernel-dev, syzbot+3b3852c6031d0f30dfaf
In-Reply-To: <CAJZ5v0iBh6BjtwhS7RK_GE8QmYLAwW71P+YpA92SiGu4wy+syw@mail.gmail.com>
Thanks for looking into this.
On 2026-03-25 09:47, Rafael J. Wysocki wrote:
> On Wed, Mar 25, 2026 at 1:10 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>
>> On Wed, Mar 25, 2026 at 12:51 AM Mauricio Faria de Oliveira
>> <mfo@igalia.com> wrote:
>> >
>> > If INIT_DELAYED_WORK() is called for a currently running work item,
>> > cancel_delayed_work_sync() is unable to cancel/wait for it anymore,
>> > as the work item's data bits required for that are cleared.
>> >
>> > In the resume path, INIT_DELAYED_WORK() is called twice:
>> > 1) to replace the work function: thermal_zone_device_check/resume()
>> > 2) to restore it.
>> >
>> > Both cases might race with the unregister path and bypass the call to
>> > cancel_delayed_work_sync(),
>>
>> So this is the problem, isn't it?
>>
>> > after which struct thermal_zone_device *tz
>> > is freed, and the non-canceled/non-waited for work hits use-after-free.
>>
>> Which basically means that a TZ_STATE_FLAG_EXIT check is missing in
>> both thermal_zone_pm_complete() and thermal_zone_device_resume().
>
> Actually, thermal_zone_pm_complete() runs under thermal_list_lock and
> thermal_zone_device_unregister() removes the zone from
> thermal_tz_list, also under thermal_list_lock, before calling
> cancel_delayed_work_sync().
>
> So either thermal_zone_device_unregister() removes the zone from the
> list before thermal_zone_pm_complete() can run, in which case the
> latter won't run for the given zone at all because that zone is not
> there in thermal_tz_list, or the cancel_delayed_work_sync() will see
> the work item queued up by thermal_zone_pm_complete().
(I think this reply clarifies your previous points too; or tell me.)
Regarding the latter: yes, cancel_delayed_work_sync() will see the
work item queued up by thermal_zone_pm_complete(), but the issue is
it will not see the _previously queued up (and running)_ work item,
due to the INIT_DELAYED_WORK() in thermal_zone_pm_complete().
That work item, thermal_zone_device_check() might be past a check
for TZ_STATE_FLAG_EXIT by the time it is set. Please read on.
> So where's the race between thermal_zone_pm_complete() and
> thermal_zone_device_unregister()?
I'll append a more detailed view below, but the summary is:
thermal_pm_notify_complete() can acquire thermal_list_lock before
thermal_zone_device_unregister() -> thermal_zone_exit() does,
thus proceed to thermal_zone_pm_complete() which calls
INIT_DELAYED_WORK().
This makes future calls to cancel_delayed_work_sync() to wait only
for the newly initialized work function, not a previous one, which
might be running -- in this case, thermal_zone_device_check().
Then thermal_zone_pm_complete() and thermal_pm_notify_complete()
return, releasing thermal_list_lock.
Now, thermal_zone_device_unregister() -> thermal_zone_exit()
can set finally set TZ_STATE_FLAG_EXIT, but it might be too late.
The previously queued, currently running, work item might be past
any check for it (say, it acquired the lock to read it earlier,
or before thermal_zone_device_unregister() was even called).
So, thermal_zone_device_unregister() reaches cancel_delayed_work_sync(),
which waits for thermal_zone_device_resume() (new work function),
but _not_ for thermal_zone_device_check() (old work function).
That finishes, tz is freed, and now the old work funtion continues
to run and accesses tz -- hitting a use-after-free.
> I can see the one between thermal_zone_device_unregister() and
> thermal_zone_device_resume(), but that can be addressed by adding a
> TZ_STATE_FLAG_EXIT check to the latter AFAICS.
In the example describe above and detailed below, apparently that
is not sufficient, if I'm not missing anything. See, if _resume()
is reached with thermal_list_lock held, thermal_zone_device_exit()
is waiting for thermal_list_lock before setting TZ_STATE_FLAG_EXIT,
thus a check for it in _resume() would find it clear yet.
Hope this helps. Please let me know your thoughts. Thanks!
Detailed view)
Thread A:
...
WORK: tz->poll_queue
thermal_zone_device_check() // started running before
TZ_STATE_FLAG_EXIT could be set
... // wait a bit
Thread B:
thermal_pm_notify()
thermal_pm_notify_complete()
guard(mutex)(&thermal_list_lock) // acquires the lock
...
Thread C:
thermal_zone_device_unregister()
thermal_zone_exit()
guard(mutex)(&thermal_list_lock) // blocks waiting for the lock
...
tz->state |= TZ_STATE_FLAG_EXIT // not run yet
list_del_init(&tz->node) // not run yet
Thread B: // continues with tz in thermal_tz_list and without
TZ_STATE_FLAG_EXIT
...
list_for_each_entry(tz, &thermal_tz_list, node)
thermal_zone_pm_complete(tz)
cancel_delayed_work(&tz->poll_queue) // does not wait for
thermal_zone_device_check() to finish
INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_resume)
...
returns, releases the thermal_list_lock.
From now on, cancel_delayed_work_sync(&tz->poll_queue)
will _not_ wait for thermal_zone_device_check() anymore
(note it _is_ running in Thread A)
it will only wait for thermal_zone_device_resume().
Thread C:
tz->state |= TZ_STATE_FLAG_EXIT
list_del_init(&tz->node)
These are now set, but no longer effective for the already running
thermal_zone_device_check().
cancel_delayed_work_sync(&tz->poll_queue) // as mentioned, waits
for the other function.
kfree(tz)
Thread A: // not waited for
...
thermal_zone_device_update()
// access to tz (freed)
--
Mauricio
^ permalink raw reply
* [PATCH v2 5/5] regulator: sc2731: Add platform_device_id table
From: Otto Pflüger @ 2026-03-25 13:53 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger
In-Reply-To: <20260325-sc27xx-mfd-cells-v2-0-d0ebb60aa4a7@abscue.de>
Make the regulator driver for the SC2731 PMIC probe automatically. Using
a platform_device_id table instead of DT compatible matching avoids the
need for a separate compatible property in the "regulators" node, which
simplifies the DT bindings and makes the parent MFD device responsible
for selecting the correct regulator driver for the PMIC.
However, this means that the regulator device is not automatically
associated with the "regulators" node. Tell the regulator core to
perform device tree lookups using the parent MFD device instead of
the regulator sub-device and set the .regulators_node member in all
regulator definitions so that the "regulators" sub-node is used.
Acked-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
drivers/regulator/sc2731-regulator.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/sc2731-regulator.c b/drivers/regulator/sc2731-regulator.c
index 5447e1a47d15..93c8156c5110 100644
--- a/drivers/regulator/sc2731-regulator.c
+++ b/drivers/regulator/sc2731-regulator.c
@@ -131,6 +131,7 @@ static const struct regulator_ops sc2731_regu_linear_ops = {
vstep, vmin, vmax) { \
.name = #_id, \
.of_match = of_match_ptr(#_id), \
+ .regulators_node = of_match_ptr("regulators"), \
.ops = &sc2731_regu_linear_ops, \
.type = REGULATOR_VOLTAGE, \
.id = SC2731_##_id, \
@@ -226,7 +227,7 @@ static int sc2731_regulator_probe(struct platform_device *pdev)
return ret;
}
- config.dev = &pdev->dev;
+ config.dev = pdev->dev.parent;
config.regmap = regmap;
for (i = 0; i < ARRAY_SIZE(regulators); i++) {
@@ -242,12 +243,19 @@ static int sc2731_regulator_probe(struct platform_device *pdev)
return 0;
}
+static const struct platform_device_id sc2731_regulator_id_table[] = {
+ { "sc2731-regulator" },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, sc2731_regulator_id_table);
+
static struct platform_driver sc2731_regulator_driver = {
.driver = {
.name = "sc27xx-regulator",
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
},
.probe = sc2731_regulator_probe,
+ .id_table = sc2731_regulator_id_table,
};
module_platform_driver(sc2731_regulator_driver);
--
2.51.0
^ permalink raw reply related
* [PATCH v2 4/5] power: reset: sc27xx: Add platform_device_id table
From: Otto Pflüger @ 2026-03-25 13:53 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger, Sebastian Reichel
In-Reply-To: <20260325-sc27xx-mfd-cells-v2-0-d0ebb60aa4a7@abscue.de>
Make the poweroff driver for SC27xx-series PMICs probe automatically.
Since the device representing the poweroff functionality of the SC27xx
PMIC is not supposed to have a dedicated device tree node without any
corresponding DT resources [1], an of_device_id table cannot be used
here. Instead, use a platform_device_id table to match the poweroff
sub-device instantiated by the parent MFD driver.
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
[1]: https://lore.kernel.org/all/20251002025344.GA2958334-robh@kernel.org/
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/power/reset/sc27xx-poweroff.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/power/reset/sc27xx-poweroff.c b/drivers/power/reset/sc27xx-poweroff.c
index 393bd1c33b73..6376706bf561 100644
--- a/drivers/power/reset/sc27xx-poweroff.c
+++ b/drivers/power/reset/sc27xx-poweroff.c
@@ -6,6 +6,7 @@
#include <linux/cpu.h>
#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
@@ -70,11 +71,18 @@ static int sc27xx_poweroff_probe(struct platform_device *pdev)
return 0;
}
+static const struct platform_device_id sc27xx_poweroff_id_table[] = {
+ { "sc2731-poweroff" },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, sc27xx_poweroff_id_table);
+
static struct platform_driver sc27xx_poweroff_driver = {
.probe = sc27xx_poweroff_probe,
.driver = {
.name = "sc27xx-poweroff",
},
+ .id_table = sc27xx_poweroff_id_table,
};
module_platform_driver(sc27xx_poweroff_driver);
--
2.51.0
^ permalink raw reply related
* [PATCH v2 2/5] regulator: dt-bindings: sc2731: Deprecate compatible property
From: Otto Pflüger @ 2026-03-25 13:53 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger
In-Reply-To: <20260325-sc27xx-mfd-cells-v2-0-d0ebb60aa4a7@abscue.de>
The node containing the regulators is always a child of the main PMIC
node, which already has a compatible property identifying the type of
PMIC. This makes the compatible in the child node redundant. Mark it
as deprecated and remove it from the required property list and the
examples.
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Acked-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml | 2 --
.../devicetree/bindings/regulator/sprd,sc2731-regulator.yaml | 4 +---
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml b/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml
index b023e1ef8d3c..12b3258daef5 100644
--- a/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml
+++ b/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml
@@ -222,8 +222,6 @@ examples:
};
regulators {
- compatible = "sprd,sc2731-regulator";
-
BUCK_CPU0 {
regulator-name = "vddarm0";
regulator-min-microvolt = <400000>;
diff --git a/Documentation/devicetree/bindings/regulator/sprd,sc2731-regulator.yaml b/Documentation/devicetree/bindings/regulator/sprd,sc2731-regulator.yaml
index 9bd752bab68e..7af20a4781b7 100644
--- a/Documentation/devicetree/bindings/regulator/sprd,sc2731-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/sprd,sc2731-regulator.yaml
@@ -26,6 +26,7 @@ description: |
properties:
compatible:
+ deprecated: true
const: sprd,sc2731-regulator
patternProperties:
@@ -39,8 +40,5 @@ patternProperties:
$ref: regulator.yaml#
unevaluatedProperties: false
-required:
- - compatible
-
additionalProperties: false
...
--
2.51.0
^ permalink raw reply related
* [PATCH v2 3/5] mfd: sprd-sc27xx: Switch to devm_mfd_add_devices()
From: Otto Pflüger @ 2026-03-25 13:53 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger
In-Reply-To: <20260325-sc27xx-mfd-cells-v2-0-d0ebb60aa4a7@abscue.de>
To allow instantiating subdevices such as the regulator and poweroff
devices that do not have corresponding device tree nodes with a
"compatible" property, use devm_mfd_add_devices() with MFD cells instead
of devm_of_platform_populate(). Since different PMICs in the SC27xx
series contain different components, use separate MFD cell tables for
each PMIC model. Define cells for all components that have upstream
drivers at this point.
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
drivers/mfd/sprd-sc27xx-spi.c | 62 ++++++++++++++++++++++++++++++++++++-------
1 file changed, 53 insertions(+), 9 deletions(-)
diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
index d6b4350779e6..eb57023fdc3c 100644
--- a/drivers/mfd/sprd-sc27xx-spi.c
+++ b/drivers/mfd/sprd-sc27xx-spi.c
@@ -14,6 +14,11 @@
#include <linux/spi/spi.h>
#include <uapi/linux/usb/charger.h>
+enum sprd_pmic_type {
+ PMIC_TYPE_SC2730,
+ PMIC_TYPE_SC2731,
+};
+
#define SPRD_PMIC_INT_MASK_STATUS 0x0
#define SPRD_PMIC_INT_RAW_STATUS 0x4
#define SPRD_PMIC_INT_EN 0x8
@@ -50,6 +55,29 @@ struct sprd_pmic_data {
u32 charger_det;
};
+static const struct mfd_cell sc2730_devices[] = {
+ MFD_CELL_OF("sc2730-adc", NULL, NULL, 0, 0, "sprd,sc2730-adc"),
+ MFD_CELL_OF("sc2730-bltc", NULL, NULL, 0, 0, "sprd,sc2730-bltc"),
+ MFD_CELL_OF("sc2730-efuse", NULL, NULL, 0, 0, "sprd,sc2730-efuse"),
+ MFD_CELL_OF("sc2730-eic", NULL, NULL, 0, 0, "sprd,sc2730-eic"),
+ MFD_CELL_OF("sc2730-fgu", NULL, NULL, 0, 0, "sprd,sc2730-fgu"),
+ MFD_CELL_OF("sc2730-rtc", NULL, NULL, 0, 0, "sprd,sc2730-rtc"),
+ MFD_CELL_OF("sc2730-vibrator", NULL, NULL, 0, 0, "sprd,sc2730-vibrator"),
+};
+
+static const struct mfd_cell sc2731_devices[] = {
+ MFD_CELL_OF("sc2731-adc", NULL, NULL, 0, 0, "sprd,sc2731-adc"),
+ MFD_CELL_OF("sc2731-bltc", NULL, NULL, 0, 0, "sprd,sc2731-bltc"),
+ MFD_CELL_OF("sc2731-charger", NULL, NULL, 0, 0, "sprd,sc2731-charger"),
+ MFD_CELL_OF("sc2731-efuse", NULL, NULL, 0, 0, "sprd,sc2731-efuse"),
+ MFD_CELL_OF("sc2731-eic", NULL, NULL, 0, 0, "sprd,sc2731-eic"),
+ MFD_CELL_OF("sc2731-fgu", NULL, NULL, 0, 0, "sprd,sc2731-fgu"),
+ MFD_CELL_NAME("sc2731-poweroff"),
+ MFD_CELL_NAME("sc2731-regulator"),
+ MFD_CELL_OF("sc2731-rtc", NULL, NULL, 0, 0, "sprd,sc2731-rtc"),
+ MFD_CELL_OF("sc2731-vibrator", NULL, NULL, 0, 0, "sprd,sc2731-vibrator"),
+};
+
/*
* Since different PMICs of SC27xx series can have different interrupt
* base address and irq number, we should save irq number and irq base
@@ -152,12 +180,26 @@ static const struct regmap_config sprd_pmic_config = {
static int sprd_pmic_probe(struct spi_device *spi)
{
struct sprd_pmic *ddata;
+ enum sprd_pmic_type pmic_type;
const struct sprd_pmic_data *pdata;
- int ret, i;
+ const struct mfd_cell *cells;
+ int ret, i, num_cells;
+
+ pmic_type = (enum sprd_pmic_type)of_device_get_match_data(&spi->dev);
- pdata = of_device_get_match_data(&spi->dev);
- if (!pdata) {
- dev_err(&spi->dev, "No matching driver data found\n");
+ switch (pmic_type) {
+ case PMIC_TYPE_SC2730:
+ pdata = &sc2730_data;
+ cells = sc2730_devices;
+ num_cells = ARRAY_SIZE(sc2730_devices);
+ break;
+ case PMIC_TYPE_SC2731:
+ pdata = &sc2731_data;
+ cells = sc2731_devices;
+ num_cells = ARRAY_SIZE(sc2731_devices);
+ break;
+ default:
+ dev_err(&spi->dev, "Invalid device ID\n");
return -EINVAL;
}
@@ -204,7 +246,9 @@ static int sprd_pmic_probe(struct spi_device *spi)
return ret;
}
- ret = devm_of_platform_populate(&spi->dev);
+ ret = devm_mfd_add_devices(&spi->dev, PLATFORM_DEVID_AUTO,
+ cells, num_cells, NULL, 0,
+ regmap_irq_get_domain(ddata->irq_data));
if (ret) {
dev_err(&spi->dev, "Failed to populate sub-devices %d\n", ret);
return ret;
@@ -241,15 +285,15 @@ static DEFINE_SIMPLE_DEV_PM_OPS(sprd_pmic_pm_ops,
sprd_pmic_suspend, sprd_pmic_resume);
static const struct of_device_id sprd_pmic_match[] = {
- { .compatible = "sprd,sc2730", .data = &sc2730_data },
- { .compatible = "sprd,sc2731", .data = &sc2731_data },
+ { .compatible = "sprd,sc2730", .data = (void *)PMIC_TYPE_SC2730 },
+ { .compatible = "sprd,sc2731", .data = (void *)PMIC_TYPE_SC2731 },
{},
};
MODULE_DEVICE_TABLE(of, sprd_pmic_match);
static const struct spi_device_id sprd_pmic_spi_ids[] = {
- { .name = "sc2730", .driver_data = (unsigned long)&sc2730_data },
- { .name = "sc2731", .driver_data = (unsigned long)&sc2731_data },
+ { .name = "sc2730", .driver_data = PMIC_TYPE_SC2730 },
+ { .name = "sc2731", .driver_data = PMIC_TYPE_SC2731 },
{},
};
MODULE_DEVICE_TABLE(spi, sprd_pmic_spi_ids);
--
2.51.0
^ permalink raw reply related
* [PATCH v2 0/5] mfd: sc27xx: Use MFD cells and devm_mfd_add_devices()
From: Otto Pflüger @ 2026-03-25 13:53 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger, Sebastian Reichel
These changes resulted from the need to decouple the the Linux device
driver hierarchy from the device tree bindings for two different series
introducing regulator [1] and poweroff [2] support for the SC2730 PMIC.
There are different PMICs in the SC27xx series, including SC2730 and
SC2731. These have a lot of similarities, but some differences too. For
instance, they contain compatible RTC blocks, but completely different
sets of regulators.
On the Linux side, each PMIC block needs its own driver. The MFD driver
currently uses devm_of_platform_populate() to load the drivers for the
components of the PMIC, which only works when each component has its own
sub-node with a "compatible" property that is used to select a driver
for the device.
When viewed from the device tree side, the parent node representing the
PMIC already contains a "compatible" property that distinguishes the
different PMICs. While the device tree bindings currently do require a
separate "compatible" property for each sub-node (ADC, fuel gauge,
regulators, ...), this is essentially redundant since the node name and
the parent compatible uniquely identify the component. Moreover, some
parts of the PMIC such as the poweroff/reboot controller do not even
need a corresponding device tree node.
Change the MFD driver to use MFD cells instead, which allows it to
instantiate sub-devices both with and without device tree nodes.
Devices that do not have a separate device tree node with its own
"compatible" property can be matched by their platform device ID.
Use this to hook up the existing SC2731 poweroff and regulator drivers,
which were previously not loaded at all due to the lack of an ID table.
In the device tree bindings, deprecate the redundant "compatible"
property for the "regulators" node. While it might make sense to do this
for the other components too, there are a few reasons to only change the
regulators at this point:
- The regulators node is special since it is not as independent as the
other components. For instance, it is the only child node of the PMIC
that does not have a "reg" property. The set of regulators also
differs much more between different PMIC models than the register
layout of the other components.
- We already have some other PMICs where only the regulators are
treated specially like this, such as MediaTek MT6359 and MT6370.
- It was suggested to remove the "compatible" property for the new
SC2730 regulator bindings I am preparing in [2]. The bindings for
the other components do not need any significant changes at the
moment.
- Unlike the poweroff and regulator components, the other parts are
already working with the existing drivers and bindings.
For the other components that still have a "compatible" property used
for matching MFD cells, ensure that an SC2730-specific compatible is
defined in the bindings so that it can be listed in the SC2730-specific
device table in the MFD driver.
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
[1]: https://lore.kernel.org/all/20250926-sc2730-reboot-v1-0-62ebfd3d31bb@abscue.de/
[2]: https://lore.kernel.org/all/20260220-sc2730-regulators-v1-0-3f2bbc9ecf14@abscue.de/
---
Changes in v2:
- Changed PMIC type matching in MFD driver to use an identifier like
other drivers instead of passing pointers through of_device_id.
- Rebased on next-20260324.
- Link to v1: https://lore.kernel.org/r/20260222-sc27xx-mfd-cells-v1-0-69526fe74c77@abscue.de
---
Otto Pflüger (5):
dt-bindings: rtc: sc2731: Add compatible for SC2730
regulator: dt-bindings: sc2731: Deprecate compatible property
mfd: sprd-sc27xx: Switch to devm_mfd_add_devices()
power: reset: sc27xx: Add platform_device_id table
regulator: sc2731: Add platform_device_id table
.../devicetree/bindings/mfd/sprd,sc2731.yaml | 2 -
.../bindings/regulator/sprd,sc2731-regulator.yaml | 4 +-
.../devicetree/bindings/rtc/sprd,sc2731-rtc.yaml | 7 ++-
drivers/mfd/sprd-sc27xx-spi.c | 62 ++++++++++++++++++----
drivers/power/reset/sc27xx-poweroff.c | 8 +++
drivers/regulator/sc2731-regulator.c | 10 +++-
6 files changed, 77 insertions(+), 16 deletions(-)
---
base-commit: 85964cdcad0fac9a0eb7b87a0f9d88cc074b854c
change-id: 20260221-sc27xx-mfd-cells-dab7905f3aae
Best regards,
--
Otto Pflüger <otto.pflueger@abscue.de>
^ permalink raw reply
* [PATCH v2 1/5] dt-bindings: rtc: sc2731: Add compatible for SC2730
From: Otto Pflüger @ 2026-03-25 13:53 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Lee Jones, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel
Cc: linux-rtc, devicetree, linux-kernel, linux-leds, linux-pm,
Otto Pflüger
In-Reply-To: <20260325-sc27xx-mfd-cells-v2-0-d0ebb60aa4a7@abscue.de>
The RTC block found in the SC2730 PMIC is compatible with the one found
in the SC2731 PMIC.
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
---
Documentation/devicetree/bindings/rtc/sprd,sc2731-rtc.yaml | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/rtc/sprd,sc2731-rtc.yaml b/Documentation/devicetree/bindings/rtc/sprd,sc2731-rtc.yaml
index 5756f617df36..1deae2f4f09d 100644
--- a/Documentation/devicetree/bindings/rtc/sprd,sc2731-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/sprd,sc2731-rtc.yaml
@@ -13,7 +13,12 @@ maintainers:
properties:
compatible:
- const: sprd,sc2731-rtc
+ oneOf:
+ - items:
+ - enum:
+ - sprd,sc2730-rtc
+ - const: sprd,sc2731-rtc
+ - const: sprd,sc2731-rtc
reg:
maxItems: 1
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v10 00/12] barrier: Add smp_cond_load_{relaxed,acquire}_timeout()
From: Catalin Marinas @ 2026-03-25 13:53 UTC (permalink / raw)
To: David Laight
Cc: Ankur Arora, Andrew Morton, linux-kernel, linux-arch,
linux-arm-kernel, linux-pm, bpf, arnd, will, peterz, mark.rutland,
harisokn, cl, ast, rafael, daniel.lezcano, memxor, zhenglifeng1,
xueshuai, rdunlap, joao.m.martins, boris.ostrovsky, konrad.wilk
In-Reply-To: <20260317091705.5a64fc56@pumpkin>
On Tue, Mar 17, 2026 at 09:17:05AM +0000, David Laight wrote:
> On Mon, 16 Mar 2026 23:53:22 -0700
> Ankur Arora <ankur.a.arora@oracle.com> wrote:
> > David Laight <david.laight.linux@gmail.com> writes:
> > > On arm64 I think you could use explicit sev and wfe - but that will wake all
> > > 'sleeping' cpu; and you may not want the 'thundering herd'.
> >
> > Wouldn't we still have the same narrow window where the CPU disregards the IPI?
>
> You need a 'sevl' in the interrupt exit path.
No need to, see the rule below in
https://developer.arm.com/documentation/ddi0487/maa/2983-beijhbbd:
R_XRZRK
The Event Register for a PE is set by any of the following:
[...]
- An exception return.
--
Catalin
^ permalink raw reply
* Re: [PATCH v3 08/12] amd-pstate-ut: Add ability to run a single testcase
From: Mario Limonciello @ 2026-03-25 13:45 UTC (permalink / raw)
To: K Prateek Nayak, Gautham R. Shenoy
Cc: Rafael J . Wysocki, Viresh Kumar, linux-kernel, linux-pm
In-Reply-To: <554baa52-c181-4dcc-a77a-ef8f4e0be763@amd.com>
On 3/24/26 23:28, K Prateek Nayak wrote:
> Hello Gautham,
>
> On 3/24/2026 9:59 AM, Gautham R. Shenoy wrote:
>> +static bool test_in_list(const char *list, const char *name)
>> +{
>> + size_t name_len = strlen(name);
>> + const char *p = list;
>> +
>> + while (*p) {
>> + const char *sep = strchr(p, ';');
>
> Any particular reason for using a ";" as the separator instead of ","?
>
> I personally prefer "," because with ";", I need to explicitly add
> '' around the test_list otherwise bash thinks the command ends at ";"
> but with "," that is avoided.
>
> Thoughts?
>
Sure, that sounds like a good reason to use a comma instead.
>> + size_t token_len = sep ? sep - p : strlen(p);
>> +
>> + if (token_len == name_len && !strncmp(p, name, token_len))
>> + return true;
>> +
>> + if (!sep)
>> + break;
>> + p = sep + 1;
>> + }
>> +
>> + return false;
>> +}
^ permalink raw reply
* Re: [PATCH v2] cpufreq: acpi-cpufreq: use DMI max speed when CPPC is unavailable
From: Rafael J. Wysocki @ 2026-03-25 13:32 UTC (permalink / raw)
To: Henry Tseng
Cc: Rafael J. Wysocki, Viresh Kumar, Len Brown, linux-pm, linux-acpi,
SW Chen, Kevin Ko
In-Reply-To: <20260324090948.1667340-1-henrytseng@qnap.com>
On Tue, Mar 24, 2026 at 10:11 AM Henry Tseng <henrytseng@qnap.com> wrote:
>
> On AMD Ryzen Embedded V1780B (Family 17h, Zen 1), the BIOS does not
> provide ACPI _CPC objects and the CPU does not support MSR-based CPPC
> (X86_FEATURE_CPPC). The _PSS table only lists nominal P-states
> (P0 = 3350 MHz), so when get_max_boost_ratio() fails at
> cppc_get_perf_caps(), cpuinfo_max_freq reports only the base frequency
> instead of the rated boost frequency (3600 MHz).
>
> dmesg:
> ACPI CPPC: No CPC descriptor for CPU:0
> acpi_cpufreq: CPU0: Unable to get performance capabilities (-19)
>
> cppc-cpufreq already has a DMI fallback (cppc_get_dmi_max_khz()) that
> reads the processor max speed from SMBIOS Type 4. Export it and reuse
> it in acpi-cpufreq as a last-resort source for the boost frequency.
>
> A sanity check ensures the DMI value is above the _PSS P0 frequency
> and within 2x of it; values outside that range are ignored and the
> existing arch_set_max_freq_ratio() path is taken instead. The 2x
> upper bound is based on a survey of the AMD Ryzen Embedded V1000
> series, where the highest boost-to-base ratio is 1.8x (V1404I:
> 2.0 GHz base / 3.6 GHz boost).
>
> The DMI lookup and sanity check are wrapped in a helper,
> acpi_cpufreq_resolve_max_freq(), which falls through to
> arch_set_max_freq_ratio() if the DMI value is absent or
> out of range.
>
> Tested on AMD Ryzen Embedded V1780B with v7.0-rc4:
>
> Before: cpuinfo_max_freq = 3350000 (base only)
> After: cpuinfo_max_freq = 3600000 (includes boost)
>
> Link: https://www.amd.com/en/products/embedded/ryzen/ryzen-v1000-series.html#specifications
> Signed-off-by: Henry Tseng <henrytseng@qnap.com>
> ---
> v2:
> - Extract else block into acpi_cpufreq_resolve_max_freq()
> to avoid #ifdef in the main init path (Rafael)
> v1:
> - https://lore.kernel.org/linux-pm/20260320095648.3598007-1-henrytseng@qnap.com/
>
> drivers/acpi/cppc_acpi.c | 3 ++-
> drivers/cpufreq/acpi-cpufreq.c | 31 ++++++++++++++++++++++++-------
> include/acpi/cppc_acpi.h | 1 +
> 3 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index f0e513e9ed5d..f53de414acf2 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1944,7 +1944,7 @@ static void cppc_find_dmi_mhz(const struct dmi_header *dm, void *private)
> }
>
> /* Look up the max frequency in DMI */
> -static u64 cppc_get_dmi_max_khz(void)
> +u64 cppc_get_dmi_max_khz(void)
> {
> u16 mhz = 0;
>
> @@ -1958,6 +1958,7 @@ static u64 cppc_get_dmi_max_khz(void)
>
> return KHZ_PER_MHZ * mhz;
> }
> +EXPORT_SYMBOL_GPL(cppc_get_dmi_max_khz);
>
> /*
> * If CPPC lowest_freq and nominal_freq registers are exposed then we can
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index e7eff6c2f092..21639d9ac753 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -675,6 +675,29 @@ static inline u64 get_max_boost_ratio(unsigned int cpu, u64 *nominal_freq)
> }
> #endif
>
> +static void acpi_cpufreq_resolve_max_freq(struct cpufreq_policy *policy,
> + unsigned int pss_max_freq)
> +{
> +#ifdef CONFIG_ACPI_CPPC_LIB
> + u64 max_speed = cppc_get_dmi_max_khz();
> + /*
> + * Use DMI "Max Speed" if it looks plausible: must be
> + * above _PSS P0 frequency and within 2x of it.
> + */
> + if (max_speed > pss_max_freq && max_speed < pss_max_freq * 2) {
> + policy->cpuinfo.max_freq = max_speed;
> + return;
> + }
> +#endif
> + /*
> + * If the maximum "boost" frequency is unknown, ask the arch
> + * scale-invariance code to use the "nominal" performance for
> + * CPU utilization scaling so as to prevent the schedutil
> + * governor from selecting inadequate CPU frequencies.
> + */
> + arch_set_max_freq_ratio(true);
> +}
> +
> static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
> {
> struct cpufreq_frequency_table *freq_table;
> @@ -849,13 +872,7 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
>
> policy->cpuinfo.max_freq = freq * max_boost_ratio >> SCHED_CAPACITY_SHIFT;
> } else {
> - /*
> - * If the maximum "boost" frequency is unknown, ask the arch
> - * scale-invariance code to use the "nominal" performance for
> - * CPU utilization scaling so as to prevent the schedutil
> - * governor from selecting inadequate CPU frequencies.
> - */
> - arch_set_max_freq_ratio(true);
> + acpi_cpufreq_resolve_max_freq(policy, freq_table[0].frequency);
> }
>
> policy->freq_table = freq_table;
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 4d644f03098e..e6c5ef3173c5 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -156,6 +156,7 @@ extern int cppc_set_enable(int cpu, bool enable);
> extern int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps);
> extern bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu);
> extern bool cppc_perf_ctrs_in_pcc(void);
> +extern u64 cppc_get_dmi_max_khz(void);
> extern unsigned int cppc_perf_to_khz(struct cppc_perf_caps *caps, unsigned int perf);
> extern unsigned int cppc_khz_to_perf(struct cppc_perf_caps *caps, unsigned int freq);
> extern bool acpi_cpc_valid(void);
> --
Applied as 7.1 material, thanks!
^ permalink raw reply
* Re: [PATCH] dt-bindings: reset: st: convert to dtschema
From: Gopi Krishna Menon @ 2026-03-25 13:13 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: sre, robh, krzk+dt, lee, conor+dt, daniel.baluta, simona.toaca,
d-gole, m-chawdhry, linux-pm, devicetree, linux-kernel
In-Reply-To: <57e3e846-d679-4201-921c-69cdd4040d13@kernel.org>
On Wed, Mar 25, 2026 at 01:36:15PM +0100, Krzysztof Kozlowski wrote:
> On 25/03/2026 13:04, Gopi Krishna Menon wrote:
> > On Wed, Mar 25, 2026 at 12:39:36PM +0100, Krzysztof Kozlowski wrote:
> >
> >> On Tue, Mar 24, 2026 at 09:29:30PM +0530, Gopi Krishna Menon wrote:
> >>> Convert the STiH4xx reset controller bindings to DT schema.
> >>>
> >>> Suggested-by: Daniel Baluta <daniel.baluta@nxp.com>
> >>> Suggested-by: Dhruva Gole <d-gole@ti.com>
> >>
> >> Both suggested you to write this patch?
> >>
> > Hi Krzysztof,
> >
> > Thanks for the review, They helped me to improve the PATCH,
> >
> > - Dhruva suggested me to change the subject from dt-bindings: power:
> > reset: st: convert to dtschema to dt-bindings: reset: st: convert to
> > dtschema as that was the general trend followed with similar files.
> > - Daniel suggested me to use the word 'reset' instead of 'restart' in
> > the patch (whereever possible) as that is more accurate here.
> >
> > That's why I added those Suggested-by tags.
>
> This is not the meaning of Suggested-by. Drop the tags.
>
Understood. Will remove it and send the v2.
> I gave you now review, so you will add "Suggested-by: Krzysztof ..."?
>
Earlier I would have. Didnt understand the use and meaning of this tag properly.
But will take care not to let this happen in future.
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: reset: st: convert to dtschema
From: Dhruva Gole @ 2026-03-25 13:10 UTC (permalink / raw)
To: Gopi Krishna Menon
Cc: sre, robh, krzk+dt, lee, conor+dt, daniel.baluta, simona.toaca,
m-chawdhry, linux-pm, devicetree, linux-kernel
In-Reply-To: <20260325130623.36710-1-krishnagopi487@gmail.com>
On Mar 25, 2026 at 18:36:21 +0530, Gopi Krishna Menon wrote:
> Convert the STiH4xx reset controller bindings to DT schema.
>
> Signed-off-by: Gopi Krishna Menon <krishnagopi487@gmail.com>
> ---
> Changes since v1:
> - Changed unevaluatedProperties to additionalProperties
> - Removed the Suggested-by tags
Reviewed-by: Dhruva Gole <d-gole@ti.com>
>
> Note:
> * This patch is part of the GSoC2026 application process for device tree bindings conversions
> * https://github.com/LinuxFoundationGSoC/ProjectIdeas/wiki/GSoC-2026-Device-Tree-Bindings
>
> .../power/reset/st,stih407-restart.yaml | 31 +++++++++++++++++++
> .../bindings/power/reset/st-reset.txt | 11 -------
> 2 files changed, 31 insertions(+), 11 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
> delete mode 100644 Documentation/devicetree/bindings/power/reset/st-reset.txt
>
> diff --git a/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml b/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
> new file mode 100644
> index 000000000000..0dd7f5e98157
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/reset/st,stih407-restart.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ST SW reset controller
> +
> +maintainers:
> + - Lee Jones <lee@kernel.org>
> +
> +properties:
> + compatible:
> + const: st,stih407-restart
> +
> + st,syscfg:
> + description: phandle of the syscfg node
> + $ref: /schemas/types.yaml#/definitions/phandle
> +
> +required:
> + - compatible
> + - st,syscfg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + reset {
> + compatible = "st,stih407-restart";
> + st,syscfg = <&syscfg_sbc_reg>;
> + };
> diff --git a/Documentation/devicetree/bindings/power/reset/st-reset.txt b/Documentation/devicetree/bindings/power/reset/st-reset.txt
> deleted file mode 100644
> index b63948737d80..000000000000
> --- a/Documentation/devicetree/bindings/power/reset/st-reset.txt
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -*Device-Tree bindings for ST SW reset functionality
> -
> -Required properties:
> -- compatible: should be "stih407-restart".
> -- st,syscfg: should be a phandle of the syscfg node.
> -
> -Example node:
> - restart {
> - compatible = "st,stih407-restart";
> - st,syscfg = <&syscfg_sbc_reg>;
> - };
> --
> 2.52.0
>
--
Best regards,
Dhruva Gole
Texas Instruments Incorporated
^ permalink raw reply
* [PATCH v2] dt-bindings: reset: st: convert to dtschema
From: Gopi Krishna Menon @ 2026-03-25 13:06 UTC (permalink / raw)
To: sre, robh, krzk+dt, lee, conor+dt
Cc: Gopi Krishna Menon, daniel.baluta, simona.toaca, d-gole,
m-chawdhry, linux-pm, devicetree, linux-kernel
Convert the STiH4xx reset controller bindings to DT schema.
Signed-off-by: Gopi Krishna Menon <krishnagopi487@gmail.com>
---
Changes since v1:
- Changed unevaluatedProperties to additionalProperties
- Removed the Suggested-by tags
Note:
* This patch is part of the GSoC2026 application process for device tree bindings conversions
* https://github.com/LinuxFoundationGSoC/ProjectIdeas/wiki/GSoC-2026-Device-Tree-Bindings
.../power/reset/st,stih407-restart.yaml | 31 +++++++++++++++++++
.../bindings/power/reset/st-reset.txt | 11 -------
2 files changed, 31 insertions(+), 11 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
delete mode 100644 Documentation/devicetree/bindings/power/reset/st-reset.txt
diff --git a/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml b/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
new file mode 100644
index 000000000000..0dd7f5e98157
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/reset/st,stih407-restart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ST SW reset controller
+
+maintainers:
+ - Lee Jones <lee@kernel.org>
+
+properties:
+ compatible:
+ const: st,stih407-restart
+
+ st,syscfg:
+ description: phandle of the syscfg node
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+required:
+ - compatible
+ - st,syscfg
+
+additionalProperties: false
+
+examples:
+ - |
+ reset {
+ compatible = "st,stih407-restart";
+ st,syscfg = <&syscfg_sbc_reg>;
+ };
diff --git a/Documentation/devicetree/bindings/power/reset/st-reset.txt b/Documentation/devicetree/bindings/power/reset/st-reset.txt
deleted file mode 100644
index b63948737d80..000000000000
--- a/Documentation/devicetree/bindings/power/reset/st-reset.txt
+++ /dev/null
@@ -1,11 +0,0 @@
-*Device-Tree bindings for ST SW reset functionality
-
-Required properties:
-- compatible: should be "stih407-restart".
-- st,syscfg: should be a phandle of the syscfg node.
-
-Example node:
- restart {
- compatible = "st,stih407-restart";
- st,syscfg = <&syscfg_sbc_reg>;
- };
--
2.52.0
^ permalink raw reply related
* Re: [PATCH] thermal: core: fix use-after-free due to init/cancel delayed_work race
From: Rafael J. Wysocki @ 2026-03-25 12:47 UTC (permalink / raw)
To: Mauricio Faria de Oliveira
Cc: Daniel Lezcano, Zhang Rui, Lukasz Luba, linux-pm, linux-kernel,
kernel-dev, syzbot+3b3852c6031d0f30dfaf
In-Reply-To: <CAJZ5v0h8Q-Cs-05hwKUF9wPbOzg2gyG8yH6tOfzJgXQCh5Ohbg@mail.gmail.com>
On Wed, Mar 25, 2026 at 1:10 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Wed, Mar 25, 2026 at 12:51 AM Mauricio Faria de Oliveira
> <mfo@igalia.com> wrote:
> >
> > If INIT_DELAYED_WORK() is called for a currently running work item,
> > cancel_delayed_work_sync() is unable to cancel/wait for it anymore,
> > as the work item's data bits required for that are cleared.
> >
> > In the resume path, INIT_DELAYED_WORK() is called twice:
> > 1) to replace the work function: thermal_zone_device_check/resume()
> > 2) to restore it.
> >
> > Both cases might race with the unregister path and bypass the call to
> > cancel_delayed_work_sync(),
>
> So this is the problem, isn't it?
>
> > after which struct thermal_zone_device *tz
> > is freed, and the non-canceled/non-waited for work hits use-after-free.
>
> Which basically means that a TZ_STATE_FLAG_EXIT check is missing in
> both thermal_zone_pm_complete() and thermal_zone_device_resume().
Actually, thermal_zone_pm_complete() runs under thermal_list_lock and
thermal_zone_device_unregister() removes the zone from
thermal_tz_list, also under thermal_list_lock, before calling
cancel_delayed_work_sync().
So either thermal_zone_device_unregister() removes the zone from the
list before thermal_zone_pm_complete() can run, in which case the
latter won't run for the given zone at all because that zone is not
there in thermal_tz_list, or the cancel_delayed_work_sync() will see
the work item queued up by thermal_zone_pm_complete().
So where's the race between thermal_zone_pm_complete() and
thermal_zone_device_unregister()?
I can see the one between thermal_zone_device_unregister() and
thermal_zone_device_resume(), but that can be addressed by adding a
TZ_STATE_FLAG_EXIT check to the latter AFAICS.
^ permalink raw reply
* Re: [PATCH] dt-bindings: reset: st: convert to dtschema
From: Krzysztof Kozlowski @ 2026-03-25 12:36 UTC (permalink / raw)
To: Gopi Krishna Menon
Cc: sre, robh, krzk+dt, lee, conor+dt, daniel.baluta, simona.toaca,
d-gole, m-chawdhry, linux-pm, devicetree, linux-kernel
In-Reply-To: <acPMx9NZBehAzkBp@toolbx>
On 25/03/2026 13:04, Gopi Krishna Menon wrote:
> On Wed, Mar 25, 2026 at 12:39:36PM +0100, Krzysztof Kozlowski wrote:
>
>> On Tue, Mar 24, 2026 at 09:29:30PM +0530, Gopi Krishna Menon wrote:
>>> Convert the STiH4xx reset controller bindings to DT schema.
>>>
>>> Suggested-by: Daniel Baluta <daniel.baluta@nxp.com>
>>> Suggested-by: Dhruva Gole <d-gole@ti.com>
>>
>> Both suggested you to write this patch?
>>
> Hi Krzysztof,
>
> Thanks for the review, They helped me to improve the PATCH,
>
> - Dhruva suggested me to change the subject from dt-bindings: power:
> reset: st: convert to dtschema to dt-bindings: reset: st: convert to
> dtschema as that was the general trend followed with similar files.
> - Daniel suggested me to use the word 'reset' instead of 'restart' in
> the patch (whereever possible) as that is more accurate here.
>
> That's why I added those Suggested-by tags.
This is not the meaning of Suggested-by. Drop the tags.
I gave you now review, so you will add "Suggested-by: Krzysztof ..."?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 0/5] mm: zone lock tracepoint instrumentation
From: Dmitry Ilvokhin @ 2026-03-25 12:14 UTC (permalink / raw)
To: Andrew Morton
Cc: Steven Rostedt, Matthew Wilcox, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Masami Hiramatsu, Mathieu Desnoyers, Rafael J. Wysocki,
Pavel Machek, Len Brown, Brendan Jackman, Johannes Weiner, Zi Yan,
Oscar Salvador, Qi Zheng, Shakeel Butt, linux-kernel, linux-mm,
linux-trace-kernel, linux-pm
In-Reply-To: <20260324163918.1a3c5c960d85a4243c9ae314@linux-foundation.org>
On Tue, Mar 24, 2026 at 04:39:18PM -0700, Andrew Morton wrote:
> On Thu, 19 Mar 2026 13:22:54 +0000 Dmitry Ilvokhin <d@ilvokhin.com> wrote:
>
> > On Mon, Mar 16, 2026 at 05:40:50PM +0000, Dmitry Ilvokhin wrote:
> >
> > [...]
> >
> > > A possible generic solution is a trace_contended_release() for spin
> > > locks, for example:
> > >
> > > if (trace_contended_release_enabled() &&
> > > atomic_read(&lock->val) & ~_Q_LOCKED_MASK)
> > > trace_contended_release(lock);
> > >
> > > This might work on x86, but could increase code size and regress
> > > performance on arches where spin_unlock() is inlined, such as arm64
> > > under !PREEMPTION.
> >
> > I took a stab at this idea and submitted an RFC [1].
> >
> > The implementation builds on your earlier observation from Matthew that
> > _raw_spin_unlock() is not inlined in most configurations. In those
> > cases, when the tracepoint is disabled, this adds a single NOP on the
> > fast path, with the conditional check staying out of line. The measured
> > text size increase in this configuration is +983 bytes.
> >
> > For configurations where _raw_spin_unlock() is inlined, the
> > instrumentation does increase code size more noticeably
> > (+71 KB in my measurements), since the check and out of line call is
> > replicated at each call site.
> >
> > This provides a generic release-side signal for contended locks,
> > allowing: correlation of lock holders with waiters and measurement of
> > contended hold times
> >
> > This RFC addressing the same visibility gap without introducing per-lock
> > instrumentation.
> >
> > If this tradeoff is acceptable, this could be a generic alternative to
> > lock-specific tracepoints.
> >
> > [1]: https://lore.kernel.org/all/51aad0415b78c5a39f2029722118fa01eac77538.1773858853.git.d@ilvokhin.com
>
> That submission has met a disappointing response.
>
> How should I proceed with this series "mm: zone lock tracepoint
> instrumentation"? It's not urgent so I'm inclined to put this on hold
> while you pursue "locking: Add contended_release tracepoint to spinning
> locks"?
Thanks for the follow-up, Andrew.
My current plan is to focus on the "locking: Add contended_release
tracepoint to spinning locks" work and drive it to a clear conclusion:
either by getting feedback that it's not a good direction, or by getting
it into mainline.
In the meantime, it seems reasonable to drop the "mm: zone lock
tracepoint instrumentation" patchset from mm-new to avoid confusion
until the direction is clearer. I can revisit and respin it if the more
generic locking approach doesn't pan out.
>
> Please send that v2 sometime and hopefully Steven can help push it along?
I'll send the next version of the generic locking series soon. Any help
in pushing it along would be appreciated.
^ permalink raw reply
* Re: [PATCH v8 8/9] dax/hmem, cxl: Defer and resolve Soft Reserved ownership
From: Jonathan Cameron @ 2026-03-25 12:12 UTC (permalink / raw)
To: Koralahalli Channabasappa, Smita
Cc: Smita Koralahalli, linux-cxl, linux-kernel, nvdimm, linux-fsdevel,
linux-pm, Ard Biesheuvel, Alison Schofield, Vishal Verma,
Ira Weiny, Dan Williams, Yazen Ghannam, Dave Jiang,
Davidlohr Bueso, Matthew Wilcox, Jan Kara, Rafael J . Wysocki,
Len Brown, Pavel Machek, Li Ming, Jeff Johnson, Ying Huang,
Yao Xingtao, Peter Zijlstra, Greg Kroah-Hartman, Nathan Fontenot,
Terry Bowman, Robert Richter, Benjamin Cheatham, Zhijian Li,
Borislav Petkov, Tomasz Wolski
In-Reply-To: <403241c1-36f7-4fd6-bc99-d7dbf30e58f4@amd.com>
On Tue, 24 Mar 2026 14:50:59 -0700
"Koralahalli Channabasappa, Smita" <skoralah@amd.com> wrote:
> Hi Jonathan,
>
> On 3/23/2026 11:13 AM, Jonathan Cameron wrote:
> > On Sun, 22 Mar 2026 19:53:41 +0000
> > Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> wrote:
> >
> >> The current probe time ownership check for Soft Reserved memory based
> >> solely on CXL window intersection is insufficient. dax_hmem probing is not
> >> always guaranteed to run after CXL enumeration and region assembly, which
> >> can lead to incorrect ownership decisions before the CXL stack has
> >> finished publishing windows and assembling committed regions.
> >>
> >> Introduce deferred ownership handling for Soft Reserved ranges that
> >> intersect CXL windows. When such a range is encountered during the
> >> initial dax_hmem probe, schedule deferred work to wait for the CXL stack
> >> to complete enumeration and region assembly before deciding ownership.
> >>
> >> Once the deferred work runs, evaluate each Soft Reserved range
> >> individually: if a CXL region fully contains the range, skip it and let
> >> dax_cxl bind. Otherwise, register it with dax_hmem. This per-range
> >> ownership model avoids the need for CXL region teardown and
> >> alloc_dax_region() resource exclusion prevents double claiming.
> >>
> >> Introduce a boolean flag dax_hmem_initial_probe to live inside device.c
> >> so it survives module reload. Ensure dax_cxl defers driver registration
> >> until dax_hmem has completed ownership resolution. dax_cxl calls
> >> dax_hmem_flush_work() before cxl_driver_register(), which both waits for
> >> the deferred work to complete and creates a module symbol dependency that
> >> forces dax_hmem.ko to load before dax_cxl.
> >>
> >> Co-developed-by: Dan Williams <dan.j.williams@intel.com>
> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> >> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> >
> > https://sashiko.dev/#/patchset/20260322195343.206900-1-Smita.KoralahalliChannabasappa%40amd.com
> > Might be worth a look. I think the last comment is potentially correct
> > though unlikely a platform_driver_register() actually fails.
> >
> > I've not looked too closely at the others. Given this was doing something
> > unusual I thought I'd see what it found. Looks like some interesting
> > questions if nothing else.
>
> Thanks for pointing this out. I went through the findings:
>
> The init error path one is valid I think, if
> platform_driver_register(&dax_hmem_driver) fails after
> dax_hmem_platform_driver has already probed and queued work, the error
> path doesn't flush the work or release the pdev reference.
>
> I was thinking something like below for v9:
>
> @@ -258,8 +262,13 @@ static __init int dax_hmem_init(void)
> return rc;
>
> rc = platform_driver_register(&dax_hmem_driver);
> - if (rc)
> + if (rc) {
> + if (dax_hmem_work.pdev) {
> + flush_work(&dax_hmem_work.work);
> + put_device(&dax_hmem_work.pdev->dev);
> + }
> platform_driver_unregister(&dax_hmem_platform_driver);
> + }
>
> return rc;
> }
>
>
> Worth adding considering the unlikeliness?
I think so. Alternative would be a very obvious comment to say we've
deliberately not handled this corner. Code seems easier to me and
lines up with what remove is doing.
>
> The others I looked at the IS_ENABLED vs IS_REACHABLE question is
> something I'm discussing with Dan in 3/9 (there's a Kconfig dependency
> and CXL_BUS dependency fix needed I guess), the module reload behavior
> is intentional and others are mostly false positives I think..
I was more suspicious of those ones as can never remember exactly what
the effective rule are.
Thanks,
J
>
> Thanks,
> Smita
>
> >
> >> ---
> >> drivers/dax/bus.h | 7 ++++
> >> drivers/dax/cxl.c | 1 +
> >> drivers/dax/hmem/device.c | 3 ++
> >> drivers/dax/hmem/hmem.c | 74 +++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 85 insertions(+)
> >>
> >> diff --git a/drivers/dax/bus.h b/drivers/dax/bus.h
> >> index cbbf64443098..ebbfe2d6da14 100644
> >> --- a/drivers/dax/bus.h
> >> +++ b/drivers/dax/bus.h
> >> @@ -49,6 +49,13 @@ void dax_driver_unregister(struct dax_device_driver *dax_drv);
> >> void kill_dev_dax(struct dev_dax *dev_dax);
> >> bool static_dev_dax(struct dev_dax *dev_dax);
> >>
> >> +#if IS_ENABLED(CONFIG_DEV_DAX_HMEM)
> >> +extern bool dax_hmem_initial_probe;
> >> +void dax_hmem_flush_work(void);
> >> +#else
> >> +static inline void dax_hmem_flush_work(void) { }
> >> +#endif
> >> +
> >> #define MODULE_ALIAS_DAX_DEVICE(type) \
> >> MODULE_ALIAS("dax:t" __stringify(type) "*")
> >> #define DAX_DEVICE_MODALIAS_FMT "dax:t%d"
> >> diff --git a/drivers/dax/cxl.c b/drivers/dax/cxl.c
> >> index a2136adfa186..3ab39b77843d 100644
> >> --- a/drivers/dax/cxl.c
> >> +++ b/drivers/dax/cxl.c
> >> @@ -44,6 +44,7 @@ static struct cxl_driver cxl_dax_region_driver = {
> >>
> >> static void cxl_dax_region_driver_register(struct work_struct *work)
> >> {
> >> + dax_hmem_flush_work();
> >> cxl_driver_register(&cxl_dax_region_driver);
> >> }
> >>
> >> diff --git a/drivers/dax/hmem/device.c b/drivers/dax/hmem/device.c
> >> index 56e3cbd181b5..991a4bf7d969 100644
> >> --- a/drivers/dax/hmem/device.c
> >> +++ b/drivers/dax/hmem/device.c
> >> @@ -8,6 +8,9 @@
> >> static bool nohmem;
> >> module_param_named(disable, nohmem, bool, 0444);
> >>
> >> +bool dax_hmem_initial_probe;
> >> +EXPORT_SYMBOL_GPL(dax_hmem_initial_probe);
> >> +
> >> static bool platform_initialized;
> >> static DEFINE_MUTEX(hmem_resource_lock);
> >> static struct resource hmem_active = {
> >> diff --git a/drivers/dax/hmem/hmem.c b/drivers/dax/hmem/hmem.c
> >> index ca752db03201..9ceda6b5cadf 100644
> >> --- a/drivers/dax/hmem/hmem.c
> >> +++ b/drivers/dax/hmem/hmem.c
> >> @@ -3,6 +3,7 @@
> >> #include <linux/memregion.h>
> >> #include <linux/module.h>
> >> #include <linux/dax.h>
> >> +#include <cxl/cxl.h>
> >> #include "../bus.h"
> >>
> >> static bool region_idle;
> >> @@ -58,6 +59,23 @@ static void release_hmem(void *pdev)
> >> platform_device_unregister(pdev);
> >> }
> >>
> >> +struct dax_defer_work {
> >> + struct platform_device *pdev;
> >> + struct work_struct work;
> >> +};
> >> +
> >> +static void process_defer_work(struct work_struct *w);
> >> +
> >> +static struct dax_defer_work dax_hmem_work = {
> >> + .work = __WORK_INITIALIZER(dax_hmem_work.work, process_defer_work),
> >> +};
> >> +
> >> +void dax_hmem_flush_work(void)
> >> +{
> >> + flush_work(&dax_hmem_work.work);
> >> +}
> >> +EXPORT_SYMBOL_GPL(dax_hmem_flush_work);
> >> +
> >> static int __hmem_register_device(struct device *host, int target_nid,
> >> const struct resource *res)
> >> {
> >> @@ -122,6 +140,11 @@ static int hmem_register_device(struct device *host, int target_nid,
> >> if (IS_ENABLED(CONFIG_DEV_DAX_CXL) &&
> >> region_intersects(res->start, resource_size(res), IORESOURCE_MEM,
> >> IORES_DESC_CXL) != REGION_DISJOINT) {
> >> + if (!dax_hmem_initial_probe) {
> >> + dev_dbg(host, "await CXL initial probe: %pr\n", res);
> >> + queue_work(system_long_wq, &dax_hmem_work.work);
> >> + return 0;
> >> + }
> >> dev_dbg(host, "deferring range to CXL: %pr\n", res);
> >> return 0;
> >> }
> >> @@ -129,8 +152,54 @@ static int hmem_register_device(struct device *host, int target_nid,
> >> return __hmem_register_device(host, target_nid, res);
> >> }
> >>
> >> +static int hmem_register_cxl_device(struct device *host, int target_nid,
> >> + const struct resource *res)
> >> +{
> >> + if (region_intersects(res->start, resource_size(res), IORESOURCE_MEM,
> >> + IORES_DESC_CXL) == REGION_DISJOINT)
> >> + return 0;
> >> +
> >> + if (cxl_region_contains_resource((struct resource *)res)) {
> >> + dev_dbg(host, "CXL claims resource, dropping: %pr\n", res);
> >> + return 0;
> >> + }
> >> +
> >> + dev_dbg(host, "CXL did not claim resource, registering: %pr\n", res);
> >> + return __hmem_register_device(host, target_nid, res);
> >> +}
> >> +
> >> +static void process_defer_work(struct work_struct *w)
> >> +{
> >> + struct dax_defer_work *work = container_of(w, typeof(*work), work);
> >> + struct platform_device *pdev;
> >> +
> >> + if (!work->pdev)
> >> + return;
> >> +
> >> + pdev = work->pdev;
> >> +
> >> + /* Relies on cxl_acpi and cxl_pci having had a chance to load */
> >> + wait_for_device_probe();
> >> +
> >> + guard(device)(&pdev->dev);
> >> + if (!pdev->dev.driver)
> >> + return;
> >> +
> >> + if (!dax_hmem_initial_probe) {
> >> + dax_hmem_initial_probe = true;
> >> + walk_hmem_resources(&pdev->dev, hmem_register_cxl_device);
> >> + }
> >> +}
> >> +
> >> static int dax_hmem_platform_probe(struct platform_device *pdev)
> >> {
> >> + if (work_pending(&dax_hmem_work.work))
> >> + return -EBUSY;
> >> +
> >> + if (!dax_hmem_work.pdev)
> >> + dax_hmem_work.pdev =
> >> + to_platform_device(get_device(&pdev->dev));
> >> +
> >> return walk_hmem_resources(&pdev->dev, hmem_register_device);
> >> }
> >>
> >> @@ -168,6 +237,11 @@ static __init int dax_hmem_init(void)
> >>
> >> static __exit void dax_hmem_exit(void)
> >> {
> >> + if (dax_hmem_work.pdev) {
> >> + flush_work(&dax_hmem_work.work);
> >> + put_device(&dax_hmem_work.pdev->dev);
> >> + }
> >> +
> >> platform_driver_unregister(&dax_hmem_driver);
> >> platform_driver_unregister(&dax_hmem_platform_driver);
> >> }
> >
>
>
^ permalink raw reply
* Re: [PATCH] thermal: core: fix use-after-free due to init/cancel delayed_work race
From: Rafael J. Wysocki @ 2026-03-25 12:10 UTC (permalink / raw)
To: Mauricio Faria de Oliveira
Cc: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
linux-pm, linux-kernel, kernel-dev, syzbot+3b3852c6031d0f30dfaf
In-Reply-To: <20260324-thermal-core-uaf-init_delayed_work-v1-1-6611ae76a8a1@igalia.com>
On Wed, Mar 25, 2026 at 12:51 AM Mauricio Faria de Oliveira
<mfo@igalia.com> wrote:
>
> If INIT_DELAYED_WORK() is called for a currently running work item,
> cancel_delayed_work_sync() is unable to cancel/wait for it anymore,
> as the work item's data bits required for that are cleared.
>
> In the resume path, INIT_DELAYED_WORK() is called twice:
> 1) to replace the work function: thermal_zone_device_check/resume()
> 2) to restore it.
>
> Both cases might race with the unregister path and bypass the call to
> cancel_delayed_work_sync(),
So this is the problem, isn't it?
> after which struct thermal_zone_device *tz
> is freed, and the non-canceled/non-waited for work hits use-after-free.
Which basically means that a TZ_STATE_FLAG_EXIT check is missing in
both thermal_zone_pm_complete() and thermal_zone_device_resume().
> Fix the first case with a dedicated work item for the resume function,
> and the second case by initializing the work item(s) only during init.
So why not add those missing checks instead of complicating the code even more?
^ permalink raw reply
* Re: [PATCH v6 6/9] dt-bindings: connector: m2: Add M.2 1620 LGA soldered down connector
From: Manivannan Sadhasivam @ 2026-03-25 12:06 UTC (permalink / raw)
To: Mark Pearson
Cc: Dmitry Baryshkov, Rob Herring, Manivannan Sadhasivam, Greg KH,
Jiri Slaby, Nathan Chancellor, Nicolas Schier, Hans de Goede,
Ilpo Järvinen, Derek J . Clark, Krzysztof Kozlowski,
Conor Dooley, Marcel Holtmann, Luiz Augusto von Dentz,
Bartosz Golaszewski, Andy Shevchenko, Bartosz Golaszewski,
linux-serial, linux-kernel, linux-kbuild,
platform-driver-x86@vger.kernel.org, linux-pci, devicetree,
linux-arm-msm, linux-bluetooth, linux-pm, Stephan Gerhold,
linux-acpi@vger.kernel.org
In-Reply-To: <3faffec9-dc9d-4eec-a652-a84d30d85c96@app.fastmail.com>
On Mon, Mar 23, 2026 at 01:23:07PM -0400, Mark Pearson wrote:
>
>
> On Mon, Mar 23, 2026, at 12:52 PM, Manivannan Sadhasivam wrote:
> > On Mon, Mar 23, 2026 at 06:45:15PM +0200, Dmitry Baryshkov wrote:
> >> On Mon, Mar 23, 2026 at 09:26:04PM +0530, Manivannan Sadhasivam wrote:
> >> > On Mon, Mar 23, 2026 at 05:14:30PM +0200, Dmitry Baryshkov wrote:
> >> > > On Mon, Mar 23, 2026 at 07:14:25PM +0530, Manivannan Sadhasivam wrote:
> >> > > > On Mon, Mar 23, 2026 at 08:39:55AM -0500, Rob Herring wrote:
> >> > > > > On Mon, Mar 23, 2026 at 7:16 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >> > > > > >
> >> > > > > > On Sun, Mar 22, 2026 at 06:37:13PM -0500, Rob Herring wrote:
> >> > > > > > > On Tue, Mar 17, 2026 at 09:59:56AM +0530, Manivannan Sadhasivam wrote:
> >> > > > > > > > Lenovo Thinkpad T14s is found to have a soldered down version of M.2 1620
> >> > > > > > > > LGA connector. Though, there is no 1620 LGA form factor defined in the M.2
> >> > > > > > > > spec, it looks very similar to the M.2 Key E connector. So add the
> >> > > > > > > > "pcie-m2-1620-lga-connector" compatible with "pcie-m2-e-connector" fallback
> >> > > > > > > > to reuse the Key E binding.
> >> > > > > > >
> >> > > > > > > What is LGA?
> >> > > > > > >
> >> > > > > >
> >> > > > > > Land Grid Array
> >> > > > > >
> >> > > > > > > If not in the spec, is it really something generic?
> >> > > > > > >
> >> > > > > >
> >> > > > > > Good question. Yes and No! LGA is not something that Lenovo only uses. Other
> >> > > > > > vendors may also use this form factor. PCIe connectors are full of innovation as
> >> > > > > > the spec gives room for hardware designers to be as innovative as possible to
> >> > > > > > save the BOM cost.
> >> > > > >
> >> > > > > innovation == incompatible changes
> >> > > > >
> >> > > >
> >> > > > Yes, I was trying to sound nice :)
> >> > > >
> >> > > > > > This is why I do not want to make it Lenovo specific. But if you prefer that, I
> >> > > > > > can name it as "lenovo,pcie-m2-1620-lga-connector".
> >> > > > >
> >> > > > > Depends if you think that s/w needs to know the differences. Hard to
> >> > > > > say with a sample size of 1.
> >> > > > >
> >> > > >
> >> > > > Sure. Will add the 'lenovo' prefix then.
> >> > >
> >> > > Is it really Lenovo? Or is it some other module vendor, whose LGAs are
> >> > > being used by Lenovo?
> >> > >
> >> > > I remember that DB820c also used some kind of a module for the WiFi card
> >> > > (which might be M.2 compatible or might not, I can't find exact docs at
> >> > > this point).
> >> > >
> >> >
> >> > I don't know. These kind of designs might be reused by several vendors. But
> >> > considering that we should not make it generic, I'd go with Lenovo as that's
> >> > the only vendor we know as of now.
> >>
> >> ... and later we learn that other vendors use the same idea /pinout,
> >> then nothing stops us from still telling that it's a
> >> "lenovo,pcie-m2-something-lga".
> >>
> >
> > How do you possibly know whether a single vendor has introduced this form factor
> > or reused by multiple ones? Atleast, I don't have access to such a source to
> > confirm.
> >
> I've not really been following this thread/patchset in detail; but want me to try and check with the T14s platform team if this device is specifically made for us (Lenovo) or not?
> I doubt it is - we just don't do that usually, but I can go and ask the question if it will help resolve this (with the caveat that it could hold up the review for a bit and I may not be able to get a straight answer)
>
I can drop this specific patch in the meantime.
> My vote (for what little it's worth) would be to make it non-Lenovo specific. Then when the same part causes issues on another vendors platform I won't get asked questions about why Lenovo is breaking <other vendor> :)
>
Even if Lenovo prefix is used, it won't break other vendors. Just that we will
end up adding more compatibles.
Anyhow, I'll wait for your reply and drop this patch for next revision.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH] dt-bindings: reset: st: convert to dtschema
From: Gopi Krishna Menon @ 2026-03-25 12:04 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: sre, robh, krzk+dt, lee, conor+dt, daniel.baluta, simona.toaca,
d-gole, m-chawdhry, linux-pm, devicetree, linux-kernel
In-Reply-To: <20260325-speedy-amethyst-beaver-08a3a4@quoll>
On Wed, Mar 25, 2026 at 12:39:36PM +0100, Krzysztof Kozlowski wrote:
> On Tue, Mar 24, 2026 at 09:29:30PM +0530, Gopi Krishna Menon wrote:
> > Convert the STiH4xx reset controller bindings to DT schema.
> >
> > Suggested-by: Daniel Baluta <daniel.baluta@nxp.com>
> > Suggested-by: Dhruva Gole <d-gole@ti.com>
>
> Both suggested you to write this patch?
>
Hi Krzysztof,
Thanks for the review, They helped me to improve the PATCH,
- Dhruva suggested me to change the subject from dt-bindings: power:
reset: st: convert to dtschema to dt-bindings: reset: st: convert to
dtschema as that was the general trend followed with similar files.
- Daniel suggested me to use the word 'reset' instead of 'restart' in
the patch (whereever possible) as that is more accurate here.
That's why I added those Suggested-by tags.
> > Signed-off-by: Gopi Krishna Menon <krishnagopi487@gmail.com>
> > ---
> > Note:
> > * This patch is part of the GSoC2026 application process for device tree bindings conversions
> > * https://github.com/LinuxFoundationGSoC/ProjectIdeas/wiki/GSoC-2026-Device-Tree-Bindings
> >
> > .../power/reset/st,stih407-restart.yaml | 31 +++++++++++++++++++
> > .../bindings/power/reset/st-reset.txt | 11 -------
> > 2 files changed, 31 insertions(+), 11 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
> > delete mode 100644 Documentation/devicetree/bindings/power/reset/st-reset.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml b/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
> > new file mode 100644
> > index 000000000000..d7adbc00f5c3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
> > @@ -0,0 +1,31 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/power/reset/st,stih407-restart.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ST SW reset controller
> > +
> > +maintainers:
> > + - Lee Jones <lee@kernel.org>
> > +
> > +properties:
> > + compatible:
> > + const: st,stih407-restart
> > +
> > + st,syscfg:
> > + description: phandle of the syscfg node
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > +
> > +required:
> > + - compatible
> > + - st,syscfg
> > +
> > +unevaluatedProperties: false
>
> additionalProperties instead
>
Ok will send a V2 for this.
>
> Best regards,
> Krzysztof
>
Thanks,
Gopi Krishna Menon
^ permalink raw reply
* Re: [PATCH] dt-bindings: reset: st: convert to dtschema
From: Krzysztof Kozlowski @ 2026-03-25 11:39 UTC (permalink / raw)
To: Gopi Krishna Menon
Cc: sre, robh, krzk+dt, lee, conor+dt, daniel.baluta, simona.toaca,
d-gole, m-chawdhry, linux-pm, devicetree, linux-kernel
In-Reply-To: <20260324155935.183952-1-krishnagopi487@gmail.com>
On Tue, Mar 24, 2026 at 09:29:30PM +0530, Gopi Krishna Menon wrote:
> Convert the STiH4xx reset controller bindings to DT schema.
>
> Suggested-by: Daniel Baluta <daniel.baluta@nxp.com>
> Suggested-by: Dhruva Gole <d-gole@ti.com>
Both suggested you to write this patch?
> Signed-off-by: Gopi Krishna Menon <krishnagopi487@gmail.com>
> ---
> Note:
> * This patch is part of the GSoC2026 application process for device tree bindings conversions
> * https://github.com/LinuxFoundationGSoC/ProjectIdeas/wiki/GSoC-2026-Device-Tree-Bindings
>
> .../power/reset/st,stih407-restart.yaml | 31 +++++++++++++++++++
> .../bindings/power/reset/st-reset.txt | 11 -------
> 2 files changed, 31 insertions(+), 11 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
> delete mode 100644 Documentation/devicetree/bindings/power/reset/st-reset.txt
>
> diff --git a/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml b/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
> new file mode 100644
> index 000000000000..d7adbc00f5c3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/st,stih407-restart.yaml
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/reset/st,stih407-restart.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ST SW reset controller
> +
> +maintainers:
> + - Lee Jones <lee@kernel.org>
> +
> +properties:
> + compatible:
> + const: st,stih407-restart
> +
> + st,syscfg:
> + description: phandle of the syscfg node
> + $ref: /schemas/types.yaml#/definitions/phandle
> +
> +required:
> + - compatible
> + - st,syscfg
> +
> +unevaluatedProperties: false
additionalProperties instead
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v5 2/4] cpupower-frequency-info.1: use the proper name of the --perf option
From: Roberto Ricci @ 2026-03-25 11:25 UTC (permalink / raw)
To: Shuah Khan
Cc: Thomas Renninger, Shuah Khan, John B. Wyatt IV, John Kacur,
linux-pm, linux-kernel
In-Reply-To: <32cdfa7a-765c-4f74-9839-d2e0b01435dc@linuxfoundation.org>
On 2026-03-24 17:06 -0600, Shuah Khan wrote:
> On 3/24/26 16:39, Roberto Ricci wrote:
> > The cpupower-frequency-info(1) man page describes a '--perf' option.
> > Even though this form is accepted by the program, its proper name is
> > '--performance'.
> >
> > cpufreq-info.c:
> > {"performance", no_argument, NULL, 'c'},
> > [...]
> > --- a/tools/power/cpupower/man/cpupower-frequency-info.1
> > +++ b/tools/power/cpupower/man/cpupower-frequency-info.1
> > @@ -53,7 +53,7 @@ human\-readable output for the \-f, \-w, \-s and \-y parameters.
> > \fB\-n\fR \fB\-\-no-rounding\fR
> > Output frequencies and latencies without rounding off values.
> > .TP
> > -\fB\-c\fR \fB\-\-perf\fR
> > +\fB\-c\fR \fB\-\-performance\fR
>
> I would keep perf and also add performance since --perf and --performance
> work - it is lot easier to type --perf
--perf would still be accepted by the program, whether or not the man
page mentions it. getopt_long() accepts any abbreviation which is not
ambiguous. While I agree that it would be nice to remind users about
abbreviations, why should the man page suggest an arbitrary one out of
the many accepted? And what about the other options (such as
--governors)? Suggesting an abbreviated long form only for --performance
would be inconsistent. The general convention is to list the one-letter
form and the complete long form. Also, if you want to save typing during
interactive use, you can use -c.
^ permalink raw reply
* Re: [PATCH 4/6] mfd: sprd-sc27xx: Switch to devm_mfd_add_devices()
From: Lee Jones @ 2026-03-25 11:22 UTC (permalink / raw)
To: Otto Pflüger
Cc: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang, Pavel Machek,
Liam Girdwood, Mark Brown, Sebastian Reichel, linux-rtc,
devicetree, linux-kernel, linux-leds, linux-pm
In-Reply-To: <ab2i6i2D5q0t0xZ5@abscue.de>
> Could you clarify what should be changed?
Sure.
> On Mon, Mar 09, 2026 at 06:58:56PM +0000, Lee Jones wrote:
> > On Sun, 22 Feb 2026, Otto Pflüger wrote:
> >
> > > To allow instantiating subdevices such as the regulator and poweroff
> > > devices that do not have corresponding device tree nodes with a
> > > "compatible" property, use devm_mfd_add_devices() with MFD cells instead
> > > of devm_of_platform_populate(). Since different PMICs in the SC27xx
> > > series contain different components, use separate MFD cell tables for
> > > each PMIC model. Define cells for all components that have upstream
> > > drivers at this point.
> >
> > We're not passing one device registration API's data (MFD)
> > through another (Device Tree).
> >
> > Pass an identifier through and match on that instead.
> >
> > Look at how all of the other drivers in MFD do it.
> >
> > [...]
> > > +static const struct mfd_cell sc2730_devices[] = {
> > > + MFD_CELL_OF("sc2730-adc", NULL, NULL, 0, 0, "sprd,sc2730-adc"),
> > > + MFD_CELL_OF("sc2730-bltc", NULL, NULL, 0, 0, "sprd,sc2730-bltc"),
> > > + MFD_CELL_OF("sc2730-efuse", NULL, NULL, 0, 0, "sprd,sc2730-efuse"),
> > > + MFD_CELL_OF("sc2730-eic", NULL, NULL, 0, 0, "sprd,sc2730-eic"),
> > > + MFD_CELL_OF("sc2730-fgu", NULL, NULL, 0, 0, "sprd,sc2730-fgu"),
> > > + MFD_CELL_OF("sc2730-rtc", NULL, NULL, 0, 0, "sprd,sc2730-rtc"),
> > > + MFD_CELL_OF("sc2730-vibrator", NULL, NULL, 0, 0, "sprd,sc2730-vibrator"),
> > > +};
> > > +
> > > +static const struct mfd_cell sc2731_devices[] = {
> > > + MFD_CELL_OF("sc2731-adc", NULL, NULL, 0, 0, "sprd,sc2731-adc"),
> > > + MFD_CELL_OF("sc2731-bltc", NULL, NULL, 0, 0, "sprd,sc2731-bltc"),
> > > + MFD_CELL_OF("sc2731-charger", NULL, NULL, 0, 0, "sprd,sc2731-charger"),
> > > + MFD_CELL_OF("sc2731-efuse", NULL, NULL, 0, 0, "sprd,sc2731-efuse"),
> > > + MFD_CELL_OF("sc2731-eic", NULL, NULL, 0, 0, "sprd,sc2731-eic"),
> > > + MFD_CELL_OF("sc2731-fgu", NULL, NULL, 0, 0, "sprd,sc2731-fgu"),
> > > + MFD_CELL_NAME("sc2731-poweroff"),
> > > + MFD_CELL_NAME("sc2731-regulator"),
> > > + MFD_CELL_OF("sc2731-rtc", NULL, NULL, 0, 0, "sprd,sc2731-rtc"),
> > > + MFD_CELL_OF("sc2731-vibrator", NULL, NULL, 0, 0, "sprd,sc2731-vibrator"),
> > > };
>
> Assuming that these tables are the "registration API's data", I don't
> see where it is being passed through the device tree. The device tree
> contains nodes for some of these MFD components, and I've listed their
> compatibles here so that the MFD core finds these nodes and registers
> them with the corresponding devices (which was previously done
> automatically by devm_of_platform_populate).
>
> > >
> > > /*
> > > @@ -59,12 +84,16 @@ static const struct sprd_pmic_data sc2730_data = {
> > > .irq_base = SPRD_SC2730_IRQ_BASE,
> > > .num_irqs = SPRD_SC2730_IRQ_NUMS,
> > > .charger_det = SPRD_SC2730_CHG_DET,
> > > + .cells = sc2730_devices,
> > > + .num_cells = ARRAY_SIZE(sc2730_devices),
Remove these from here.
Either replace them with an ID that you can match on or stop passing
'sc2730_data' through .data and pass an ID through there instead. Then
choose 'sc2730_data' and 'sc2730_devices' in an switch() statement
instead, just like the vast majority of existing MFD drivers do.
> > > };
> > >
> > > static const struct sprd_pmic_data sc2731_data = {
> > > .irq_base = SPRD_SC2731_IRQ_BASE,
> > > .num_irqs = SPRD_SC2731_IRQ_NUMS,
> > > .charger_det = SPRD_SC2731_CHG_DET,
> > > + .cells = sc2731_devices,
> > > + .num_cells = ARRAY_SIZE(sc2731_devices),
> > > };
>
> Here I am simply referencing the tables above in the device-specific
> MFD data. These structs containing device-specific data already exist,
> they are private to the MFD driver, and I wouldn't consider them part
> of the device tree.
>
> I've looked at mt6397-core.c and it seems to be doing the exact same
> thing with its "struct chip_data".
That was a momentary oversight. It's also passing a driver-level
call-back which I despise. However, past mistakes are not good
justifications for new ones.
> Some other drivers use a numeric ID
> for this purpose, but how would that be different from a pointer as long
> as it identifies the same data within the MFD driver?
The point is that sc2731_data->cells would be passed through the Device
Tree's .data attribute, which is not allowed.
--
Lee Jones [李琼斯]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox