[PATCH 1/3] ASoC: dapm: connect virtual mux with default value

[PATCH 1/3] ASoC: dapm: connect virtual mux with default value

[이경택]

Since a virtual mixer has no backing registers
to decide which path to connect,
it will try to match with initial value 0.
This is to ensure that the default mixer choice will be
correctly powered up during initialization.

Signed-off-by: Gyeongtaek Lee <gt82.lee@samsung.com>
---
 sound/soc/soc-dapm.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 9fb54e6fe254..fc55a0534252 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -802,7 +802,16 @@ static void dapm_set_mixer_path_status(struct
snd_soc_dapm_path *p, int i,
 			val = max - val;
 		p->connect = !!val;
 	} else {
-		p->connect = 0;
+		/* since a virtual mixer has no backing registers to
+		 * decide which path to connect, it will try to match
+		 * with initial value 0.  This is to ensure
+		 * that the default mixer choice will be
+		 * correctly powered up during initialization.
+		 */
+		val = 0;
+		if (invert)
+			val = max - val;
+		p->connect = !!val;
 	}
 }
 
-- 
2.21.0

Re: [PATCH 1/3] ASoC: dapm: connect virtual mux with default value

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

On Mon, Mar 30, 2020 at 04:35:47PM +0900, 이경택 wrote:

> +++ b/sound/soc/soc-dapm.c
> @@ -802,7 +802,16 @@ static void dapm_set_mixer_path_status(struct
> snd_soc_dapm_path *p, int i,

It looks like your client corrupted this by at least word wrapping it,
can you resend please?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 1/3] ASoC: dapm: connect virtual mux with default value

[이경택]

Since a virtual mixer has no backing registers to decide which path to
connect, it will try to match with initial value 0.
This is to ensure that the default mixer choice will be correctly powered
up during initialization.

Signed-off-by: Gyeongtaek Lee <gt82.lee@samsung.com>
---
 sound/soc/soc-dapm.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
9fb54e6fe254..fc55a0534252 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -802,7 +802,16 @@ static void dapm_set_mixer_path_status(struct
snd_soc_dapm_path *p, int i,
 			val = max - val;
 		p->connect = !!val;
 	} else {
-		p->connect = 0;
+		/* since a virtual mixer has no backing registers to
+		 * decide which path to connect, it will try to match
+		 * with initial value 0.  This is to ensure
+		 * that the default mixer choice will be
+		 * correctly powered up during initialization.
+		 */
+		val = 0;
+		if (invert)
+			val = max - val;
+		p->connect = !!val;
 	}
 }
 
--
2.21.0

Re: [PATCH 1/3] ASoC: dapm: connect virtual mux with default value

[Lars-Peter Clausen]

