diff mbox series

[RFC,v2,3/6] ASoC: dt-bindings: Extend clock bindings of rt5659

Message ID 1648448050-15237-4-git-send-email-spujar@nvidia.com
State Not Applicable
Headers show
Series Flexible codec clock configuration | expand

Commit Message

Sameer Pujar March 28, 2022, 6:14 a.m. UTC
The rt5658 or rt5659 CODEC system clock (SYSCLK) can be derived from
various clock sources. For example it can be derived either from master
clock (MCLK) or by internal PLL. The internal PLL again can take input
clock references from bit clocks (BCLKs) and MCLK. To enable a flexible
clocking configuration the DT binding is extended here.

It makes use of standard clock bindings and sets up the clock relation
via DT.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
Cc: Oder Chiou <oder_chiou@realtek.com>
---
 .../devicetree/bindings/sound/realtek,rt5659.yaml  | 53 ++++++++++++++++++++--
 1 file changed, 49 insertions(+), 4 deletions(-)

Comments

Krzysztof Kozlowski March 28, 2022, 7:06 a.m. UTC | #1
On 28/03/2022 08:14, Sameer Pujar wrote:
> The rt5658 or rt5659 CODEC system clock (SYSCLK) can be derived from
> various clock sources. For example it can be derived either from master
> clock (MCLK) or by internal PLL. The internal PLL again can take input
> clock references from bit clocks (BCLKs) and MCLK. To enable a flexible
> clocking configuration the DT binding is extended here.
> 
> It makes use of standard clock bindings and sets up the clock relation
> via DT.
> 
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> Cc: Oder Chiou <oder_chiou@realtek.com>
> ---
>  .../devicetree/bindings/sound/realtek,rt5659.yaml  | 53 ++++++++++++++++++++--
>  1 file changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
> index b0485b8..0c2f3cb 100644
> --- a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
> @@ -29,12 +29,28 @@ properties:
>      maxItems: 1
>  
>    clocks:
> -    items:
> -      - description: Master clock (MCLK) to the CODEC
> +    description: |
> +      CODEC can receive multiple clock inputs like Master
> +      clock (MCLK), I2S bit clocks (BCLK1, BCLK2, BCLK3,
> +      BCLK4). The CODEC SYSCLK can be generated from MCLK
> +      or internal PLL. In turn PLL can reference from MCLK
> +      and BCLKs.
>  
>    clock-names:
> -    items:
> -      - const: mclk
> +    description: |
> +      The clock names can be combination of following:
> +        "mclk"        : Master clock
> +        "pll_ref"     : Reference to CODEC PLL clock
> +        "sysclk"      : CODEC SYSCLK
> +        "^bclk[1-4]$" : Bit clocks to CODEC

No, that does not look correct. You allow anything as clock input (even
20 clocks, different names, any order). That's not how DT schema should
work and that's not how hardware looks like.

Usually the clock inputs are always there which also you mentioned in
description - "multiple clock inputs". All these clocks should be
expected, unless really the wires (physical wires) can be left disconnected.

Best regards,
Krzysztof
Sameer Pujar March 28, 2022, 7:58 a.m. UTC | #2
On 28-03-2022 12:36, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
>
>
> On 28/03/2022 08:14, Sameer Pujar wrote:
>> The rt5658 or rt5659 CODEC system clock (SYSCLK) can be derived from
>> various clock sources. For example it can be derived either from master
>> clock (MCLK) or by internal PLL. The internal PLL again can take input
>> clock references from bit clocks (BCLKs) and MCLK. To enable a flexible
>> clocking configuration the DT binding is extended here.
>>
>> It makes use of standard clock bindings and sets up the clock relation
>> via DT.
>>
>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>> Cc: Oder Chiou <oder_chiou@realtek.com>
>> ---
>>   .../devicetree/bindings/sound/realtek,rt5659.yaml  | 53 ++++++++++++++++++++--
>>   1 file changed, 49 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>> index b0485b8..0c2f3cb 100644
>> --- a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>> @@ -29,12 +29,28 @@ properties:
>>       maxItems: 1
>>
>>     clocks:
>> -    items:
>> -      - description: Master clock (MCLK) to the CODEC
>> +    description: |
>> +      CODEC can receive multiple clock inputs like Master
>> +      clock (MCLK), I2S bit clocks (BCLK1, BCLK2, BCLK3,
>> +      BCLK4). The CODEC SYSCLK can be generated from MCLK
>> +      or internal PLL. In turn PLL can reference from MCLK
>> +      and BCLKs.
>>
>>     clock-names:
>> -    items:
>> -      - const: mclk
>> +    description: |
>> +      The clock names can be combination of following:
>> +        "mclk"        : Master clock
>> +        "pll_ref"     : Reference to CODEC PLL clock
>> +        "sysclk"      : CODEC SYSCLK
>> +        "^bclk[1-4]$" : Bit clocks to CODEC
> No, that does not look correct. You allow anything as clock input (even
> 20 clocks, different names, any order). That's not how DT schema should
> work and that's not how hardware looks like.

