| Message ID | 20240813025436.52410-3-kiseok.jo@irondevice.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add a driver for the Iron Device SMA1307 Amp | expand |
On Tue, 13 Aug 2024 11:54:36 +0900, Kiseok Jo wrote: > Signed-off-by: Kiseok Jo <kiseok.jo@irondevice.com> > --- > .../bindings/sound/irondevice,sma1307.yaml | 66 +++++++++++++++++++ > 1 file changed, 66 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/irondevice,sma1307.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/irondevice,sma1307.yaml: use-binary: missing type definition doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240813025436.52410-3-kiseok.jo@irondevice.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
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
> 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 Thank you for the advise. I'll make the corrections and resend the patch with the changes. Like below: ASoC: dt-bindings: irondevice,sma1307: Add sma1307 amplifier > <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> I sent the emails using get_maintainers.pl, but I'll try using b4 as you suggested. Thank you for the helpful information. > > + 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? They are all different products. For example, there is no sma1307aq-w, so I listed them all to distinguish between them. There are hardware differences between the products. > > + > > + '#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. This is an option for choosing whether to use a binary file or default values when configuring the hardware, so I considered it to be related to hardware and included it in the device tree. If it is not appropriate as a device tree entry, I will consider moving it to sysfs. > > + > > +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. I will remove the interrupt as it is not being used. If needed in the future, I will add the header for gpio.h. Thank you for your kind feedback. I'll make the corrections and distribute the updated version. Best regards, Kiseok Jo
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. + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + description: + only in sma1307 + + '#sound-dai-cells': + const: 1 + + use-binary: + description: + whether to use binary files for device settings. + +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>; + }; + };
Signed-off-by: Kiseok Jo <kiseok.jo@irondevice.com> --- .../bindings/sound/irondevice,sma1307.yaml | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/irondevice,sma1307.yaml