2023-10-18 10:01:22

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH] ASoC: codecs: wsa884x: allow sharing reset GPIO

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 <[email protected]>
---
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


2023-10-18 12:32:16

by Srinivas Kandagatla

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



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 <[email protected]>
> ---

Reviewed-by: Srinivas Kandagatla <[email protected]>


--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");

2023-10-18 12:36:03

by Mark Brown

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

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?


Attachments:
(No filename) (451.00 B)
signature.asc (499.00 B)
Download all attachments

2023-10-18 12:38:17

by Krzysztof Kozlowski

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

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

2023-10-18 12:56:29

by Mark Brown

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

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.


Attachments:
(No filename) (932.00 B)
signature.asc (499.00 B)
Download all attachments

2023-10-18 12:58:11

by Krzysztof Kozlowski

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

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

2023-10-18 15:08:29

by Mark Brown

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

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.


Attachments:
(No filename) (785.00 B)
signature.asc (499.00 B)
Download all attachments