* [PATCH v1 0/2] Handle shared reset GPIO for WSA883x speakers @ 2025-06-20 10:30 Mohammad Rafi Shaik 2025-06-20 10:30 ` [PATCH v1 1/2] ASoC: dt-bindings: qcom,wsa8830: Add reset-gpios for shared line Mohammad Rafi Shaik 2025-06-20 10:30 ` [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers Mohammad Rafi Shaik 0 siblings, 2 replies; 11+ messages in thread From: Mohammad Rafi Shaik @ 2025-06-20 10:30 UTC (permalink / raw) To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai, Philipp Zabel, Linus Walleij, Bartosz Golaszewski Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio, quic_pkumpatl, kernel On some Qualcomm platforms, such as QCS6490-RB3Gen2 and QCM6490-IDP, multiple WSA8830/WSA8835 speakers share a common reset (shutdown) GPIO. To handle such cases, use the reset controller framework along with the "reset-gpio" driver. Tested on: - QCS6490-RB3Gen2 - QCM6490-IDP Mohammad Rafi Shaik (2): ASoC: dt-bindings: qcom,wsa8830: Add reset-gpios for shared line ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers .../bindings/sound/qcom,wsa883x.yaml | 11 +++- sound/soc/codecs/wsa883x.c | 57 ++++++++++++++++--- 2 files changed, 58 insertions(+), 10 deletions(-) base-commit: 2c923c845768a0f0e34b8161d70bc96525385782 -- 2.34.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 1/2] ASoC: dt-bindings: qcom,wsa8830: Add reset-gpios for shared line 2025-06-20 10:30 [PATCH v1 0/2] Handle shared reset GPIO for WSA883x speakers Mohammad Rafi Shaik @ 2025-06-20 10:30 ` Mohammad Rafi Shaik 2025-06-23 8:03 ` Krzysztof Kozlowski 2025-06-20 10:30 ` [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers Mohammad Rafi Shaik 1 sibling, 1 reply; 11+ messages in thread From: Mohammad Rafi Shaik @ 2025-06-20 10:30 UTC (permalink / raw) To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai, Philipp Zabel, Linus Walleij, Bartosz Golaszewski Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio, quic_pkumpatl, kernel On Qualcomm platforms, like QCS6490-RB3Gen2 and QCM6490-IDP, the WSA884x speakers share SD_N GPIOs between two speakers, thus a coordinated assertion is needed. Linux supports handling shared GPIO lines through "reset-gpios"property, thus allow specifying either powerdown or reset GPIOs (these are the same). Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com> --- .../devicetree/bindings/sound/qcom,wsa883x.yaml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/sound/qcom,wsa883x.yaml b/Documentation/devicetree/bindings/sound/qcom,wsa883x.yaml index 14d312f9c345..098f1df62c8c 100644 --- a/Documentation/devicetree/bindings/sound/qcom,wsa883x.yaml +++ b/Documentation/devicetree/bindings/sound/qcom,wsa883x.yaml @@ -29,6 +29,10 @@ properties: description: GPIO spec for Powerdown/Shutdown line to use (pin SD_N) maxItems: 1 + reset-gpios: + description: Powerdown/Shutdown line to use (pin SD_N) + maxItems: 1 + vdd-supply: description: VDD Supply for the Codec @@ -50,10 +54,15 @@ required: - compatible - reg - vdd-supply - - powerdown-gpios - "#thermal-sensor-cells" - "#sound-dai-cells" +oneOf: + - required: + - powerdown-gpios + - required: + - reset-gpios + unevaluatedProperties: false examples: -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/2] ASoC: dt-bindings: qcom,wsa8830: Add reset-gpios for shared line 2025-06-20 10:30 ` [PATCH v1 1/2] ASoC: dt-bindings: qcom,wsa8830: Add reset-gpios for shared line Mohammad Rafi Shaik @ 2025-06-23 8:03 ` Krzysztof Kozlowski 2025-06-23 8:41 ` Mohammad Rafi Shaik 0 siblings, 1 reply; 11+ messages in thread From: Krzysztof Kozlowski @ 2025-06-23 8:03 UTC (permalink / raw) To: Mohammad Rafi Shaik, Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai, Philipp Zabel, Linus Walleij, Bartosz Golaszewski Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio, quic_pkumpatl, kernel On 20/06/2025 12:30, Mohammad Rafi Shaik wrote: > On Qualcomm platforms, like QCS6490-RB3Gen2 and QCM6490-IDP, the > WSA884x speakers share SD_N GPIOs between two speakers, thus You copied everything from my commit 26c8a435fce6 ... even device name. Please don't blindly copy, but check what you are actually pasting. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/2] ASoC: dt-bindings: qcom,wsa8830: Add reset-gpios for shared line 2025-06-23 8:03 ` Krzysztof Kozlowski @ 2025-06-23 8:41 ` Mohammad Rafi Shaik 0 siblings, 0 replies; 11+ messages in thread From: Mohammad Rafi Shaik @ 2025-06-23 8:41 UTC (permalink / raw) To: Krzysztof Kozlowski, Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai, Philipp Zabel, Linus Walleij, Bartosz Golaszewski Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio, quic_pkumpatl, kernel On 6/23/2025 1:33 PM, Krzysztof Kozlowski wrote: > On 20/06/2025 12:30, Mohammad Rafi Shaik wrote: >> On Qualcomm platforms, like QCS6490-RB3Gen2 and QCM6490-IDP, the >> WSA884x speakers share SD_N GPIOs between two speakers, thus > > You copied everything from my commit 26c8a435fce6 ... even device name. > Please don't blindly copy, but check what you are actually pasting. > Ack, Sorry about that Will take care and update the proper commit message. Thanks & Regards, Rafi. > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers 2025-06-20 10:30 [PATCH v1 0/2] Handle shared reset GPIO for WSA883x speakers Mohammad Rafi Shaik 2025-06-20 10:30 ` [PATCH v1 1/2] ASoC: dt-bindings: qcom,wsa8830: Add reset-gpios for shared line Mohammad Rafi Shaik @ 2025-06-20 10:30 ` Mohammad Rafi Shaik 2025-06-20 18:35 ` Dmitry Baryshkov ` (2 more replies) 1 sibling, 3 replies; 11+ messages in thread From: Mohammad Rafi Shaik @ 2025-06-20 10:30 UTC (permalink / raw) To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai, Philipp Zabel, Linus Walleij, Bartosz Golaszewski Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio, quic_pkumpatl, kernel On some Qualcomm platforms, such as QCS6490-RB3Gen2 and QCM6490-IDP, multiple WSA8830/WSA8835 speakers share a common reset (shutdown) GPIO. To handle such cases, use the reset controller framework along with the "reset-gpio" driver. Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com> --- sound/soc/codecs/wsa883x.c | 57 ++++++++++++++++++++++++++++++++------ 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/sound/soc/codecs/wsa883x.c b/sound/soc/codecs/wsa883x.c index 13c9d4a6f015..b82b925c1f8d 100644 --- a/sound/soc/codecs/wsa883x.c +++ b/sound/soc/codecs/wsa883x.c @@ -14,6 +14,7 @@ #include <linux/printk.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> +#include <linux/reset.h> #include <linux/slab.h> #include <linux/soundwire/sdw.h> #include <linux/soundwire/sdw_registers.h> @@ -468,6 +469,7 @@ struct wsa883x_priv { struct sdw_stream_runtime *sruntime; struct sdw_port_config port_config[WSA883X_MAX_SWR_PORTS]; struct gpio_desc *sd_n; + struct reset_control *sd_reset; bool port_prepared[WSA883X_MAX_SWR_PORTS]; bool port_enable[WSA883X_MAX_SWR_PORTS]; int active_ports; @@ -1547,6 +1549,44 @@ static const struct hwmon_chip_info wsa883x_hwmon_chip_info = { .info = wsa883x_hwmon_info, }; +static void wsa883x_reset_powerdown(void *data) +{ + struct wsa883x_priv *wsa883x = data; + + if (wsa883x->sd_reset) + reset_control_assert(wsa883x->sd_reset); + else + gpiod_direction_output(wsa883x->sd_n, 1); +} + +static void wsa883x_reset_deassert(struct wsa883x_priv *wsa883x) +{ + if (wsa883x->sd_reset) + reset_control_deassert(wsa883x->sd_reset); + else + gpiod_direction_output(wsa883x->sd_n, 0); +} + +static int wsa883x_get_reset(struct device *dev, struct wsa883x_priv *wsa883x) +{ + wsa883x->sd_reset = devm_reset_control_get_optional_shared(dev, NULL); + if (IS_ERR(wsa883x->sd_reset)) + return dev_err_probe(dev, PTR_ERR(wsa883x->sd_reset), + "Failed to get reset\n"); + else if (wsa883x->sd_reset) + return 0; + /* + * else: NULL, so use the backwards compatible way for powerdown-gpios, + * which does not handle sharing GPIO properly. + */ + wsa883x->sd_n = devm_gpiod_get_optional(dev, "powerdown", + GPIOD_FLAGS_BIT_NONEXCLUSIVE | GPIOD_OUT_HIGH); + if (IS_ERR(wsa883x->sd_n)) + return dev_err_probe(dev, PTR_ERR(wsa883x->sd_n), + "Shutdown Control GPIO not found\n"); + return 0; +} + static int wsa883x_probe(struct sdw_slave *pdev, const struct sdw_device_id *id) { @@ -1567,13 +1607,9 @@ static int wsa883x_probe(struct sdw_slave *pdev, if (ret) return dev_err_probe(dev, ret, "Failed to enable vdd regulator\n"); - wsa883x->sd_n = devm_gpiod_get_optional(dev, "powerdown", - GPIOD_FLAGS_BIT_NONEXCLUSIVE | GPIOD_OUT_HIGH); - if (IS_ERR(wsa883x->sd_n)) { - ret = dev_err_probe(dev, PTR_ERR(wsa883x->sd_n), - "Shutdown Control GPIO not found\n"); - goto err; - } + ret = wsa883x_get_reset(dev, wsa883x); + if (ret) + return ret; dev_set_drvdata(dev, wsa883x); wsa883x->slave = pdev; @@ -1596,11 +1632,14 @@ static int wsa883x_probe(struct sdw_slave *pdev, pdev->prop.simple_clk_stop_capable = true; pdev->prop.sink_dpn_prop = wsa_sink_dpn_prop; pdev->prop.scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; - gpiod_direction_output(wsa883x->sd_n, 0); + + wsa883x_reset_deassert(wsa883x); + ret = devm_add_action_or_reset(dev, wsa883x_reset_powerdown, wsa883x); + if (ret) + return ret; wsa883x->regmap = devm_regmap_init_sdw(pdev, &wsa883x_regmap_config); if (IS_ERR(wsa883x->regmap)) { - gpiod_direction_output(wsa883x->sd_n, 1); ret = dev_err_probe(dev, PTR_ERR(wsa883x->regmap), "regmap_init failed\n"); goto err; -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers 2025-06-20 10:30 ` [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers Mohammad Rafi Shaik @ 2025-06-20 18:35 ` Dmitry Baryshkov 2025-06-23 7:11 ` Philipp Zabel 2025-06-23 8:01 ` Krzysztof Kozlowski 2025-06-23 7:00 ` Philipp Zabel 2025-06-24 2:43 ` Dmitry Baryshkov 2 siblings, 2 replies; 11+ messages in thread From: Dmitry Baryshkov @ 2025-06-20 18:35 UTC (permalink / raw) To: Mohammad Rafi Shaik, Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai, Philipp Zabel, Linus Walleij, Bartosz Golaszewski Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio, quic_pkumpatl, kernel On 20/06/2025 13:30, Mohammad Rafi Shaik wrote: > On some Qualcomm platforms, such as QCS6490-RB3Gen2 and QCM6490-IDP, > multiple WSA8830/WSA8835 speakers share a common reset (shutdown) GPIO. > To handle such cases, use the reset controller framework along with the > "reset-gpio" driver. How does this handle the fact that resetting one codec will also silently reset another one? > > Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com> > --- > sound/soc/codecs/wsa883x.c | 57 ++++++++++++++++++++++++++++++++------ > 1 file changed, 48 insertions(+), 9 deletions(-) -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers 2025-06-20 18:35 ` Dmitry Baryshkov @ 2025-06-23 7:11 ` Philipp Zabel 2025-06-23 8:01 ` Krzysztof Kozlowski 1 sibling, 0 replies; 11+ messages in thread From: Philipp Zabel @ 2025-06-23 7:11 UTC (permalink / raw) To: Dmitry Baryshkov, Mohammad Rafi Shaik, Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai, Linus Walleij, Bartosz Golaszewski Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio, quic_pkumpatl, kernel On Fr, 2025-06-20 at 21:35 +0300, Dmitry Baryshkov wrote: > On 20/06/2025 13:30, Mohammad Rafi Shaik wrote: > > On some Qualcomm platforms, such as QCS6490-RB3Gen2 and QCM6490-IDP, > > multiple WSA8830/WSA8835 speakers share a common reset (shutdown) GPIO. > > To handle such cases, use the reset controller framework along with the > > "reset-gpio" driver. > > How does this handle the fact that resetting one codec will also > silently reset another one? It's the other way around. Since shared reset controls have a common deassertion refcount, actual reset assertion only happens once reset_control_assert() has been called on all shared reset controls [1]. The speakers would only be put back into reset once the last one is unbound. [1] https://docs.kernel.org/driver-api/reset.html#shared-and-exclusive-resets regards Philipp ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers 2025-06-20 18:35 ` Dmitry Baryshkov 2025-06-23 7:11 ` Philipp Zabel @ 2025-06-23 8:01 ` Krzysztof Kozlowski 1 sibling, 0 replies; 11+ messages in thread From: Krzysztof Kozlowski @ 2025-06-23 8:01 UTC (permalink / raw) To: Dmitry Baryshkov, Mohammad Rafi Shaik, Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai, Philipp Zabel, Linus Walleij, Bartosz Golaszewski Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio, quic_pkumpatl, kernel On 20/06/2025 20:35, Dmitry Baryshkov wrote: > On 20/06/2025 13:30, Mohammad Rafi Shaik wrote: >> On some Qualcomm platforms, such as QCS6490-RB3Gen2 and QCM6490-IDP, >> multiple WSA8830/WSA8835 speakers share a common reset (shutdown) GPIO. >> To handle such cases, use the reset controller framework along with the >> "reset-gpio" driver. > > How does this handle the fact that resetting one codec will also > silently reset another one? Will not, that's the entire point of reset framework. See also my talk from last year OSS NA for some graphs/explanations. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers 2025-06-20 10:30 ` [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers Mohammad Rafi Shaik 2025-06-20 18:35 ` Dmitry Baryshkov @ 2025-06-23 7:00 ` Philipp Zabel 2025-06-24 2:43 ` Dmitry Baryshkov 2 siblings, 0 replies; 11+ messages in thread From: Philipp Zabel @ 2025-06-23 7:00 UTC (permalink / raw) To: Mohammad Rafi Shaik, Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai, Linus Walleij, Bartosz Golaszewski Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio, quic_pkumpatl, kernel On Fr, 2025-06-20 at 16:00 +0530, Mohammad Rafi Shaik wrote: > On some Qualcomm platforms, such as QCS6490-RB3Gen2 and QCM6490-IDP, > multiple WSA8830/WSA8835 speakers share a common reset (shutdown) GPIO. > To handle such cases, use the reset controller framework along with the > "reset-gpio" driver. > > Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com> > --- > sound/soc/codecs/wsa883x.c | 57 ++++++++++++++++++++++++++++++++------ > 1 file changed, 48 insertions(+), 9 deletions(-) > > diff --git a/sound/soc/codecs/wsa883x.c b/sound/soc/codecs/wsa883x.c > index 13c9d4a6f015..b82b925c1f8d 100644 > --- a/sound/soc/codecs/wsa883x.c > +++ b/sound/soc/codecs/wsa883x.c [...] > @@ -1547,6 +1549,44 @@ static const struct hwmon_chip_info wsa883x_hwmon_chip_info = { > .info = wsa883x_hwmon_info, > }; > > +static void wsa883x_reset_powerdown(void *data) > +{ > + struct wsa883x_priv *wsa883x = data; > + > + if (wsa883x->sd_reset) > + reset_control_assert(wsa883x->sd_reset); > + else > + gpiod_direction_output(wsa883x->sd_n, 1); > +} > + > +static void wsa883x_reset_deassert(struct wsa883x_priv *wsa883x) > +{ > + if (wsa883x->sd_reset) > + reset_control_deassert(wsa883x->sd_reset); > + else > + gpiod_direction_output(wsa883x->sd_n, 0); > +} > + > +static int wsa883x_get_reset(struct device *dev, struct wsa883x_priv *wsa883x) > +{ > + wsa883x->sd_reset = devm_reset_control_get_optional_shared(dev, NULL); Since this driver only deasserts/asserts on probe/remove, you could use devm_reset_control_get_optional_shared_deasserted() and let the framework code handle the (de)assertion of sd_reset. I wonder if the codec could also be put into reset during (runtime) suspend. regards Philipp ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers 2025-06-20 10:30 ` [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers Mohammad Rafi Shaik 2025-06-20 18:35 ` Dmitry Baryshkov 2025-06-23 7:00 ` Philipp Zabel @ 2025-06-24 2:43 ` Dmitry Baryshkov 2025-06-25 9:08 ` Mohammad Rafi Shaik 2 siblings, 1 reply; 11+ messages in thread From: Dmitry Baryshkov @ 2025-06-24 2:43 UTC (permalink / raw) To: Mohammad Rafi Shaik Cc: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai, Philipp Zabel, Linus Walleij, Bartosz Golaszewski, linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio, quic_pkumpatl, kernel On Fri, Jun 20, 2025 at 04:00:12PM +0530, Mohammad Rafi Shaik wrote: > On some Qualcomm platforms, such as QCS6490-RB3Gen2 and QCM6490-IDP, > multiple WSA8830/WSA8835 speakers share a common reset (shutdown) GPIO. > To handle such cases, use the reset controller framework along with the > "reset-gpio" driver. > > Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com> > --- > sound/soc/codecs/wsa883x.c | 57 ++++++++++++++++++++++++++++++++------ > 1 file changed, 48 insertions(+), 9 deletions(-) > > diff --git a/sound/soc/codecs/wsa883x.c b/sound/soc/codecs/wsa883x.c > index 13c9d4a6f015..b82b925c1f8d 100644 > --- a/sound/soc/codecs/wsa883x.c > +++ b/sound/soc/codecs/wsa883x.c > @@ -14,6 +14,7 @@ > #include <linux/printk.h> > #include <linux/regmap.h> > #include <linux/regulator/consumer.h> > +#include <linux/reset.h> > #include <linux/slab.h> > #include <linux/soundwire/sdw.h> > #include <linux/soundwire/sdw_registers.h> > @@ -468,6 +469,7 @@ struct wsa883x_priv { > struct sdw_stream_runtime *sruntime; > struct sdw_port_config port_config[WSA883X_MAX_SWR_PORTS]; > struct gpio_desc *sd_n; > + struct reset_control *sd_reset; > bool port_prepared[WSA883X_MAX_SWR_PORTS]; > bool port_enable[WSA883X_MAX_SWR_PORTS]; > int active_ports; > @@ -1547,6 +1549,44 @@ static const struct hwmon_chip_info wsa883x_hwmon_chip_info = { > .info = wsa883x_hwmon_info, > }; > > +static void wsa883x_reset_powerdown(void *data) > +{ > + struct wsa883x_priv *wsa883x = data; > + > + if (wsa883x->sd_reset) > + reset_control_assert(wsa883x->sd_reset); > + else > + gpiod_direction_output(wsa883x->sd_n, 1); > +} > + > +static void wsa883x_reset_deassert(struct wsa883x_priv *wsa883x) Please name these two functions in using antonyms (e.g. init/fini, powerup / powerdown, assert / deassert, etc). > +{ > + if (wsa883x->sd_reset) > + reset_control_deassert(wsa883x->sd_reset); > + else > + gpiod_direction_output(wsa883x->sd_n, 0); > +} > + > +static int wsa883x_get_reset(struct device *dev, struct wsa883x_priv *wsa883x) > +{ > + wsa883x->sd_reset = devm_reset_control_get_optional_shared(dev, NULL); > + if (IS_ERR(wsa883x->sd_reset)) > + return dev_err_probe(dev, PTR_ERR(wsa883x->sd_reset), > + "Failed to get reset\n"); > + else if (wsa883x->sd_reset) No need for 'else' here. > + return 0; > + /* > + * else: NULL, so use the backwards compatible way for powerdown-gpios, > + * which does not handle sharing GPIO properly. > + */ > + wsa883x->sd_n = devm_gpiod_get_optional(dev, "powerdown", > + GPIOD_FLAGS_BIT_NONEXCLUSIVE | GPIOD_OUT_HIGH); > + if (IS_ERR(wsa883x->sd_n)) > + return dev_err_probe(dev, PTR_ERR(wsa883x->sd_n), > + "Shutdown Control GPIO not found\n"); > + return 0; > +} > + > static int wsa883x_probe(struct sdw_slave *pdev, > const struct sdw_device_id *id) > { > @@ -1567,13 +1607,9 @@ static int wsa883x_probe(struct sdw_slave *pdev, > if (ret) > return dev_err_probe(dev, ret, "Failed to enable vdd regulator\n"); > > - wsa883x->sd_n = devm_gpiod_get_optional(dev, "powerdown", > - GPIOD_FLAGS_BIT_NONEXCLUSIVE | GPIOD_OUT_HIGH); > - if (IS_ERR(wsa883x->sd_n)) { > - ret = dev_err_probe(dev, PTR_ERR(wsa883x->sd_n), > - "Shutdown Control GPIO not found\n"); > - goto err; > - } > + ret = wsa883x_get_reset(dev, wsa883x); > + if (ret) > + return ret; > > dev_set_drvdata(dev, wsa883x); > wsa883x->slave = pdev; > @@ -1596,11 +1632,14 @@ static int wsa883x_probe(struct sdw_slave *pdev, > pdev->prop.simple_clk_stop_capable = true; > pdev->prop.sink_dpn_prop = wsa_sink_dpn_prop; > pdev->prop.scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; > - gpiod_direction_output(wsa883x->sd_n, 0); > + > + wsa883x_reset_deassert(wsa883x); > + ret = devm_add_action_or_reset(dev, wsa883x_reset_powerdown, wsa883x); > + if (ret) > + return ret; > > wsa883x->regmap = devm_regmap_init_sdw(pdev, &wsa883x_regmap_config); > if (IS_ERR(wsa883x->regmap)) { > - gpiod_direction_output(wsa883x->sd_n, 1); > ret = dev_err_probe(dev, PTR_ERR(wsa883x->regmap), > "regmap_init failed\n"); > goto err; > -- > 2.34.1 > -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers 2025-06-24 2:43 ` Dmitry Baryshkov @ 2025-06-25 9:08 ` Mohammad Rafi Shaik 0 siblings, 0 replies; 11+ messages in thread From: Mohammad Rafi Shaik @ 2025-06-25 9:08 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai, Philipp Zabel, Linus Walleij, Bartosz Golaszewski, linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio, quic_pkumpatl, kernel On 6/24/2025 8:13 AM, Dmitry Baryshkov wrote: > On Fri, Jun 20, 2025 at 04:00:12PM +0530, Mohammad Rafi Shaik wrote: >> On some Qualcomm platforms, such as QCS6490-RB3Gen2 and QCM6490-IDP, >> multiple WSA8830/WSA8835 speakers share a common reset (shutdown) GPIO. >> To handle such cases, use the reset controller framework along with the >> "reset-gpio" driver. >> >> Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com> >> --- >> sound/soc/codecs/wsa883x.c | 57 ++++++++++++++++++++++++++++++++------ >> 1 file changed, 48 insertions(+), 9 deletions(-) >> >> diff --git a/sound/soc/codecs/wsa883x.c b/sound/soc/codecs/wsa883x.c >> index 13c9d4a6f015..b82b925c1f8d 100644 >> --- a/sound/soc/codecs/wsa883x.c >> +++ b/sound/soc/codecs/wsa883x.c >> @@ -14,6 +14,7 @@ >> #include <linux/printk.h> >> #include <linux/regmap.h> >> #include <linux/regulator/consumer.h> >> +#include <linux/reset.h> >> #include <linux/slab.h> >> #include <linux/soundwire/sdw.h> >> #include <linux/soundwire/sdw_registers.h> >> @@ -468,6 +469,7 @@ struct wsa883x_priv { >> struct sdw_stream_runtime *sruntime; >> struct sdw_port_config port_config[WSA883X_MAX_SWR_PORTS]; >> struct gpio_desc *sd_n; >> + struct reset_control *sd_reset; >> bool port_prepared[WSA883X_MAX_SWR_PORTS]; >> bool port_enable[WSA883X_MAX_SWR_PORTS]; >> int active_ports; >> @@ -1547,6 +1549,44 @@ static const struct hwmon_chip_info wsa883x_hwmon_chip_info = { >> .info = wsa883x_hwmon_info, >> }; >> >> +static void wsa883x_reset_powerdown(void *data) >> +{ >> + struct wsa883x_priv *wsa883x = data; >> + >> + if (wsa883x->sd_reset) >> + reset_control_assert(wsa883x->sd_reset); >> + else >> + gpiod_direction_output(wsa883x->sd_n, 1); >> +} >> + >> +static void wsa883x_reset_deassert(struct wsa883x_priv *wsa883x) > > Please name these two functions in using antonyms (e.g. init/fini, > powerup / powerdown, assert / deassert, etc). > Ack, sure, will update the function names. >> +{ >> + if (wsa883x->sd_reset) >> + reset_control_deassert(wsa883x->sd_reset); >> + else >> + gpiod_direction_output(wsa883x->sd_n, 0); >> +} >> + >> +static int wsa883x_get_reset(struct device *dev, struct wsa883x_priv *wsa883x) >> +{ >> + wsa883x->sd_reset = devm_reset_control_get_optional_shared(dev, NULL); >> + if (IS_ERR(wsa883x->sd_reset)) >> + return dev_err_probe(dev, PTR_ERR(wsa883x->sd_reset), >> + "Failed to get reset\n"); >> + else if (wsa883x->sd_reset) > > No need for 'else' here. Ack, will make the changes. Thanks & best regards, Rafi. > >> + return 0; >> + /* >> + * else: NULL, so use the backwards compatible way for powerdown-gpios, >> + * which does not handle sharing GPIO properly. >> + */ >> + wsa883x->sd_n = devm_gpiod_get_optional(dev, "powerdown", >> + GPIOD_FLAGS_BIT_NONEXCLUSIVE | GPIOD_OUT_HIGH); >> + if (IS_ERR(wsa883x->sd_n)) >> + return dev_err_probe(dev, PTR_ERR(wsa883x->sd_n), >> + "Shutdown Control GPIO not found\n"); >> + return 0; >> +} >> + >> static int wsa883x_probe(struct sdw_slave *pdev, >> const struct sdw_device_id *id) >> { >> @@ -1567,13 +1607,9 @@ static int wsa883x_probe(struct sdw_slave *pdev, >> if (ret) >> return dev_err_probe(dev, ret, "Failed to enable vdd regulator\n"); >> >> - wsa883x->sd_n = devm_gpiod_get_optional(dev, "powerdown", >> - GPIOD_FLAGS_BIT_NONEXCLUSIVE | GPIOD_OUT_HIGH); >> - if (IS_ERR(wsa883x->sd_n)) { >> - ret = dev_err_probe(dev, PTR_ERR(wsa883x->sd_n), >> - "Shutdown Control GPIO not found\n"); >> - goto err; >> - } >> + ret = wsa883x_get_reset(dev, wsa883x); >> + if (ret) >> + return ret; >> >> dev_set_drvdata(dev, wsa883x); >> wsa883x->slave = pdev; >> @@ -1596,11 +1632,14 @@ static int wsa883x_probe(struct sdw_slave *pdev, >> pdev->prop.simple_clk_stop_capable = true; >> pdev->prop.sink_dpn_prop = wsa_sink_dpn_prop; >> pdev->prop.scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; >> - gpiod_direction_output(wsa883x->sd_n, 0); >> + >> + wsa883x_reset_deassert(wsa883x); >> + ret = devm_add_action_or_reset(dev, wsa883x_reset_powerdown, wsa883x); >> + if (ret) >> + return ret; >> >> wsa883x->regmap = devm_regmap_init_sdw(pdev, &wsa883x_regmap_config); >> if (IS_ERR(wsa883x->regmap)) { >> - gpiod_direction_output(wsa883x->sd_n, 1); >> ret = dev_err_probe(dev, PTR_ERR(wsa883x->regmap), >> "regmap_init failed\n"); >> goto err; >> -- >> 2.34.1 >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-06-25 9:09 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-20 10:30 [PATCH v1 0/2] Handle shared reset GPIO for WSA883x speakers Mohammad Rafi Shaik 2025-06-20 10:30 ` [PATCH v1 1/2] ASoC: dt-bindings: qcom,wsa8830: Add reset-gpios for shared line Mohammad Rafi Shaik 2025-06-23 8:03 ` Krzysztof Kozlowski 2025-06-23 8:41 ` Mohammad Rafi Shaik 2025-06-20 10:30 ` [PATCH v1 2/2] ASoC: codecs: wsa883x: Handle shared reset GPIO for WSA883x speakers Mohammad Rafi Shaik 2025-06-20 18:35 ` Dmitry Baryshkov 2025-06-23 7:11 ` Philipp Zabel 2025-06-23 8:01 ` Krzysztof Kozlowski 2025-06-23 7:00 ` Philipp Zabel 2025-06-24 2:43 ` Dmitry Baryshkov 2025-06-25 9:08 ` Mohammad Rafi Shaik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).