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: <ZvsYk4aGCPvR2kyf@finisterre.sirena.org.uk>
Date: Mon, 30 Sep 2024 22:30:59 +0100
From: Mark Brown <broonie@...nel.org>
To: Benjamin Hahn <B.Hahn@...tec.de>
Cc: Shenghao Ding <shenghao-ding@...com>, Kevin Lu <kevin-lu@...com>,
	Baojun Xu <baojun.xu@...com>, Liam Girdwood <lgirdwood@...il.com>,
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
	Teresa Remmet <t.remmet@...tec.de>, alsa-devel@...a-project.org,
	linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org,
	upstream@...ts.phytec.de
Subject: Re: [PATCH] sound: soc: codecs: tlv320aic3x: Fix codec gpio-reset

On Mon, Sep 30, 2024 at 09:16:46AM +0200, Benjamin Hahn wrote:

> The TLV320AIC3007 requires a hardware reset after power-up for proper
> operation. After all power supplies are at their specified values,
> the RESET pin must be driven low for at least 10 ns. Even though the
> datasheet sais min 10ns, I needed more than 10ns when testing this out.
> 15ns worked for me.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

> -		if (aic3x->gpio_reset)
> +		if (aic3x->gpio_reset) {
>  			gpiod_set_value(aic3x->gpio_reset, 1);
> +			ndelay(15);
> +			gpiod_set_value(aic3x->gpio_reset, 0);
> +		}

This seems obviously buggy, it leaves the GPIO with the opposite state
to that it would have prior to the patch.  It's also not joined up with
your changelog, that talks about actions taken after powering up the
device but this is a callback run after power has been removed from the
device so nothing in your changelog motivates leaving reset deasserted.

>  		regcache_mark_dirty(aic3x->regmap);
>  	}
>  
> @@ -1813,6 +1816,10 @@ int aic3x_probe(struct device *dev, struct regmap *regmap, kernel_ulong_t driver
>  
>  	gpiod_set_consumer_name(aic3x->gpio_reset, "tlv320aic3x reset");
>  
> +	/* CODEC datasheet says minimum of 10ns */
> +	ndelay(15);
> +	gpiod_set_value(aic3x->gpio_reset, 0);
> +

This seems more relevant to your changelog, but I don't understand why
aic3x_set_power() is not also (instead?) updated?

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