[PATCH 0/3] dt-bindings: rockchip: Add support for rk809 audio codec

[PATCH 0/3] dt-bindings: rockchip: Add support for rk809 audio codec

[Tim Lunn]

Rockchip RK809 shares the same audio codec as the rk817 mfd, it is also
using the same rk817_codec driver. However it is missing from the
bindings.

This series documents the audio codec properties in rockchip,rk809.yaml
bindings and updates example.


Tim Lunn (3):
  dt-bindings: rockchip: Add rk809 support for rk817 audio codec
  dt-bindings: rockchip: rk809 fix compatible string in examples
  dt-bindings: rockchip: Update rk809 example with audio codec
    properties

 .../bindings/mfd/rockchip,rk809.yaml          | 40 +++++++++++++++++--
 1 file changed, 37 insertions(+), 3 deletions(-)

-- 
2.40.1

[PATCH 1/3] dt-bindings: rockchip: Add rk809 support for rk817 audio codec

[Tim Lunn]

Rockchip RK809 shares the same audio codec as the rk817 mfd, it is also
using the same rk817_codec driver. However it is missing from the
bindings.

Update dt-binding documentation for rk809 to include the audio codec
properties. This fixes the following warning from dtb check:

pmic@20: '#sound-dai-cells', 'assigned-clock-parents', 'assigned-clocks',
   'clock-names', 'clocks', 'codec' do not match any of the regexes:
   'pinctrl-[0-9]+'

Signed-off-by: Tim Lunn <tim@feathertop.org>
---

 .../bindings/mfd/rockchip,rk809.yaml          | 30 ++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
index 839c0521f1e5..bac2e751e2f2 100644
--- a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
+++ b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
@@ -12,7 +12,7 @@ maintainers:
 
 description: |
   Rockchip RK809 series PMIC. This device consists of an i2c controlled MFD
-  that includes regulators, an RTC, and power button.
+  that includes regulators, an RTC, a power button, and an audio codec.
 
 properties:
   compatible:
@@ -93,6 +93,34 @@ properties:
         unevaluatedProperties: false
     unevaluatedProperties: false
 
+  clocks:
+    description:
+      The input clock for the audio codec.
+
+  clock-names:
+    description:
+      The clock name for the codec clock.
+    items:
+      - const: mclk
+
+  '#sound-dai-cells':
+    description:
+      Needed for the interpretation of sound dais.
+    const: 0
+
+  codec:
+    description: |
+      The child node for the codec to hold additional properties. If no
+      additional properties are required for the codec, this node can be
+      omitted.
+    type: object
+    additionalProperties: false
+    properties:
+      rockchip,mic-in-differential:
+        type: boolean
+        description:
+          Describes if the microphone uses differential mode.
+
 allOf:
   - if:
       properties:
-- 
2.40.1

[PATCH 2/3] dt-bindings: rockchip: rk809 fix compatible string in examples

[Tim Lunn]

Fix typo in the example specifying wrong compatible string

Signed-off-by: Tim Lunn <tim@feathertop.org>
---

 Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
index bac2e751e2f2..3f31478932c2 100644
--- a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
+++ b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
@@ -154,8 +154,8 @@ examples:
         #address-cells = <1>;
         #size-cells = <0>;
 
-        rk808: pmic@1b {
-            compatible = "rockchip,rk808";
+        rk809: pmic@1b {
+            compatible = "rockchip,rk809";
             reg = <0x1b>;
             #clock-cells = <1>;
             clock-output-names = "xin32k", "rk808-clkout2";
-- 
2.40.1

[PATCH 3/3] dt-bindings: rockchip: Update rk809 example with audio codec properties

[Tim Lunn]

Update the example provided to include the properties for using
rk817 audio codec.

Signed-off-by: Tim Lunn <tim@feathertop.org>
---

 Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
index 3f31478932c2..c9c676d0922d 100644
--- a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
+++ b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
@@ -161,10 +161,13 @@ examples:
             clock-output-names = "xin32k", "rk808-clkout2";
             interrupt-parent = <&gpio3>;
             interrupts = <10 IRQ_TYPE_LEVEL_LOW>;
+            clock-names = "mclk";
+            clocks = <&cru SCLK_I2S1_OUT>;
             pinctrl-names = "default";
             pinctrl-0 = <&pmic_int_l_pin>;
             rockchip,system-power-controller;
             wakeup-source;
+            #sound-dai-cells = <0>;
 
             vcc1-supply = <&vcc_sysin>;
             vcc2-supply = <&vcc_sysin>;
@@ -312,5 +315,8 @@ examples:
                     };
                 };
             };
