diff mbox series

[V2,1/5] ASoC: dt-bindings: Adds properties to "awinic,aw88395"

Message ID 20230912065852.347000-2-wangweidong.a@awinic.com (mailing list archive)
State Superseded
Headers show
Series ASoC: codecs: Add aw87390 amplifier driver | expand

Commit Message

wangweidong.a@awinic.com Sept. 12, 2023, 6:58 a.m. UTC
From: Weidong Wang <wangweidong.a@awinic.com>

Adds properties to "awinic,aw88395" to make files more complete

Signed-off-by: Weidong Wang <wangweidong.a@awinic.com>
---
 .../bindings/sound/awinic,aw88395.yaml        | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Rob Herring Sept. 12, 2023, 4:21 p.m. UTC | #1
On Tue, Sep 12, 2023 at 02:58:48PM +0800, wangweidong.a@awinic.com wrote:
> From: Weidong Wang <wangweidong.a@awinic.com>

The subject is still pretty generic. Ideally, we'd never have the same 
subject twice. I'd do something like this:

ASoC: dt-bindings: awinic,aw88395: Add properties for multiple PA support

> 
> Adds properties to "awinic,aw88395" to make files more complete

Complete in what way? multiple PAs, right. It's not really clear 
to me though how these properties enable support for multiple PAs.

> 
> Signed-off-by: Weidong Wang <wangweidong.a@awinic.com>
> ---
>  .../bindings/sound/awinic,aw88395.yaml        | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
> index 4051c2538caf..4965aa4a5370 100644
> --- a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
> +++ b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
> @@ -32,11 +32,28 @@ properties:
>    reset-gpios:
>      maxItems: 1
>  
> +  awinic,audio-channel:
> +    description:
> +      It is used to distinguish multiple PA devices, so that different
> +      configurations can be loaded to different PA devices
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 7
> +
> +  awinic,sync-flag:
> +    description:
> +      Flag bit used to keep the phase synchronized in the case of multiple PA
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 1
> +
>  required:
>    - compatible
>    - reg
>    - '#sound-dai-cells'
>    - reset-gpios
> +  - awinic,audio-channel
> +  - awinic,sync-flag
>  
>  unevaluatedProperties: false
>  
> @@ -51,5 +68,7 @@ examples:
>              reg = <0x34>;
>              #sound-dai-cells = <0>;
>              reset-gpios = <&gpio 10 GPIO_ACTIVE_LOW>;
> +            awinic,audio-channel = <0>;
> +            awinic,sync-flag = <0>;
>          };
>      };
> -- 
> 2.41.0
>
Krzysztof Kozlowski Sept. 13, 2023, 7:02 a.m. UTC | #2
On 12/09/2023 08:58, wangweidong.a@awinic.com wrote:
> From: Weidong Wang <wangweidong.a@awinic.com>
> 
> Adds properties to "awinic,aw88395" to make files more complete
> 
> Signed-off-by: Weidong Wang <wangweidong.a@awinic.com>
> ---
>  .../bindings/sound/awinic,aw88395.yaml        | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
> index 4051c2538caf..4965aa4a5370 100644
> --- a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
> +++ b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
> @@ -32,11 +32,28 @@ properties:
>    reset-gpios:
>      maxItems: 1
>  
> +  awinic,audio-channel:
> +    description:
> +      It is used to distinguish multiple PA devices, so that different
> +      configurations can be loaded to different PA devices
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 7
> +
> +  awinic,sync-flag:
> +    description:
> +      Flag bit used to keep the phase synchronized in the case of multiple PA
> +    $ref: /schemas/types.yaml#/definitions/uint32

Looks like bool, not uint32. If you made it uint32 for some future
purpose, then the name "flag" is misleading and anyway what would be the
third option here for sync?

Best regards,
Krzysztof
wangweidong.a@awinic.com Sept. 13, 2023, 8:03 a.m. UTC | #3
Thank you very much for your review, but I have a question that I would like to consult

