diff mbox series

[v3,2/3] dt-bindings: rockchip: rk809: Document audio codec properties

Message ID 20240120135529.899403-3-tim@feathertop.org
State New
Headers
Series dt-bindings: rockchip: Add support for rk809 audio codec |

Commit Message

Tim Lunn Jan. 20, 2024, 1:55 p.m. UTC 
  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 <tim@feathertop.org>

---

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(-)

Comments

Krzysztof Kozlowski Jan. 22, 2024, 8:14 a.m. UTC | #1 
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
Tim Lunn Jan. 23, 2024, 4:10 a.m. UTC | #2 
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
>
Krzysztof Kozlowski Jan. 23, 2024, 7:37 a.m. UTC | #3 
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
Tim Lunn Jan. 23, 2024, 8:33 a.m. UTC | #4 
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
>
diff mbox series

Patch

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;
+            };
         };
     };