Re: [PATCH v3 12/17] ASoC: simple-amplifier: gpio-audio-amp: Add support for bypass gpio

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The bypass GPIO can drive a physical logic HIGH into an unpowered amplifier chip, creating a severe risk of back-powering and permanent hardware damage.
--

commit 802f9d8e2a1a80c9fdc791af2e538c251cfdc8e9
Author: Herve Codina <herve.codina@bootlin.com>

ASoC: simple-amplifier: gpio-audio-amp: Add support for bypass gpio

This commit adds support for controlling an amplifier bypass feature using
a GPIO. It duplicates the initialization pattern used for the mute GPIO.

> diff --git a/sound/soc/codecs/simple-amplifier.c b/sound/soc/codecs/simple-amplifier.c
> index c8991e340def5..b9c09a04832a8 100644
> --- a/sound/soc/codecs/simple-amplifier.c
> +++ b/sound/soc/codecs/simple-amplifier.c
[ ... ]
> @@ -352,8 +360,15 @@ static int simple_amp_probe(struct platform_device *pdev)
>  			return ret;
>  	}
>  
> +	if (simple_amp->data->supports & SIMPLE_AUDIO_SUPPORT_BYPASS) {
> +		ret = simple_amp_parse_single_gpio(dev, &simple_amp->bypass, "bypass");

Looking at simple_amp_parse_single_gpio(), it requests the GPIO
synchronously with GPIOD_OUT_LOW:

sound/soc/codecs/simple-amplifier.c:simple_amp_parse_single_gpio() {
    ...
    single->gpio = devm_gpiod_get_optional(dev, gpio_property, GPIOD_OUT_LOW);
    ...
}

Since this is called during probe, before the DAPM powers up the device,
can this cause hardware damage if the device tree defines the GPIO as
active low?

If the GPIO is defined as active low, gpiolib will immediately drive the pin
to a physical high voltage. Because the amplifier's power supplies and enable
pin are managed by DAPM and disabled at probe time, driving a physical high
signal into an unpowered CMOS device might forward-bias the internal ESD
protection diodes.

This could back-power the chip from the SoC's GPIO pad and cause latch-up.
Would it be safer to strictly sequence the GPIO state changes with the
device's power state? For example, by deferring the GPIO output configuration
until DAPM powers up the device, or implementing the controls as DAPM widgets.

Note that this identical behavior appears to exist for the mute GPIO as
well, which this bypass GPIO implementation is modeled after.

> +		if (ret)
> +			return ret;
> +	}
> +
>  	/* Set controls name */
>  	simple_amp->mute.control_name = "Switch";
> +	simple_amp->bypass.control_name = "Bypass Switch";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513081702.317117-1-herve.codina@bootlin.com?part=12