[PATCH] ASoC: codecs: wsa884x: allow sharing reset GPIO

[PATCH] ASoC: codecs: wsa884x: allow sharing reset GPIO

[Krzysztof Kozlowski]

On some boards with multiple WSA8840/WSA8845 speakers, the reset
(shutdown) GPIO is shared between two speakers.  Request it as
GPIOD_FLAGS_BIT_NONEXCLUSIVE to allow such configurations.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 sound/soc/codecs/wsa884x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wsa884x.c b/sound/soc/codecs/wsa884x.c
index 993d76b18b53..bee6e763c700 100644
--- a/sound/soc/codecs/wsa884x.c
+++ b/sound/soc/codecs/wsa884x.c
@@ -1844,7 +1844,7 @@ static int wsa884x_probe(struct sdw_slave *pdev,
 		return ret;
 
 	wsa884x->sd_n = devm_gpiod_get_optional(dev, "powerdown",
-						GPIOD_OUT_HIGH);
+						GPIOD_FLAGS_BIT_NONEXCLUSIVE | GPIOD_OUT_HIGH);
 	if (IS_ERR(wsa884x->sd_n))
 		return dev_err_probe(dev, PTR_ERR(wsa884x->sd_n),
 				     "Shutdown Control GPIO not found\n");
-- 
2.34.1

Re: [PATCH] ASoC: codecs: wsa884x: allow sharing reset GPIO

[Srinivas Kandagatla]

On 18/10/2023 11:00, Krzysztof Kozlowski wrote:
> On some boards with multiple WSA8840/WSA8845 speakers, the reset
> (shutdown) GPIO is shared between two speakers.  Request it as
> GPIOD_FLAGS_BIT_NONEXCLUSIVE to allow such configurations.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---

Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>


--srini
>   sound/soc/codecs/wsa884x.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/wsa884x.c b/sound/soc/codecs/wsa884x.c
> index 993d76b18b53..bee6e763c700 100644
> --- a/sound/soc/codecs/wsa884x.c
> +++ b/sound/soc/codecs/wsa884x.c
> @@ -1844,7 +1844,7 @@ static int wsa884x_probe(struct sdw_slave *pdev,
>   		return ret;
>   
>   	wsa884x->sd_n = devm_gpiod_get_optional(dev, "powerdown",
> -						GPIOD_OUT_HIGH);
> +						GPIOD_FLAGS_BIT_NONEXCLUSIVE | GPIOD_OUT_HIGH);
>   	if (IS_ERR(wsa884x->sd_n))
>   		return dev_err_probe(dev, PTR_ERR(wsa884x->sd_n),
>   				     "Shutdown Control GPIO not found\n");

Re: [PATCH] ASoC: codecs: wsa884x: allow sharing reset GPIO

[Mark Brown]

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

On Wed, Oct 18, 2023 at 12:00:55PM +0200, Krzysztof Kozlowski wrote:
> On some boards with multiple WSA8840/WSA8845 speakers, the reset
> (shutdown) GPIO is shared between two speakers.  Request it as
> GPIOD_FLAGS_BIT_NONEXCLUSIVE to allow such configurations.

>  	wsa884x->sd_n = devm_gpiod_get_optional(dev, "powerdown",
> -						GPIOD_OUT_HIGH);
> +						GPIOD_FLAGS_BIT_NONEXCLUSIVE | GPIOD_OUT_HIGH);

How do the speakers coordinate?

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

Re: [PATCH] ASoC: codecs: wsa884x: allow sharing reset GPIO

[Krzysztof Kozlowski]

On 18/10/2023 14:35, Mark Brown wrote:
> On Wed, Oct 18, 2023 at 12:00:55PM +0200, Krzysztof Kozlowski wrote:
>> On some boards with multiple WSA8840/WSA8845 speakers, the reset
>> (shutdown) GPIO is shared between two speakers.  Request it as
>> GPIOD_FLAGS_BIT_NONEXCLUSIVE to allow such configurations.
> 
>>  	wsa884x->sd_n = devm_gpiod_get_optional(dev, "powerdown",
>> -						GPIOD_OUT_HIGH);
>> +						GPIOD_FLAGS_BIT_NONEXCLUSIVE | GPIOD_OUT_HIGH);
> 
> How do the speakers coordinate?

They don't and that's the generic problem of many Linux drivers. Not
only this one, but others as well.

Device unbind (remove()) or runtime suspend of one speaker will affect
other speaker. I don't think any other drivers solved this, because this
is rather core's GPIO issue, thus I am not solving it here either. :(

Best regards,
Krzysztof

Re: [PATCH] ASoC: codecs: wsa884x: allow sharing reset GPIO

[Mark Brown]

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

On Wed, Oct 18, 2023 at 02:38:00PM +0200, Krzysztof Kozlowski wrote:
> On 18/10/2023 14:35, Mark Brown wrote:

> > How do the speakers coordinate?

> They don't and that's the generic problem of many Linux drivers. Not
> only this one, but others as well.

> Device unbind (remove()) or runtime suspend of one speaker will affect
> other speaker. I don't think any other drivers solved this, because this
> is rather core's GPIO issue, thus I am not solving it here either. :(

I'd expect that the GPIO users should coordiante directly rather than
rely on the GPIO API to do the coordination for them - there aren't
enough semantics in the GPIO itself to do much more except possibly
provide discovery services (which would be nice).  Look at how the
regulator API manages multiple regulators sharing an enable GPIO for
example, it adds an additional layer of reference counting when it
identifies a shared GPIO.

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

Re: [PATCH] ASoC: codecs: wsa884x: allow sharing reset GPIO

[Krzysztof Kozlowski]

On 18/10/2023 14:56, Mark Brown wrote:
> On Wed, Oct 18, 2023 at 02:38:00PM +0200, Krzysztof Kozlowski wrote:
>> On 18/10/2023 14:35, Mark Brown wrote:
> 
>>> How do the speakers coordinate?
> 
>> They don't and that's the generic problem of many Linux drivers. Not
>> only this one, but others as well.
> 
>> Device unbind (remove()) or runtime suspend of one speaker will affect
>> other speaker. I don't think any other drivers solved this, because this
>> is rather core's GPIO issue, thus I am not solving it here either. :(
> 
> I'd expect that the GPIO users should coordiante directly rather than
> rely on the GPIO API to do the coordination for them - there aren't
> enough semantics in the GPIO itself to do much more except possibly
> provide discovery services (which would be nice).  Look at how the
> regulator API manages multiple regulators sharing an enable GPIO for
> example, it adds an additional layer of reference counting when it
> identifies a shared GPIO.

OK, it is still regulator core, though. Not individual drivers problem.

Several other existing drivers have the same issue, so this should be
solved in a generic or shared way.

Best regards,
Krzysztof

Re: [PATCH] ASoC: codecs: wsa884x: allow sharing reset GPIO

[Mark Brown]

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

On Wed, Oct 18, 2023 at 02:57:59PM +0200, Krzysztof Kozlowski wrote:
> On 18/10/2023 14:56, Mark Brown wrote:

> > I'd expect that the GPIO users should coordiante directly rather than
> > rely on the GPIO API to do the coordination for them - there aren't
> > enough semantics in the GPIO itself to do much more except possibly
> > provide discovery services (which would be nice).  Look at how the
> > regulator API manages multiple regulators sharing an enable GPIO for
> > example, it adds an additional layer of reference counting when it
> > identifies a shared GPIO.

> OK, it is still regulator core, though. Not individual drivers problem.

> Several other existing drivers have the same issue, so this should be
> solved in a generic or shared way.

Indeed.

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