diff mbox series

[v7,2/3] ASoC: dt-bindings: mt6358: Convert to DT Schema

Message ID 20240930073311.1486-2-macpaul.lin@mediatek.com (mailing list archive)
State New, archived
Headers show
Series [v7,1/3] regulator: dt-bindings: mt6323: Convert to DT schema | expand

Commit Message

Macpaul Lin Sept. 30, 2024, 7:33 a.m. UTC
Convert the MediaTek MT6358 Audio CODEC bindings to DT schema.

This change implements the following updates:
1. Compatible property: Added the const 'mediatek,mt6358-sound'
   to ensure alignment with the schema in the actual (DTS) file
   "mt8186-corsola.dtsi" with 'mediatek,mt6366-sound'.
2. Example: Removed the example section, as it should be relocated to
   the MT6397 PMIC file 'mfd/mediatek,mt6397.yaml'.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
 .../devicetree/bindings/sound/mt6358.txt      | 26 ------------
 .../devicetree/bindings/sound/mt6358.yaml     | 41 +++++++++++++++++++
 2 files changed, 41 insertions(+), 26 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/sound/mt6358.txt
 create mode 100644 Documentation/devicetree/bindings/sound/mt6358.yaml

Changess for v1~v6:
 - It was not in the origin patch set.

Changes for v7:
 - It is seperated from the origin patch set (2/2, v6).
   It could simplifies the audio-codec node in next patch (3/3, v7)
   of 'mfd/mediatek,mt6397.yaml'

Comments

Krzysztof Kozlowski Oct. 1, 2024, 6:34 a.m. UTC | #1
On Mon, Sep 30, 2024 at 03:33:10PM +0800, Macpaul Lin wrote:
> Convert the MediaTek MT6358 Audio CODEC bindings to DT schema.
> 
> This change implements the following updates:
> 1. Compatible property: Added the const 'mediatek,mt6358-sound'
>    to ensure alignment with the schema in the actual (DTS) file
>    "mt8186-corsola.dtsi" with 'mediatek,mt6366-sound'.
> 2. Example: Removed the example section, as it should be relocated to
>    the MT6397 PMIC file 'mfd/mediatek,mt6397.yaml'.
> 
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> ---
>  .../devicetree/bindings/sound/mt6358.txt      | 26 ------------
>  .../devicetree/bindings/sound/mt6358.yaml     | 41 +++++++++++++++++++
>  2 files changed, 41 insertions(+), 26 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/sound/mt6358.txt
>  create mode 100644 Documentation/devicetree/bindings/sound/mt6358.yaml
> 
> Changess for v1~v6:
>  - It was not in the origin patch set.
> 
> Changes for v7:
>  - It is seperated from the origin patch set (2/2, v6).
>    It could simplifies the audio-codec node in next patch (3/3, v7)
>    of 'mfd/mediatek,mt6397.yaml'
> 
> diff --git a/Documentation/devicetree/bindings/sound/mt6358.txt b/Documentation/devicetree/bindings/sound/mt6358.txt
> deleted file mode 100644
> index fbe9e55c68f5..000000000000
> --- a/Documentation/devicetree/bindings/sound/mt6358.txt
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -Mediatek MT6358 Audio Codec
> -
> -The communication between MT6358 and SoC is through Mediatek PMIC wrapper.
> -For more detail, please visit Mediatek PMIC wrapper documentation.
> -
> -Must be a child node of PMIC wrapper.
> -
> -Required properties:
> -
> -- compatible - "string" - One of:
> -    "mediatek,mt6358-sound"
> -    "mediatek,mt6366-sound"
> -- Avdd-supply : power source of AVDD
> -
> -Optional properties:
> -- mediatek,dmic-mode : Indicates how many data pins are used to transmit two
> -	channels of PDM signal. 0 means two wires, 1 means one wire. Default
> -	value is 0.
> -
> -Example:
> -
> -mt6358_snd {
> -	compatible = "mediatek,mt6358-sound";
> -	Avdd-supply = <&mt6358_vaud28_reg>;
> -	mediatek,dmic-mode = <0>;
> -};
> diff --git a/Documentation/devicetree/bindings/sound/mt6358.yaml b/Documentation/devicetree/bindings/sound/mt6358.yaml
> new file mode 100644
> index 000000000000..5f7611e26a89
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mt6358.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/mt6358.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mediatek MT6358 Audio Codec
> +
> +maintainers:
> +  - Jiaxin Yu <jiaxin.yu@mediatek.com>
> +  - Macpaul Lin <macpaul.lin@mediatek.com>
> +
> +description: |
> +  The communication between MT6358 and SoC is through Mediatek PMIC wrapper.
> +  For more detail, please visit Mediatek PMIC wrapper documentation.
> +  Must be a child node of PMIC wrapper.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mediatek,mt6366-sound
> +      - mediatek,mt6358-sound
> +    const: mediatek,mt6358-sound