On 3/31/20 7:42 AM, 이경택 wrote:
> Since a virtual mixer has no backing registers to decide which path to
> connect, it will try to match with initial value 0.
> This is to ensure that the default mixer choice will be correctly powered
> up during initialization.
>
> Signed-off-by: Gyeongtaek Lee <gt82.lee@samsung.com>
> ---
>   sound/soc/soc-dapm.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
> 9fb54e6fe254..fc55a0534252 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -802,7 +802,16 @@ static void dapm_set_mixer_path_status(struct
> snd_soc_dapm_path *p, int i,
>   			val = max - val;
>   		p->connect = !!val;
>   	} else {
> -		p->connect = 0;
> +		/* since a virtual mixer has no backing registers to
> +		 * decide which path to connect, it will try to match
> +		 * with initial value 0.  This is to ensure
> +		 * that the default mixer choice will be
> +		 * correctly powered up during initialization.
> +		 */
> +		val = 0;
> +		if (invert)
> +			val = max - val;
> +		p->connect = !!val;
Do you have virtual mixers that have the inverted flag set and if yes 
why? Cause otherwise this is exactly the same as before.

RE: [PATCH 1/3] ASoC: dapm: connect virtual mux with default value

[이경택]

On 3/31/20 7:55 AM, Lars-Peter Clausen wrote:
>On 3/31/20 7:42 AM, 이경택 wrote:
>> Since a virtual mixer has no backing registers to decide which path to 
>> connect, it will try to match with initial value 0.
>> This is to ensure that the default mixer choice will be correctly 
>> powered up during initialization.
>>
>> Signed-off-by: Gyeongtaek Lee <gt82.lee@samsung.com>
>> ---
>>   sound/soc/soc-dapm.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
>> 9fb54e6fe254..fc55a0534252 100644
>> --- a/sound/soc/soc-dapm.c
>> +++ b/sound/soc/soc-dapm.c
>> @@ -802,7 +802,16 @@ static void dapm_set_mixer_path_status(struct 
>> snd_soc_dapm_path *p, int i,
>>   			val = max - val;
>>   		p->connect = !!val;
>>   	} else {
>> -		p->connect = 0;
>> +		/* since a virtual mixer has no backing registers to
>> +		 * decide which path to connect, it will try to match
>> +		 * with initial value 0.  This is to ensure
>> +		 * that the default mixer choice will be
>> +		 * correctly powered up during initialization.
>> +		 */
>> +		val = 0;
>> +		if (invert)
>> +			val = max - val;
>> +		p->connect = !!val;
>Do you have virtual mixers that have the inverted flag set and if yes why? Cause otherwise this is exactly the same as before.
Yes, I have virtual mixers with inverted flag.
It is used to create a switch which is connected by default and can be disconnected when user application needs it.
Actually hardware can't be disconnected by virtual switch.
So, connected was better for default value for virtual mixers in my experience.

[PATCH 1/3] ASoC: dapm: connect virtual mux with default value

[이경택]

Since a virtual mixer has no backing registers to decide which path to connect, it will try to match with initial value 0.
This is to ensure that the default mixer choice will be correctly powered up during initialization.

Signed-off-by: Gyeongtaek Lee <gt82.lee@samsung.com>
---
 sound/soc/soc-dapm.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 9fb54e6fe254..fc55a0534252 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -802,7 +802,16 @@ static void dapm_set_mixer_path_status(struct snd_soc_dapm_path *p, int i,
 			val = max - val;
 		p->connect = !!val;
 	} else {
-		p->connect = 0;
+		/* since a virtual mixer has no backing registers to
+		 * decide which path to connect, it will try to match
+		 * with initial value 0.  This is to ensure
+		 * that the default mixer choice will be
+		 * correctly powered up during initialization.
+		 */
+		val = 0;
+		if (invert)
+			val = max - val;
+		p->connect = !!val;
 	}
 }
 
--
2.21.0

Re: [PATCH 1/3] ASoC: dapm: connect virtual mux with default value

[Lars-Peter Clausen]

On 3/31/20 8:35 AM, 이경택 wrote:
> On 3/31/20 7:55 AM, Lars-Peter Clausen wrote:
>> On 3/31/20 7:42 AM, 이경택 wrote:
>>> Since a virtual mixer has no backing registers to decide which path to
>>> connect, it will try to match with initial value 0.
>>> This is to ensure that the default mixer choice will be correctly
>>> powered up during initialization.
>>>
>>> Signed-off-by: Gyeongtaek Lee <gt82.lee@samsung.com>
>>> ---
>>>    sound/soc/soc-dapm.c | 11 ++++++++++-
>>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
>>> 9fb54e6fe254..fc55a0534252 100644
>>> --- a/sound/soc/soc-dapm.c
>>> +++ b/sound/soc/soc-dapm.c
>>> @@ -802,7 +802,16 @@ static void dapm_set_mixer_path_status(struct
>>> snd_soc_dapm_path *p, int i,
>>>    			val = max - val;
>>>    		p->connect = !!val;
>>>    	} else {
>>> -		p->connect = 0;
>>> +		/* since a virtual mixer has no backing registers to
>>> +		 * decide which path to connect, it will try to match
>>> +		 * with initial value 0.  This is to ensure
>>> +		 * that the default mixer choice will be
>>> +		 * correctly powered up during initialization.
>>> +		 */
>>> +		val = 0;
>>> +		if (invert)
>>> +			val = max - val;
>>> +		p->connect = !!val;
>> Do you have virtual mixers that have the inverted flag set and if yes why? Cause otherwise this is exactly the same as before.
> Yes, I have virtual mixers with inverted flag.
> It is used to create a switch which is connected by default and can be disconnected when user application needs it.
> Actually hardware can't be disconnected by virtual switch.
> So, connected was better for default value for virtual mixers in my experience.

It would be good to mention this in the commit message, that the purpose 
of the patch is to allow virtual mixers with a default state of on.

I think you can also simplify the expression to just `p->connect = invert;`

RE: [PATCH 1/3] ASoC: dapm: connect virtual mux with default value

[이경택]

On 3/31/20 9:10 AM, Lars-Peter Clausen wrote:
>On 3/31/20 8:35 AM, 이경택 wrote:
>> On 3/31/20 7:55 AM, Lars-Peter Clausen wrote:
>>> On 3/31/20 7:42 AM, 이경택 wrote:
>>>> Since a virtual mixer has no backing registers to decide which path to
>>>> connect, it will try to match with initial value 0.
>>>> This is to ensure that the default mixer choice will be correctly
>>>> powered up during initialization.
>>>>
>>>> Signed-off-by: Gyeongtaek Lee <gt82.lee@samsung.com>
>>>> ---
>>>>    sound/soc/soc-dapm.c | 11 ++++++++++-
>>>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
>>>> 9fb54e6fe254..fc55a0534252 100644
>>>> --- a/sound/soc/soc-dapm.c
>>>> +++ b/sound/soc/soc-dapm.c
>>>> @@ -802,7 +802,16 @@ static void dapm_set_mixer_path_status(struct
>>>> snd_soc_dapm_path *p, int i,
>>>>    			val = max - val;
>>>>    		p->connect = !!val;
>>>>    	} else {
>>>> -		p->connect = 0;
>>>> +		/* since a virtual mixer has no backing registers to
>>>> +		 * decide which path to connect, it will try to match
>>>> +		 * with initial value 0.  This is to ensure
>>>> +		 * that the default mixer choice will be
>>>> +		 * correctly powered up during initialization.
>>>> +		 */
>>>> +		val = 0;
>>>> +		if (invert)
>>>> +			val = max - val;
>>>> +		p->connect = !!val;
>>> Do you have virtual mixers that have the inverted flag set and if yes why? Cause otherwise this is exactly the same as before.
>> Yes, I have virtual mixers with inverted flag.
>> It is used to create a switch which is connected by default and can be disconnected when user application needs it.
>> Actually hardware can't be disconnected by virtual switch.
>> So, connected was better for default value for virtual mixers in my experience.
>
>It would be good to mention this in the commit message, that the purpose 
>of the patch is to allow virtual mixers with a default state of on.
>
>I think you can also simplify the expression to just `p->connect = invert;`
>
Ok. I'll resend this patch with the purpose in commit message, and simplified code.
Thank you for your quick response!