>
> Usually the clock inputs are always there which also you mentioned in
> description - "multiple clock inputs". All these clocks should be
> expected, unless really the wires (physical wires) can be left disconnected.

The CODEC can receive multiple clocks but all the input clocks need not 
be present or connected always. If a specific configuration is needed 
and platform supports such an input, then all these inputs can be added. 
I don't know how to define this detail in the schema. If I make all of 
them expected, then binding check throws errors. If I were to list all 
the possible combinations, the list is going to be big (not sure if this 
would be OK?).
Krzysztof Kozlowski March 28, 2022, 8:07 a.m. UTC | #3
On 28/03/2022 09:58, Sameer Pujar wrote:
> 
> On 28-03-2022 12:36, Krzysztof Kozlowski wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 28/03/2022 08:14, Sameer Pujar wrote:
>>> The rt5658 or rt5659 CODEC system clock (SYSCLK) can be derived from
>>> various clock sources. For example it can be derived either from master
>>> clock (MCLK) or by internal PLL. The internal PLL again can take input
>>> clock references from bit clocks (BCLKs) and MCLK. To enable a flexible
>>> clocking configuration the DT binding is extended here.
>>>
>>> It makes use of standard clock bindings and sets up the clock relation
>>> via DT.
>>>
>>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>>> Cc: Oder Chiou <oder_chiou@realtek.com>
>>> ---
>>>   .../devicetree/bindings/sound/realtek,rt5659.yaml  | 53 ++++++++++++++++++++--
>>>   1 file changed, 49 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>> index b0485b8..0c2f3cb 100644
>>> --- a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>> @@ -29,12 +29,28 @@ properties:
>>>       maxItems: 1
>>>
>>>     clocks:
>>> -    items:
>>> -      - description: Master clock (MCLK) to the CODEC
>>> +    description: |
>>> +      CODEC can receive multiple clock inputs like Master
>>> +      clock (MCLK), I2S bit clocks (BCLK1, BCLK2, BCLK3,
>>> +      BCLK4). The CODEC SYSCLK can be generated from MCLK
>>> +      or internal PLL. In turn PLL can reference from MCLK
>>> +      and BCLKs.
>>>
>>>     clock-names:
>>> -    items:
>>> -      - const: mclk
>>> +    description: |
>>> +      The clock names can be combination of following:
>>> +        "mclk"        : Master clock
>>> +        "pll_ref"     : Reference to CODEC PLL clock
>>> +        "sysclk"      : CODEC SYSCLK
>>> +        "^bclk[1-4]$" : Bit clocks to CODEC
>> No, that does not look correct. You allow anything as clock input (even
>> 20 clocks, different names, any order). That's not how DT schema should
>> work and that's not how hardware looks like.
> 
>>
>> Usually the clock inputs are always there which also you mentioned in
>> description - "multiple clock inputs". All these clocks should be
>> expected, unless really the wires (physical wires) can be left disconnected.
> 
> The CODEC can receive multiple clocks but all the input clocks need not 
> be present or connected always. If a specific configuration is needed 
> and platform supports such an input, then all these inputs can be added. 
> I don't know how to define this detail in the schema. If I make all of 
> them expected, then binding check throws errors. If I were to list all 
> the possible combinations, the list is going to be big (not sure if this 
> would be OK?).

Thanks for explanation. Please differentiate between these two:
1. clock inputs connected, but unused (not needed for driver or for
particular use case),
2. clock inputs really not connected.

For the 1. above, such clock inputs should still be listed in the
bindings and DTS. For the 2. above, such clocks should actually not be
there. How to achieve this depends on number of your combinations. IOW,
how many clocks are physically optional. For some small number of
variations this can be:
oneOf:
 - const: mclk
 - items:
   - const: mclk
   - enum:
       - bclk1
       - bclk2
       - bclk3
       - bclk4
 - items:
   - const: mclk
   - const: pll_ref
   - enum:
       - bclk1
       - bclk2
       - bclk3
       - bclk4

