Re: [PATCH v2 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] Property `everest,mclk-src` uses an unidiomatic `uint8` type to model a boolean flag, which is highly error-prone due to DT endianness.
- [Low] Property `everest,hpf-frq` lacks the standard `-hz` unit suffix and uses an uncommon abbreviation.
--

commit 733db5a41798dd67759483d1a1aa543394f0ddf2
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

> diff --git a/Documentation/devicetree/bindings/sound/everest,es8389.yaml b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> --- a/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> +++ b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> @@ -42,6 +42,21 @@ properties:
>      description:
>        Interface power supply.
>  
> +  everest,mclk-src:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    description:
> +      Indicates that SCLK is used as the internal clock.
> +    minimum: 0
> +    maximum: 0x01
> +    default: 0x00

[Severity: Medium]
Since this functions conceptually as a boolean toggle, should this be
defined as a standard boolean property instead of a uint8?

If a developer intuitively writes "everest,mclk-src = <1>;" in the Device
Tree, standard angle-bracket integer cells evaluate to 32-bit big-endian
integers (0x00 0x00 0x00 0x01).

When device_property_read_u8() reads this in the driver:
    ret = device_property_read_u8(component->dev, "everest,mclk-src", &es8389->mclk_src);

It will read the first byte (0x00) and silently misinterpret the flag as 0.
To correctly set the flag as a uint8, users would be forced to use the unwieldy
syntax "/bits/ 8 <1>;". Using a boolean property (where presence indicates true)
would prevent this silent configuration failure.

Also, prior discussions upstream noted a bitmask mismatch (0x40) related to this
property, but it was resolved in a subsequent patch in this series ("ASoC:
codecs: ES8389: Fix the issue about mclk_src").

> +
> +  everest,hpf-frq:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      The frequency of HPF in Hz.
> +    maximum: 1020
> +    default: 16

[Severity: Low]
Does this property name need the standard "-hz" unit suffix?

According to Device Tree schema guidelines, properties representing frequencies
should use the "-hz" suffix to be self-documenting (e.g., "everest,hpf-hz").
Additionally, "frq" is a non-standard abbreviation for frequency.

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