On Mon, 3 Mar 2025 19:26:37 +0530
Jishnu Prakash <jishnu.prakash@xxxxxxxxxxxxxxxx> wrote:
> Hi Jonathan,
>
> On 3/1/2025 8:55 AM, Jonathan Cameron wrote:
> > On Wed, 26 Feb 2025 14:22:05 +0530
> > Jishnu Prakash <jishnu.prakash@xxxxxxxxxxxxxxxx> wrote:
> >
> ...
> >>>> +void adc5_take_mutex_lock(struct device *dev, bool lock)
> >>>> +{
> >>>> + struct iio_dev *indio_dev = dev_get_drvdata(dev->parent);
> >>>> + struct adc5_chip *adc = iio_priv(indio_dev);
> >>>> +
> >>>> + if (lock)
> >>>> + mutex_lock(&adc->lock);
> >>>> + else
> >>>> + mutex_unlock(&adc->lock);
> >>>> +}
> >>>> +EXPORT_SYMBOL_NS_GPL(adc5_take_mutex_lock, "QCOM_SPMI_ADC5_GEN3");
> >>>
> >>> This is potentially going to make a mess for sparse. Might be better to split
> >>> it in two so you can had __acquires and __releases markings.
> >>>
> >>> If you don't get any warnings with sparse then I guess we are fine.
> >>>
> >>
> >> I had tried building with sparse in my local workspace and I did not get any errors in this file. Do you think I can keep this unchanged?
> >> Also, would any kernel bots run sparse later on this patch, if it's not already done?
> >
> > Problems around this tend to turn up a bit late in build tests as requires
> > particular combinations of features. Here you may not see problems because
> > sparse can't see far enough to understand the locking.
> >
> > I would still split this into lock / unlock as that matches better
> > with common syntax for locks. We can then add markings
> > as necessary later.
> >
>
> OK, I can split this into separate lock and unlock functions.
> And for markings, you mean I should add these:
>
> __acquires(&adc->lock)
> __releases(&adc->lock)
>
> under the lock and unlock functions respectively?
yes
>
> Thanks,
> Jishnu
>
> >>>> +/*
> >>>
> >>> Looks like valid kernel doc, so /** and check it builds fine
> >>> with the kernel-doc script.
> >>>
>