For a total flexibility that any clock input can be disconnected, this
should be a list of enums I guess (with minItems). However please find
the clocks always connected and include them if possible in a fixed way
(like this oneOf above).


Best regards,
Krzysztof
Sameer Pujar March 28, 2022, 1:19 p.m. UTC | #4
On 28-03-2022 13:37, Krzysztof Kozlowski wrote:
> On 28/03/2022 09:58, Sameer Pujar wrote:
>> On 28-03-2022 12:36, Krzysztof Kozlowski wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 28/03/2022 08:14, Sameer Pujar wrote:
>>>> The rt5658 or rt5659 CODEC system clock (SYSCLK) can be derived from
>>>> various clock sources. For example it can be derived either from master
>>>> clock (MCLK) or by internal PLL. The internal PLL again can take input
>>>> clock references from bit clocks (BCLKs) and MCLK. To enable a flexible
>>>> clocking configuration the DT binding is extended here.
>>>>
>>>> It makes use of standard clock bindings and sets up the clock relation
>>>> via DT.
>>>>
>>>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>>>> Cc: Oder Chiou <oder_chiou@realtek.com>
>>>> ---
>>>>    .../devicetree/bindings/sound/realtek,rt5659.yaml  | 53 ++++++++++++++++++++--
>>>>    1 file changed, 49 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>>> index b0485b8..0c2f3cb 100644
>>>> --- a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>>> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>>> @@ -29,12 +29,28 @@ properties:
>>>>        maxItems: 1
>>>>
>>>>      clocks:
>>>> -    items:
>>>> -      - description: Master clock (MCLK) to the CODEC
>>>> +    description: |
>>>> +      CODEC can receive multiple clock inputs like Master
>>>> +      clock (MCLK), I2S bit clocks (BCLK1, BCLK2, BCLK3,
>>>> +      BCLK4). The CODEC SYSCLK can be generated from MCLK
>>>> +      or internal PLL. In turn PLL can reference from MCLK
>>>> +      and BCLKs.
>>>>
>>>>      clock-names:
>>>> -    items:
>>>> -      - const: mclk
>>>> +    description: |
>>>> +      The clock names can be combination of following:
>>>> +        "mclk"        : Master clock
>>>> +        "pll_ref"     : Reference to CODEC PLL clock
>>>> +        "sysclk"      : CODEC SYSCLK
>>>> +        "^bclk[1-4]$" : Bit clocks to CODEC
>>> No, that does not look correct. You allow anything as clock input (even
>>> 20 clocks, different names, any order). That's not how DT schema should
>>> work and that's not how hardware looks like.
>>> Usually the clock inputs are always there which also you mentioned in
>>> description - "multiple clock inputs". All these clocks should be
>>> expected, unless really the wires (physical wires) can be left disconnected.
>> The CODEC can receive multiple clocks but all the input clocks need not
>> be present or connected always. If a specific configuration is needed
>> and platform supports such an input, then all these inputs can be added.
>> I don't know how to define this detail in the schema. If I make all of
>> them expected, then binding check throws errors. If I were to list all
>> the possible combinations, the list is going to be big (not sure if this
>> would be OK?).
> Thanks for explanation. Please differentiate between these two:
> 1. clock inputs connected, but unused (not needed for driver or for
> particular use case),
> 2. clock inputs really not connected.
>
> For the 1. above, such clock inputs should still be listed in the
> bindings and DTS. For the 2. above, such clocks should actually not be
> there.

Thank you for the suggestion.

> How to achieve this depends on number of your combinations. IOW,
> how many clocks are physically optional.

 From CODEC point of view all these clock inputs are possible and a 
platform may choose to connect a subset of it depending on the 
application. The binding is expected to support all such cases. To 
support all possibilities, the total combinations can be very big (100+).

> For some small number of
> variations this can be:
> oneOf:
>   - const: mclk
>   - items:
>     - const: mclk
>     - enum:
>         - bclk1
>         - bclk2
>         - bclk3
>         - bclk4
>   - items:
>     - const: mclk
>     - const: pll_ref
>     - enum:
>         - bclk1
>         - bclk2
>         - bclk3
>         - bclk4
>
> For a total flexibility that any clock input can be disconnected, this
> should be a list of enums I guess (with minItems). However please find
> the clocks always connected and include them if possible in a fixed way
> (like this oneOf above).

