Re: [PATCH 2/2] ASoC: sma1307: Add bindings for Iron Device SMA1307 amplifier

[Krzysztof Kozlowski <krzk@kernel.org>]

On 13/08/2024 04:54, Kiseok Jo wrote:
> Signed-off-by: Kiseok Jo <kiseok.jo@irondevice.com>

Missing commit msg. Please order the patches as asked in submitting
bindings - bindings before users.

A nit, subject: drop second/last, redundant "bindings for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.
</form letter>



> ---
>  .../bindings/sound/irondevice,sma1307.yaml    | 66 +++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/irondevice,sma1307.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/irondevice,sma1307.yaml b/Documentation/devicetree/bindings/sound/irondevice,sma1307.yaml
> new file mode 100644
> index 000000000000..a2bcbdc3444e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/irondevice,sma1307.yaml
> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/irondevice,sma1307.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Iron Device SMA1307 Audio Amplifier
> +
> +maintainers:
> +  - Kiseok Jo <kiseok.jo@irondevice.com>
> +
> +description:
> +  SMA1307 boosted digital speaker amplifier
> +  with feedback-loop.
> +
> +allOf:
> +  - $ref: dai-common.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - irondevice,sma1307a-w
> +      - irondevice,sma1307a-f
> +      - irondevice,sma1307aq-f
> +    description:
> +      It is divided according to the package.
> +      The WLCSP packages are denoted with 'w', and the QFN packages are denoted
> +      with 'f'. If a 'q' is added, it indicated the product is AEC-Q100
> +      qualified for automotive applications.

Package usually does not mean different compatibles. Aren't they all
compatible? Or even the same?

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +    description:
> +      only in sma1307

??? Drop

> +
> +  '#sound-dai-cells':
> +    const: 1
> +
> +  use-binary:
> +    description:
> +      whether to use binary files for device settings.

Drop property. You described the desired Linux feature or behavior, not
the actual hardware. The bindings are about the latter, so instead you
need to rephrase the property and its description to match actual
hardware capabilities/features/configuration etc.

> +
> +required:
> +  - compatible
> +  - reg
> +  - '#sound-dai-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        amplifier@1e {
> +            compatible = "irondevice,sma1307a-w";
> +            reg = <0x1e>;
> +            #sound-dai-cells = <1>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <4 0>;

Include proper header and use defines for flags.



Best regards,
Krzysztof