Re: [PATCH 07/10] ASoC: soc-dai: rename snd_soc_dai_stream_active()

["Alvin Šipraga" <alvin.sipraga@analog.com>]

Hi Kuninori,

On Wed, Sep 16, 2026 at 12:48:52AM +0000, Kuninori Morimoto wrote:
> 
> Hi Alvin
> 
> Thank you for your feedback.
> 
> > I agree _active() <=> _action() is not an obvious relationship. But the
> > _action() form is just an internal thing to avoid code duplication in
> > snd_soc_runtime_activate()/deactivate():
> (snip)
> > Since it's internal, I wouldn't care too much about the naming. It could
> > also be snd_soc_dai_active_update(dai, +/-1). Or maybe _push/_pop? But
> > even as a native English speaker, I think _action() is quite fine.
> (snip)
> > Most users of snd_soc_dai_active() just treat it as a boolean.
> > changing it from "active" to "count", we would be advertising the
> > internal reference counter, which suggests to the author that he/she
> > should maybe care about it. I don't think drivers should care about it
> > though. Especially for more niche things like DPCM, the reference
> > counter value can be surprising.
> 
> Basically I have no big objection from my side. But I have few concern.
> 
> 1. Most users are using it as boolean. This is OK. But it is count
>    up/down on .active value. It is confusable ? Indeed it is internal
>    things, but this kind of confusion load some issue.
> 
>    I want to rename .active to .count
> 
> -	xxx->active += xxx;
> +	xxx->count  += xxx;
> 
>    But the function is keeping _active().  Maybe _is_active() is more
>    good naming (?), and use bool.
> 	
> -	unsigned int xxx_active()
> +	bool         xxx_is_active()
> 	{
> -		return   xxx->count;
> +		return !!xxx->count;
> 	}

Making it a bool would be an improvement I think. But it will require a
few driver changes, and I think it's out of the scope of your
series. But good you agree with the reasoning.

> 
> 2. From non-English speaker (or only me ?) point of view,
>    active / action are very confusable. The diff is "ve" / "on" only :(

That's a good reason to change it then. And to be honest, I can't give
you a good reason for why it feels OK to me... I guess that's the
problem with English ;-D

> 
> 	snd_soc_dai_active()
> 	snd_soc_dai_action()
> 	                ^^
> 
>    I personally would like to grep related things by same grep.
>    So I would like to use active_update() instead.

I think changing _action() => _active_update() is intuitive and makes
good sense.

>    # Ah, so _is_active() is not good match to this naming...
> 
> 	snd_soc_dai_active()
> 	snd_soc_dai_active_update()
> 	^^^^^^^^^^^^^^^^^^
> 
>    # this is one of the reason why I want to use get/set at end of
>    # function. But is a little bit strange name and different from
>    # traditional naming
>    #	snd_soc_dai_xxx_get()		snd_soc_dai_get_xxx()
>    #	snd_soc_dai_xxx_set()		snd_soc_dai_set_xxx()
>    #	^^^^^^^^^^^^^^^^		^^^^^^^^^^^^   ^^^^

Yea, I see your grepping point. I see from your mail agent that you are
an emacs user. Normally I do a fuzzy projectile grep[*] of the form:

snd_soc    dai_fmt      set

       ^^^^       ^^^^^^ -------- extra spaces for emphasis, I only use one

[*] helm-projectile-rg

and it gets me quite far (set can be before dai_fmt and it still matches).

But agree the get_xxx/xxx_get inconsistency is unfortunate.

> 
> These are not a big deal, and I don't want to start bicycle shed mail here.
> If you have no big objection about above, I will use it.
> If you have, I will use your idea.

No objection

> 
> > As best as I can tell, it's a marker saying that the DAI's .startup op
> > has been called successfully. If it goes from nonzero to zero, then it's
> > guaranteed that .shutdown has also been called. Nonzero is also
> > equivalent to PCM .open having been called, except for codec2codec where
> > there's no PCM.
> 
> Yes.
> 
> > snd_soc_component_active() always follows snd_soc_dai_active(). That is,
> > the following condition is always true:
> > 
> >   !!snd_soc_component_active(dai->component) == !!snd_soc_dai_active(dai)
> 
> Maybe not always ?
> For example, if Component has many DAIs or, Component is used as "platform"
> (= DAI is not working, but Component is working)
> 
> 	component (1)
> 	  - DAI1 (1)
> =>	  - DAI2 (0)

Yes, of course, forgot about multiple DAIis per component.

> 
> > Personally I always found it surprising that DAI .startup can get called
> > multiple times on the same substream. (I think this is just a DPCM
> > thing though?) Unlike .hw_params, here is no new info to act on I think.
> > So maybe the core can guarantee balanced calls of .startup/.shutdown
> > per-substream, and the guards can be removed. Equally for .set_sysclk,
> > it's mostly the machine driver's fault if it does that when the PCM is
> > open. Along the same lines, maybe the core can help here.
> 
> Yeah, agree.
> I will focus to capsuling for a while, but this can be next focus point.
> (Actually I already have next topic, after capsuling)

Of course. I think it's a big/messy topic I'm bringing anyway. I will
look into a bit more and maybe try to submit some improvements if the
idea makes sense.

Kind regards,
Alvin