On 13/09/2023 09:03, krzysztof.kozlowski@linaro.org wrote:
> On 12/09/2023 08:58, wangweidong.a@awinic.com wrote:
>> From: Weidong Wang <wangweidong.a@awinic.com>
>> 
>> Adds properties to "awinic,aw88395" to make files more complete
>> 
>> Signed-off-by: Weidong Wang <wangweidong.a@awinic.com>
>> ---
>>  .../bindings/sound/awinic,aw88395.yaml        | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
>> index 4051c2538caf..4965aa4a5370 100644
>> --- a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
>> +++ b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
>> @@ -32,11 +32,28 @@ properties:
>>    reset-gpios:
>>     maxItems: 1
>>  
>> +  awinic,audio-channel:
>> +    description:
>> +      It is used to distinguish multiple PA devices, so that different
>> +      configurations can be loaded to different PA devices
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 7
>> +
>> +  awinic,sync-flag:
>> +    description:
>> +      Flag bit used to keep the phase synchronized in the case of multiple PA
>> +    $ref: /schemas/types.yaml#/definitions/uint32

> Looks like bool, not uint32. If you made it uint32 for some future
> purpose, then the name "flag" is misleading and anyway what would be the
> third option here for sync?

Thank you very much. Can I Change it to "$ref: /schemas/types.yaml#/definitions/bool"?

Best regards,
Weidong Wang
wangweidong.a@awinic.com Sept. 13, 2023, 8:06 a.m. UTC | #4
Thank you very much for your advice, and I will modify it in the next patch.

On Tue, Sep 12, 2023 at 11:21:22PM -0500, robh@kernel.org wrote:
> On Tue, Sep 12, 2023 at 02:58:48PM +0800, wangweidong.a@awinic.com wrote:
>> From: Weidong Wang <wangweidong.a@awinic.com>

> The subject is still pretty generic. Ideally, we'd never have the same 
> subject twice. I'd do something like this:

> ASoC: dt-bindings: awinic,aw88395: Add properties for multiple PA support

Thank you very much. I will modify according to your suggestion in the next patch

>> 
>> Adds properties to "awinic,aw88395" to make files more complete

> Complete in what way? multiple PAs, right. It's not really clear 
> to me though how these properties enable support for multiple PAs.

Thank you very much. I will change the following content in the next patch:
Add two properties, the "awinic,audio-channel" property and the
"awinic,sync-flag". The "awinic,audio-channel" is used to make 
different PA load different configurations, the "awinic,sync-flag"
is used to synchronize the phases of multiple PA. These two properties 
will be read by the corresponding driver, so that the multi-PA to 
achieve better playback effect.

>> 
>> Signed-off-by: Weidong Wang <wangweidong.a@awinic.com>
>> ---
>>  .../bindings/sound/awinic,aw88395.yaml        | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
>> index 4051c2538caf..4965aa4a5370 100644
>> --- a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
>> +++ b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
>> @@ -32,11 +32,28 @@ properties:
>>    reset-gpios:
>>      maxItems: 1
>>  
>> +  awinic,audio-channel:
>> +    description:
>> +      It is used to distinguish multiple PA devices, so that different
>> +      configurations can be loaded to different PA devices
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 7
>> +
>> +  awinic,sync-flag:
>> +    description:
>> +      Flag bit used to keep the phase synchronized in the case of multiple PA
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 1
>> +
>>  required:
>>    - compatible
>>    - reg
>>    - '#sound-dai-cells'
>>    - reset-gpios
>> +  - awinic,audio-channel
>> +  - awinic,sync-flag
>>  
>>  unevaluatedProperties: false
>>  
>> @@ -51,5 +68,7 @@ examples:
>>              reg = <0x34>;
>>              #sound-dai-cells = <0>;
>>              reset-gpios = <&gpio 10 GPIO_ACTIVE_LOW>;
>> +            awinic,audio-channel = <0>;
>> +            awinic,sync-flag = <0>;
>>          };
>>      };
>> -- 
>> 2.41.0
>>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
index 4051c2538caf..4965aa4a5370 100644
--- a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
+++ b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml
@@ -32,11 +32,28 @@  properties:
   reset-gpios:
     maxItems: 1
 
+  awinic,audio-channel:
+    description:
+      It is used to distinguish multiple PA devices, so that different
+      configurations can be loaded to different PA devices
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 7
+
+  awinic,sync-flag:
+    description:
+      Flag bit used to keep the phase synchronized in the case of multiple PA
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 1
+
 required:
   - compatible
   - reg
   - '#sound-dai-cells'
   - reset-gpios
+  - awinic,audio-channel
+  - awinic,sync-flag
 
 unevaluatedProperties: false
 
@@ -51,5 +68,7 @@  examples:
             reg = <0x34>;
             #sound-dai-cells = <0>;
             reset-gpios = <&gpio 10 GPIO_ACTIVE_LOW>;
+            awinic,audio-channel = <0>;
+            awinic,sync-flag = <0>;
         };
     };