May be I can list the most commonly required combinations like below and 
extend it whenever there is a need for specific combination?

   clock-names:
     oneOf:
       - const: mclk
       - pattern: '^bclk[1-4]$'
       - items:
           - const: mclk
           - pattern: '^bclk[1-4]$'
       - items:
           - const: mclk
           - const: sysclk
       - items:
           - const: mclk
           - const: pll_ref
           - const: sysclk
       - items:
           - pattern: '^bclk[1-4]$'
           - const: pll_ref
           - const: sysclk
       - items:
           - const: mclk
           - pattern: '^bclk[1-4]$'
           - const: pll_ref
           - const: sysclk
Krzysztof Kozlowski March 28, 2022, 1:28 p.m. UTC | #5
On 28/03/2022 15:19, Sameer Pujar wrote:
> 
> On 28-03-2022 13:37, Krzysztof Kozlowski wrote:
>> On 28/03/2022 09:58, Sameer Pujar wrote:
>>> On 28-03-2022 12:36, Krzysztof Kozlowski wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On 28/03/2022 08:14, Sameer Pujar wrote:
>>>>> The rt5658 or rt5659 CODEC system clock (SYSCLK) can be derived from
>>>>> various clock sources. For example it can be derived either from master
>>>>> clock (MCLK) or by internal PLL. The internal PLL again can take input
>>>>> clock references from bit clocks (BCLKs) and MCLK. To enable a flexible
>>>>> clocking configuration the DT binding is extended here.
>>>>>
>>>>> It makes use of standard clock bindings and sets up the clock relation
>>>>> via DT.
>>>>>
>>>>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>>>>> Cc: Oder Chiou <oder_chiou@realtek.com>
>>>>> ---
>>>>>    .../devicetree/bindings/sound/realtek,rt5659.yaml  | 53 ++++++++++++++++++++--
>>>>>    1 file changed, 49 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>>>> index b0485b8..0c2f3cb 100644
>>>>> --- a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>>>> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>>>> @@ -29,12 +29,28 @@ properties:
>>>>>        maxItems: 1
>>>>>
>>>>>      clocks:
>>>>> -    items:
>>>>> -      - description: Master clock (MCLK) to the CODEC
>>>>> +    description: |
>>>>> +      CODEC can receive multiple clock inputs like Master
>>>>> +      clock (MCLK), I2S bit clocks (BCLK1, BCLK2, BCLK3,
>>>>> +      BCLK4). The CODEC SYSCLK can be generated from MCLK
>>>>> +      or internal PLL. In turn PLL can reference from MCLK
>>>>> +      and BCLKs.
>>>>>
>>>>>      clock-names:
>>>>> -    items:
>>>>> -      - const: mclk
>>>>> +    description: |
>>>>> +      The clock names can be combination of following:
>>>>> +        "mclk"        : Master clock
>>>>> +        "pll_ref"     : Reference to CODEC PLL clock
>>>>> +        "sysclk"      : CODEC SYSCLK
>>>>> +        "^bclk[1-4]$" : Bit clocks to CODEC
>>>> No, that does not look correct. You allow anything as clock input (even
>>>> 20 clocks, different names, any order). That's not how DT schema should
>>>> work and that's not how hardware looks like.
>>>> Usually the clock inputs are always there which also you mentioned in
>>>> description - "multiple clock inputs". All these clocks should be
>>>> expected, unless really the wires (physical wires) can be left disconnected.
>>> The CODEC can receive multiple clocks but all the input clocks need not
>>> be present or connected always. If a specific configuration is needed
>>> and platform supports such an input, then all these inputs can be added.
>>> I don't know how to define this detail in the schema. If I make all of
>>> them expected, then binding check throws errors. If I were to list all
>>> the possible combinations, the list is going to be big (not sure if this
>>> would be OK?).
>> Thanks for explanation. Please differentiate between these two:
>> 1. clock inputs connected, but unused (not needed for driver or for
>> particular use case),
>> 2. clock inputs really not connected.
>>
>> For the 1. above, such clock inputs should still be listed in the
>> bindings and DTS. For the 2. above, such clocks should actually not be
>> there.
> 
> Thank you for the suggestion.
> 
>> How to achieve this depends on number of your combinations. IOW,
>> how many clocks are physically optional.
> 
>  From CODEC point of view all these clock inputs are possible and a 
> platform may choose to connect a subset of it depending on the 
> application. The binding is expected to support all such cases. To 
> support all possibilities, the total combinations can be very big (100+).
> 
>> For some small number of
>> variations this can be:
>> oneOf:
>>   - const: mclk
>>   - items:
>>     - const: mclk
>>     - enum:
>>         - bclk1
>>         - bclk2
>>         - bclk3
>>         - bclk4
>>   - items:
>>     - const: mclk
>>     - const: pll_ref
>>     - enum:
>>         - bclk1
>>         - bclk2
>>         - bclk3
>>         - bclk4
>>
>> For a total flexibility that any clock input can be disconnected, this
>> should be a list of enums I guess (with minItems). However please find
>> the clocks always connected and include them if possible in a fixed way
>> (like this oneOf above).
> 
> May be I can list the most commonly required combinations like below and 
> extend it whenever there is a need for specific combination?

