[PATCH 0/3] Add missing dt-binding properties to rt5682(s)

[PATCH 0/3] Add missing dt-binding properties to rt5682(s)

[Nícolas F. R. A. Prado]

This series adds the missing sound-dai-cells to rt5682s and supplies for
both rt5682s and rt5682.


Nícolas F. R. A. Prado (3):
  ASoC: dt-bindings: realtek,rt5682s: Add #sound-dai-cells
  ASoC: dt-bindings: realtek,rt5682s: Add AVDD and MICVDD supplies
  ASoC: dt-bindings: rt5682: Add AVDD, MICVDD and VBAT supplies

 .../devicetree/bindings/sound/realtek,rt5682s.yaml         | 7 +++++++
 Documentation/devicetree/bindings/sound/rt5682.txt         | 6 ++++++
 2 files changed, 13 insertions(+)

-- 
2.38.1

[PATCH 1/3] ASoC: dt-bindings: realtek,rt5682s: Add #sound-dai-cells

[Nícolas F. R. A. Prado]

The rt5682s codec can be pointed to through a sound-dai property to be
used as part of a machine sound driver. dtc expects #sound-dai-cells to
be defined in the codec's node in those cases, so add it in the
dt-binding and set it to 0.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

---

 Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
index ca5b8987b749..ea53a55015c4 100644
--- a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
+++ b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
@@ -87,6 +87,9 @@ properties:
     maxItems: 2
     description: Name given for DAI word clock and bit clock outputs.
 
+  "#sound-dai-cells":
+    const: 0
+
 additionalProperties: false
 
 required:
-- 
2.38.1

Re: [PATCH 1/3] ASoC: dt-bindings: realtek,rt5682s: Add #sound-dai-cells

[David Heidelberg]

[-- Attachment #1.1.1: Type: text/plain, Size: 1312 bytes --]

Reviewed-by: David Heidelberg <david.heidelberg@collabora.com>

On 21/10/2022 21:09, Nícolas F. R. A. Prado wrote:
> The rt5682s codec can be pointed to through a sound-dai property to be
> used as part of a machine sound driver. dtc expects #sound-dai-cells to
> be defined in the codec's node in those cases, so add it in the
> dt-binding and set it to 0.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>
> ---
>
>   Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> index ca5b8987b749..ea53a55015c4 100644
> --- a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> @@ -87,6 +87,9 @@ properties:
>       maxItems: 2
>       description: Name given for DAI word clock and bit clock outputs.
>   
> +  "#sound-dai-cells":
> +    const: 0
> +
>   additionalProperties: false
>   
>   required:

-- 
David Heidelberg
Consultant Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 695 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] ASoC: dt-bindings: realtek,rt5682s: Add #sound-dai-cells

[Krzysztof Kozlowski]

On 21/10/2022 15:09, Nícolas F. R. A. Prado wrote:
> The rt5682s codec can be pointed to through a sound-dai property to be
> used as part of a machine sound driver. dtc expects #sound-dai-cells to
> be defined in the codec's node in those cases, so add it in the
> dt-binding and set it to 0.

Drop the entire last sentence, it's not really relevant to the problem.
What if we name compiler not dtc, but ctd? It's redundant and actually
forces reader to read unrelated stuff, instead of focusing on the root
problem - this is a DAI provider.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

[PATCH 2/3] ASoC: dt-bindings: realtek,rt5682s: Add AVDD and MICVDD supplies

[Nícolas F. R. A. Prado]

The rt5682s codec can have two supplies: AVDD and MICVDD. Add properties
for them.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

 Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
index ea53a55015c4..ca1037e76f96 100644
--- a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
+++ b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
@@ -90,6 +90,10 @@ properties:
   "#sound-dai-cells":
     const: 0
 
+  AVDD-supply: true
+
+  MICVDD-supply: true
+
 additionalProperties: false
 
 required:
-- 
2.38.1

Re: [PATCH 2/3] ASoC: dt-bindings: realtek,rt5682s: Add AVDD and MICVDD supplies

[David Heidelberg]

[-- Attachment #1.1.1: Type: text/plain, Size: 926 bytes --]

Reviewed-by: David Heidelberg <david.heidelberg@collabora.com>

On 21/10/2022 21:09, Nícolas F. R. A. Prado wrote:
> The rt5682s codec can have two supplies: AVDD and MICVDD. Add properties
> for them.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>
>   Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> index ea53a55015c4..ca1037e76f96 100644
> --- a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> @@ -90,6 +90,10 @@ properties:
>     "#sound-dai-cells":
>       const: 0
>   
> +  AVDD-supply: true
> +
> +  MICVDD-supply: true
> +
>   additionalProperties: false
>   
>   required:

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 695 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/3] ASoC: dt-bindings: realtek,rt5682s: Add AVDD and MICVDD supplies