+            rk817_codec: codec {
+                rockchip,mic-in-differential;
+            };
         };
     };
-- 
2.40.1

Re: [PATCH 2/3] dt-bindings: rockchip: rk809 fix compatible string in examples

[Rob Herring]

On Tue, 16 Jan 2024 19:46:17 +1100, Tim Lunn wrote:
> Fix typo in the example specifying wrong compatible string
> 
> Signed-off-by: Tim Lunn <tim@feathertop.org>
> ---
> 
>  Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/rockchip,rk809.example.dtb: pmic@1b: 'vcc10-supply', 'vcc11-supply', 'vcc12-supply' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/mfd/rockchip,rk809.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240116084618.3112410-3-tim@feathertop.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 3/3] dt-bindings: rockchip: Update rk809 example with audio codec properties

[Rob Herring]

On Tue, 16 Jan 2024 19:46:18 +1100, Tim Lunn wrote:
> Update the example provided to include the properties for using
> rk817 audio codec.
> 
> Signed-off-by: Tim Lunn <tim@feathertop.org>
> ---
> 
>  Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/mfd/rockchip,rk809.example.dts:39.32-33 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/mfd/rockchip,rk809.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2
make: *** [Makefile:234: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240116084618.3112410-4-tim@feathertop.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/3] dt-bindings: rockchip: Add rk809 support for rk817 audio codec

[Rob Herring]

On Tue, Jan 16, 2024 at 07:46:16PM +1100, Tim Lunn wrote:
> Rockchip RK809 shares the same audio codec as the rk817 mfd, it is also
> using the same rk817_codec driver. However it is missing from the
> bindings.
> 
> Update dt-binding documentation for rk809 to include the audio codec
> properties. This fixes the following warning from dtb check:
> 
> pmic@20: '#sound-dai-cells', 'assigned-clock-parents', 'assigned-clocks',
>    'clock-names', 'clocks', 'codec' do not match any of the regexes:
>    'pinctrl-[0-9]+'
> 
> Signed-off-by: Tim Lunn <tim@feathertop.org>
> ---
> 
>  .../bindings/mfd/rockchip,rk809.yaml          | 30 ++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
> index 839c0521f1e5..bac2e751e2f2 100644
> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
> @@ -12,7 +12,7 @@ maintainers:
>  
>  description: |
>    Rockchip RK809 series PMIC. This device consists of an i2c controlled MFD
> -  that includes regulators, an RTC, and power button.
> +  that includes regulators, an RTC, a power button, and an audio codec.
>  
>  properties:
>    compatible:
> @@ -93,6 +93,34 @@ properties:
>          unevaluatedProperties: false
>      unevaluatedProperties: false
>  
> +  clocks:
> +    description:
> +      The input clock for the audio codec.

How many clocks? (maxItems: 1)

You can drop the description.

> +
> +  clock-names:
> +    description:
> +      The clock name for the codec clock.

Drop.

> +    items:
> +      - const: mclk
> +
> +  '#sound-dai-cells':
> +    description:
> +      Needed for the interpretation of sound dais.

Common property, don't need the description.


> +    const: 0
> +
> +  codec:
> +    description: |
> +      The child node for the codec to hold additional properties. If no
> +      additional properties are required for the codec, this node can be
> +      omitted.

Why do you need a child node here? Just put the 1 property in the parent 
node.

> +    type: object
> +    additionalProperties: false
> +    properties:
> +      rockchip,mic-in-differential:
> +        type: boolean
> +        description:
> +          Describes if the microphone uses differential mode.
> +
>  allOf:
>    - if:
>        properties:
> -- 
> 2.40.1
> 

Re: [PATCH 2/3] dt-bindings: rockchip: rk809 fix compatible string in examples

[Rob Herring]

On Tue, 16 Jan 2024 19:46:17 +1100, Tim Lunn wrote:
> Fix typo in the example specifying wrong compatible string
> 
> Signed-off-by: Tim Lunn <tim@feathertop.org>
> ---
> 
>  Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/3] dt-bindings: rockchip: rk809 fix compatible string in examples

[Rob Herring]

On Tue, Jan 16, 2024 at 1:37 PM Rob Herring <robh@kernel.org> wrote:
>
>
> On Tue, 16 Jan 2024 19:46:17 +1100, Tim Lunn wrote:
> > Fix typo in the example specifying wrong compatible string
> >
> > Signed-off-by: Tim Lunn <tim@feathertop.org>
> > ---
> >
> >  Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
>
> Acked-by: Rob Herring <robh@kernel.org>

Err, withdrawn. This doesn't pass tests.