Yes, this would work. Relaxing such constraints is possible.


Best regards,
Krzysztof
Sameer Pujar March 29, 2022, 8:27 a.m. UTC | #6
On 28-03-2022 18:58, Krzysztof Kozlowski wrote:
> On 28/03/2022 15:19, Sameer Pujar wrote:
>> On 28-03-2022 13:37, Krzysztof Kozlowski wrote:
>>> On 28/03/2022 09:58, Sameer Pujar wrote:
>>>> On 28-03-2022 12:36, Krzysztof Kozlowski wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> On 28/03/2022 08:14, Sameer Pujar wrote:
>>>>>> The rt5658 or rt5659 CODEC system clock (SYSCLK) can be derived from
>>>>>> various clock sources. For example it can be derived either from master
>>>>>> clock (MCLK) or by internal PLL. The internal PLL again can take input
>>>>>> clock references from bit clocks (BCLKs) and MCLK. To enable a flexible
>>>>>> clocking configuration the DT binding is extended here.
>>>>>>
>>>>>> It makes use of standard clock bindings and sets up the clock relation
>>>>>> via DT.
>>>>>>
>>>>>> Signed-off-by: Sameer Pujar<spujar@nvidia.com>
>>>>>> Cc: Oder Chiou<oder_chiou@realtek.com>
>>>>>> ---
>>>>>>     .../devicetree/bindings/sound/realtek,rt5659.yaml  | 53 ++++++++++++++++++++--
>>>>>>     1 file changed, 49 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>>>>> index b0485b8..0c2f3cb 100644
>>>>>> --- a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
>>>>>> @@ -29,12 +29,28 @@ properties:
>>>>>>         maxItems: 1
>>>>>>
>>>>>>       clocks:
>>>>>> -    items:
>>>>>> -      - description: Master clock (MCLK) to the CODEC
>>>>>> +    description: |
>>>>>> +      CODEC can receive multiple clock inputs like Master
>>>>>> +      clock (MCLK), I2S bit clocks (BCLK1, BCLK2, BCLK3,
>>>>>> +      BCLK4). The CODEC SYSCLK can be generated from MCLK
>>>>>> +      or internal PLL. In turn PLL can reference from MCLK
>>>>>> +      and BCLKs.
>>>>>>
>>>>>>       clock-names:
>>>>>> -    items:
>>>>>> -      - const: mclk
>>>>>> +    description: |
>>>>>> +      The clock names can be combination of following:
>>>>>> +        "mclk"        : Master clock
>>>>>> +        "pll_ref"     : Reference to CODEC PLL clock
>>>>>> +        "sysclk"      : CODEC SYSCLK
>>>>>> +        "^bclk[1-4]$" : Bit clocks to CODEC
>>>>> No, that does not look correct. You allow anything as clock input (even
>>>>> 20 clocks, different names, any order). That's not how DT schema should
>>>>> work and that's not how hardware looks like.
>>>>> Usually the clock inputs are always there which also you mentioned in
>>>>> description - "multiple clock inputs". All these clocks should be
>>>>> expected, unless really the wires (physical wires) can be left disconnected.
>>>> The CODEC can receive multiple clocks but all the input clocks need not
>>>> be present or connected always. If a specific configuration is needed
>>>> and platform supports such an input, then all these inputs can be added.
>>>> I don't know how to define this detail in the schema. If I make all of
>>>> them expected, then binding check throws errors. If I were to list all
>>>> the possible combinations, the list is going to be big (not sure if this
>>>> would be OK?).
>>> Thanks for explanation. Please differentiate between these two:
>>> 1. clock inputs connected, but unused (not needed for driver or for
>>> particular use case),
>>> 2. clock inputs really not connected.
>>>
>>> For the 1. above, such clock inputs should still be listed in the
>>> bindings and DTS. For the 2. above, such clocks should actually not be
>>> there.
>> Thank you for the suggestion.
>>
>>> How to achieve this depends on number of your combinations. IOW,
>>> how many clocks are physically optional.
>>   From CODEC point of view all these clock inputs are possible and a
>> platform may choose to connect a subset of it depending on the
>> application. The binding is expected to support all such cases. To
>> support all possibilities, the total combinations can be very big (100+).
>>
>>> For some small number of
>>> variations this can be:
>>> oneOf:
>>>    - const: mclk
>>>    - items:
>>>      - const: mclk
>>>      - enum:
>>>          - bclk1
>>>          - bclk2
>>>          - bclk3
>>>          - bclk4
>>>    - items:
>>>      - const: mclk
>>>      - const: pll_ref
>>>      - enum:
>>>          - bclk1
>>>          - bclk2
>>>          - bclk3
>>>          - bclk4
>>>
>>> For a total flexibility that any clock input can be disconnected, this
>>> should be a list of enums I guess (with minItems). However please find
>>> the clocks always connected and include them if possible in a fixed way
>>> (like this oneOf above).
>> May be I can list the most commonly required combinations like below and
>> extend it whenever there is a need for specific combination?
> Yes, this would work. Relaxing such constraints is possible.

