* [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
@ 2023-05-08 5:45 Shenghao Ding
2023-05-08 6:26 ` Rob Herring
2023-05-10 7:04 ` Krzysztof Kozlowski
0 siblings, 2 replies; 12+ messages in thread
From: Shenghao Ding @ 2023-05-08 5:45 UTC (permalink / raw)
To: broonie, devicetree, krzysztof.kozlowski+dt, robh+dt, lgirdwood,
perex, pierre-louis.bossart
Cc: kevin-lu, shenghao-ding, alsa-devel, linux-kernel, x1077012,
peeyush, navada, gentuser, Ryan_Chu, Sam_Wu, Shenghao Ding
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>;
+ };
+ };
+...
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
2023-05-08 5:45 [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier Shenghao Ding
@ 2023-05-08 6:26 ` Rob Herring
2023-05-08 9:32 ` Krzysztof Kozlowski
2023-05-10 7:04 ` Krzysztof Kozlowski
1 sibling, 1 reply; 12+ messages in thread
From: Rob Herring @ 2023-05-08 6:26 UTC (permalink / raw)
To: Shenghao Ding
Cc: Ryan_Chu, navada, gentuser, alsa-devel, linux-kernel, robh+dt,
shenghao-ding, Sam_Wu, lgirdwood, krzysztof.kozlowski+dt,
x1077012, broonie, kevin-lu, perex, devicetree,
pierre-louis.bossart, peeyush
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
2023-05-08 6:26 ` Rob Herring
@ 2023-05-08 9:32 ` Krzysztof Kozlowski
0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-08 9:32 UTC (permalink / raw)
To: Rob Herring, Shenghao Ding
Cc: Ryan_Chu, navada, gentuser, alsa-devel, linux-kernel, robh+dt,
shenghao-ding, Sam_Wu, lgirdwood, krzysztof.kozlowski+dt,
x1077012, broonie, kevin-lu, perex, devicetree,
pierre-louis.bossart, peeyush
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
2023-05-08 5:45 [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier Shenghao Ding
2023-05-08 6:26 ` Rob Herring
@ 2023-05-10 7:04 ` Krzysztof Kozlowski
2023-05-10 14:03 ` [EXTERNAL] " Ding, Shenghao
2023-05-11 5:49 ` Mark Brown
1 sibling, 2 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-10 7:04 UTC (permalink / raw)
To: Shenghao Ding, broonie, devicetree, krzysztof.kozlowski+dt,
robh+dt, lgirdwood, perex, pierre-louis.bossart
Cc: kevin-lu, shenghao-ding, alsa-devel, linux-kernel, x1077012,
peeyush, navada, gentuser, Ryan_Chu, Sam_Wu
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [EXTERNAL] Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
2023-05-10 7:04 ` Krzysztof Kozlowski
@ 2023-05-10 14:03 ` Ding, Shenghao
2023-05-11 9:01 ` Krzysztof Kozlowski
2023-05-11 5:49 ` Mark Brown
1 sibling, 1 reply; 12+ messages in thread
From: Ding, Shenghao @ 2023-05-10 14:03 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lu, Kevin, alsa-devel@alsa-project.org,
linux-kernel@vger.kernel.org, Xu, Baojun, Gupta, Peeyush,
Navada Kanyana, Mukund, gentuser@gmail.com, Ryan_Chu@wistron.com,
Sam_Wu@wistron.com, Shenghao Ding, 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
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: 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.
[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 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
> +
> + '#sound-dai-cells':
> + const: 1
> +
> +required:
> + - compatible
> + - reg
> +
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
2023-05-10 7:04 ` Krzysztof Kozlowski
2023-05-10 14:03 ` [EXTERNAL] " Ding, Shenghao
@ 2023-05-11 5:49 ` Mark Brown
2023-05-11 9:37 ` Krzysztof Kozlowski
1 sibling, 1 reply; 12+ messages in thread
From: Mark Brown @ 2023-05-11 5:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Shenghao Ding, devicetree, krzysztof.kozlowski+dt, robh+dt,
lgirdwood, perex, pierre-louis.bossart, kevin-lu, shenghao-ding,
alsa-devel, linux-kernel, x1077012, peeyush, navada, gentuser,
Ryan_Chu, Sam_Wu
[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]
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 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...
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
2023-05-10 14:03 ` [EXTERNAL] " Ding, Shenghao
@ 2023-05-11 9:01 ` Krzysztof Kozlowski
0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-11 9:01 UTC (permalink / raw)
To: Ding, Shenghao
Cc: Lu, Kevin, alsa-devel@alsa-project.org,
linux-kernel@vger.kernel.org, Xu, Baojun, Gupta, Peeyush,
Navada Kanyana, Mukund, gentuser@gmail.com, Ryan_Chu@wistron.com,
Sam_Wu@wistron.com, Shenghao Ding, 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
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
2023-05-11 5:49 ` Mark Brown
@ 2023-05-11 9:37 ` Krzysztof Kozlowski
2023-05-11 13:19 ` [EXTERNAL] " Ding, Shenghao
0 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-11 9:37 UTC (permalink / raw)
To: Mark Brown
Cc: Shenghao Ding, devicetree, krzysztof.kozlowski+dt, robh+dt,
lgirdwood, perex, pierre-louis.bossart, kevin-lu, shenghao-ding,
alsa-devel, linux-kernel, x1077012, peeyush, navada, gentuser,
Ryan_Chu, Sam_Wu
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [EXTERNAL] Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
2023-05-11 9:37 ` Krzysztof Kozlowski
@ 2023-05-11 13:19 ` Ding, Shenghao
2023-05-11 14:48 ` Krzysztof Kozlowski
0 siblings, 1 reply; 12+ messages in thread
From: Ding, Shenghao @ 2023-05-11 13:19 UTC (permalink / raw)
To: Krzysztof Kozlowski, Mark Brown
Cc: Shenghao Ding, 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, Lu, Kevin,
alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
Xu, Baojun, Gupta, Peeyush, Navada Kanyana, Mukund,
gentuser@gmail.com, Ryan_Chu@wistron.com, Sam_Wu@wistron.com
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
2023-05-11 13:19 ` [EXTERNAL] " Ding, Shenghao
@ 2023-05-11 14:48 ` Krzysztof Kozlowski
2023-05-17 12:24 ` Ding, Shenghao
0 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-11 14:48 UTC (permalink / raw)
To: Ding, Shenghao, Mark Brown
Cc: Shenghao Ding, 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, Lu, Kevin,
alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
Xu, Baojun, Gupta, Peeyush, Navada Kanyana, Mukund,
gentuser@gmail.com, Ryan_Chu@wistron.com, Sam_Wu@wistron.com
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [EXTERNAL] Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
2023-05-11 14:48 ` Krzysztof Kozlowski
@ 2023-05-17 12:24 ` Ding, Shenghao
2023-05-17 15:05 ` Krzysztof Kozlowski
0 siblings, 1 reply; 12+ messages in thread
From: Ding, Shenghao @ 2023-05-17 12:24 UTC (permalink / raw)
To: Krzysztof Kozlowski, Mark Brown
Cc: Shenghao Ding, 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, Lu, Kevin,
alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
Xu, Baojun, Gupta, Peeyush, Navada Kanyana, Mukund,
gentuser@gmail.com, Ryan_Chu@wistron.com, Sam_Wu@wistron.com
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...
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier
2023-05-17 12:24 ` Ding, Shenghao
@ 2023-05-17 15:05 ` Krzysztof Kozlowski
0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-17 15:05 UTC (permalink / raw)
To: Ding, Shenghao, Mark Brown
Cc: Shenghao Ding, 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, Lu, Kevin,
alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
Xu, Baojun, Gupta, Peeyush, Navada Kanyana, Mukund,
gentuser@gmail.com, Ryan_Chu@wistron.com, Sam_Wu@wistron.com
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: 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...
>
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-05-17 15:06 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-08 5:45 [PATCH v2 2/5] ASoC: dt-bindings: Add tas2781 amplifier Shenghao Ding
2023-05-08 6:26 ` Rob Herring
2023-05-08 9:32 ` Krzysztof Kozlowski
2023-05-10 7:04 ` Krzysztof Kozlowski
2023-05-10 14:03 ` [EXTERNAL] " Ding, Shenghao
2023-05-11 9:01 ` Krzysztof Kozlowski
2023-05-11 5:49 ` Mark Brown
2023-05-11 9:37 ` Krzysztof Kozlowski
2023-05-11 13:19 ` [EXTERNAL] " Ding, Shenghao
2023-05-11 14:48 ` Krzysztof Kozlowski
2023-05-17 12:24 ` Ding, Shenghao
2023-05-17 15:05 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).