Re: [PATCH v4 5/9] iio: adc: meson_saradc: annotate the lock helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] iio: adc: meson_saradc: context analysis warning in meson_sar_adc_hw_disable()
--

commit ab37c3948447f440afeebef2fb0a7a0fdf5e9e63
Author: Rupesh Majhi <zoone.rupert@gmail.com>

iio: adc: meson_saradc: annotate the lock helpers

This patch adds __cond_acquires() and __releases() context analysis
annotations to meson_sar_adc_lock() and meson_sar_adc_unlock() so that
compiler context analysis tracks priv->lock across their callers.

> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 2b003fd5afeba..3c1bc22741296 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -578,6 +578,7 @@ static void meson_sar_adc_stop_sample_engine(struct iio_dev *indio_dev)
>  }
>  
>  static int meson_sar_adc_lock(struct iio_dev *indio_dev)
> +	__cond_acquires(0, &((struct meson_sar_adc_priv *)iio_priv(indio_dev))->lock)
>  {
>  	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
>  	int val, ret;
> @@ -608,6 +609,7 @@ static int meson_sar_adc_lock(struct iio_dev *indio_dev)
>  }
>  
>  static void meson_sar_adc_unlock(struct iio_dev *indio_dev)
> +	__releases(&((struct meson_sar_adc_priv *)iio_priv(indio_dev))->lock)

[Severity: Medium]
Does adding these annotations trigger a -Wthread-safety-analysis warning in
meson_sar_adc_hw_disable() when context analysis is enabled?

In drivers/iio/adc/meson_saradc.c:meson_sar_adc_hw_disable(), the caller logs
an error if meson_sar_adc_lock() fails, but continues execution rather than
returning early:

	ret = meson_sar_adc_lock(indio_dev);
	if (ret)
		dev_err(indio_dev->dev.parent, "Failed to lock ADC (%pE)\n", ERR_PTR(ret));

	clk_disable_unprepare(priv->adc_clk);

	regmap_clear_bits(priv->regmap, MESON_SAR_ADC_REG3,
			  MESON_SAR_ADC_REG3_ADC_EN);

	meson_sar_adc_set_bandgap(indio_dev, false);

	regulator_disable(priv->vref);

	if (!ret)
		meson_sar_adc_unlock(indio_dev);

Because the error branch falls through, the locked (ret == 0) and unlocked
(ret != 0) control-flow paths merge at clk_disable_unprepare(priv->adc_clk)
with mismatched lock sets, followed by a conditional unlock at if (!ret).

When building with CONFIG_MESON_SARADC=y and CONFIG_WARN_CONTEXT_ANALYSIS=y
(and CONTEXT_ANALYSIS := y enabled for IIO drivers), wouldn't Clang warn that
priv->lock is not held on every path through here and on the conditional
release in meson_sar_adc_unlock(), unless meson_sar_adc_hw_disable() is also
annotated with __context_unsafe() or its control flow is restructured?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027124954.110697-1-zoone.rupert@gmail.com?part=5

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic