2024-01-20 13:55:54

by Tim Lunn

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

Rockchip RK809 shares the same audio codec block as the rk817 mfd, and
is compatible with the existing rk817_codec driver. There are already
about seven rk809 based boards that implement this codec in existing
device tree files.

This series documents the audio codec properties in rockchip,rk809.yaml
bindings and updates the example with their usage. It also fixes a typo
in the existing example that was causing the example to be validated
against the wrong binding.

Changes in v3:
- split out clocks into separate patch and group example properties
where properties are introduced.
- Address review comments
- drop clock descriptions that arent required
- set maxitems on clocks

Changes in v2:
- Fix vcc-supply warning detected by dt_binding bot
- Fix missing include and pinctrl for codec example

Tim Lunn (3):
dt-bindings: rockchip: rk809 fix existing example
dt-bindings: rockchip: rk809: Document audio codec properties
dt-bindings: rockchip: rk809: Document audio codec clock

.../bindings/mfd/rockchip,rk809.yaml | 42 +++++++++++++++----
1 file changed, 35 insertions(+), 7 deletions(-)

--
2.40.1



2024-01-20 13:56:07

by Tim Lunn

[permalink] [raw]
Subject: [PATCH v3 1/3] dt-bindings: rockchip: rk809 fix existing example

The example for rk809 picked up the wrong compatible string when the
binding was converted to yaml. As a result it also specified too many
vccX-supply properties.

Fix typo in the example specifying wrong compatible string for rk809.
Remove additional vccX-supply properties that dont exist on rk809 so
that binding checks pass again.

Signed-off-by: Tim Lunn <[email protected]>
Acked-by: Conor Dooley <[email protected]>
Fixes: 6c38ca03406e ("dt-bindings: mfd: rk808: Convert bindings to yaml")

---

Changes in v3:
- Drop label from rk809 node

Changes in v2:
- Fix vcc-supply warning detected by dt_binding bot

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

diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
index 839c0521f1e5..eb057607dc54 100644
--- a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
+++ b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
@@ -126,8 +126,8 @@ examples:
#address-cells = <1>;
#size-cells = <0>;

- rk808: pmic@1b {
- compatible = "rockchip,rk808";
+ pmic@1b {
+ compatible = "rockchip,rk809";
reg = <0x1b>;
#clock-cells = <1>;
clock-output-names = "xin32k", "rk808-clkout2";
@@ -146,9 +146,6 @@ examples:
vcc7-supply = <&vcc_sysin>;
vcc8-supply = <&vcc3v3_sys>;
vcc9-supply = <&vcc_sysin>;
- vcc10-supply = <&vcc_sysin>;
- vcc11-supply = <&vcc_sysin>;
- vcc12-supply = <&vcc3v3_sys>;

regulators {
vdd_center: DCDC_REG1 {
--
2.40.1


2024-01-20 13:56:20

by Tim Lunn

[permalink] [raw]
Subject: [PATCH v3 2/3] dt-bindings: rockchip: rk809: Document audio codec properties

Rockchip RK809 shares the same audio codec block as the rk817 mfd, and
is compatible with the existing rk817_codec driver.

This patch introduces to the binding the standard property #sound-dai-cells
and also an optional codec child node to hold codec specific properties.
Currently there is only one property in this node however the downstream
driver shows a number of other properties that are supported by the codec
hardware, that could be implemented in the future. This maintains the
existing driver ABI and keeps consistency with the rk817 bindings.

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

---

Changes in v3:
- split out clocks into separate patch and group example properties
where properties are introduced.
- remove descriptions from #sound-dai-cells node

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

diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
index eb057607dc54..be0616201f52 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,22 @@ properties:
unevaluatedProperties: false
unevaluatedProperties: false

+ '#sound-dai-cells':
+ 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:
@@ -137,6 +153,7 @@ examples:
pinctrl-0 = <&pmic_int_l_pin>;
rockchip,system-power-controller;
wakeup-source;
+ #sound-dai-cells = <0>;

vcc1-supply = <&vcc_sysin>;
vcc2-supply = <&vcc_sysin>;
@@ -281,5 +298,9 @@ examples:
};
};
};
+
+ rk817_codec: codec {
+ rockchip,mic-in-differential;
+ };
};
};
--
2.40.1


2024-01-20 13:56:34

by Tim Lunn

[permalink] [raw]
Subject: [PATCH v3 3/3] dt-bindings: rockchip: rk809: Document audio codec clock

Rockchip RK809 shares the same audio codec block as the rk817 mfd, and
is compatible with the existing rk817_codec driver. This patch
introduces the clock required for the audio codec.

This clock provides the I2S master clock for the audio data. The codec
driver finds the clock by the name "mclk" and will fail to register if
this is missing. Clock-names is kept here to keep compatibility with the
exisitng driver ABI and also to be consistent with the rk817 binding.

