On Fri, Dec 16, 2022 at 05:24:02PM +0000, Biju Das wrote:
> Hi William Breathitt Gray,
>
> > Subject: Re: [PATCH v9 4/5] counter: Add Renesas RZ/G2L MTU3a counter driver
> >
> > On Fri, Dec 16, 2022 at 05:00:30PM +0000, Biju Das wrote:
> > > > Hello Biju,
> > > >
> > > > Do you need to take the ch->lock before checking ch->is_busy to
> > > > ensure it does not change?
> > >
> > > priv->count_is_enabled[count->id]-> true means channel is held by counter.
> > > So pwm won't be able to change the state ch->is_busy.
> > >
> > > priv->count_is_enabled[count->id]-> false and if there is contention
> > > priv->for ch->busy
> > > whoever is first calling rz_mtu3_request_channel() will get the channel.
> > > among pwm_request and counter_enable.
> > >
> > > So I think it is safe here. Please correct me if I am missing something.
> > >
> > > static inline bool rz_mtu3_request_channel(struct rz_mtu3_channel *ch)
> > > {
> > > bool is_idle;
> > >
> > > mutex_lock(&ch->lock);
> > > is_idle = !ch->is_busy;
> > > if (is_idle)
> > > ch->is_busy = true;
> > > mutex_unlock(&ch->lock);
> > >
> > > return is_idle;
> > > }
> >
> > Okay seems safe then. If the respective count_is_enabled will only be true
> > when the respective channel is held by the counter, is there a need to check
> > ch->is_busy, or would checking count_is_enabled alone suffice?
>
> We still can configure below properties before enabling the count.
> That can be done only when ch->is_busy = false or ch->is_busy= true and count_is_enabled.
> That is channel is used by the counter.
>
> /sys/bus/counter/devices/counterX/external_input_phase_clock_select
> /sys/bus/counter/devices/counterX/long_word_access_ctrl_mode
> /sys/bus/counter/devices/counterX/count2/count
> /sys/bus/counter/devices/counterX/count2/ceiling
>
> Cheers,
> Biju
I see what you mean now, we'll need to check ch->is_busy regardless and
not just the count_is_enable state. That should be fine then, thanks for
explaining.
William Breathitt Gray
Attachment:
signature.asc
Description: PGP signature
