Re: [PATCH v4 5/6] ASoC: codecs: wcd938x: add mux control support for hp audio mux

[Srinivas Kandagatla]

On 24/03/2025 16:33, Peter Rosin wrote:
> Hi!
>
> 2025-03-24 at 16:58, Srinivas Kandagatla wrote:
> > On 24/03/2025 15:18, Dmitry Baryshkov wrote:
> > > On Mon, Mar 24, 2025 at 01:58:06PM +0000, Srinivas Kandagatla wrote:
> > > > On 24/03/2025 13:50, Dmitry Baryshkov wrote:
> > > > > On Mon, 24 Mar 2025 at 15:01, <srinivas.kandagatla@xxxxxxxxxx> wrote:
> > > > > > From: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
>
> *snip*
>
> > > > > > +       int ret = mux_control_try_select(wcd938x->us_euro_mux, state);
> > > > >
> > > > > Hmm. Does this really work? You have selected the mux in probe
> > > > > function, now you are trying to select it again. If I'm reading the
> > > > > code correctly, you will get -EBUSY here.
> > > >
> > > > On successful selection of mux state, the mux will be kept available
> > > > (mux_control_deselect) for any new callers.
> > > >
> > > > So we will not get EBUSY for the second caller.
> > >
> > > No. wcd938x_populate_dt_data() selects the state by calling
> > > wcd938x_select_mux_state().
> >
> > At this point we also released it (both in success and error case).
> >
> > This will hold on to the previous state unless we have defined a fallback idle-state.
> >
> >
> >   Then you call mux_control_try_select() here.
> > > As far as I understand, it will return -EBUSY as the sempahore is > already taken. Moreover, this is not how the MUX API is supposed to be
> > > used. The driver is supposed to hold a state while it is still in use.
>
> Dmitry is correct. A mux consumer is supposed to keep the mux selected
> while it needs the mux to remain in a certain state. Relying on details
> such as idle as-is and that no other consumer butts in and clobbers the
> state is fragile. Mux access is not exclusive, at least not until a
> mux state is selected.
Thanks Peter,  I agree that its fragile to depend on idle as-is flags.

Will update accordingly.

--srini
> Cheers,
> Peter