Re: [PATCH 2/2] dt-bindings: mfd: twl4030-audio: document preferred name for external mute

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: sound: ti,twl4030-audio: invalid $ref and missing deprecated tag
- [Medium] ASoC: twl4030: legacy ti,hs_extmute_gpio property not matched by gpiolib
--

commit 43d9eacfe52fdbf41c021df1d5b980ba9e4220b9
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

dt-bindings: mfd: twl4030-audio: document preferred name for external mute

This commit updates the ti,twl4030-audio binding schema to document
ti,hs_extmute-gpios as the preferred external mute gpio property name and
retain ti,hs_extmute_gpio as a deprecated legacy property.

> diff --git a/Documentation/devicetree/bindings/sound/ti,twl4030-audio.yaml b/Documentation/devicetree/bindings/sound/ti,twl4030-audio.yaml
> index c9c3f7513ad42..1d41f582261db 100644
> --- a/Documentation/devicetree/bindings/sound/ti,twl4030-audio.yaml
> +++ b/Documentation/devicetree/bindings/sound/ti,twl4030-audio.yaml
> @@ -39,12 +39,18 @@ properties:
>          description:
>            Enable the use of an external mute for headset pop reduction.
>  
> -      ti,hs_extmute_gpio:
> +      ti,hs_extmute-gpios:
>          $ref: /schemas/types.yaml#/definitions/phandle-array
>          description:
>            The GPIO specifier for the external mute control.
>          maxItems: 1

[Severity: Medium]
Will keeping $ref: /schemas/types.yaml#/definitions/phandle-array under
properties.codec.properties.ti,hs_extmute-gpios in
Documentation/devicetree/bindings/sound/ti,twl4030-audio.yaml cause a schema
validation error when running dt_binding_check?

The explicit phandle-array $ref was previously required on ti,hs_extmute_gpio
because its underscore _gpio suffix did not match standard gpio suffix
patterns. Because ti,hs_extmute-gpios uses the standard -gpios suffix, the
core dtschema meta-schema (meta-schemas/gpios.yaml and
schemas/gpio/gpio-consumer.yaml) automatically assigns its type and sets
additionalProperties: false, which disallows an explicit $ref to
phandle-array.

Also, for properties.codec.properties.ti,hs_extmute_gpio below, should
deprecated: true be added to the property definition in addition to the
description text so that dtschema validation tools recognize
ti,hs_extmute_gpio as deprecated?

>  
> +      ti,hs_extmute_gpio:
> +        $ref: /schemas/types.yaml#/definitions/phandle-array
> +        description:
> +          Deprecated legacy name for ti,hs_extmute-gpios.
> +        maxItems: 1

[Severity: Medium]
Can gpiolib actually resolve the deprecated ti,hs_extmute_gpio property
documented here at properties.codec.properties.ti,hs_extmute_gpio when a
device tree uses the legacy underscore name (for example, under twl_audio in
arch/arm/boot/dts/ti/omap/logicpd-som-lv.dtsi)?

During driver probe, twl4030_get_hw_params() calls
twl4030_get_board_params() in sound/soc/codecs/twl4030.c (updated in commit
"ASoC: codecs: twl4030: fix external mute GPIO lookup"):

	/*
	 * 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");

This calls into gpiod_find_and_request() -> of_find_gpio() in
drivers/gpio/gpiolib-of.c:

	/* Try GPIO property "foo-gpios" and "foo-gpio" */
	for_each_gpio_property_name(propname, con_id) {
		desc = of_get_named_gpiod_flags(np, propname, idx, &of_flags);
		if (!gpiod_not_found(desc))
			break;
	}

	/* Properly named GPIO was not found, try workarounds */
	for (q = of_find_gpio_quirks; gpiod_not_found(desc) && *q; q++)
		desc = (*q)(np, con_id, idx, &of_flags);

However, for_each_gpio_property_name() in drivers/gpio/gpiolib.h formats
candidate names using a hyphen:

#define for_each_gpio_property_name(propname, con_id)					\
	for (const char * const *__suffixes = gpio_suffixes;				\
	     *__suffixes && ({								\
		const char *__gs = *__suffixes;						\
											\
		if (con_id)								\
			snprintf(propname, sizeof(propname), "%s-%s", con_id, __gs);	\

Because this only checks ti,hs_extmute-gpios and ti,hs_extmute-gpio, and
of_find_gpio_rename() in drivers/gpio/gpiolib-of.c has no entry mapping
ti,hs_extmute to ti,hs_extmute_gpio, devm_fwnode_gpiod_get_optional() returns
NULL for boards specifying ti,hs_extmute_gpio.

When ti,hs_extmute is absent, external mute is silently skipped in
headset_ramp(). When ti,hs_extmute is set alongside ti,hs_extmute_gpio,
twl4030_get_hw_params() in sound/soc/codecs/twl4030.c sees a null
hs_extmute_gpio and reprograms TWL4030_PMBR1_REG to mux the internal
gpio6/pwm0 pin as extmute instead of using the external gpio:

	if (board_params && board_params->hs_extmute &&
	    !board_params->hs_extmute_gpio) {
		u8 pin_mux;

		dev_info(dev, "use TWL4030 GPIO6\n");

		/* Set TWL4030 GPIO6 as EXTMUTE signal */
		twl_i2c_read_u8(TWL4030_MODULE_INTBR, &pin_mux,
				TWL4030_PMBR1_REG);
		pin_mux &= ~TWL4030_GPIO6_PWM0_MUTE(0x03);
		pin_mux |= TWL4030_GPIO6_PWM0_MUTE(0x02);
		twl_i2c_write_u8(TWL4030_MODULE_INTBR, pin_mux,
				 TWL4030_PMBR1_REG);
	}

Does of_find_gpio_rename() in drivers/gpio/gpiolib-of.c (or a fallback lookup
in twl4030_get_board_params()) need to handle ti,hs_extmute_gpio so legacy
device trees continue to work as documented by this binding?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-twl4030-extmute-v1-0-3d2dbe260df9@gmail.com?part=2