2024-05-18 08:18:01

by Kartik Agarwala

[permalink] [raw]
Subject: [PATCH] ASoC: dt-bindings: mt6358: Convert to dtschema

Convert Mediatek MT6358 Audio Codec bindings from text to dtschema.

Signed-off-by: Kartik Agarwala <[email protected]>
---
.../bindings/sound/mediatek,mt6358.yaml | 47 +++++++++++++++++++
.../devicetree/bindings/sound/mt6358.txt | 26 ----------
2 files changed, 47 insertions(+), 26 deletions(-)
create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
delete mode 100644 Documentation/devicetree/bindings/sound/mt6358.txt

diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
new file mode 100644
index 000000000..f57ef2aa5
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/mediatek,mt6358.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek MT6358 Audio Codec
+
+maintainers:
+ - Kartik Agarwala <[email protected]>
+
+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,mt6358-sound
+ - mediatek,mt6366-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 # one wire
+ - 1 # two wires
+
+required:
+ - compatible
+ - Avdd-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ mt6358_snd {
+ compatible = "mediatek,mt6358-sound";
+ Avdd-supply = <&mt6358_vaud28_reg>;
+ mediatek,dmic-mode = <0>;
+ };
diff --git a/Documentation/devicetree/bindings/sound/mt6358.txt b/Documentation/devicetree/bindings/sound/mt6358.txt
deleted file mode 100644
index fbe9e55c6..000000000
--- 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>;
-};
--
2.34.1



2024-05-20 07:09:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] ASoC: dt-bindings: mt6358: Convert to dtschema

On 18/05/2024 10:16, Kartik Agarwala wrote:
> Convert Mediatek MT6358 Audio Codec bindings from text to dtschema.
>
> Signed-off-by: Kartik Agarwala <[email protected]>
> ---
> .../bindings/sound/mediatek,mt6358.yaml | 47 +++++++++++++++++++
> .../devicetree/bindings/sound/mt6358.txt | 26 ----------
> 2 files changed, 47 insertions(+), 26 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
> delete mode 100644 Documentation/devicetree/bindings/sound/mt6358.txt
>
> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
> new file mode 100644
> index 000000000..f57ef2aa5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/mediatek,mt6358.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mediatek MT6358 Audio Codec
> +
> +maintainers:
> + - Kartik Agarwala <[email protected]>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> + 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.

Did you update the PMIC wrapper binding with ref to this?

> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt6358-sound
> + - mediatek,mt6366-sound

You did not test the DTS.

I think I raised the issue already: please make necessary fixes to the
binding (with explanation) or to the DTS, when converting the binding.

> +
> + Avdd-supply:
> + description: power source of AVDD
> +
> + mediatek,dmic-mode:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |

Do not need '|' unless you need to preserve formatting.

