[PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
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
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>; + }; + }; +...
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/2023050805451...
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 08:26, Rob Herring wrote:
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
Ignore failures.
Best regards, Krzysztof
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: 0x38maximum: 0x3f- reset-gpios:
- maxItems: 1
- interrupts:
- maxItems: 1
- ti,audio-slots:
- description:
Multiple tas2781s aggregate as one Audio Amp to supportmultiple 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: 0x38maximum: 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 inpurpose of I2C broadcast during the multi-devicewrites, 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
Answer· embeded
-----Original Message----- From: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org Sent: Wednesday, May 10, 2023 3:04 PM To: Shenghao Ding 13916275206@139.com; broonie@kernel.org; devicetree@vger.kernel.org; krzysztof.kozlowski+dt@linaro.org; robh+dt@kernel.org; lgirdwood@gmail.com; perex@perex.cz; pierre-louis.bossart@linux.intel.com Cc: Lu, Kevin kevin-lu@ti.com; Ding, Shenghao shenghao-ding@ti.com; alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org; Xu, Baojun x1077012@ti.com; Gupta, Peeyush peeyush@ti.com; Navada Kanyana, Mukund navada@ti.com; gentuser@gmail.com; Ryan_Chu@wistron.com; Sam_Wu@wistron.com Subject: [EXTERNAL] Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
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. [Ding] v2 is the version no for all of the pathes, in each patch there is separated version, v7 is the version number for yaml
- 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. [Ding] Add it in next patch
+properties:
- compatible:
- enum:
- ti,tas2781- reg:
- description: I2C address of the primary device.
- items:
minimum: 0x38maximum: 0x3f- reset-gpios:
- maxItems: 1
- interrupts:
- maxItems: 1
- ti,audio-slots:
- description:
Multiple tas2781s aggregate as one Audio Amp to supportmultiple 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: 0x38maximum: 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.
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 inpurpose of I2C broadcast during the multi-devicewrites, 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
- '#sound-dai-cells':
- const: 1
+required:
- compatible
- reg
Best regards, Krzysztof
On 10/05/2023 16:03, Ding, Shenghao wrote:
- items:
minimum: 0x38maximum: 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 inpurpose of I2C broadcast during the multi-devicewrites, 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 Wed, May 10, 2023 at 09:04:26AM +0200, Krzysztof Kozlowski wrote:
On 08/05/2023 07:45, Shenghao Ding wrote:
- Submit together with tas2781 codec driver code
Fix your patchset threading. I don't think it's possible to apply your one.
While it would be nice if things were fixed (if git send-email is being used then passing all the patches to one git send-email command should do the trick) it's not an insurmountable obstacle.
- ti,audio-slots:
- description:
Multiple tas2781s aggregate as one Audio Amp to supportmultiple 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: 0x38maximum: 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.
On 11/05/2023 07:49, Mark Brown wrote:
Missing minItems, but...
- items:
minimum: 0x38maximum: 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: 0x38maximum: 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: 0x38maximum: 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
On 11/05/2023 15:19, Ding, Shenghao wrote:
On 11/05/2023 07:49, Mark Brown wrote:
Missing minItems, but...
- items:
minimum: 0x38maximum: 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...
All the addresses of tas2781 are in range from 0x38 to 0x 3f, the order of them in the audio-slots item are up to the hardware connections. I have studied the reg item to save multiple i2c addresses for multiple pieces of tas2781 and found that "'#address-cells': maximum: 3" that means "reg" store not more than three addresses, this can't support the more than 3 pieces of tas2781, such as 4-slot TDM case or multiple dual-membrane speakers case, in such a case, one speaker will use two pieces of tas2781 to boost, usually at least 6 pieces of tas2781 will be used in a laptop or other device.
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?
The reason to define this item and add it in dts is that tell tas2781 driver code to enable broadcast and its address. Removing this item means disabling broadcast. Do you want to hardcode the global address in the code? And this item only used as a flag to enable or disable?
Best regards, Krzysztof
On 17/05/2023 14:24, Ding, Shenghao wrote:
On 11/05/2023 15:19, Ding, Shenghao wrote:
On 11/05/2023 07:49, Mark Brown wrote:
Missing minItems, but...
- items:
minimum: 0x38maximum: 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...
All the addresses of tas2781 are in range from 0x38 to 0x 3f, the order of them in the audio-slots item are up to the hardware connections. I have studied the reg item to save multiple i2c addresses for multiple pieces of tas2781 and found that "'#address-cells': maximum: 3" that means "reg" store not more than three addresses, this can't support the more than 3 pieces of tas2781,
No entirely. This determines the size of each address, not the number of addresses.
such as 4-slot TDM case or multiple dual-membrane speakers case, in such a case, one speaker will use two pieces of tas2781 to boost, usually at least 6 pieces of tas2781 will be used in a laptop or other device.
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?
The reason to define this item and add it in dts is that tell tas2781 driver code to enable broadcast and its address. Removing this item means disabling broadcast. Do you want to hardcode the global address in the code? And this item only used as a flag to enable or disable?
Best regards, Krzysztof
Hi Krzysztof, nice to talk with you!
I really do not know what is here mine what's yours. I could guess, but we are all a bit busy, so I would appreciate if reading your email was easier for me.
Best regards, Krzysztof
participants (5)
-
Ding, Shenghao -
Krzysztof Kozlowski -
Mark Brown -
Rob Herring -
Shenghao Ding