linux-kernel - Re: [PATCH 05/10] ASoC: codecs: wsa881x: add runtime pm support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <a2a6f631-b1b6-e015-f6bd-0c3851fdaa83@linux.intel.com>
Date:   Tue, 22 Feb 2022 13:53:54 -0600
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        broonie@...nel.org
Cc:     alsa-devel@...a-project.org, lgirdwood@...il.com,
        linux-kernel@...r.kernel.org, tiwai@...e.com,
        quic_srivasam@...cinc.com
Subject: Re: [PATCH 05/10] ASoC: codecs: wsa881x: add runtime pm support



On 2/21/22 07:10, Srinivas Kandagatla wrote:
> WSA881x codecs can not cope up with clk stop and requires a full reset after suspend.

isn't clock stop mode0 a peripheral requirement in SoundWire 1.x? I
don't see any permission to skip this mode, the only thing I see in the
spec is the permission to implement an always-ready capability (SCSP_SM)

The current assumption is that ALL peripherals support that mode, but
that Managers may or may not support clock stop.

It wouldn't be a big deal to add a quirk, but IMHO this needs to be
captured at the bus/manager level since the decision to enter this mode
is made at that level.

> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> ---
>  sound/soc/codecs/wsa881x.c | 54 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c
> index 0222370ff95d..d851ba14fbdd 100644
> --- a/sound/soc/codecs/wsa881x.c
> +++ b/sound/soc/codecs/wsa881x.c
> @@ -11,6 +11,7 @@
>  #include <linux/of_gpio.h>
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/soundwire/sdw.h>
>  #include <linux/soundwire/sdw_registers.h>
>  #include <linux/soundwire/sdw_type.h>
> @@ -198,6 +199,7 @@
>  #define WSA881X_OCP_CTL_TIMER_SEC 2
>  #define WSA881X_OCP_CTL_TEMP_CELSIUS 25
>  #define WSA881X_OCP_CTL_POLL_TIMER_SEC 60
> +#define WSA881X_PROBE_TIMEOUT 1000
>  
>  #define WSA881X_PA_GAIN_TLV(xname, reg, shift, max, invert, tlv_array) \
>  {	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
> @@ -747,6 +749,12 @@ static int wsa881x_put_pa_gain(struct snd_kcontrol *kc,
>  	unsigned int mask = (1 << fls(max)) - 1;
>  	int val, ret, min_gain, max_gain;
>  
> +	ret = pm_runtime_get_sync(comp->dev);
> +	if (ret < 0 && ret != -EACCES) {
> +		pm_runtime_put_noidle(comp->dev);
> +		return ret;
> +	}
> +
>  	max_gain = (max - ucontrol->value.integer.value[0]) & mask;
>  	/*
>  	 * Gain has to set incrementally in 4 steps
> @@ -773,6 +781,9 @@ static int wsa881x_put_pa_gain(struct snd_kcontrol *kc,
>  		usleep_range(1000, 1010);
>  	}
>  
> +	pm_runtime_mark_last_busy(comp->dev);
> +	pm_runtime_put_autosuspend(comp->dev);
> +
>  	return 1;
>  }
>  
> @@ -1101,6 +1112,7 @@ static int wsa881x_probe(struct sdw_slave *pdev,
>  			 const struct sdw_device_id *id)
>  {
>  	struct wsa881x_priv *wsa881x;
> +	struct device *dev = &pdev->dev;
>  
>  	wsa881x = devm_kzalloc(&pdev->dev, sizeof(*wsa881x), GFP_KERNEL);
>  	if (!wsa881x)
> @@ -1124,6 +1136,7 @@ static int wsa881x_probe(struct sdw_slave *pdev,
>  	pdev->prop.sink_ports = GENMASK(WSA881X_MAX_SWR_PORTS, 0);
>  	pdev->prop.sink_dpn_prop = wsa_sink_dpn_prop;
>  	pdev->prop.scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY;
> +	pdev->prop.simple_clk_stop_capable = true;
>  	gpiod_direction_output(wsa881x->sd_n, 1);
>  
>  	wsa881x->regmap = devm_regmap_init_sdw(pdev, &wsa881x_regmap_config);
> @@ -1132,12 +1145,52 @@ static int wsa881x_probe(struct sdw_slave *pdev,
>  		return PTR_ERR(wsa881x->regmap);
>  	}
>  
> +	pm_runtime_set_autosuspend_delay(dev, 3000);
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +
>  	return devm_snd_soc_register_component(&pdev->dev,
>  					       &wsa881x_component_drv,
>  					       wsa881x_dais,
>  					       ARRAY_SIZE(wsa881x_dais));
>  }
>  
> +static int __maybe_unused wsa881x_runtime_suspend(struct device *dev)
> +{
> +	struct regmap *regmap = dev_get_regmap(dev, NULL);
> +	struct wsa881x_priv *wsa881x = dev_get_drvdata(dev);
> +
> +	gpiod_direction_output(wsa881x->sd_n, 0);
> +
> +	regcache_cache_only(regmap, true);
> +	regcache_mark_dirty(regmap);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused wsa881x_runtime_resume(struct device *dev)
> +{
> +	struct sdw_slave *slave = dev_to_sdw_dev(dev);
> +	struct regmap *regmap = dev_get_regmap(dev, NULL);
> +	struct wsa881x_priv *wsa881x = dev_get_drvdata(dev);
> +
> +	gpiod_direction_output(wsa881x->sd_n, 1);
> +
> +	wait_for_completion_timeout(&slave->initialization_complete,
> +				    msecs_to_jiffies(WSA881X_PROBE_TIMEOUT));
> +
> +	regcache_cache_only(regmap, false);
> +	regcache_sync(regmap);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops wsa881x_pm_ops = {
> +	SET_RUNTIME_PM_OPS(wsa881x_runtime_suspend, wsa881x_runtime_resume, NULL)
> +};
> +
>  static const struct sdw_device_id wsa881x_slave_id[] = {
>  	SDW_SLAVE_ENTRY(0x0217, 0x2010, 0),
>  	SDW_SLAVE_ENTRY(0x0217, 0x2110, 0),
> @@ -1151,6 +1204,7 @@ static struct sdw_driver wsa881x_codec_driver = {
>  	.id_table = wsa881x_slave_id,
>  	.driver = {
>  		.name	= "wsa881x-codec",
> +		.pm = &wsa881x_pm_ops,
>  	}
>  };
>  module_sdw_driver(wsa881x_codec_driver);