Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ronald Claveau" <linux-kernel-dev@aliel.fr>
Cc: neil.armstrong@linaro.org, devicetree@vger.kernel.org,
	lee@kernel.org, linux-amlogic@lists.infradead.org,
	conor+dt@kernel.org, robh@kernel.org, mfd@lists.linux.dev
Subject: Re: [PATCH v9 6/6] arm64: dts: amlogic: t7: khadas-vim4: Add i2c MCU fan node
Date: Wed, 05 Aug 2026 08:24:15 +0000	[thread overview]
Message-ID: <20260805082416.884591F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260805-add-mcu-fan-khadas-vim4-v9-6-75df78710b9e@aliel.fr>

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

      reply	other threads:[~2026-08-05  8:24 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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: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
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
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
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: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
2026-08-05  8:24   ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260805082416.884591F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-kernel-dev@aliel.fr \
    --cc=mfd@lists.linux.dev \
    --cc=neil.armstrong@linaro.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox