Re: [PATCH v1] Asoc: dt_bindings: Add tas2781 yaml

[Krzysztof Kozlowski]

On 15/01/2023 13:16, Kevin Lu wrote:
> Complete the DTS for tas2781

1. This is not a v1 but v2.
2. Subject - still wrong.
3. Commit msg - does not make sense to me. I don't understand it.
4. Other comments - also not implemented.

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.



> 
> Signed-off-by: Kevin Lu <luminlong@xxxxxxx>
> ---
>  .../devicetree/bindings/sound/ti,tas2781.yaml | 122 ++++++++++++++++++
>  1 file changed, 122 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 0000000..7d73f46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/ti,tas2781.yaml
> @@ -0,0 +1,122 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 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 Smart PA
> +
> +maintainers:
> +  - Shenghao Ding <shenghao-ding@xxxxxx>
> +  - Kevin Lu <kevin-lu@xxxxxx>
> +
> +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:
> +    maxItems: 1
> +    description: |
> +      I2C address of the device can be in range from 0x38 to 0x40.
> +
> +  ti,audio-slots:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 4
> +    description: |
> +      This item is used to store the i2c address of the device
> +      for deifferent audio slots. It is not required for Mono case.
> +
> +  ti,global-addr:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      This item is used to store the generic i2c address of
> +      all the tas2781 devices for I2C broadcast during the multi-device
> +      writes, useless in mono case.
> +
> +  ti,reset-gpios:

I asked you to use existing property. Drop prefix.

> +    minItems: 1
> +    maxItems: 4
> +    description: GPIO specifier for the reset pin.
> +
> +  ti,irq-gpio:
> +    maxItems: 1
> +    description: GPIO used to interrupts the device.

So you ignored around 3 or four my comments. I'll stop the review - it
does not make sense.

NAK - this is not correct property. Implement the feedback.

Best regards,
Krzysztof