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.1On 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
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
>>
>
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
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
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
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
© 2016 - 2025 Red Hat, Inc.