linux-kernel - Re: [PATCH] ASoC: cs42xx8: Move reset operation to runtime resume

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aFEu1dsFNSSN8vKo@opensource.cirrus.com>
Date: Tue, 17 Jun 2025 10:01:09 +0100
From: Charles Keepax <ckeepax@...nsource.cirrus.com>
To: Shengjiu Wang <shengjiu.wang@....com>
Cc: david.rhodes@...rus.com, rf@...nsource.cirrus.com, lgirdwood@...il.com,
        broonie@...nel.org, perex@...ex.cz, tiwai@...e.com,
        linux-sound@...r.kernel.org, patches@...nsource.cirrus.com,
        linux-kernel@...r.kernel.org, shengjiu.wang@...il.com
Subject: Re: [PATCH] ASoC: cs42xx8: Move reset operation to runtime resume

On Tue, Jun 17, 2025 at 03:43:34PM +0800, Shengjiu Wang wrote:
> When codec is in idle state, the runtime suspend is executed, if codec
> has been reset and user wants to use i2ctransfer command to get the
> codec registers, i2c access will fail.
> 
> Move reset operation to the runtime resume stage to avoid such issues.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
> ---
>  sound/soc/codecs/cs42xx8.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/sound/soc/codecs/cs42xx8.c b/sound/soc/codecs/cs42xx8.c
> index 6a925f3f7137..be215cd2a714 100644
> --- a/sound/soc/codecs/cs42xx8.c
> +++ b/sound/soc/codecs/cs42xx8.c
> @@ -617,6 +617,7 @@ static int cs42xx8_runtime_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	gpiod_set_value_cansleep(cs42xx8->gpiod_reset, 1);
>  	gpiod_set_value_cansleep(cs42xx8->gpiod_reset, 0);
>  
>  	ret = regulator_bulk_enable(ARRAY_SIZE(cs42xx8->supplies),
> @@ -658,8 +659,6 @@ static int cs42xx8_runtime_suspend(struct device *dev)
>  	regulator_bulk_disable(ARRAY_SIZE(cs42xx8->supplies),
>  			       cs42xx8->supplies);
>  
> -	gpiod_set_value_cansleep(cs42xx8->gpiod_reset, 1);
> -

This makes no sense the line before this is disabling all the
supplies. This code is parking the chip in a turned off state
and it is better for it to be in reset for that. If you want to
access the chip directly from user-space during runtime suspend
you need to runtime resume it first, I believe that can be done
through sysfs.

Thanks,
Charles