Thanks. I will update this in next revision.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
index b0485b8..0c2f3cb 100644
--- a/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
+++ b/Documentation/devicetree/bindings/sound/realtek,rt5659.yaml
@@ -29,12 +29,28 @@  properties:
     maxItems: 1
 
   clocks:
-    items:
-      - description: Master clock (MCLK) to the CODEC
+    description: |
+      CODEC can receive multiple clock inputs like Master
+      clock (MCLK), I2S bit clocks (BCLK1, BCLK2, BCLK3,
+      BCLK4). The CODEC SYSCLK can be generated from MCLK
+      or internal PLL. In turn PLL can reference from MCLK
+      and BCLKs.
 
   clock-names:
-    items:
-      - const: mclk
+    description: |
+      The clock names can be combination of following:
+        "mclk"        : Master clock
+        "pll_ref"     : Reference to CODEC PLL clock
+        "sysclk"      : CODEC SYSCLK
+        "^bclk[1-4]$" : Bit clocks to CODEC
+
+  "#clock-cells":
+    const: 1
+
+  clock-output-names:
+    description: PLL output clock
+    $ref: /schemas/types.yaml#/definitions/string
+    const: rt5659_pll_out
 
   realtek,in1-differential:
     description: MIC1 input is differntial and not single-ended.
@@ -97,6 +113,7 @@  required:
 
 examples:
   - |
+    /* SYSCLK derived from MCLK */
     #include<dt-bindings/gpio/tegra194-gpio.h>
     #include<dt-bindings/clock/tegra194-clock.h>
 
@@ -114,3 +131,31 @@  examples:
             realtek,jd-src = <2>;
         };
     };
+
+  - |
+    /*
+     * SYSCLK is derived from CODEC internal PLL and PLL uses I2S1 BCLK
+     * as the clock reference.
+     */
+    #include<dt-bindings/gpio/tegra194-gpio.h>
+    #include<dt-bindings/clock/tegra194-clock.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rt5658: audio-codec@1a {
+            compatible = "realtek,rt5658";
+            reg = <0x1a>;
+            interrupt-parent = <&gpio>;
+            interrupts = <TEGRA194_MAIN_GPIO(S, 5) GPIO_ACTIVE_HIGH>;
+            clocks = <&bpmp TEGRA194_CLK_AUD_MCLK>,
+                     <&bpmp TEGRA194_CLK_I2S1>,
+                     <&bpmp TEGRA194_CLK_I2S1>,
+                     <&rt5658 0>;
+            clock-names = "mclk", "bclk1", "pll_ref", "sysclk";
+            #clock-cells = <1>;
+            clock-output-names = "rt5659_pll_out";
+            realtek,jd-src = <2>;
+        };
+    };