| Message ID | 20230508054512.719-1-13916275206@139.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/5] ASoC: tas2781: Add Header file for tas2781 driver | expand |
On Mon, 08 May 2023 13:45:12 +0800, Shenghao Ding wrote: > Create tas2781.yaml for tas2781 driver. > > Signed-off-by: Shenghao Ding <13916275206@139.com> > > --- > Changes in v7: > - Submit together with tas2781 codec driver code > - Add more detail description for ti,audio-slots > - Keep consistent for "I2C" > - remove reset-gpios description > - For reg, express as constraints instead > - remove unnecessary '|' > Changes to be committed: > new file: Documentation/devicetree/bindings/sound/ti,tas2781.yaml > --- > .../devicetree/bindings/sound/ti,tas2781.yaml | 90 +++++++++++++++++++ > 1 file changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/ti,tas2781.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/rockchip-isp1.example.dtb: camera@3c: port:endpoint:data-lanes: [[1]] is too short From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.example.dtb: camera-sensor@3c: port:endpoint:data-lanes: [[1]] is too short From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/ovti,ov2685.yaml /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.example.dtb: pcie-ep@33800000: Unevaluated properties are not allowed ('assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' were unexpected) From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml doc reference errors (make refcheckdocs): Documentation/usb/gadget_uvc.rst: Documentation/userspace-api/media/v4l/pixfmt-packed.yuv.rst MAINTAINERS: Documentation/devicetree/bindings/pwm/pwm-apple.yaml See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230508054512.719-1-13916275206@139.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 08/05/2023 07:45, Shenghao Ding wrote: > Create tas2781.yaml for tas2781 driver. > > Signed-off-by: Shenghao Ding <13916275206@139.com> > > --- > Changes in v7: Your subject says it is v2 and previously it was v6. Make it consistent. > - Submit together with tas2781 codec driver code Fix your patchset threading. I don't think it's possible to apply your one. > - Add more detail description for ti,audio-slots > - Keep consistent for "I2C" > - remove reset-gpios description > - For reg, express as constraints instead > - remove unnecessary '|' > Changes to be committed: > new file: Documentation/devicetree/bindings/sound/ti,tas2781.yaml > --- > .../devicetree/bindings/sound/ti,tas2781.yaml | 90 +++++++++++++++++++ > 1 file changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/ti,tas2781.yaml > > diff --git a/Documentation/devicetree/bindings/sound/ti,tas2781.yaml b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml > new file mode 100644 > index 000000000000..96c2584855d4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml > @@ -0,0 +1,90 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright (C) 2022 - 2023 Texas Instruments Incorporated > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/ti,tas2781.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Texas Instruments TAS2781 SmartAMP > + > +maintainers: > + - Shenghao Ding <shenghao-ding@ti.com> > + > +description: > + The TAS2781 is a mono, digital input Class-D audio amplifier > + optimized for efficiently driving high peak power into small > + loudspeakers. Integrated an on-chip DSP supports Texas Instruments > + Smart Amp speaker protection algorithm. The integrated speaker > + voltage and current sense provides for real time > + monitoring of loudspeaker behavior. > + you miss allOf with ref to saound-dai-common. > +properties: > + compatible: > + enum: > + - ti,tas2781 > + > + reg: > + description: I2C address of the primary device. > + items: > + minimum: 0x38 > + maximum: 0x3f > + > + reset-gpios: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + ti,audio-slots: > + description: > + Multiple tas2781s aggregate as one Audio Amp to support > + multiple audio slots > + $ref: /schemas/types.yaml#/definitions/uint32-array > + maxItems: 4 You said you can have here two addresses. You don't allow it, test it. Missing minItems, but... > + items: > + minimum: 0x38 > + maximum: 0x3f ... So these are fixed? No need to encode them in such case... and anyway actually I agree with Rob here - these addresses should be put in reg. > + description: > + I2C address of the device for different audio slots, > + useless in mono case. > + > + ti,broadcast-addr: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Generic I2C address for all the tas2781 devices in > + purpose of I2C broadcast during the multi-device > + writes, useless in mono case. Probably you can figure it out from previous addresses and there is no need for this property. > + > + '#sound-dai-cells': > + const: 1 > + > +required: > + - compatible > + - reg > + Best regards, Krzysztof
On 10/05/2023 16:03, Ding, Shenghao wrote: > >> + items: >> + minimum: 0x38 >> + maximum: 0x3f > > ... So these are fixed? No need to encode them in such case... > > and anyway actually I agree with Rob here - these addresses should be put in reg. > [DING] if all the put in reg, the i2c_probe will be called several time. The code don not want to register several codecs, but one codec including several tas2781s. Eh? Why and what would call probe multiple times for one device? It's the first time I hear... I don't know, maybe Linux I2C has some specifics, but sounds surprising. > >> + description: >> + I2C address of the device for different audio slots, >> + useless in mono case. >> + >> + ti,broadcast-addr: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + Generic I2C address for all the tas2781 devices in >> + purpose of I2C broadcast during the multi-device >> + writes, useless in mono case. > > Probably you can figure it out from previous addresses and there is no need for this property. > [Ding] this address is the common address for all the tas2781, it can be used for dsp firmware downloading to all the tas2781s in parallel, which can save more downloading time So the answer is yes? Then please drop it... If not, say why it cannot be deducted from other addresses (e.g. last address + 1). Best regards, Krzysztof
On 11/05/2023 07:49, Mark Brown wrote: >> Missing minItems, but... > >>> + items: >>> + minimum: 0x38 >>> + maximum: 0x3f > >> ... So these are fixed? No need to encode them in such case... > > I'm not sure I understand your concern here, there's up to 4 possible > values from 0x38-0x3f which has more than 4 possible values. Aren't the addresses going to be incremented by one (up to 8 of devices in total)? Best regards, Krzysztof
On 11/05/2023 07:49, Mark Brown wrote: >> Missing minItems, but... > >>> + items: >>> + minimum: 0x38 >>> + maximum: 0x3f > >> ... So these are fixed? No need to encode them in such case... > > I'm not sure I understand your concern here, there's up to 4 possible > values from 0x38-0x3f which has more than 4 possible values. Aren't the addresses going to be incremented by one (up to 8 of devices in total)? No, the i2c address order is not always monotonic increase or decrease, sometime it would be disorder, according to the application. Each device would have eight possible i2c address, the final address depends on the hardware connections. Best regards, Krzysztof
On 11/05/2023 15:19, Ding, Shenghao wrote: > > On 11/05/2023 07:49, Mark Brown wrote: >>> Missing minItems, but... >> >>>> + items: >>>> + minimum: 0x38 >>>> + maximum: 0x3f >> >>> ... So these are fixed? No need to encode them in such case... >> >> I'm not sure I understand your concern here, there's up to 4 possible >> values from 0x38-0x3f which has more than 4 possible values. > > Aren't the addresses going to be incremented by one (up to 8 of devices in total)? With your style of replies, it looks like you wrote it... > > No, the i2c address order is not always monotonic increase or decrease, sometime it would be disorder, according to the application. > Each device would have eight possible i2c address, the final address depends on the hardware connections. OK, the question about the broadcast is still there - cannot it be deduced? Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/sound/ti,tas2781.yaml b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml new file mode 100644 index 000000000000..96c2584855d4 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml @@ -0,0 +1,90 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright (C) 2022 - 2023 Texas Instruments Incorporated +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/ti,tas2781.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Texas Instruments TAS2781 SmartAMP + +maintainers: + - Shenghao Ding <shenghao-ding@ti.com> + +description: + The TAS2781 is a mono, digital input Class-D audio amplifier + optimized for efficiently driving high peak power into small + loudspeakers. Integrated an on-chip DSP supports Texas Instruments + Smart Amp speaker protection algorithm. The integrated speaker + voltage and current sense provides for real time + monitoring of loudspeaker behavior. + +properties: + compatible: + enum: + - ti,tas2781 + + reg: + description: I2C address of the primary device. + items: + minimum: 0x38 + maximum: 0x3f + + reset-gpios: + maxItems: 1 + + interrupts: + maxItems: 1 + + ti,audio-slots: + description: + Multiple tas2781s aggregate as one Audio Amp to support + multiple audio slots + $ref: /schemas/types.yaml#/definitions/uint32-array + maxItems: 4 + items: + minimum: 0x38 + maximum: 0x3f + description: + I2C address of the device for different audio slots, + useless in mono case. + + ti,broadcast-addr: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Generic I2C address for all the tas2781 devices in + purpose of I2C broadcast during the multi-device + writes, useless in mono case. + + '#sound-dai-cells': + const: 1 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + i2c { + /* example with quad support, such as tablet or pad device */ + #address-cells = <1>; + #size-cells = <0>; + quad: codec@38 { + compatible = "ti,tas2781"; + reg = <0x38>; + #sound-dai-cells = <1>; + reset-gpios = < &gpio1 10 GPIO_ACTIVE_HIGH >; + interrupt-parent = <&gpio1>; + interrupts = <15>; + ti,audio-slots = < 0x38 /* Audio slot 0 */ + 0x3a /* Audio slot 1 */ + 0x39 /* Audio slot 2 */ + 0x3b /* Audio slot 3 */ + >; + /* Generic I2C addr among all the tas2781 for i2c data broadcast */ + ti,broadcast-addr = <0x40>; + }; + }; +...
Create tas2781.yaml for tas2781 driver. Signed-off-by: Shenghao Ding <13916275206@139.com> --- Changes in v7: - Submit together with tas2781 codec driver code - Add more detail description for ti,audio-slots - Keep consistent for "I2C" - remove reset-gpios description - For reg, express as constraints instead - remove unnecessary '|' Changes to be committed: new file: Documentation/devicetree/bindings/sound/ti,tas2781.yaml --- .../devicetree/bindings/sound/ti,tas2781.yaml | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/ti,tas2781.yaml