[Krzysztof Kozlowski]

On 21/10/2022 15:09, Nícolas F. R. A. Prado wrote:
> The rt5682s codec can have two supplies: AVDD and MICVDD. Add properties
> for them.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
> 
>  Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> index ea53a55015c4..ca1037e76f96 100644
> --- a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> @@ -90,6 +90,10 @@ properties:
>    "#sound-dai-cells":
>      const: 0
>  
> +  AVDD-supply: true
> +
> +  MICVDD-supply: true
> +

How about keeping some order in the list of properties?


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 2/3] ASoC: dt-bindings: realtek,rt5682s: Add AVDD and MICVDD supplies

[Nícolas F. R. A. Prado]

On Sat, Oct 22, 2022 at 12:39:56PM -0400, Krzysztof Kozlowski wrote:
> On 21/10/2022 15:09, Nícolas F. R. A. Prado wrote:
> > The rt5682s codec can have two supplies: AVDD and MICVDD. Add properties
> > for them.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> > 
> >  Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> > index ea53a55015c4..ca1037e76f96 100644
> > --- a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> > +++ b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> > @@ -90,6 +90,10 @@ properties:
> >    "#sound-dai-cells":
> >      const: 0
> >  
> > +  AVDD-supply: true
> > +
> > +  MICVDD-supply: true
> > +
> 
> How about keeping some order in the list of properties?

The current properties don't seem to follow any particular order and keeping the
supplies grouped together seemed reasonable. What ordering do you suggest?

Thanks,
Nícolas

Re: [PATCH 2/3] ASoC: dt-bindings: realtek,rt5682s: Add AVDD and MICVDD supplies

[Krzysztof Kozlowski]

On 24/10/2022 11:06, Nícolas F. R. A. Prado wrote:
> On Sat, Oct 22, 2022 at 12:39:56PM -0400, Krzysztof Kozlowski wrote:
>> On 21/10/2022 15:09, Nícolas F. R. A. Prado wrote:
>>> The rt5682s codec can have two supplies: AVDD and MICVDD. Add properties
>>> for them.
>>>
>>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>>> ---
>>>
>>>  Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
>>> index ea53a55015c4..ca1037e76f96 100644
>>> --- a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
>>> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
>>> @@ -90,6 +90,10 @@ properties:
>>>    "#sound-dai-cells":
>>>      const: 0
>>>  
>>> +  AVDD-supply: true
>>> +
>>> +  MICVDD-supply: true
>>> +
>>
>> How about keeping some order in the list of properties?
> 
> The current properties don't seem to follow any particular order and keeping the
> supplies grouped together seemed reasonable. What ordering do you suggest?

That's true...  :/

Best regards,
Krzysztof

Re: [PATCH 2/3] ASoC: dt-bindings: realtek,rt5682s: Add AVDD and MICVDD supplies

[Krzysztof Kozlowski]

On 21/10/2022 15:09, Nícolas F. R. A. Prado wrote:
> The rt5682s codec can have two supplies: AVDD and MICVDD. Add properties
> for them.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
> 
>  Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> index ea53a55015c4..ca1037e76f96 100644
> --- a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml
> @@ -90,6 +90,10 @@ properties:
>    "#sound-dai-cells":
>      const: 0
>  
> +  AVDD-supply: true
> +
> +  MICVDD-supply: true

Ach, unacked. Please use lowercase names.

Best regards,
Krzysztof

[PATCH 3/3] ASoC: dt-bindings: rt5682: Add AVDD, MICVDD and VBAT supplies

[Nícolas F. R. A. Prado]

The rt5682 codec can have three supplies: AVDD, MICVDD and VBAT. Add
properties for them.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

 Documentation/devicetree/bindings/sound/rt5682.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/rt5682.txt b/Documentation/devicetree/bindings/sound/rt5682.txt
index c5f2b8febcee..5ccf4eaf12a9 100644
--- a/Documentation/devicetree/bindings/sound/rt5682.txt
+++ b/Documentation/devicetree/bindings/sound/rt5682.txt
@@ -48,6 +48,12 @@ Optional properties:
 
 - #sound-dai-cells: Should be set to '<0>'.
 
+- AVDD-supply: phandle to the regulator supplying AVDD
+
+- MICVDD-supply: phandle to the regulator supplying MICVDD
+
+- VBAT-supply: phandle to the regulator supplying VBAT
+
 Pins on the device (for linking into audio routes) for RT5682:
 
   * DMIC L1