Rob

Re: [PATCH 2/3] dt-bindings: rockchip: rk809 fix compatible string in examples

[Krzysztof Kozlowski]

On 16/01/2024 09:46, Tim Lunn wrote:
> Fix typo in the example specifying wrong compatible string
> 
> Signed-off-by: Tim Lunn <tim@feathertop.org>
> ---
> 
>  Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
> index bac2e751e2f2..3f31478932c2 100644
> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
> @@ -154,8 +154,8 @@ examples:
>          #address-cells = <1>;
>          #size-cells = <0>;
>  
> -        rk808: pmic@1b {
> -            compatible = "rockchip,rk808";
> +        rk809: pmic@1b {

You can just drop the label... Is it used here?

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: rockchip: Add rk809 support for rk817 audio codec

[Tim Lunn]

Hi Rob,

On 1/17/24 06:37, Rob Herring wrote:
> On Tue, Jan 16, 2024 at 07:46:16PM +1100, Tim Lunn wrote:
>> Rockchip RK809 shares the same audio codec as the rk817 mfd, it is also
>> using the same rk817_codec driver. However it is missing from the
>> bindings.
>>
>> Update dt-binding documentation for rk809 to include the audio codec
>> properties. This fixes the following warning from dtb check:
>>
>> pmic@20: '#sound-dai-cells', 'assigned-clock-parents', 'assigned-clocks',
>>     'clock-names', 'clocks', 'codec' do not match any of the regexes:
>>     'pinctrl-[0-9]+'
>>
>> Signed-off-by: Tim Lunn <tim@feathertop.org>
>> ---
>>
>>   .../bindings/mfd/rockchip,rk809.yaml          | 30 ++++++++++++++++++-
>>   1 file changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
>> index 839c0521f1e5..bac2e751e2f2 100644
>> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
>> @@ -12,7 +12,7 @@ maintainers:
>>   
>>   description: |
>>     Rockchip RK809 series PMIC. This device consists of an i2c controlled MFD
>> -  that includes regulators, an RTC, and power button.
>> +  that includes regulators, an RTC, a power button, and an audio codec.
>>   
>>   properties:
>>     compatible:
>> @@ -93,6 +93,34 @@ properties:
>>           unevaluatedProperties: false
>>       unevaluatedProperties: false
>>   
>> +  clocks:
>> +    description:
>> +      The input clock for the audio codec.
> How many clocks? (maxItems: 1)
>
> You can drop the description.
Yes just 1 clock, i will fix this.
>
>> +
>> +  clock-names:
>> +    description:
>> +      The clock name for the codec clock.
> Drop.
Just drop the description? I dont think can drop the clock names as the 
driver use the name to lookup clock:

devm_clk_get(pdev->dev.parent, "mclk");
>
>> +    items:
>> +      - const: mclk
>> +
>> +  '#sound-dai-cells':
>> +    description:
>> +      Needed for the interpretation of sound dais.
> Common property, don't need the description.
Ok
>
>> +    const: 0
>> +
>> +  codec:
>> +    description: |
>> +      The child node for the codec to hold additional properties. If no
>> +      additional properties are required for the codec, this node can be
>> +      omitted.
> Why do you need a child node here? Just put the 1 property in the parent
> node.
This is how the existing rk817 codec driver was setup. I suppose it was 
copied from downstream, where there are more properties than just the 
one. I don't know if there was any intention (or need) to implement 
those other properties.
>
>> +    type: object
>> +    additionalProperties: false
>> +    properties:
>> +      rockchip,mic-in-differential:
>> +        type: boolean
>> +        description:
>> +          Describes if the microphone uses differential mode.
>> +
>>   allOf:
>>     - if:
>>         properties:
>> -- 
>> 2.40.1
>>

Re: [PATCH 1/3] dt-bindings: rockchip: Add rk809 support for rk817 audio codec

[Krzysztof Kozlowski]

On 17/01/2024 10:37, Tim Lunn wrote:
>> You can drop the description.
> Yes just 1 clock, i will fix this.
>>
>>> +
>>> +  clock-names:
>>> +    description:
>>> +      The clock name for the codec clock.
>> Drop.
> Just drop the description? I dont think can drop the clock names as the 
> driver use the name to lookup clock:

Description. But anyway the problem is that adding clocks should be
separate patch with its own explanation.


> 
> devm_clk_get(pdev->dev.parent, "mclk");
>>
>>> +    items:
>>> +      - const: mclk
>>> +
>>> +  '#sound-dai-cells':
>>> +    description:
>>> +      Needed for the interpretation of sound dais.
>> Common property, don't need the description.
> Ok
>>
>>> +    const: 0
>>> +
>>> +  codec:
>>> +    description: |
>>> +      The child node for the codec to hold additional properties. If no
>>> +      additional properties are required for the codec, this node can be
>>> +      omitted.
>> Why do you need a child node here? Just put the 1 property in the parent
>> node.
> This is how the existing rk817 codec driver was setup. I suppose it was 
> copied from downstream, where there are more properties than just the 
> one. I don't know if there was any intention (or need) to implement 
> those other properties.
>>

You need to clearly express ABI requirements in the commit msg.
Otherwise you will get a review like for new bindings.

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: rockchip: Add rk809 support for rk817 audio codec

[Tim Lunn]

On 1/17/24 21:12, Krzysztof Kozlowski wrote:
> On 17/01/2024 10:37, Tim Lunn wrote:
>>> You can drop the description.
>> Yes just 1 clock, i will fix this.
>>>> +
>>>> +  clock-names:
>>>> +    description:
>>>> +      The clock name for the codec clock.
>>> Drop.
>> Just drop the description? I dont think can drop the clock names as the
>> driver use the name to lookup clock:
> Description. But anyway the problem is that adding clocks should be
> separate patch with its own explanation.
>
Right, but I am not actually adding any clocks, just documenting what is 
already there.
There are already boards using this codec with rk809 in dts files and is 
working fine from driver side.
>
>
>> devm_clk_get(pdev->dev.parent, "mclk");
>>>> +    items:
>>>> +      - const: mclk
>>>> +
>>>> +  '#sound-dai-cells':
>>>> +    description:
>>>> +      Needed for the interpretation of sound dais.
>>> Common property, don't need the description.
>> Ok
>>>> +    const: 0
>>>> +
>>>> +  codec:
>>>> +    description: |
>>>> +      The child node for the codec to hold additional properties. If no
>>>> +      additional properties are required for the codec, this node can be
>>>> +      omitted.
>>> Why do you need a child node here? Just put the 1 property in the parent
>>> node.
>> This is how the existing rk817 codec driver was setup. I suppose it was
>> copied from downstream, where there are more properties than just the
>> one. I don't know if there was any intention (or need) to implement
>> those other properties.
> You need to clearly express ABI requirements in the commit msg.
> Otherwise you will get a review like for new bindings.
Got it, I will clarify this and future commit messages

Regards
    Tim
>
> Best regards,
> Krzysztof
>

Re: [PATCH 1/3] dt-bindings: rockchip: Add rk809 support for rk817 audio codec

[Krzysztof Kozlowski]

On 17/01/2024 11:38, Tim Lunn wrote:
> 
> On 1/17/24 21:12, Krzysztof Kozlowski wrote:
>> On 17/01/2024 10:37, Tim Lunn wrote:
>>>> You can drop the description.
>>> Yes just 1 clock, i will fix this.
>>>>> +
>>>>> +  clock-names:
>>>>> +    description:
>>>>> +      The clock name for the codec clock.
>>>> Drop.
>>> Just drop the description? I dont think can drop the clock names as the
>>> driver use the name to lookup clock:
>> Description. But anyway the problem is that adding clocks should be
>> separate patch with its own explanation.
>>
> Right, but I am not actually adding any clocks, just documenting what is 
> already there.

You are. Binding did not have any clocks, now it has.

> There are already boards using this codec with rk809 in dts files and is 
> working fine from driver side.



Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: rockchip: Add rk809 support for rk817 audio codec

[Tim Lunn]

On 1/17/24 21:57, Krzysztof Kozlowski wrote:
> On 17/01/2024 11:38, Tim Lunn wrote:
>> On 1/17/24 21:12, Krzysztof Kozlowski wrote:
>>> On 17/01/2024 10:37, Tim Lunn wrote:
>>>>> You can drop the description.
>>>> Yes just 1 clock, i will fix this.
>>>>>> +
>>>>>> +  clock-names:
>>>>>> +    description:
>>>>>> +      The clock name for the codec clock.
>>>>> Drop.
>>>> Just drop the description? I dont think can drop the clock names as the
>>>> driver use the name to lookup clock:
>>> Description. But anyway the problem is that adding clocks should be
>>> separate patch with its own explanation.
>>>
>> Right, but I am not actually adding any clocks, just documenting what is
>> already there.
> You are. Binding did not have any clocks, now it has.
Ok, I will split the clocks into a separate patch.
>
>> There are already boards using this codec with rk809 in dts files and is
>> working fine from driver side.
>
>
> Best regards,
> Krzysztof
>