[alsa-devel] [PATCH 04/12] ALSA: hda: check stream decoupled register state

[Takashi Iwai]

On Fri, 02 Dec 2016 18:41:41 +0100,
jeeja.kp at intel.com wrote:
> 
> From: Jeeja KP <jeeja.kp at intel.com>
> 
> Check stream decoupled register value with requested value
> before decoupling/coupling the stream.
> 
> Signed-off-by: Jeeja KP <jeeja.kp at intel.com>
> ---
>  sound/hda/ext/hdac_ext_stream.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
> index 3be051a..bd8187b 100644
> --- a/sound/hda/ext/hdac_ext_stream.c
> +++ b/sound/hda/ext/hdac_ext_stream.c
> @@ -128,14 +128,17 @@ void snd_hdac_ext_stream_decouple(struct hdac_ext_bus *ebus,
>  {
>  	struct hdac_stream *hstream = &stream->hstream;
>  	struct hdac_bus *bus = &ebus->bus;
> +	u32 val;
> +	int mask = AZX_PPCTL_PROCEN(hstream->index);
>  
>  	spin_lock_irq(&bus->reg_lock);
> -	if (decouple)
> -		snd_hdac_updatel(bus->ppcap, AZX_REG_PP_PPCTL, 0,
> -				AZX_PPCTL_PROCEN(hstream->index));
> -	else
> -		snd_hdac_updatel(bus->ppcap, AZX_REG_PP_PPCTL,
> -					AZX_PPCTL_PROCEN(hstream->index), 0);
> +	val = ((readw(bus->ppcap + AZX_REG_PP_PPCTL) & mask) >> mask);
> +
> +	if (decouple && (val == 0))
> +		snd_hdac_updatel(bus->ppcap, AZX_REG_PP_PPCTL, 0, mask);
> +	else if (!decouple && (val > 0))
> +		snd_hdac_updatel(bus->ppcap, AZX_REG_PP_PPCTL, mask, 0);
> +

The usage of snd_hdac_updatel() looks strange.
The third argument must be the mask bits and the fourth be the value
bits.  So, usually for clearing a bit

	snd_hdac_update(pcap, REG, mask, 0);

and for setting a bit

	snd_hdac_update(pcap, REG, mask, mask);

Passing 0 to the mask bits means to replace the whole bits.

This usage pattern is found already in the old code, so it's not new,
and I'm not sure whether this behavior is intentional...


thanks,

Takashi