This wasn't ever tested.

Do not send untested code, it's a waste of reviewers' time.

Best regards,
Krzysztof
Macpaul Lin Oct. 1, 2024, 7:05 a.m. UTC | #2
On 10/1/24 14:34, Krzysztof Kozlowski wrote:

[snip]

>> +description: |
>> +  The communication between MT6358 and SoC is through Mediatek PMIC wrapper.
>> +  For more detail, please visit Mediatek PMIC wrapper documentation.
>> +  Must be a child node of PMIC wrapper.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - mediatek,mt6366-sound
>> +      - mediatek,mt6358-sound
>> +    const: mediatek,mt6358-sound
> 
> This wasn't ever tested.

Hum, I have indeed tested it with linux-next/master branch.
Ran dt_binding_check with dtschema trunk with this single file
but didn't get any warning or errors.
'make dt_binding_check DT_SCHEMA_FILES=mt6358.yaml'

Could you please help to paste the error log for me?
If there are new errors, I need to check if there is any
environment issue.

> Do not send untested code, it's a waste of reviewers' time.
> 
> Best regards,
> Krzysztof
> 
> 
Thanks
Macpaul Lin
Macpaul Lin Oct. 1, 2024, 7:50 a.m. UTC | #3
On 10/1/24 15:05, Macpaul Lin wrote:
> On 10/1/24 14:34, Krzysztof Kozlowski wrote:
> 
> [snip]
> 
>>> +description: |
>>> +  The communication between MT6358 and SoC is through Mediatek PMIC 
>>> wrapper.
>>> +  For more detail, please visit Mediatek PMIC wrapper documentation.
>>> +  Must be a child node of PMIC wrapper.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - mediatek,mt6366-sound
>>> +      - mediatek,mt6358-sound
>>> +    const: mediatek,mt6358-sound
>>
>> This wasn't ever tested.
> 
> Hum, I have indeed tested it with linux-next/master branch.
> Ran dt_binding_check with dtschema trunk with this single file
> but didn't get any warning or errors.
> 'make dt_binding_check DT_SCHEMA_FILES=mt6358.yaml'
> 
> Could you please help to paste the error log for me?
> If there are new errors, I need to check if there is any
> environment issue.

I've both tested both of the following format pass dt_binding_check.
#1.
properties:
   compatible:
     items:
       - enum:
           - mediatek,mt6366-sound
           - mediatek,mt6358-sound
       - const: mediatek,mt6358-sound

#2.
properties:
   compatible:
     enum:
       - mediatek,mt6366-sound
       - mediatek,mt6358-sound
     const: mediatek,mt6358-sound

>> Do not send untested code, it's a waste of reviewers' time.
>>
>> Best regards,
>> Krzysztof
>>
>>
> Thanks
> Macpaul Lin

Should I update it with format #1?

Thanks
Macpaul Lin
Krzysztof Kozlowski Oct. 1, 2024, 8:56 a.m. UTC | #4
On 01/10/2024 09:05, Macpaul Lin wrote:
> On 10/1/24 14:34, Krzysztof Kozlowski wrote:
> 
> [snip]
> 
>>> +description: |
>>> +  The communication between MT6358 and SoC is through Mediatek PMIC wrapper.
>>> +  For more detail, please visit Mediatek PMIC wrapper documentation.
>>> +  Must be a child node of PMIC wrapper.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - mediatek,mt6366-sound
>>> +      - mediatek,mt6358-sound
>>> +    const: mediatek,mt6358-sound
>>
>> This wasn't ever tested.
> 
> Hum, I have indeed tested it with linux-next/master branch.
> Ran dt_binding_check with dtschema trunk with this single file
> but didn't get any warning or errors.
> 'make dt_binding_check DT_SCHEMA_FILES=mt6358.yaml'
> 
> Could you please help to paste the error log for me?
> If there are new errors, I need to check if there is any
> environment issue.