> + 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 # one wire
> + - 1 # two wires
> +
> +required:
> + - compatible
> + - Avdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + mt6358_snd {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Definitely no underscores. Probably this is "codec" or "audio-codec".

Best regards,
Krzysztof


2024-05-22 21:05:28

by Kartik Agarwala

[permalink] [raw]
Subject: Re: [PATCH] ASoC: dt-bindings: mt6358: Convert to dtschema

On 5/20/24 12:39 PM, Krzysztof Kozlowski wrote:
> On 18/05/2024 10:16, Kartik Agarwala wrote:
>> Convert Mediatek MT6358 Audio Codec bindings from text to dtschema.
>>
>> Signed-off-by: Kartik Agarwala <[email protected]>
>> ---
>> .../bindings/sound/mediatek,mt6358.yaml | 47 +++++++++++++++++++
>> .../devicetree/bindings/sound/mt6358.txt | 26 ----------
>> 2 files changed, 47 insertions(+), 26 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
>> delete mode 100644 Documentation/devicetree/bindings/sound/mt6358.txt
>>
>> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
>> new file mode 100644
>> index 000000000..f57ef2aa5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
>> @@ -0,0 +1,47 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/sound/mediatek,mt6358.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Mediatek MT6358 Audio Codec
>> +
>> +maintainers:
>> + - Kartik Agarwala <[email protected]>
>> +
>> +description: |
>
> Do not need '|' unless you need to preserve formatting.

Noted

>
>> + 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.
>
> Did you update the PMIC wrapper binding with ref to this?

I am sorry but if I understand this comment, you are asking me to update this
file [1], correct?

1. https://www.kernel.org/doc/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt

>
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - mediatek,mt6358-sound
>> + - mediatek,mt6366-sound
>
> You did not test the DTS.
>
> I think I raised the issue already: please make necessary fixes to the
> binding (with explanation) or to the DTS, when converting the binding.
>

Apologies again. Just to be sure, am I correct to assume that you want
me to fix the dts file [1] as it has both these compatibles
mentioned instead of only one and I should fix that by dropping one of
the two compatibles?

[1] https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8186-corsola.dtsi#L1246

>> +
>> + Avdd-supply:
>> + description: power source of AVDD
>> +
>> + mediatek,dmic-mode:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: |
>
> Do not need '|' unless you need to preserve formatting.
>
>> + 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 # one wire
>> + - 1 # two wires
>> +
>> +required:
>> + - compatible
>> + - Avdd-supply
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + mt6358_snd {
>
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
> Definitely no underscores. Probably this is "codec" or "audio-codec".

Noted

>
> Best regards,
> Krzysztof
>

Thanks for the review!

Regards,
Kartik Agarwala

2024-05-23 06:06:02

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] ASoC: dt-bindings: mt6358: Convert to dtschema

On 22/05/2024 23:04, Kartik Agarwala wrote:
> On 5/20/24 12:39 PM, Krzysztof Kozlowski wrote:
>> On 18/05/2024 10:16, Kartik Agarwala wrote:
>>> Convert Mediatek MT6358 Audio Codec bindings from text to dtschema.
>>>
>>> Signed-off-by: Kartik Agarwala <[email protected]>
>>> ---
>>> .../bindings/sound/mediatek,mt6358.yaml | 47 +++++++++++++++++++
>>> .../devicetree/bindings/sound/mt6358.txt | 26 ----------
>>> 2 files changed, 47 insertions(+), 26 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
>>> delete mode 100644 Documentation/devicetree/bindings/sound/mt6358.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
>>> new file mode 100644
>>> index 000000000..f57ef2aa5
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
>>> @@ -0,0 +1,47 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/sound/mediatek,mt6358.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Mediatek MT6358 Audio Codec
>>> +
>>> +maintainers:
>>> + - Kartik Agarwala <[email protected]>
>>> +
>>> +description: |
>>
>> Do not need '|' unless you need to preserve formatting.
>
> Noted
>
>>
>>> + 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.
>>
>> Did you update the PMIC wrapper binding with ref to this?
>
> I am sorry but if I understand this comment, you are asking me to update this
> file [1], correct?
>
> 1. https://www.kernel.org/doc/Documentation/devicetree/bindings/soc/mediatek/pwrap.txt
>
>>
>>> +
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - mediatek,mt6358-sound
>>> + - mediatek,mt6366-sound
>>
>> You did not test the DTS.
>>
>> I think I raised the issue already: please make necessary fixes to the
>> binding (with explanation) or to the DTS, when converting the binding.
>>
>
> Apologies again. Just to be sure, am I correct to assume that you want
> me to fix the dts file [1] as it has both these compatibles
> mentioned instead of only one and I should fix that by dropping one of
> the two compatibles?
>
> [1] https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8186-corsola.dtsi#L1246

No. I want these to be in sync and correct. dtbs_check must pass, which
requires either changing binding or DTS, whichever is the correct action
to do.

https://social.kernel.org/notice/Ai9hYRUKo8suzX3zNY

Best regards,
Krzysztof


2024-06-06 14:42:07

by Kartik Agarwala

[permalink] [raw]
Subject: Re: [PATCH] ASoC: dt-bindings: mt6358: Convert to dtschema

On 5/20/24 12:39 PM, Krzysztof Kozlowski wrote:
> On 18/05/2024 10:16, Kartik Agarwala wrote:
>> Convert Mediatek MT6358 Audio Codec bindings from text to dtschema.
>>
>> Signed-off-by: Kartik Agarwala <[email protected]>
>> ---
>> .../bindings/sound/mediatek,mt6358.yaml | 47 +++++++++++++++++++
>> .../devicetree/bindings/sound/mt6358.txt | 26 ----------
>> 2 files changed, 47 insertions(+), 26 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
>> delete mode 100644 Documentation/devicetree/bindings/sound/mt6358.txt
>>
>> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
>> new file mode 100644
>> index 000000000..f57ef2aa5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt6358.yaml
>> @@ -0,0 +1,47 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/sound/mediatek,mt6358.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Mediatek MT6358 Audio Codec
>> +
>> +maintainers:
>> + - Kartik Agarwala <[email protected]>
>> +
>> +description: |
>
> Do not need '|' unless you need to preserve formatting.
>
>> + 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.
>
> Did you update the PMIC wrapper binding with ref to this?
>

Hi Krzysztof,

I apologize incase this is something obvious but I am still not sure
if I understand what you expect here. Could you please explain this
a bit more? I thought that you wanted me to convert the Mediatek PMIC
wrapper but it already seems to be in DT-Schema format.[1]

[1]: https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/soc/mediatek/mediatek%2Cpwrap.yaml

Thanks and Regards,
Kartik Agarwala

2024-06-07 06:56:26

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] ASoC: dt-bindings: mt6358: Convert to dtschema

On 06/06/2024 16:19, Kartik Agarwala wrote:
>>> + Must be a child node of PMIC wrapper.
>>
>> Did you update the PMIC wrapper binding with ref to this?
>>
>
> Hi Krzysztof,
>
> I apologize incase this is something obvious but I am still not sure
> if I understand what you expect here. Could you please explain this
> a bit more? I thought that you wanted me to convert the Mediatek PMIC
> wrapper but it already seems to be in DT-Schema format.[1]
>
> [1]: https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/soc/mediatek/mediatek%2Cpwrap.yaml

If this is a child of some node (PMIC wrapper), then parent node should
reference this schema. Please make the parent example complete and
validate it. Otherwise above statement - must be a child ...- is false.

Best regards,
Krzysztof