diff mbox series

ASoC: codecs: wsa884x: allow sharing reset GPIO

Message ID 20231018100055.140847-1-krzysztof.kozlowski@linaro.org
State New
Headers
Series ASoC: codecs: wsa884x: allow sharing reset GPIO |

Commit Message

Krzysztof Kozlowski Oct. 18, 2023, 10 a.m. UTC 
  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(-)

Comments

Srinivas Kandagatla Oct. 18, 2023, 12:31 p.m. UTC | #1 
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");
Mark Brown Oct. 18, 2023, 12:35 p.m. UTC | #2 
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?
Krzysztof Kozlowski Oct. 18, 2023, 12:38 p.m. UTC | #3 
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
Mark Brown Oct. 18, 2023, 12:56 p.m. UTC | #4 
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.
Krzysztof Kozlowski Oct. 18, 2023, 12:57 p.m. UTC | #5 
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
Mark Brown Oct. 18, 2023, 3:07 p.m. UTC | #6 
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.
diff mbox series

Patch

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