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

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/bi...

<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