Re: [PATCH v2 1/2] ASoC: codecs: twl4030: fix external mute GPIO lookup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ASoC: twl4030: broken ti,hs_extmute_gpio lookup in twl4030_get_board_params()
--

commit 4ac8bbc6b258588c0a2286c88c8f200492ec2c36
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

ASoC: codecs: twl4030: fix external mute GPIO lookup

This commit switches twl4030_get_board_params() to fwnode APIs and acquires
the external mute GPIO descriptor directly from the "codec" child firmware
node using devm_fwnode_gpiod_get_optional().

> diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
> index 7117af5a1a569..9541c8a5a24e8 100644
> --- a/sound/soc/codecs/twl4030.c
> +++ b/sound/soc/codecs/twl4030.c

[ ... ]

> @@ -198,41 +199,53 @@ static void twl4030_codec_enable(struct snd_soc_component *component, int enable
>  	udelay(10);
>  }
>  
> -static void
> -twl4030_get_board_param_values(struct twl4030_board_params *board_params,
> -			       struct device_node *node)
> +static struct twl4030_board_params *twl4030_get_board_params(struct device *dev)
>  {
> -	of_property_read_u32(node, "ti,digimic_delay", &board_params->digimic_delay);
> -	of_property_read_u32(node, "ti,ramp_delay_value", &board_params->ramp_delay_value);
> -	of_property_read_u32(node, "ti,offset_cncl_path", &board_params->offset_cncl_path);
> -	board_params->hs_extmute = of_property_read_bool(node, "ti,hs_extmute");
> +	struct twl4030_board_params *params;
> +	int error;
>  
> -	if (of_property_present(node, "ti,hs_extmute_gpio"))
> -		board_params->hs_extmute = 1;
> -}

[ ... ]

> +	/*
> +	 * This will attempt to parse "ti,hs_extmute-gpios" or
> +	 * "ti,hs_extmute_gpio".
> +	 */
> +	params->hs_extmute_gpio = devm_fwnode_gpiod_get_optional(dev, codec_node,
> +								 "ti,hs_extmute",
> +								 GPIOD_OUT_LOW,
> +								 "hs_extmute");

[Severity: High]
Will devm_fwnode_gpiod_get_optional() with con_id "ti,hs_extmute" actually
find the legacy "ti,hs_extmute_gpio" property (with an underscore)?

In drivers/gpio/gpiolib.h, for_each_gpio_property_name() constructs candidate
property names using a hyphen separator:

	if (con_id)
		snprintf(propname, sizeof(propname), "%s-%s", con_id, __gs);

With gpio_suffixes[] containing "gpios" and "gpio", passing "ti,hs_extmute"
makes of_find_gpio() in drivers/gpio/gpiolib-of.c look up
"ti,hs_extmute-gpios" and "ti,hs_extmute-gpio", and there is no entry in
of_find_gpio_rename() mapping "ti,hs_extmute" to "ti,hs_extmute_gpio".

However, both Documentation/devicetree/bindings/sound/ti,twl4030-audio.yaml
and arch/arm/boot/dts/ti/omap/logicpd-som-lv.dtsi spell the legacy property
with an underscore:

	twl_audio: audio {
		compatible = "ti,twl4030-audio";
		codec {
			ti,hs_extmute_gpio = <&gpio2 25 GPIO_ACTIVE_HIGH>;
		};
	};

On boards using "ti,hs_extmute_gpio", devm_fwnode_gpiod_get_optional() will
return NULL. When "ti,hs_extmute" is absent (as in logicpd-som-lv.dtsi), both
params->hs_extmute_gpio and params->hs_extmute remain unset, leaving external
mute disabled. When "ti,hs_extmute" is present alongside
"ti,hs_extmute_gpio", twl4030_get_hw_params() sees params->hs_extmute == 1 and
!params->hs_extmute_gpio, causing it to mux TWL4030's internal GPIO6 as
EXTMUTE instead of driving the external GPIO.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-twl4030-extmute-v2-0-c7e0c90509a1@gmail.com?part=1