-- 
2.38.1

Re: [PATCH 3/3] ASoC: dt-bindings: rt5682: Add AVDD, MICVDD and VBAT supplies

[Krzysztof Kozlowski]

On 21/10/2022 15:09, Nícolas F. R. A. Prado wrote:
> The rt5682 codec can have three supplies: AVDD, MICVDD and VBAT. Add
> properties for them.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
> 
>  Documentation/devicetree/bindings/sound/rt5682.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/rt5682.txt b/Documentation/devicetree/bindings/sound/rt5682.txt
> index c5f2b8febcee..5ccf4eaf12a9 100644
> --- a/Documentation/devicetree/bindings/sound/rt5682.txt
> +++ b/Documentation/devicetree/bindings/sound/rt5682.txt
> @@ -48,6 +48,12 @@ Optional properties:
>  
>  - #sound-dai-cells: Should be set to '<0>'.
>  
> +- AVDD-supply: phandle to the regulator supplying AVDD
> +
> +- MICVDD-supply: phandle to the regulator supplying MICVDD
> +
> +- VBAT-supply: phandle to the regulator supplying VBAT

Lowercase.

Best regards,
Krzysztof

Re: [PATCH 3/3] ASoC: dt-bindings: rt5682: Add AVDD, MICVDD and VBAT supplies

[Nícolas F. R. A. Prado]

On Sat, Oct 22, 2022 at 12:41:01PM -0400, Krzysztof Kozlowski wrote:
> On 21/10/2022 15:09, Nícolas F. R. A. Prado wrote:
> > The rt5682 codec can have three supplies: AVDD, MICVDD and VBAT. Add
> > properties for them.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> > 
> >  Documentation/devicetree/bindings/sound/rt5682.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/rt5682.txt b/Documentation/devicetree/bindings/sound/rt5682.txt
> > index c5f2b8febcee..5ccf4eaf12a9 100644
> > --- a/Documentation/devicetree/bindings/sound/rt5682.txt
> > +++ b/Documentation/devicetree/bindings/sound/rt5682.txt
> > @@ -48,6 +48,12 @@ Optional properties:
> >  
> >  - #sound-dai-cells: Should be set to '<0>'.
> >  
> > +- AVDD-supply: phandle to the regulator supplying AVDD
> > +
> > +- MICVDD-supply: phandle to the regulator supplying MICVDD
> > +
> > +- VBAT-supply: phandle to the regulator supplying VBAT
> 
> Lowercase.

Actually looks like there's already a DT using these properties before the
binding was added:

arm64/boot/dts/qcom/sc7180-trogdor.dtsi

In this case should we keep them uppercase for compatibility or carry on with
the name changes and also update the DT? (the driver also uses uppercase names)


Also noticed that dai-cells should actually be 1. Will fix in next version.

Thanks,
Nícolas

Re: [PATCH 3/3] ASoC: dt-bindings: rt5682: Add AVDD, MICVDD and VBAT supplies

[Krzysztof Kozlowski]

On 24/10/2022 12:00, Nícolas F. R. A. Prado wrote:
> On Sat, Oct 22, 2022 at 12:41:01PM -0400, Krzysztof Kozlowski wrote:
>> On 21/10/2022 15:09, Nícolas F. R. A. Prado wrote:
>>> The rt5682 codec can have three supplies: AVDD, MICVDD and VBAT. Add
>>> properties for them.
>>>
>>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>>> ---
>>>
>>>  Documentation/devicetree/bindings/sound/rt5682.txt | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/rt5682.txt b/Documentation/devicetree/bindings/sound/rt5682.txt
>>> index c5f2b8febcee..5ccf4eaf12a9 100644
>>> --- a/Documentation/devicetree/bindings/sound/rt5682.txt
>>> +++ b/Documentation/devicetree/bindings/sound/rt5682.txt
>>> @@ -48,6 +48,12 @@ Optional properties:
>>>  
>>>  - #sound-dai-cells: Should be set to '<0>'.
>>>  
>>> +- AVDD-supply: phandle to the regulator supplying AVDD
>>> +
>>> +- MICVDD-supply: phandle to the regulator supplying MICVDD
>>> +
>>> +- VBAT-supply: phandle to the regulator supplying VBAT
>>
>> Lowercase.
> 
> Actually looks like there's already a DT using these properties before the
> binding was added:
> 
> arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> 
> In this case should we keep them uppercase for compatibility or carry on with
> the name changes and also update the DT? (the driver also uses uppercase names)

Driver seems to use them as well, then uppercase is fine. But you need
to describe it in commit msg, that you document existing usage, not
adding new properties.

Best regards,
Krzysztof