2024-01-16 08:46:51

by Tim Lunn

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

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



2024-01-16 08:47:01

by Tim Lunn

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

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 <[email protected]>
---

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


2024-01-16 08:47:28

by Tim Lunn

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

Fix typo in the example specifying wrong compatible string

Signed-off-by: Tim Lunn <[email protected]>
---

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


2024-01-16 08:47:48

by Tim Lunn

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

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

Signed-off-by: Tim Lunn <[email protected]>
---

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


2024-01-16 11:08:54

by Rob Herring (Arm)

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


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 <[email protected]>
> ---
>
> 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/[email protected]

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.


2024-01-16 11:09:21

by Rob Herring (Arm)

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


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 <[email protected]>
> ---
>
> 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/[email protected]

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.


2024-01-16 19:37:10

by Rob Herring (Arm)

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

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 <[email protected]>
> ---
>
> .../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
>

2024-01-16 19:37:51

by Rob Herring (Arm)

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


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 <[email protected]>
> ---
>
> Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>

Acked-by: Rob Herring <[email protected]>


2024-01-16 19:39:39

by Rob Herring

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

On Tue, Jan 16, 2024 at 1:37 PM Rob Herring <[email protected]> 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 <[email protected]>
> > ---
> >
> > Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
>
> Acked-by: Rob Herring <[email protected]>

Err, withdrawn. This doesn't pass tests.

Rob

2024-01-17 07:42:28

by Krzysztof Kozlowski

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

On 16/01/2024 09:46, Tim Lunn wrote:
> Fix typo in the example specifying wrong compatible string
>
> Signed-off-by: Tim Lunn <[email protected]>
> ---
>
> 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


2024-01-17 09:38:03

by Tim Lunn

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

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 <[email protected]>
>> ---
>>
>> .../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
>>

2024-01-17 10:12:27

by Krzysztof Kozlowski

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

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


2024-01-17 10:39:02

by Tim Lunn

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


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
>

2024-01-17 10:58:10

by Krzysztof Kozlowski

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

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


2024-01-17 11:08:49

by Tim Lunn

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


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
>