Re: [PATCH v1 3/4] ASoC: dt-bindings: Add dt bindings for FS2104/5S audio amplifiers

[Krzysztof Kozlowski <krzk@kernel.org>]

On 03/07/2025 05:56, Nick wrote:
> From: Nick Li <nick.li@foursemi.com>
> 
> This patch adds bindings for FourSemi FS2104/5S audio amplifiers
> which can support both I2S and I2C interface.
> 
> Signed-off-by: Nick Li <nick.li@foursemi.com>

Your patches are not correctly ordered. Vendor prefix is supposed to be
before is being used. Bindings are before their users.

> ---
>  .../bindings/sound/foursemi,fs210x.yaml       | 95 +++++++++++++++++++
>  1 file changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/foursemi,fs210x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/foursemi,fs210x.yaml b/Documentation/devicetree/bindings/sound/foursemi,fs210x.yaml
> new file mode 100644
> index 000000000..3a1aba9b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/foursemi,fs210x.yaml

Choose one compatible as filename.

> @@ -0,0 +1,95 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/foursemi,fs210x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: FourSemi FS2104/5S Digital Audio Amplifier
> +
> +maintainers:
> +  - Nick <nick.li@foursemi.com>
> +
> +description: |
> +    The FS2104 is a 15W Inductor-Less, Stereo, Closed-Loop,
> +    Digital Input Class-D Power Amplifier with Enhanced Signal Processing.
> +    The FS2105S is a 30W Inductor-Less, Stereo, Closed-Loop,
> +    Digital Input Class-D Power Amplifier with Enhanced Signal Processing.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - foursemi,fs2104
> +      - foursemi,fs2105s

Your driver suggests these are compatible, so express that with fallback.

> +
> +  reg:
> +    maxItems: 1
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      I2C address of the amplifier. Refer to datasheet for possible values:
> +      0x68/0x69/0x6A/0x6B
> +
> +  clocks:
> +    items:
> +      - description: The clock of I2S BCLK
> +
> +  clock-names:
> +    items:
> +      - const: bclk
> +
> +  '#sound-dai-cells':
> +    const: 0
> +
> +  pvdd-supply:
> +    description: |

Do not need '|' unless you need to preserve formatting.


> +      Regulator for power supply(PVDD in datasheet).
> +
> +  dvdd-supply:
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      Regulator for digital supply(DVDD in datasheet).
> +
> +  sdz-gpios:

Use standard properties - see gpio-consumer-common

> +    maxItems: 1
> +    description: |
> +      SDZ(Shut Down) pin is active low, it will power down the chip,
> +      and then reset the chip to shut down state.
> +
> +  fs,fwm-name:

No, use standard properties (see writing bindings)

> +    description: |
> +      The name of firmware that should be loaded for this
> +      instance. The firmware is gernerated by FourSemi's tuning tool.
> +    $ref: /schemas/types.yaml#/definitions/string
> +
> +  fs,dai-name:

No, it's already in dai-common

> +    description: |
> +      The name of FS210x DAI, it can be set different name when
> +      there are multiple FS210x devices in the system.
> +    $ref: /schemas/types.yaml#/definitions/string
> +
> +required:
> +  - compatible
> +  - reg
> +  - sdz-gpios
> +  - fs,fwm-name
> +  - '#sound-dai-cells'
> +

Missing ref to dai common

> +additionalProperties: false

And this should be unevaluatedProperties instead


Best regards,
Krzysztof