This series 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]>

---

Changes in v3:
- split out clocks into separate patch and group example properties
where properties are introduced.
- Address review comments
- drop clock descriptions that arent required
- set maxitems on clocks

Changes in v2:
- Fix missing include and pinctrl for codec example

.../devicetree/bindings/mfd/rockchip,rk809.yaml | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
index be0616201f52..0174261449ab 100644
--- a/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
+++ b/Documentation/devicetree/bindings/mfd/rockchip,rk809.yaml
@@ -93,6 +93,13 @@ properties:
unevaluatedProperties: false
unevaluatedProperties: false

+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: mclk
+
'#sound-dai-cells':
const: 0

@@ -135,6 +142,7 @@ additionalProperties: false

examples:
- |
+ #include <dt-bindings/clock/px30-cru.h>
#include <dt-bindings/pinctrl/rockchip.h>
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/gpio/gpio.h>
@@ -149,8 +157,10 @@ 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>;
+ pinctrl-0 = <&pmic_int_l_pin>, <&i2s1_2ch_mclk>;
rockchip,system-power-controller;
wakeup-source;
#sound-dai-cells = <0>;
--
2.40.1


2024-01-22 08:16:22

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] dt-bindings: rockchip: rk809: Document audio codec clock

On 20/01/2024 14:55, Tim Lunn wrote:
> Rockchip RK809 shares the same audio codec block as the rk817 mfd, and

What rk817 has anything to do with this?

> is compatible with the existing rk817_codec driver. This patch

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95


> introduces the clock required for the audio codec.
>
> This clock provides the I2S master clock for the audio data. The codec
> driver finds the clock by the name "mclk" and will fail to register if
> this is missing. Clock-names is kept here to keep compatibility with the
> exisitng driver ABI and also to be consistent with the rk817 binding.

Typo.

Also, what consistency with rk817 driver?

I really do not understand which problem you are solving here.



Best regards,
Krzysztof


2024-01-22 08:21:47

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] dt-bindings: rockchip: rk809: Document audio codec properties

On 20/01/2024 14:55, Tim Lunn wrote:
> Rockchip RK809 shares the same audio codec block as the rk817 mfd, and
> is compatible with the existing rk817_codec driver.

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.

>
> This patch introduces to the binding the standard property #sound-dai-cells
> and also an optional codec child node to hold codec specific properties.
> Currently there is only one property in this node however the downstream
> driver shows a number of other properties that are supported by the codec
> hardware, that could be implemented in the future. This maintains the
> existing driver ABI and keeps consistency with the rk817 bindings.

So you are adding a new node? Just for one property? No, just put it
into parent node.

Downstream driver does not matter at all in that aspect.

Best regards,
Krzysztof


2024-01-23 04:10:57

by Tim Lunn

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] dt-bindings: rockchip: rk809: Document audio codec properties


On 1/22/24 19:14, Krzysztof Kozlowski wrote:
> On 20/01/2024 14:55, Tim Lunn wrote:
>> Rockchip RK809 shares the same audio codec block as the rk817 mfd, and
>> is compatible with the existing rk817_codec driver.
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching.
Ok I will check this.
>
>> This patch introduces to the binding the standard property #sound-dai-cells
>> and also an optional codec child node to hold codec specific properties.
>> Currently there is only one property in this node however the downstream
>> driver shows a number of other properties that are supported by the codec
>> hardware, that could be implemented in the future. This maintains the
>> existing driver ABI and keeps consistency with the rk817 bindings.
> So you are adding a new node? Just for one property? No, just put it
> into parent node.
The existing upstream codec driver parses the property from the "codec"
sub-node, if I
move it to the parent node here, I will need to patch the codec driver
to search in both locations,
so as to not break the rk817 bindings.  If that is preferred, I can do
it that way.
>
> Downstream driver does not matter at all in that aspect.
>
The codec hardware supports additional properties but they are not
implemented currently in
upstream driver.


>
> Best regards,
> Krzysztof
>

2024-01-23 04:25:54

by Tim Lunn

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] dt-bindings: rockchip: rk809: Document audio codec clock


On 1/22/24 19:16, Krzysztof Kozlowski wrote:
> On 20/01/2024 14:55, Tim Lunn wrote:
>> Rockchip RK809 shares the same audio codec block as the rk817 mfd, and
> What rk817 has anything to do with this?

The existing codec driver in linux already is from the rk817 and thus
called rk817, however
that driver is also compatible with the codec in rk809.