Hm, I don't know, indeed Rob's does not report failure here, so my
comment was not right. Syntax is unexpected or wrong - const with enum.
Is this a list? If list then you duplicated compatible. Then maybe
implicit oneOf?

BTW, filenames are expected to match compatible.

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 1, 2024, 9:19 a.m. UTC | #5
On 01/10/2024 09:50, Macpaul Lin wrote:
> 
> 
> On 10/1/24 15:05, Macpaul Lin wrote:
>> On 10/1/24 14:34, Krzysztof Kozlowski wrote:
>>
>> [snip]
>>
>>>> +description: |
>>>> +  The communication between MT6358 and SoC is through Mediatek PMIC 
>>>> wrapper.
>>>> +  For more detail, please visit Mediatek PMIC wrapper documentation.
>>>> +  Must be a child node of PMIC wrapper.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - mediatek,mt6366-sound
>>>> +      - mediatek,mt6358-sound
>>>> +    const: mediatek,mt6358-sound
>>>
>>> This wasn't ever tested.
>>
>> Hum, I have indeed tested it with linux-next/master branch.
>> Ran dt_binding_check with dtschema trunk with this single file
>> but didn't get any warning or errors.
>> 'make dt_binding_check DT_SCHEMA_FILES=mt6358.yaml'
>>
>> Could you please help to paste the error log for me?
>> If there are new errors, I need to check if there is any
>> environment issue.
> 
> I've both tested both of the following format pass dt_binding_check.
> #1.
> properties:
>    compatible:
>      items:
>        - enum:
>            - mediatek,mt6366-sound
>            - mediatek,mt6358-sound
>        - const: mediatek,mt6358-sound

I don't understand what you wanted here. neither oneOf nor ietms make
any sense. Why "mediatek,mt6358-sound", "mediatek,mt6358-sound" is a
correct compatible?

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/mt6358.txt b/Documentation/devicetree/bindings/sound/mt6358.txt
deleted file mode 100644
index fbe9e55c68f5..000000000000
--- a/Documentation/devicetree/bindings/sound/mt6358.txt
+++ /dev/null
@@ -1,26 +0,0 @@ 
-Mediatek MT6358 Audio Codec
-
-The communication between MT6358 and SoC is through Mediatek PMIC wrapper.
-For more detail, please visit Mediatek PMIC wrapper documentation.
-
-Must be a child node of PMIC wrapper.
-
-Required properties:
-
-- compatible - "string" - One of:
-    "mediatek,mt6358-sound"
-    "mediatek,mt6366-sound"
-- Avdd-supply : power source of AVDD
-
-Optional properties:
-- mediatek,dmic-mode : Indicates how many data pins are used to transmit two
-	channels of PDM signal. 0 means two wires, 1 means one wire. Default
-	value is 0.
-
-Example:
-
-mt6358_snd {
-	compatible = "mediatek,mt6358-sound";
-	Avdd-supply = <&mt6358_vaud28_reg>;
-	mediatek,dmic-mode = <0>;
-};
diff --git a/Documentation/devicetree/bindings/sound/mt6358.yaml b/Documentation/devicetree/bindings/sound/mt6358.yaml
new file mode 100644
index 000000000000..5f7611e26a89
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mt6358.yaml
@@ -0,0 +1,41 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/mt6358.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek MT6358 Audio Codec
+
+maintainers:
+  - Jiaxin Yu <jiaxin.yu@mediatek.com>
+  - Macpaul Lin <macpaul.lin@mediatek.com>
+
+description: |
+  The communication between MT6358 and SoC is through Mediatek PMIC wrapper.
+  For more detail, please visit Mediatek PMIC wrapper documentation.
+  Must be a child node of PMIC wrapper.
+
+properties:
+  compatible:
+    enum:
+      - mediatek,mt6366-sound
+      - mediatek,mt6358-sound
+    const: mediatek,mt6358-sound
+
+  Avdd-supply:
+    description: Power source of AVDD
+
+  mediatek,dmic-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Indicates how many data pins are used to transmit two channels of PDM
+      signal. 0 means two wires, 1 means one wire. Default value is 0.
+    enum:
+      - 0 # two wires
+      - 1 # one wire
+
+required:
+  - compatible
+  - Avdd-supply
+
+additionalProperties: false