From: sashiko-bot@kernel.org
To: "Chen-Yu Tsai" <wenst@chromium.org>
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org,
krzk+dt@kernel.org
Subject: Re: [PATCH v4 6/6] regulator: mt6359: Add proper ldo_vcn33_[12] regulators
Date: Thu, 14 May 2026 19:13:59 +0000 [thread overview]
Message-ID: <20260514191400.18C78C2BCB3@smtp.kernel.org> (raw)
In-Reply-To: <20260514091520.2718987-7-wenst@chromium.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `mt6359_noop_ops` struct is empty, which causes the dummy regulators to be treated as always-on and prevents them from being disabled, permanently locking the shared `ldo_vcn33_[12]` supply in the enabled state.
--
commit fb19e47461ecd9ff9e40d149eebb44edec04294e
Author: Chen-Yu Tsai <wenst@chromium.org>
regulator: mt6359: Add proper ldo_vcn33_[12] regulators
This commit replaces the separate WiFi and BT regulators for ldo_vcn33_1 and
ldo_vcn33_2 with single regulators. It retains backward compatibility by
converting the existing regulators to no-op regulators that are fed from their
respective new parent regulators.
> diff --git a/drivers/regulator/mt6359-regulator.c b/drivers/regulator/mt6359-regulator.c
> index 46cafe93b24e4..af0e0339fbdd6 100644
> --- a/drivers/regulator/mt6359-regulator.c
> +++ b/drivers/regulator/mt6359-regulator.c
[ ... ]
> @@ -475,6 +489,9 @@ static const struct regulator_ops mt6359p_vemc_ops = {
> .get_status = mt6359_get_status,
> };
>
> +/* Used for backward-compatible placeholder regulators */
> +static const struct regulator_ops mt6359_noop_ops = {};
Could this empty structure cause the parent supply to remain enabled
permanently?
Without an .is_enabled callback, _regulator_is_enabled() defaults to
returning 1. During registration, set_supply() checks if the regulator is
enabled. Since it appears to be enabled, it proactively calls
regulator_enable() on the parent supply (ldo_vcn33_1 or ldo_vcn33_2),
incrementing its use_count.
Later, if a consumer calls regulator_disable() on the dummy regulator,
_regulator_do_disable() returns -EINVAL because the .disable callback is
NULL. Due to this error, _regulator_disable() aborts early without
decrementing its own use_count or calling regulator_disable() on the parent
supply.
Would it make sense to add dummy .enable, .disable, and .is_enabled callbacks
that return 0 to mt6359_noop_ops?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260514091520.2718987-1-wenst@chromium.org?part=6
prev parent reply other threads:[~2026-05-14 19:14 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-14 9:15 [PATCH v4 0/6] regulator: mt6359: cleanup and add supplies Chen-Yu Tsai
2026-05-14 9:15 ` [PATCH v4 1/6] mfd: dt-bindings: mt6397: Add regulator supplies Chen-Yu Tsai
2026-05-14 9:15 ` [PATCH v4 2/6] regulator: dt-bindings: mt6359: Drop regulator-name pattern restrictions Chen-Yu Tsai
2026-05-14 9:15 ` [PATCH v4 3/6] regulator: dt-bindings: mt6359: Deprecate bogus vcn33_[12]_* split regulators Chen-Yu Tsai
2026-05-14 9:15 ` [PATCH v4 4/6] regulator: mt6359: const-ify regulator descriptions Chen-Yu Tsai
2026-05-14 9:15 ` [PATCH v4 5/6] regulator: mt6359: Add regulator supply names Chen-Yu Tsai
2026-05-14 18:34 ` sashiko-bot
2026-05-15 4:22 ` Chen-Yu Tsai
2026-05-14 9:15 ` [PATCH v4 6/6] regulator: mt6359: Add proper ldo_vcn33_[12] regulators Chen-Yu Tsai
2026-05-14 19:13 ` 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=20260514191400.18C78C2BCB3@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzk+dt@kernel.org \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=wenst@chromium.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.