* [PATCH v9 1/6] mfd: khadas-mcu: Add per-variant configuration infrastructure and VIM4 support
2026-08-05 8:06 [PATCH v9 0/6] Add VIM4 MCU/FAN support Ronald Claveau via B4 Relay
@ 2026-08-05 8:06 ` Ronald Claveau via B4 Relay
2026-08-05 8:16 ` sashiko-bot
2026-08-05 8:06 ` [PATCH v9 2/6] mfd: khadas-mcu: Use MFD_CELL_* macros for cell declarations Ronald Claveau via B4 Relay
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ronald Claveau via B4 Relay @ 2026-08-05 8:06 UTC (permalink / raw)
To: Neil Armstrong, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andi Shyti, Kevin Hilman, Jerome Brunet,
Martin Blumenstingl, Beniamino Galvani, Rafael J. Wysocki,
Daniel Lezcano, Zhang Rui, Lukasz Luba, Liam Girdwood, Mark Brown
Cc: linux-amlogic, devicetree, linux-kernel, linux-i2c,
linux-arm-kernel, linux-pm, mfd, Ronald Claveau
From: Ronald Claveau <linux-kernel-dev@aliel.fr>
Some values change per variant, add the structure to manage it.
Add an enum khadas_mcu_type used as value to match.
Also introduce khadas_mcu_fan_pdata to pass fan register address, a
table named levels to get fan-supported levels on the MCU, and the number
of levels to the fan sub-driver, removing the hardcoded constants.
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
include/linux/mfd/khadas-mcu.h | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/include/linux/mfd/khadas-mcu.h b/include/linux/mfd/khadas-mcu.h
index a99ba2ed0e4e0..e4044c251b97e 100644
--- a/include/linux/mfd/khadas-mcu.h
+++ b/include/linux/mfd/khadas-mcu.h
@@ -70,6 +70,13 @@
#define KHADAS_MCU_WOL_INIT_START_REG 0x87 /* WO */
#define KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG 0x88 /* WO */
+/* VIM4 specific registers */
+#define KHADAS_MCU_VIM4_REST_CONF_REG 0x2c /* WO - reset EEPROM */
+#define KHADAS_MCU_VIM4_LED_ON_RAM_REG 0x89 /* WO - LED volatile */
+#define KHADAS_MCU_VIM4_FAN_CTRL_REG 0x8a /* WO */
+#define KHADAS_MCU_VIM4_WDT_EN_REG 0x8b /* WO */
+#define KHADAS_MCU_VIM4_SYS_RST_REG 0x91 /* WO */
+
enum {
KHADAS_BOARD_VIM1 = 0x1,
KHADAS_BOARD_VIM2,
@@ -88,4 +95,21 @@ struct khadas_mcu {
struct regmap *regmap;
};
+/**
+ * struct khadas_mcu_fan_pdata - fan sub-driver configuration
+ * @fan_reg: register address to write the fan level
+ * @levels: table mapping logical cooling-device states to raw fan levels
+ * @nlevels: number of entries in @levels
+ */
+struct khadas_mcu_fan_pdata {
+ unsigned int fan_reg;
+ const unsigned int *levels;
+ unsigned int nlevels;
+};
+
+enum khadas_mcu_type {
+ KHADAS_MCU_GENERIC, /* VIM1/2/3, Edge, Edge-V */
+ KHADAS_MCU_VIM4,
+};
+
#endif /* MFD_KHADAS_MCU_H */
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v9 1/6] mfd: khadas-mcu: Add per-variant configuration infrastructure and VIM4 support
2026-08-05 8:06 ` [PATCH v9 1/6] mfd: khadas-mcu: Add per-variant configuration infrastructure and VIM4 support Ronald Claveau via B4 Relay
@ 2026-08-05 8:16 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-05 8:16 UTC (permalink / raw)
To: Ronald Claveau
Cc: mfd, neil.armstrong, linux-amlogic, lee, robh, conor+dt,
devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Passing child-specific configuration (fan registers and cooling levels) from the core MFD parent driver to the child violates MFD separation of concerns.
- [Medium] The patch claims to remove hardcoded constants and add VIM4 support but fails to include the corresponding C code modifications, leaving the feature entirely unimplemented.
--
--- Patch [1]: [PATCH v9 1/6] mfd: khadas-mcu: Add per-variant configuration infrastructure and VIM4 support ---
commit 9c30c9c20ad7d8a42c271a8ebbe94020ab58ae90
Author: Ronald Claveau <linux-kernel-dev@aliel.fr>
mfd: khadas-mcu: Add per-variant configuration infrastructure and VIM4 support
This commit adds per-variant configuration structures and enums to manage
different MCU variants, specifically adding VIM4 support. It also introduces
a structure to pass fan configuration to the fan sub-driver to remove
hardcoded constants.
> diff --git a/include/linux/mfd/khadas-mcu.h b/include/linux/mfd/khadas-mcu.h
> index a99ba2ed0e4e0..e4044c251b97e 100644
> --- a/include/linux/mfd/khadas-mcu.h
> +++ b/include/linux/mfd/khadas-mcu.h
> @@ -70,6 +70,13 @@
> #define KHADAS_MCU_WOL_INIT_START_REG 0x87 /* WO */
> #define KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG 0x88 /* WO */
>
> +/* VIM4 specific registers */
> +#define KHADAS_MCU_VIM4_REST_CONF_REG 0x2c /* WO - reset EEPROM */
> +#define KHADAS_MCU_VIM4_LED_ON_RAM_REG 0x89 /* WO - LED volatile */
> +#define KHADAS_MCU_VIM4_FAN_CTRL_REG 0x8a /* WO */
> +#define KHADAS_MCU_VIM4_WDT_EN_REG 0x8b /* WO */
> +#define KHADAS_MCU_VIM4_SYS_RST_REG 0x91 /* WO */
[Severity: Medium]
Will these new VIM4 specific registers exceed the unchanged max_register
(0x88) in the core driver's regmap configuration?
It looks like the C file modifications are missing from this patch, meaning
these new registers might be rejected by the regmap API if the configuration
is not updated to allow access up to 0x91.
[ ... ]
> @@ -88,4 +95,21 @@ struct khadas_mcu {
> struct regmap *regmap;
> };
>
> +/**
> + * struct khadas_mcu_fan_pdata - fan sub-driver configuration
> + * @fan_reg: register address to write the fan level
> + * @levels: table mapping logical cooling-device states to raw fan levels
> + * @nlevels: number of entries in @levels
> + */
> +struct khadas_mcu_fan_pdata {
> + unsigned int fan_reg;
> + const unsigned int *levels;
> + unsigned int nlevels;
> +};
[Severity: Medium]
Does passing child-specific configuration like fan registers and cooling
levels from the core MFD parent driver to the child violate MFD separation
of concerns?
The MFD subsystem guidelines suggest that the core MFD parent driver should
only handle shared core resources. Private resources consumed exclusively by
a single child driver should typically be initialized directly within that
child rather than passed from the parent.
[Severity: Medium]
The commit message mentions removing hardcoded constants and passing these
parameters to the fan sub-driver, but is this structure actually used
anywhere in the code?
The corresponding C file modifications appear to be missing from the commit,
which leaves this structure entirely unused.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-add-mcu-fan-khadas-vim4-v9-0-75df78710b9e@aliel.fr?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v9 2/6] mfd: khadas-mcu: Use MFD_CELL_* macros for cell declarations
2026-08-05 8:06 [PATCH v9 0/6] Add VIM4 MCU/FAN support Ronald Claveau via B4 Relay
2026-08-05 8:06 ` [PATCH v9 1/6] mfd: khadas-mcu: Add per-variant configuration infrastructure and VIM4 support Ronald Claveau via B4 Relay
@ 2026-08-05 8:06 ` Ronald Claveau via B4 Relay
2026-08-05 8:16 ` sashiko-bot
2026-08-05 8:06 ` [PATCH v9 3/6] mfd: khadas-mcu: Add support for VIM4 MCU variant Ronald Claveau via B4 Relay
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ronald Claveau via B4 Relay @ 2026-08-05 8:06 UTC (permalink / raw)
To: Neil Armstrong, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andi Shyti, Kevin Hilman, Jerome Brunet,
Martin Blumenstingl, Beniamino Galvani, Rafael J. Wysocki,
Daniel Lezcano, Zhang Rui, Lukasz Luba, Liam Girdwood, Mark Brown
Cc: linux-amlogic, devicetree, linux-kernel, linux-i2c,
linux-arm-kernel, linux-pm, mfd, Ronald Claveau
From: Ronald Claveau <linux-kernel-dev@aliel.fr>
Convert khadas_mcu_fan_cells[] and khadas_mcu_cells[] to use the
MFD_CELL_NAME() helper macro instead of open-coding the struct
mfd_cell initialisers. While at it, make both arrays const since
they are never modified after initialisation.
This is a pure cleanup with no functional change, done in
preparation for a subsequent patch which will need to attach
platform_data to the fan cell.
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
drivers/mfd/khadas-mcu.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mfd/khadas-mcu.c b/drivers/mfd/khadas-mcu.c
index ba981a7886921..0e74efe14f367 100644
--- a/drivers/mfd/khadas-mcu.c
+++ b/drivers/mfd/khadas-mcu.c
@@ -75,13 +75,13 @@ static const struct regmap_config khadas_mcu_regmap_config = {
.cache_type = REGCACHE_MAPLE,
};
-static struct mfd_cell khadas_mcu_fan_cells[] = {
+static const struct mfd_cell khadas_mcu_fan_cells[] = {
/* VIM1/2 Rev13+ and VIM3 only */
- { .name = "khadas-mcu-fan-ctrl", },
+ MFD_CELL_NAME("khadas-mcu-fan-ctrl"),
};
-static struct mfd_cell khadas_mcu_cells[] = {
- { .name = "khadas-mcu-user-mem", },
+static const struct mfd_cell khadas_mcu_cells[] = {
+ MFD_CELL_NAME("khadas-mcu-user-mem"),
};
static int khadas_mcu_probe(struct i2c_client *client)
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v9 3/6] mfd: khadas-mcu: Add support for VIM4 MCU variant
2026-08-05 8:06 [PATCH v9 0/6] Add VIM4 MCU/FAN support Ronald Claveau via B4 Relay
2026-08-05 8:06 ` [PATCH v9 1/6] mfd: khadas-mcu: Add per-variant configuration infrastructure and VIM4 support Ronald Claveau via B4 Relay
2026-08-05 8:06 ` [PATCH v9 2/6] mfd: khadas-mcu: Use MFD_CELL_* macros for cell declarations Ronald Claveau via B4 Relay
@ 2026-08-05 8:06 ` Ronald Claveau via B4 Relay
2026-08-05 8:19 ` sashiko-bot
2026-08-05 8:06 ` [PATCH v9 4/6] thermal: khadas-mcu-fan: Add fan config from platform data Add regulator support Ronald Claveau via B4 Relay
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ronald Claveau via B4 Relay @ 2026-08-05 8:06 UTC (permalink / raw)
To: Neil Armstrong, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andi Shyti, Kevin Hilman, Jerome Brunet,
Martin Blumenstingl, Beniamino Galvani, Rafael J. Wysocki,
Daniel Lezcano, Zhang Rui, Lukasz Luba, Liam Girdwood, Mark Brown
Cc: linux-amlogic, devicetree, linux-kernel, linux-i2c,
linux-arm-kernel, linux-pm, mfd, Ronald Claveau
From: Ronald Claveau <linux-kernel-dev@aliel.fr>
Refactor probe() to use per-variant values
instead of hardcoded globals.
Add dedicated regmap configuration for the VIM4 MCU,
with its own volatile/writeable registers.
Introduce struct khadas_mcu_fan_pdata to pass the fan
control register and duty-cycle levels per variant.
VIM4 uses a different register and level set (0-100)
declared as a list (0, 30, 40, 55, 75, 100) compared
to previous boards (0-3).
Add a new compatible string "khadas,vim4-mcu".
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
drivers/mfd/khadas-mcu.c | 120 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 106 insertions(+), 14 deletions(-)
diff --git a/drivers/mfd/khadas-mcu.c b/drivers/mfd/khadas-mcu.c
index 0e74efe14f367..4cb305ac915e1 100644
--- a/drivers/mfd/khadas-mcu.c
+++ b/drivers/mfd/khadas-mcu.c
@@ -75,47 +75,138 @@ static const struct regmap_config khadas_mcu_regmap_config = {
.cache_type = REGCACHE_MAPLE,
};
+static const unsigned int khadas_mcu_fan_levels[] = {
+ 0, 1, 2, 3,
+};
+
+static const struct khadas_mcu_fan_pdata khadas_mcu_fan_pdata = {
+ .fan_reg = KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
+ .levels = khadas_mcu_fan_levels,
+ .nlevels = ARRAY_SIZE(khadas_mcu_fan_levels),
+};
+
static const struct mfd_cell khadas_mcu_fan_cells[] = {
/* VIM1/2 Rev13+ and VIM3 only */
- MFD_CELL_NAME("khadas-mcu-fan-ctrl"),
+ MFD_CELL_BASIC("khadas-mcu-fan-ctrl", NULL, &khadas_mcu_fan_pdata,
+ sizeof(khadas_mcu_fan_pdata), 0),
};
static const struct mfd_cell khadas_mcu_cells[] = {
MFD_CELL_NAME("khadas-mcu-user-mem"),
};
+static bool khadas_mcu_vim4_reg_volatile(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case KHADAS_MCU_PWR_OFF_CMD_REG:
+ case KHADAS_MCU_VIM4_REST_CONF_REG:
+ case KHADAS_MCU_WOL_INIT_START_REG:
+ case KHADAS_MCU_VIM4_LED_ON_RAM_REG:
+ case KHADAS_MCU_VIM4_FAN_CTRL_REG:
+ case KHADAS_MCU_VIM4_WDT_EN_REG:
+ case KHADAS_MCU_VIM4_SYS_RST_REG:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool khadas_mcu_vim4_reg_writeable(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case KHADAS_MCU_VERSION_0_REG:
+ case KHADAS_MCU_VERSION_1_REG:
+ case KHADAS_MCU_SHUTDOWN_NORMAL_STATUS_REG:
+ return false;
+ default:
+ return true;
+ }
+}
+
+static const struct regmap_config khadas_mcu_vim4_regmap_config = {
+ .reg_bits = 8,
+ .reg_stride = 1,
+ .val_bits = 8,
+ .max_register = KHADAS_MCU_VIM4_SYS_RST_REG,
+ .volatile_reg = khadas_mcu_vim4_reg_volatile,
+ .writeable_reg = khadas_mcu_vim4_reg_writeable,
+ .cache_type = REGCACHE_MAPLE,
+};
+
+static const unsigned int khadas_mcu_vim4_fan_levels[] = {
+ 0, 30, 40, 55, 75, 100,
+};
+
+static const struct khadas_mcu_fan_pdata khadas_mcu_vim4_fan_pdata = {
+ .fan_reg = KHADAS_MCU_VIM4_FAN_CTRL_REG,
+ .levels = khadas_mcu_vim4_fan_levels,
+ .nlevels = ARRAY_SIZE(khadas_mcu_vim4_fan_levels),
+};
+
+static const struct mfd_cell khadas_mcu_vim4_fan_cells[] = {
+ MFD_CELL_BASIC("khadas-mcu-fan-ctrl", NULL, &khadas_mcu_vim4_fan_pdata,
+ sizeof(khadas_mcu_vim4_fan_pdata), 0),
+};
+
static int khadas_mcu_probe(struct i2c_client *client)
{
+ const struct mfd_cell *cells, *fan_cells;
+ const struct regmap_config *regmap_cfg;
struct device *dev = &client->dev;
+ int ncells, nfan_cells, ret;
struct khadas_mcu *ddata;
- int ret;
+ const void *match;
ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
if (!ddata)
return -ENOMEM;
+ match = i2c_get_match_data(client);
+ if (!match)
+ return -ENODEV;
+
+ switch ((uintptr_t)match) {
+ case KHADAS_MCU_GENERIC:
+ regmap_cfg = &khadas_mcu_regmap_config;
+ cells = khadas_mcu_cells;
+ ncells = ARRAY_SIZE(khadas_mcu_cells);
+ fan_cells = khadas_mcu_fan_cells;
+ nfan_cells = ARRAY_SIZE(khadas_mcu_fan_cells);
+ break;
+ case KHADAS_MCU_VIM4:
+ regmap_cfg = &khadas_mcu_vim4_regmap_config;
+ cells = NULL;
+ ncells = 0;
+ fan_cells = khadas_mcu_vim4_fan_cells;
+ nfan_cells = ARRAY_SIZE(khadas_mcu_vim4_fan_cells);
+ break;
+ default:
+ return -ENODEV;
+ }
+
i2c_set_clientdata(client, ddata);
ddata->dev = dev;
- ddata->regmap = devm_regmap_init_i2c(client, &khadas_mcu_regmap_config);
+ ddata->regmap = devm_regmap_init_i2c(client, regmap_cfg);
if (IS_ERR(ddata->regmap)) {
ret = PTR_ERR(ddata->regmap);
- dev_err(dev, "Failed to allocate register map: %d\n", ret);
- return ret;
+ return dev_err_probe(dev, ret, "Failed to allocate register map\n");
}
- ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
- khadas_mcu_cells,
- ARRAY_SIZE(khadas_mcu_cells),
- NULL, 0, NULL);
- if (ret)
- return ret;
+ if (cells && ncells) {
+ ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
+ cells,
+ ncells,
+ NULL, 0, NULL);
+ if (ret)
+ return ret;
+ }
if (of_property_present(dev->of_node, "#cooling-cells"))
return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
- khadas_mcu_fan_cells,
- ARRAY_SIZE(khadas_mcu_fan_cells),
+ fan_cells,
+ nfan_cells,
NULL, 0, NULL);
return 0;
@@ -123,7 +214,8 @@ static int khadas_mcu_probe(struct i2c_client *client)
#ifdef CONFIG_OF
static const struct of_device_id khadas_mcu_of_match[] = {
- { .compatible = "khadas,mcu", },
+ { .compatible = "khadas,mcu", .data = (void *)KHADAS_MCU_GENERIC },
+ { .compatible = "khadas,vim4-mcu", .data = (void *)KHADAS_MCU_VIM4 },
{},
};
MODULE_DEVICE_TABLE(of, khadas_mcu_of_match);
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v9 3/6] mfd: khadas-mcu: Add support for VIM4 MCU variant
2026-08-05 8:06 ` [PATCH v9 3/6] mfd: khadas-mcu: Add support for VIM4 MCU variant Ronald Claveau via B4 Relay
@ 2026-08-05 8:19 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-05 8:19 UTC (permalink / raw)
To: Ronald Claveau
Cc: neil.armstrong, linux-amlogic, mfd, lee, conor+dt, robh,
devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver unconditionally fails to probe on generic Khadas MCU boards (VIM1/VIM2/VIM3) due to a NULL pointer check on valid match data.
--
--- Patch [3]: [PATCH v9 3/6] mfd: khadas-mcu: Add support for VIM4 MCU variant ---
commit eab418dac1b5e94bd297b0a14e92e87b2063dc9b
Author: Ronald Claveau <linux-kernel-dev@aliel.fr>
mfd: khadas-mcu: Add support for VIM4 MCU variant
This patch refactors the probe function to use per-variant values instead of
hardcoded globals. It adds dedicated regmap configuration, fan control register,
and duty-cycle levels for the VIM4 MCU variant. A new compatible string
"khadas,vim4-mcu" is introduced for this new device.
> diff --git a/drivers/mfd/khadas-mcu.c b/drivers/mfd/khadas-mcu.c
> index 0e74efe14f367..4cb305ac915e1 100644
> --- a/drivers/mfd/khadas-mcu.c
> +++ b/drivers/mfd/khadas-mcu.c
[ ... ]
> @@ -75,47 +75,138 @@ static const struct regmap_config khadas_mcu_regmap_config = {
[ ... ]
> static int khadas_mcu_probe(struct i2c_client *client)
> {
> + const struct mfd_cell *cells, *fan_cells;
> + const struct regmap_config *regmap_cfg;
> struct device *dev = &client->dev;
> + int ncells, nfan_cells, ret;
> struct khadas_mcu *ddata;
> - int ret;
> + const void *match;
>
> ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> if (!ddata)
> return -ENOMEM;
>
> + match = i2c_get_match_data(client);
> + if (!match)
> + return -ENODEV;
[Severity: High]
Does this code inadvertently break the probe for generic Khadas MCU boards
(VIM1/VIM2/VIM3)?
Because KHADAS_MCU_GENERIC evaluates to 0 in its enum, i2c_get_match_data() will
return a NULL pointer when matching "khadas,mcu" in khadas_mcu_probe() in
drivers/mfd/khadas-mcu.c. This will cause the check here to evaluate to true and
unconditionally abort the probe with -ENODEV.
[ ... ]
> @@ -123,7 +214,8 @@ static int khadas_mcu_probe(struct i2c_client *client)
>
> #ifdef CONFIG_OF
> static const struct of_device_id khadas_mcu_of_match[] = {
> - { .compatible = "khadas,mcu", },
> + { .compatible = "khadas,mcu", .data = (void *)KHADAS_MCU_GENERIC },
> + { .compatible = "khadas,vim4-mcu", .data = (void *)KHADAS_MCU_VIM4 },
> {},
> };
> MODULE_DEVICE_TABLE(of, khadas_mcu_of_match);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-add-mcu-fan-khadas-vim4-v9-0-75df78710b9e@aliel.fr?part=3
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v9 4/6] thermal: khadas-mcu-fan: Add fan config from platform data Add regulator support
2026-08-05 8:06 [PATCH v9 0/6] Add VIM4 MCU/FAN support Ronald Claveau via B4 Relay
` (2 preceding siblings ...)
2026-08-05 8:06 ` [PATCH v9 3/6] mfd: khadas-mcu: Add support for VIM4 MCU variant Ronald Claveau via B4 Relay
@ 2026-08-05 8:06 ` Ronald Claveau via B4 Relay
2026-08-05 8:21 ` sashiko-bot
2026-08-05 8:06 ` [PATCH v9 5/6] arm64: dts: amlogic: t7: Add i2c controller node Ronald Claveau via B4 Relay
2026-08-05 8:06 ` [PATCH v9 6/6] arm64: dts: amlogic: t7: khadas-vim4: Add i2c MCU fan node Ronald Claveau via B4 Relay
5 siblings, 1 reply; 13+ messages in thread
From: Ronald Claveau via B4 Relay @ 2026-08-05 8:06 UTC (permalink / raw)
To: Neil Armstrong, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andi Shyti, Kevin Hilman, Jerome Brunet,
Martin Blumenstingl, Beniamino Galvani, Rafael J. Wysocki,
Daniel Lezcano, Zhang Rui, Lukasz Luba, Liam Girdwood, Mark Brown
Cc: linux-amlogic, devicetree, linux-kernel, linux-i2c,
linux-arm-kernel, linux-pm, mfd, Ronald Claveau
From: Ronald Claveau <linux-kernel-dev@aliel.fr>
Replace the hardcoded MAX_LEVEL constant and fan register
with values read from platform_data (fan_reg, levels[] and nlevels),
as new MCUs need different values.
The cooling device's state is stored as an index into levels[],
and khadas_mcu_fan_set_level() writes levels[state] to fan_reg,
rather than writing the raw state value as before.
Optionally acquire and enable a "fan" regulator supply
at probe time and on resume,
so boards that gate fan power through a regulator are handled.
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
drivers/thermal/khadas_mcu_fan.c | 58 ++++++++++++++++++++++++++++------------
1 file changed, 41 insertions(+), 17 deletions(-)
diff --git a/drivers/thermal/khadas_mcu_fan.c b/drivers/thermal/khadas_mcu_fan.c
index 21b3d0a71bd0d..c7f74394b823f 100644
--- a/drivers/thermal/khadas_mcu_fan.c
+++ b/drivers/thermal/khadas_mcu_fan.c
@@ -13,34 +13,30 @@
#include <linux/regmap.h>
#include <linux/sysfs.h>
#include <linux/thermal.h>
-
-#define MAX_LEVEL 3
+#include <linux/regulator/consumer.h>
struct khadas_mcu_fan_ctx {
struct khadas_mcu *mcu;
+ unsigned int fan_reg;
unsigned int level;
+ const unsigned int *levels;
+ unsigned int nlevels;
struct thermal_cooling_device *cdev;
+ struct regulator *power;
};
static int khadas_mcu_fan_set_level(struct khadas_mcu_fan_ctx *ctx,
unsigned int level)
{
- int ret;
-
- ret = regmap_write(ctx->mcu->regmap, KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
- level);
- if (ret)
- return ret;
-
- ctx->level = level;
-
- return 0;
+ return regmap_write(ctx->mcu->regmap, ctx->fan_reg, level);
}
static int khadas_mcu_fan_get_max_state(struct thermal_cooling_device *cdev,
unsigned long *state)
{
- *state = MAX_LEVEL;
+ struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
+
+ *state = ctx->nlevels - 1;
return 0;
}
@@ -60,14 +56,21 @@ khadas_mcu_fan_set_cur_state(struct thermal_cooling_device *cdev,
unsigned long state)
{
struct khadas_mcu_fan_ctx *ctx = cdev->devdata;
+ int ret;
- if (state > MAX_LEVEL)
+ if (state >= ctx->nlevels)
return -EINVAL;
if (state == ctx->level)
return 0;
- return khadas_mcu_fan_set_level(ctx, state);
+ ret = khadas_mcu_fan_set_level(ctx, ctx->levels[state]);
+ if (ret)
+ return ret;
+
+ ctx->level = state;
+
+ return 0;
}
static const struct thermal_cooling_device_ops khadas_mcu_fan_cooling_ops = {
@@ -78,6 +81,7 @@ static const struct thermal_cooling_device_ops khadas_mcu_fan_cooling_ops = {
static int khadas_mcu_fan_probe(struct platform_device *pdev)
{
+ const struct khadas_mcu_fan_pdata *pdata = dev_get_platdata(&pdev->dev);
struct khadas_mcu *mcu = dev_get_drvdata(pdev->dev.parent);
struct thermal_cooling_device *cdev;
struct device *dev = &pdev->dev;
@@ -87,7 +91,22 @@ static int khadas_mcu_fan_probe(struct platform_device *pdev)
ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
if (!ctx)
return -ENOMEM;
+
ctx->mcu = mcu;
+ ctx->fan_reg = pdata->fan_reg;
+ ctx->levels = pdata->levels;
+ ctx->nlevels = pdata->nlevels;
+
+ ctx->power = devm_regulator_get(dev->parent, "fan");
+ if (IS_ERR(ctx->power))
+ return PTR_ERR(ctx->power);
+
+ ret = regulator_enable(ctx->power);
+ if (ret) {
+ dev_err(dev, "Failed to enable fan power supply: %d\n", ret);
+ return ret;
+ }
+
platform_set_drvdata(pdev, ctx);
cdev = devm_thermal_of_child_cooling_device_register(dev->parent,
@@ -125,14 +144,19 @@ static int khadas_mcu_fan_suspend(struct device *dev)
ctx->level = level_save;
- return 0;
+ return regulator_disable(ctx->power);
}
static int khadas_mcu_fan_resume(struct device *dev)
{
struct khadas_mcu_fan_ctx *ctx = dev_get_drvdata(dev);
+ int ret;
+
+ ret = regulator_enable(ctx->power);
+ if (ret)
+ return ret;
- return khadas_mcu_fan_set_level(ctx, ctx->level);
+ return khadas_mcu_fan_set_level(ctx, ctx->levels[ctx->level]);
}
#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v9 4/6] thermal: khadas-mcu-fan: Add fan config from platform data Add regulator support
2026-08-05 8:06 ` [PATCH v9 4/6] thermal: khadas-mcu-fan: Add fan config from platform data Add regulator support Ronald Claveau via B4 Relay
@ 2026-08-05 8:21 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-05 8:21 UTC (permalink / raw)
To: Ronald Claveau
Cc: devicetree, mfd, linux-amlogic, neil.armstrong, lee, robh,
conor+dt
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Missing regulator_disable() in error paths and on driver unbind.
- [High] Regulator devres allocation is incorrectly attached to the parent device.
- [Medium] In suspend and shutdown, the code bypasses the newly introduced `levels` abstraction and contains dead state-saving logic.
Pre-existing issues:
- [Critical] Thermal cooling device devres allocation is incorrectly attached to the parent device, leading to UAF on child unbind.
--
--- Patch [4]: [PATCH v9 4/6] thermal: khadas-mcu-fan: Add fan config from platform data Add regulator support ---
commit cdd1fb53966220cb2785c705c998afe04ac55405
Author: Ronald Claveau <linux-kernel-dev@aliel.fr>
thermal: khadas-mcu-fan: Add fan config from platform data Add regulator support
This commit updates the khadas-mcu-fan driver to read fan configuration
(register and levels) from platform data instead of using hardcoded values.
The cooling device's state is stored as an index into levels[]. It also
optionally acquires and enables a "fan" regulator supply during probe and
resume.
> diff --git a/drivers/thermal/khadas_mcu_fan.c b/drivers/thermal/khadas_mcu_fan.c
> index 21b3d0a71bd0d..c7f74394b823f 100644
> --- a/drivers/thermal/khadas_mcu_fan.c
> +++ b/drivers/thermal/khadas_mcu_fan.c
> @@ -13,34 +13,30 @@
> static int khadas_mcu_fan_set_level(struct khadas_mcu_fan_ctx *ctx,
> unsigned int level)
> {
> - int ret;
> -
> - ret = regmap_write(ctx->mcu->regmap, KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
> - level);
> - if (ret)
> - return ret;
> -
> - ctx->level = level;
> -
> - return 0;
> + return regmap_write(ctx->mcu->regmap, ctx->fan_reg, level);
> }
[Severity: Medium]
Since this function no longer updates ctx->level and now takes a raw register
value instead of a state index, does this break khadas_mcu_fan_suspend() and
khadas_mcu_fan_shutdown()?
Both functions still call khadas_mcu_fan_set_level(ctx, 0), which now writes a
raw 0 to the hardware instead of ctx->levels[0]. Additionally, the state-saving
logic in suspend is now dead code since khadas_mcu_fan_set_level() no longer
mutates ctx->level.
[ ... ]
> @@ -87,7 +91,22 @@ static int khadas_mcu_fan_probe(struct platform_device *pdev)
> ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> if (!ctx)
> return -ENOMEM;
> +
> ctx->mcu = mcu;
> + ctx->fan_reg = pdata->fan_reg;
> + ctx->levels = pdata->levels;
> + ctx->nlevels = pdata->nlevels;
> +
> + ctx->power = devm_regulator_get(dev->parent, "fan");
[Severity: High]
Does devm_regulator_get() incorrectly attach the devres release action to
the parent device instead of the child device?
If the child driver is unbound or probe defers repeatedly, the memory and
regulator handle could be permanently leaked on the parent device's devres
list. Should this use dev instead of dev->parent?
> + if (IS_ERR(ctx->power))
> + return PTR_ERR(ctx->power);
> +
> + ret = regulator_enable(ctx->power);
> + if (ret) {
> + dev_err(dev, "Failed to enable fan power supply: %d\n", ret);
> + return ret;
> + }
> +
> platform_set_drvdata(pdev, ctx);
>
> cdev = devm_thermal_of_child_cooling_device_register(dev->parent,
[Severity: Critical]
This is a pre-existing issue, but does using dev->parent for the thermal
cooling device devres allocation mean the cooling device will outlive the
child device?
If the child driver is unbound via sysfs, ctx is freed, but the cooling
device remains active on the parent. Could reading the cooling device state
via sysfs then cause a use-after-free by dereferencing the freed ctx pointer?
> dev->parent->of_node,
> "khadas-mcu-fan", ctx,
> &khadas_mcu_fan_cooling_ops);
> if (IS_ERR(cdev)) {
> ret = PTR_ERR(cdev);
> dev_err(dev, "Failed to register khadas-mcu-fan as cooling device: %d\n",
> ret);
> return ret;
[Severity: High]
If devm_thermal_of_child_cooling_device_register() fails here, does the
probe function return without disabling the manually enabled regulator?
> }
[ ... ]
> @@ -125,14 +144,19 @@ static int khadas_mcu_fan_suspend(struct device *dev)
> static int khadas_mcu_fan_resume(struct device *dev)
> {
> struct khadas_mcu_fan_ctx *ctx = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = regulator_enable(ctx->power);
> + if (ret)
> + return ret;
>
> - return khadas_mcu_fan_set_level(ctx, ctx->level);
> + return khadas_mcu_fan_set_level(ctx, ctx->levels[ctx->level]);
> }
[Severity: High]
If khadas_mcu_fan_set_level() fails here and returns an error to the PM core,
does the regulator remain permanently enabled?
Additionally, it appears the driver is missing a .remove callback in the
platform_driver definition to symmetrically disable the regulator on unbind.
Could this lead to a leak of the regulator enable count?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-add-mcu-fan-khadas-vim4-v9-0-75df78710b9e@aliel.fr?part=4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v9 5/6] arm64: dts: amlogic: t7: Add i2c controller node
2026-08-05 8:06 [PATCH v9 0/6] Add VIM4 MCU/FAN support Ronald Claveau via B4 Relay
` (3 preceding siblings ...)
2026-08-05 8:06 ` [PATCH v9 4/6] thermal: khadas-mcu-fan: Add fan config from platform data Add regulator support Ronald Claveau via B4 Relay
@ 2026-08-05 8:06 ` Ronald Claveau via B4 Relay
2026-08-05 8:19 ` sashiko-bot
2026-08-05 8:06 ` [PATCH v9 6/6] arm64: dts: amlogic: t7: khadas-vim4: Add i2c MCU fan node Ronald Claveau via B4 Relay
5 siblings, 1 reply; 13+ messages in thread
From: Ronald Claveau via B4 Relay @ 2026-08-05 8:06 UTC (permalink / raw)
To: Neil Armstrong, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andi Shyti, Kevin Hilman, Jerome Brunet,
Martin Blumenstingl, Beniamino Galvani, Rafael J. Wysocki,
Daniel Lezcano, Zhang Rui, Lukasz Luba, Liam Girdwood, Mark Brown
Cc: linux-amlogic, devicetree, linux-kernel, linux-i2c,
linux-arm-kernel, linux-pm, mfd, Ronald Claveau
From: Ronald Claveau <linux-kernel-dev@aliel.fr>
Add the T7 i2c controller node used by the Khadas VIM4
for MCU communication.
Use amlogic,meson-axg-i2c as fallback compatible.
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
index c3dc479b137de..0f48e23077e99 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
+++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
@@ -724,6 +724,16 @@ pwm_ao_cd: pwm@60000 {
status = "disabled";
};
+ i2c_m_ao_a: i2c@76000 {
+ compatible = "amlogic,t7-i2c", "amlogic,meson-axg-i2c";
+ reg = <0x0 0x76000 0x0 0x48>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <GIC_SPI 330 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&clkc_periphs CLKID_SYS_I2C_AO_A>;
+ status = "disabled";
+ };
+
sd_emmc_a: mmc@88000 {
compatible = "amlogic,t7-mmc", "amlogic,meson-axg-mmc";
reg = <0x0 0x88000 0x0 0x800>;
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v9 5/6] arm64: dts: amlogic: t7: Add i2c controller node
2026-08-05 8:06 ` [PATCH v9 5/6] arm64: dts: amlogic: t7: Add i2c controller node Ronald Claveau via B4 Relay
@ 2026-08-05 8:19 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-05 8:19 UTC (permalink / raw)
To: Ronald Claveau
Cc: mfd, robh, devicetree, neil.armstrong, conor+dt, linux-amlogic,
lee
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Probe fails for generic Khadas MCU devices because match data evaluates to NULL.
- [Medium] The fan regulator is leaked on probe error and driver unbind.
--
--- Patch [5]: [PATCH v9 5/6] arm64: dts: amlogic: t7: Add i2c controller node ---
commit b6b3cf68d983172e0dc3e121aff82af7de674ddb
Author: Ronald Claveau <linux-kernel-dev@aliel.fr>
arm64: dts: amlogic: t7: Add i2c controller node
Add the T7 i2c controller node used by the Khadas VIM4 for MCU communication.
Use amlogic,meson-axg-i2c as fallback compatible.
> diff --git a/drivers/mfd/khadas-mcu.c b/drivers/mfd/khadas-mcu.c
> index ba981a7886921..4cb305ac915e1 100644
> --- a/drivers/mfd/khadas-mcu.c
> +++ b/drivers/mfd/khadas-mcu.c
[ ... ]
> @@ -75,47 +75,138 @@ static const struct regmap_config khadas_mcu_regmap_config = {
[ ... ]
> static int khadas_mcu_probe(struct i2c_client *client)
> {
> + const struct mfd_cell *cells, *fan_cells;
> + const struct regmap_config *regmap_cfg;
> struct device *dev = &client->dev;
> + int ncells, nfan_cells, ret;
> struct khadas_mcu *ddata;
> - int ret;
> + const void *match;
>
> ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> if (!ddata)
> return -ENOMEM;
>
> + match = i2c_get_match_data(client);
> + if (!match)
> + return -ENODEV;
> +
> + switch ((uintptr_t)match) {
> + case KHADAS_MCU_GENERIC:
[Severity: High]
Does this check unintentionally abort probe for generic Khadas MCU devices?
Because KHADAS_MCU_GENERIC is 0, the data field in the match table is
effectively a null pointer:
{ .compatible = "khadas,mcu", .data = (void *)KHADAS_MCU_GENERIC }
When khadas_mcu_probe() is called for a generic device, i2c_get_match_data()
returns NULL. The check for (!match) is met, and the driver unconditionally
returns -ENODEV. This causes a regression for all generic Khadas MCU boards.
[ ... ]
> diff --git a/drivers/thermal/khadas_mcu_fan.c b/drivers/thermal/khadas_mcu_fan.c
> index 21b3d0a71bd0d..c7f74394b823f 100644
> --- a/drivers/thermal/khadas_mcu_fan.c
> +++ b/drivers/thermal/khadas_mcu_fan.c
[ ... ]
> @@ -78,6 +81,7 @@ static const struct thermal_cooling_device_ops khadas_mcu_fan_cooling_ops = {
[ ... ]
> static int khadas_mcu_fan_probe(struct platform_device *pdev)
> {
> + const struct khadas_mcu_fan_pdata *pdata = dev_get_platdata(&pdev->dev);
> struct khadas_mcu *mcu = dev_get_drvdata(pdev->dev.parent);
> struct thermal_cooling_device *cdev;
> struct device *dev = &pdev->dev;
> @@ -87,7 +91,22 @@ static int khadas_mcu_fan_probe(struct platform_device *pdev)
> ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> if (!ctx)
> return -ENOMEM;
> +
> ctx->mcu = mcu;
> + ctx->fan_reg = pdata->fan_reg;
> + ctx->levels = pdata->levels;
> + ctx->nlevels = pdata->nlevels;
> +
> + ctx->power = devm_regulator_get(dev->parent, "fan");
> + if (IS_ERR(ctx->power))
> + return PTR_ERR(ctx->power);
> +
> + ret = regulator_enable(ctx->power);
> + if (ret) {
> + dev_err(dev, "Failed to enable fan power supply: %d\n", ret);
> + return ret;
> + }
> +
> platform_set_drvdata(pdev, ctx);
>
> cdev = devm_thermal_of_child_cooling_device_register(dev->parent,
[Severity: Medium]
Could this leak the regulator on probe error or driver unbind?
If devm_thermal_of_child_cooling_device_register() fails later in
khadas_mcu_fan_probe(), the function returns without calling
regulator_disable().
Additionally, there is no remove callback in khadas_mcu_fan_driver:
static struct platform_driver khadas_mcu_fan_driver = {
.probe = khadas_mcu_fan_probe,
.shutdown = khadas_mcu_fan_shutdown,
.driver = { ...
This means the regulator is not cleanly disabled when the driver is unbound,
which could leave the reference leaked and prevent the regulator from ever
being powered down.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-add-mcu-fan-khadas-vim4-v9-0-75df78710b9e@aliel.fr?part=5
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v9 6/6] arm64: dts: amlogic: t7: khadas-vim4: Add i2c MCU fan node
2026-08-05 8:06 [PATCH v9 0/6] Add VIM4 MCU/FAN support Ronald Claveau via B4 Relay
` (4 preceding siblings ...)
2026-08-05 8:06 ` [PATCH v9 5/6] arm64: dts: amlogic: t7: Add i2c controller node Ronald Claveau via B4 Relay
@ 2026-08-05 8:06 ` Ronald Claveau via B4 Relay
2026-08-05 8:24 ` sashiko-bot
5 siblings, 1 reply; 13+ messages in thread
From: Ronald Claveau via B4 Relay @ 2026-08-05 8:06 UTC (permalink / raw)
To: Neil Armstrong, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andi Shyti, Kevin Hilman, Jerome Brunet,
Martin Blumenstingl, Beniamino Galvani, Rafael J. Wysocki,
Daniel Lezcano, Zhang Rui, Lukasz Luba, Liam Girdwood, Mark Brown
Cc: linux-amlogic, devicetree, linux-kernel, linux-i2c,
linux-arm-kernel, linux-pm, mfd, Ronald Claveau
From: Ronald Claveau <linux-kernel-dev@aliel.fr>
Enable and configure i2c MCU node to get fan working on Khadas VIM4.
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
.../boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
index c41525a34b721..7d95aadcf6a66 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
+++ b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
@@ -167,6 +167,19 @@ xtal: xtal-clk {
};
};
+&i2c_m_ao_a {
+ status = "okay";
+ pinctrl-0 = <&i2c0_ao_d_pins>;
+ pinctrl-names = "default";
+
+ khadas_mcu: system-controller@18 {
+ compatible = "khadas,vim4-mcu";
+ reg = <0x18>;
+ fan-supply = <&vcc5v>;
+ #cooling-cells = <2>;
+ };
+};
+
&pwm_ab {
status = "okay";
pinctrl-0 = <&pwm_a_pins>;
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v9 6/6] arm64: dts: amlogic: t7: khadas-vim4: Add i2c MCU fan node
2026-08-05 8:06 ` [PATCH v9 6/6] arm64: dts: amlogic: t7: khadas-vim4: Add i2c MCU fan node Ronald Claveau via B4 Relay
@ 2026-08-05 8:24 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-05 8:24 UTC (permalink / raw)
To: Ronald Claveau
Cc: neil.armstrong, devicetree, lee, linux-amlogic, conor+dt, robh,
mfd
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The khadas-mcu-fan driver leaks regulator references and enable counts due to binding devres to the parent device and missing cleanup paths.
--
--- Patch [6]: [PATCH v9 6/6] arm64: dts: amlogic: t7: khadas-vim4: Add i2c MCU fan node ---
commit 69aa0768107f9fa2d0c3f9c0b9a761cb13f3890b
Author: Ronald Claveau <linux-kernel-dev@aliel.fr>
arm64: dts: amlogic: t7: khadas-vim4: Add i2c MCU fan node
Enable and configure i2c MCU node to get fan working on Khadas VIM4.
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
> index c41525a34b721..7d95aadcf6a66 100644
> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
> @@ -167,6 +167,19 @@ xtal: xtal-clk {
> };
> };
>
> +&i2c_m_ao_a {
> + status = "okay";
> + pinctrl-0 = <&i2c0_ao_d_pins>;
> + pinctrl-names = "default";
> +
> + khadas_mcu: system-controller@18 {
> + compatible = "khadas,vim4-mcu";
> + reg = <0x18>;
> + fan-supply = <&vcc5v>;
[Severity: High]
Does enabling this fan node expose a regulator leak in the khadas-mcu-fan
driver?
When the driver probes, it appears to attach the regulator resource
lifecycle to the parent device rather than the child:
drivers/thermal/khadas_mcu_fan.c:khadas_mcu_fan_probe() {
...
ctx->power = devm_regulator_get(dev->parent, "fan");
...
}
If devm_thermal_of_child_cooling_device_register() fails, the probe
function returns without calling regulator_disable():
drivers/thermal/khadas_mcu_fan.c:khadas_mcu_fan_probe() {
...
return ret;
}
Furthermore, the driver struct lacks a .remove callback entirely:
drivers/thermal/khadas_mcu_fan.c:
static struct platform_driver khadas_mcu_fan_driver = {
.probe = khadas_mcu_fan_probe,
.shutdown = khadas_mcu_fan_shutdown,
.driver = {
.name = "khadas-mcu-fan-ctrl",
.pm = &khadas_mcu_fan_pm,
},
.id_table = khadas_mcu_fan_id_table,
};
Since devres is attached to dev->parent, the driver core will not
automatically clean up the regulator when the child device probe fails or
when the child driver is unbound.
Will this cause a monotonically increasing enable count and memory leak
on the parent device during unbind/rebind cycles?
> + #cooling-cells = <2>;
> + };
> +};
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-add-mcu-fan-khadas-vim4-v9-0-75df78710b9e@aliel.fr?part=6
^ permalink raw reply [flat|nested] 13+ messages in thread