Re: [PATCH 2/2] ASoC: codecs: Add Awinic AW8738 audio amplifier driver

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 972 bytes --]

On Tue, Mar 01, 2022 at 01:37:42PM +0100, Stephan Gerhold wrote:

> The Awinic AW8738 is a simple audio amplifier using an enable GPIO.
> The main difference to simple-amplifier is that there is a "one-wire
> pulse control" that allows configuring the amplifier to one of a few
> pre-defined modes.

What exactly are the modes here?  Looking at the web site for the part
it seems like it's selecting a power limit for the speaker so it makes
sense that the mode would be fixed in DT but it's not clear from the
driver.

> +	aw->gpiod_enable = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
> +	if (IS_ERR(aw->gpiod_enable))
> +		return dev_err_probe(dev, PTR_ERR(aw->gpiod_enable),
> +				     "Failed to get 'enable' gpio");

Are we sure that enable is the best name for this pin?  It's more
complex than just an enable since it's the 1 wire data - according to
what's on the awinic web site it looks like the actual label is /SHDN
which is similarly misleading though :/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]