Re: [PATCH v1 1/7] ASoC: dt-bindings: ES8389: Add members about HPF and clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Adding Devicetree properties ('everest,hpfl', 'everest,hpfr') that represent raw register values instead of physical hardware descriptions, and statically configuring audio processing features that should be ALSA kcontrols.
- [Medium] Custom clock routing property 'everest,mclk-from-sclk' bypasses standard ASoC clock configuration APIs.
--

commit f53482d6ef6f3f66c8185a876c24dae03171cfd8
Author: Zhang Yi <zhangyi@everest-semi.com>

ASoC: dt-bindings: ES8389: Add members about HPF and clock

Add members related to HPF and mclk_source. The value of HPF is not dB, it
is just the value of registers. And add HPF settings to the example.

> diff --git a/Documentation/devicetree/bindings/sound/everest,es8389.yaml b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> index 75ce0bc489045..be92014c0c597 100644
> --- a/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> +++ b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> @@ -38,6 +38,27 @@ properties:
>      description:
>        Interface power supply.
>  
> +  everest,mclk-from-sclk:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Indicates that SCLK is used as the internal clock.

[Severity: Medium]
Does this custom boolean flag bypass standard ASoC clock configuration APIs?

Statically configuring the codec to use SCLK as the internal clock via
devicetree limits machine driver flexibility. ASoC clock routing decisions
are normally handled dynamically by the machine driver using standard APIs
like snd_soc_dai_set_sysclk() and the standard devicetree clocks property.

A later commit in the series (ASoC: codecs: ES8389: Fix the issue about
mclk_src) addresses a type mismatch where the driver initially expected a u8
instead of a boolean, but the architectural bypass issue remains.

> +
> +  everest,hpfl:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    description:
> +      the HPF value of ADCL.
> +    minimum: 0x00
> +    maximum: 0x0f
> +    default: 0x0a
> +
> +  everest,hpfr:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    description:
> +      the HPF value of ADCR.
> +    minimum: 0x00
> +    maximum: 0x0f
> +    default: 0x0a

[Severity: Medium]
Should these properties be represented as ALSA kcontrols instead?

Devicetree is designed to describe hardware topology and physical
characteristics, not to pass raw register configurations. Audio processing
parameters like a High-Pass Filter (HPF) should be exposed to userspace as
ALSA kcontrols so they can be dynamically adjusted or bypassed.

In sound/soc/codecs/es8389.c:es8389_probe(), the driver reads these custom
raw register properties directly. Can we expose these as kcontrols instead?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611031824.4628-1-zhangyi@everest-semi.com?part=1