>
>> is compatible with the existing rk817_codec driver. This patch
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
I will check this as well.
>
>> introduces the clock required for the audio codec.
>>
>> This clock provides the I2S master clock for the audio data. The codec
>> driver finds the clock by the name "mclk" and will fail to register if
>> this is missing. Clock-names is kept here to keep compatibility with the
>> exisitng driver ABI and also to be consistent with the rk817 binding.
> Typo.
>
> Also, what consistency with rk817 driver?
The rk817 codec driver that  already exists in mainline linux tree.
>
> I really do not understand which problem you are solving here.
I will fix the typo and try to clarify commit message further.
>
>
>
> Best regards,
> Krzysztof
>

2024-01-23 07:41:28

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] dt-bindings: rockchip: rk809: Document audio codec clock

On 23/01/2024 05:25, Tim Lunn wrote:
>
> On 1/22/24 19:16, Krzysztof Kozlowski wrote:
>> On 20/01/2024 14:55, Tim Lunn wrote:
>>> Rockchip RK809 shares the same audio codec block as the rk817 mfd, and
>> What rk817 has anything to do with this?
>
> The existing codec driver in linux already is from the rk817 and thus
> called rk817, however
> that driver is also compatible with the codec in rk809.

Sure, but how is this any related? Your commit msg says two independent
things:
1. Something shares same audio codec block
2. Add clocks

I don't see how they are anyhow related to each other, IOW, how from (1)
comes (2).

Best regards,
Krzysztof


2024-01-23 07:46:33

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] dt-bindings: rockchip: rk809: Document audio codec properties

On 23/01/2024 05:10, Tim Lunn wrote:
>
> On 1/22/24 19:14, Krzysztof Kozlowski wrote:
>> On 20/01/2024 14:55, Tim Lunn wrote:
>>> Rockchip RK809 shares the same audio codec block as the rk817 mfd, and
>>> is compatible with the existing rk817_codec driver.
>> Please use subject prefixes matching the subsystem. You can get them for
>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>> your patch is touching.
> Ok I will check this.
>>
>>> This patch introduces to the binding the standard property #sound-dai-cells
>>> and also an optional codec child node to hold codec specific properties.
>>> Currently there is only one property in this node however the downstream
>>> driver shows a number of other properties that are supported by the codec
>>> hardware, that could be implemented in the future. This maintains the
>>> existing driver ABI and keeps consistency with the rk817 bindings.
>> So you are adding a new node? Just for one property? No, just put it
>> into parent node.
> The existing upstream codec driver parses the property from the "codec"
> sub-node, if I
> move it to the parent node here, I will need to patch the codec driver
> to search in both locations,
> so as to not break the rk817 bindings.  If that is preferred, I can do
> it that way.

Your long commit msg has just very short mention about existing driver
and the rest is not helpful. Please rephrase to explain why and what you
are doing it.

>>
>> Downstream driver does not matter at all in that aspect.
>>
> The codec hardware supports additional properties but they are not
> implemented currently in
> upstream driver.


Again: it does not matter. Bindings are not about drivers.

Best regards,
Krzysztof


2024-01-23 08:35:36

by Tim Lunn

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] dt-bindings: rockchip: rk809: Document audio codec properties


On 1/23/24 18:37, Krzysztof Kozlowski wrote:
> On 23/01/2024 05:10, Tim Lunn wrote:
>> On 1/22/24 19:14, Krzysztof Kozlowski wrote:
>>> On 20/01/2024 14:55, Tim Lunn wrote:
>>>> Rockchip RK809 shares the same audio codec block as the rk817 mfd, and
>>>> is compatible with the existing rk817_codec driver.
>>> Please use subject prefixes matching the subsystem. You can get them for
>>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>>> your patch is touching.
>> Ok I will check this.
>>>> This patch introduces to the binding the standard property #sound-dai-cells
>>>> and also an optional codec child node to hold codec specific properties.
>>>> Currently there is only one property in this node however the downstream
>>>> driver shows a number of other properties that are supported by the codec
>>>> hardware, that could be implemented in the future. This maintains the
>>>> existing driver ABI and keeps consistency with the rk817 bindings.
>>> So you are adding a new node? Just for one property? No, just put it
>>> into parent node.
>> The existing upstream codec driver parses the property from the "codec"
>> sub-node, if I
>> move it to the parent node here, I will need to patch the codec driver
>> to search in both locations,
>> so as to not break the rk817 bindings.  If that is preferred, I can do
>> it that way.
> Your long commit msg has just very short mention about existing driver
> and the rest is not helpful. Please rephrase to explain why and what you
> are doing it.
>
OK I will rephrase both commit messages for the next version.
>>> Downstream driver does not matter at all in that aspect.
>>>
>> The codec hardware supports additional properties but they are not
>> implemented currently in
>> upstream driver.
>
> Again: it does not matter. Bindings are not about drivers.
>
> Best regards,
> Krzysztof
>