Re: [PATCH v17] pwm: airoha: Add support for EN7581 SoC

[Andy Shevchenko <andy.shevchenko@gmail.com>]

Fri, Jun 27, 2025 at 12:52:06AM +0200, Christian Marangi kirjoitti:
> On Thu, Jun 26, 2025 at 04:54:41PM +0300, Andy Shevchenko wrote:
> > On Wed, Jun 25, 2025 at 09:49:01PM +0200, Christian Marangi wrote:

...

> > > +	if (refcount_read(&pc->buckets[bucket].used) == 0) {
> > > +		pc->buckets[bucket].period_ticks = period_ticks;
> > > +		pc->buckets[bucket].duty_ticks = duty_ticks;
> > > +		ret = airoha_pwm_apply_bucket_config(pc, bucket,
> > > +						     duty_ticks,
> > > +						     period_ticks);
> > > +		if (ret)
> > > +			return ret;
> > > +
> > > +		refcount_set(&pc->buckets[bucket].used, 1);
> > 
> > What happens if refcount is updated in between? This is wrong use of atomics.
> > 
> > > +	} else {
> > > +		refcount_inc(&pc->buckets[bucket].used);
> > 
> > Ditto.
> > 
> > You probably wanted _inc_and_test() variant.
> 
> The main problem is that adding macro for refcount and atomic is that
> normaly you expect to have parallel ASM code for it to have real aotmic
> OP. Anyway I think I solved it with a simple mutex.
> 
> The usage of refcount here it's not really for atomic but for object
> tracking but refcount doesn't love to initialize from 0 and incremented
> so I have to use this _set and _inc thing.

Indeed, the main use of refcount is to start with 1, when object gets
initialised and free it on 0, hence if there are more users the count
goes up and drops when a user of the resource drops.

> > > +	}

-- 
With Best Regards,
Andy Shevchenko