Re: [PATCH v2 4/8] regulator: add Qualcomm LCDB driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] regulator: qcom-lcdb: off-by-one and window reset in qcom_lcdb_handle_sc()
--

commit 5b5d0fa4182dea42061084cb3ab6d86b894783ff
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

regulator: add Qualcomm LCDB driver

This commit adds a regulator driver for the Qualcomm LCDB (LCD bias) block
found on PMICs such as PMI632, PM6150L, and PM8150L. It manages the shared
module enable, boost headroom tracking, voltage ramp-up, and short-circuit
recovery for the LDO and NCP bias rails.

> diff --git a/drivers/regulator/qcom-lcdb-regulator.c b/drivers/regulator/qcom-lcdb-regulator.c
> new file mode 100644
> index 0000000000000..bb2d4ee23ad5c
> --- /dev/null
> +++ b/drivers/regulator/qcom-lcdb-regulator.c
> @@ -0,0 +1,494 @@

[ ... ]

> +static bool qcom_lcdb_handle_sc(struct qcom_lcdb *lcdb)
> +{
> +	guard(mutex)(&lcdb->lock);
> +
> +	if (!lcdb->enabled_mask || lcdb->sc_disabled)
> +		return false;
> +
> +	qcom_lcdb_module_disable(lcdb);
> +
> +	if (ktime_us_delta(ktime_get(), lcdb->sc_time) > LCDB_SC_RESET_CNT_DLY_US) {
> +		lcdb->sc_count = 0;
> +	} else if (lcdb->sc_count > LCDB_SC_CNT_MAX) {
> +		dev_err(lcdb->dev, "persistent short circuit, keeping the outputs off\n");
> +		lcdb->sc_disabled = true;
> +		return true;
> +	}
> +
> +	lcdb->sc_count++;
> +	lcdb->sc_time = ktime_get();

[Severity: Medium]
Does qcom_lcdb_handle_sc() have an off-by-one in the retry count and reset
the 1-second fault window on every short-circuit event?

The commit message notes that on a short circuit the module is restarted up
to ten times within a second. However, in qcom_lcdb_handle_sc(),
lcdb->sc_count > LCDB_SC_CNT_MAX is evaluated before lcdb->sc_count++,
allowing 11 restarts (for lcdb->sc_count values 0 through 10) before
disabling the outputs on the 12th fault.

In addition, lcdb->sc_time = ktime_get() is updated on every fault rather
than only when starting a new counting window. Because ktime_us_delta()
measures the time since the immediately preceding fault instead of the first
fault in the window, intermittent faults spaced less than 1 second apart
(for example, every 0.8 seconds) will never reset lcdb->sc_count and will
permanently disable the rails across multiple seconds.

> +
> +	if (qcom_lcdb_module_enable(lcdb)) {
> +		dev_err(lcdb->dev, "failed to recover from a short circuit\n");
> +		lcdb->sc_disabled = true;
> +		return true;
> +	}
> +
> +	return false;
> +}

[ ... ]

────
🤖 ThisDimensions (v1.2 · gpt-5.4) · 2026-10-19 04:01 UTC · [📚](https://github.com/masoncl/review-prompts)
  📧 dmitry.baryshkov@oss.qualcomm.com · 💬 ~420 words · 🪙 ~15k tokens · 💰 ~$0.05 ([📊](https://platform.openai.com/docs/pricing))
────

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019-sm8150-display-card-v2-0-86c05068efc5@oss.qualcomm.com?part=4