[PATCH RFC v2 2/6] ASoC: dt-bindings: qcom,sm8250: Add clocks properties for I2S

Neil Armstrong posted 6 patches 2 months ago
[PATCH RFC v2 2/6] ASoC: dt-bindings: qcom,sm8250: Add clocks properties for I2S
Posted by Neil Armstrong 2 months ago
In order to describe the block and master clock of each I2S bus, add
the first 5 I2S busses clock entries.

The names (primary, secondary, tertiarty, quaternary, quinary, senary)
uses the LPASS clock naming which were used for a long time on Qualcomm
LPASS firmware interfaces.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../devicetree/bindings/sound/qcom,sm8250.yaml      | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
index 8ac91625dce5ccba5c5f31748c36296b12fac1a6..d1420d138b7ed8152aa53769c4d495e1674275e6 100644
--- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
@@ -64,6 +64,27 @@ properties:
     $ref: /schemas/types.yaml#/definitions/string
     description: User visible long sound card name
 
+  clocks:
+    minItems: 2
+    maxItems: 12
+
+  clock-names:
+    minItems: 2
+    items:
+      # mclk is the I2S Master Clock, mi2s the I2S Bit Clock
+      - const: primary-mi2s
+      - const: primary-mclk
+      - const: secondary-mi2s
+      - const: secondary-mclk
+      - const: tertiary-mi2s
+      - const: tertiary-mclk
+      - const: quaternary-mi2s
+      - const: quaternary-mclk
+      - const: quinary-mi2s
+      - const: quinary-mclk
+      - const: senary-mi2s
+      - const: senary-mclk
+
 patternProperties:
   ".*-dai-link$":
     description:

-- 
2.34.1
Re: [PATCH RFC v2 2/6] ASoC: dt-bindings: qcom,sm8250: Add clocks properties for I2S
Posted by Srinivas Kandagatla 1 month, 4 weeks ago

On 10/8/25 7:56 PM, Neil Armstrong wrote:
> In order to describe the block and master clock of each I2S bus, add
> the first 5 I2S busses clock entries.
> 
> The names (primary, secondary, tertiarty, quaternary, quinary, senary)
> uses the LPASS clock naming which were used for a long time on Qualcomm
> LPASS firmware interfaces.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../devicetree/bindings/sound/qcom,sm8250.yaml      | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
> index 8ac91625dce5ccba5c5f31748c36296b12fac1a6..d1420d138b7ed8152aa53769c4d495e1674275e6 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
> @@ -64,6 +64,27 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/string
>      description: User visible long sound card name
>  
> +  clocks:
> +    minItems: 2
> +    maxItems: 12
> +
> +  clock-names:
> +    minItems: 2
> +    items:
> +      # mclk is the I2S Master Clock, mi2s the I2S Bit Clock
> +      - const: primary-mi2s
> +      - const: primary-mclk
> +      - const: secondary-mi2s
> +      - const: secondary-mclk
> +      - const: tertiary-mi2s
> +      - const: tertiary-mclk
> +      - const: quaternary-mi2s
> +      - const: quaternary-mclk
> +      - const: quinary-mi2s
> +      - const: quinary-mclk
> +      - const: senary-mi2s
> +      - const: senary-mclk
> +

I don't this is correct way to handling bitclk and mclks for I2S, these
are normally handled as part of snd_soc_dai_set_sysclk() transparently
without need of any device tree description.

Also doing this way is an issue as this is going to break existing Elite
based platforms, and the device description should not change across
these both audio firmwares.

thanks,
Srini

>  patternProperties:
>    ".*-dai-link$":
>      description:
>
Re: [PATCH RFC v2 2/6] ASoC: dt-bindings: qcom,sm8250: Add clocks properties for I2S
Posted by Neil Armstrong 1 month, 4 weeks ago
On 10/9/25 15:36, Srinivas Kandagatla wrote:
> 
> 
> On 10/8/25 7:56 PM, Neil Armstrong wrote:
>> In order to describe the block and master clock of each I2S bus, add
>> the first 5 I2S busses clock entries.
>>
>> The names (primary, secondary, tertiarty, quaternary, quinary, senary)
>> uses the LPASS clock naming which were used for a long time on Qualcomm
>> LPASS firmware interfaces.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   .../devicetree/bindings/sound/qcom,sm8250.yaml      | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>> index 8ac91625dce5ccba5c5f31748c36296b12fac1a6..d1420d138b7ed8152aa53769c4d495e1674275e6 100644
>> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>> @@ -64,6 +64,27 @@ properties:
>>       $ref: /schemas/types.yaml#/definitions/string
>>       description: User visible long sound card name
>>   
>> +  clocks:
>> +    minItems: 2
>> +    maxItems: 12
>> +
>> +  clock-names:
>> +    minItems: 2
>> +    items:
>> +      # mclk is the I2S Master Clock, mi2s the I2S Bit Clock
>> +      - const: primary-mi2s
>> +      - const: primary-mclk
>> +      - const: secondary-mi2s
>> +      - const: secondary-mclk
>> +      - const: tertiary-mi2s
>> +      - const: tertiary-mclk
>> +      - const: quaternary-mi2s
>> +      - const: quaternary-mclk
>> +      - const: quinary-mi2s
>> +      - const: quinary-mclk
>> +      - const: senary-mi2s
>> +      - const: senary-mclk
>> +
> 
> I don't this is correct way to handling bitclk and mclks for I2S, these
> are normally handled as part of snd_soc_dai_set_sysclk() transparently
> without need of any device tree description.
> 
> Also doing this way is an issue as this is going to break existing Elite
> based platforms, and the device description should not change across
> these both audio firmwares.

This is only for AudioReach platforms, on those platforms the
clocks are registered in DT and are not accessible by the card.

Device description is obviously different for the AudioReach platforms.

Neil

> 
> thanks,
> Srini
> 
>>   patternProperties:
>>     ".*-dai-link$":
>>       description:
>>
>
Re: [PATCH RFC v2 2/6] ASoC: dt-bindings: qcom,sm8250: Add clocks properties for I2S
Posted by Srinivas Kandagatla 1 month, 4 weeks ago

On 10/9/25 3:03 PM, Neil Armstrong wrote:
> On 10/9/25 15:36, Srinivas Kandagatla wrote:
>>
>>
>> On 10/8/25 7:56 PM, Neil Armstrong wrote:
>>> In order to describe the block and master clock of each I2S bus, add
>>> the first 5 I2S busses clock entries.
>>>
>>> The names (primary, secondary, tertiarty, quaternary, quinary, senary)
>>> uses the LPASS clock naming which were used for a long time on Qualcomm
>>> LPASS firmware interfaces.
>>>
>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>> ---
>>>   .../devicetree/bindings/sound/qcom,sm8250.yaml      | 21 ++++++++++
>>> +++++++++++
>>>   1 file changed, 21 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>> b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>> index
>>> 8ac91625dce5ccba5c5f31748c36296b12fac1a6..d1420d138b7ed8152aa53769c4d495e1674275e6 100644
>>> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>> @@ -64,6 +64,27 @@ properties:
>>>       $ref: /schemas/types.yaml#/definitions/string
>>>       description: User visible long sound card name
>>>   +  clocks:
>>> +    minItems: 2
>>> +    maxItems: 12
>>> +
>>> +  clock-names:
>>> +    minItems: 2
>>> +    items:
>>> +      # mclk is the I2S Master Clock, mi2s the I2S Bit Clock
>>> +      - const: primary-mi2s
>>> +      - const: primary-mclk
>>> +      - const: secondary-mi2s
>>> +      - const: secondary-mclk
>>> +      - const: tertiary-mi2s
>>> +      - const: tertiary-mclk
>>> +      - const: quaternary-mi2s
>>> +      - const: quaternary-mclk
>>> +      - const: quinary-mi2s
>>> +      - const: quinary-mclk
>>> +      - const: senary-mi2s
>>> +      - const: senary-mclk
>>> +
>>
>> I don't this is correct way to handling bitclk and mclks for I2S, these
>> are normally handled as part of snd_soc_dai_set_sysclk() transparently
>> without need of any device tree description.
>>
>> Also doing this way is an issue as this is going to break existing Elite
>> based platforms, and the device description should not change across
>> these both audio firmwares.
> 
> This is only for AudioReach platforms, on those platforms the
> clocks are registered in DT and are not accessible by the card.
> 
Clocks will be acessable via snd_soc_dai_set_sysclk ->
q6prm_set_lpass_clock once set_sysclk support is added to q6apm-lpass
i2s dai ops.


> Device description is obviously different for the AudioReach platforms.

Why should it be different, its same device.
We have platforms that use both Elite and Audioreach.

--srini
> 
> Neil
> 
>>
>> thanks,
>> Srini
>>
>>>   patternProperties:
>>>     ".*-dai-link$":
>>>       description:
>>>
>>
> 

Re: [PATCH RFC v2 2/6] ASoC: dt-bindings: qcom,sm8250: Add clocks properties for I2S
Posted by Neil Armstrong 1 month, 4 weeks ago
On 10/9/25 16:06, Srinivas Kandagatla wrote:
> 
> 
> On 10/9/25 3:03 PM, Neil Armstrong wrote:
>> On 10/9/25 15:36, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 10/8/25 7:56 PM, Neil Armstrong wrote:
>>>> In order to describe the block and master clock of each I2S bus, add
>>>> the first 5 I2S busses clock entries.
>>>>
>>>> The names (primary, secondary, tertiarty, quaternary, quinary, senary)
>>>> uses the LPASS clock naming which were used for a long time on Qualcomm
>>>> LPASS firmware interfaces.
>>>>
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> ---
>>>>    .../devicetree/bindings/sound/qcom,sm8250.yaml      | 21 ++++++++++
>>>> +++++++++++
>>>>    1 file changed, 21 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>> b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>> index
>>>> 8ac91625dce5ccba5c5f31748c36296b12fac1a6..d1420d138b7ed8152aa53769c4d495e1674275e6 100644
>>>> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>> @@ -64,6 +64,27 @@ properties:
>>>>        $ref: /schemas/types.yaml#/definitions/string
>>>>        description: User visible long sound card name
>>>>    +  clocks:
>>>> +    minItems: 2
>>>> +    maxItems: 12
>>>> +
>>>> +  clock-names:
>>>> +    minItems: 2
>>>> +    items:
>>>> +      # mclk is the I2S Master Clock, mi2s the I2S Bit Clock
>>>> +      - const: primary-mi2s
>>>> +      - const: primary-mclk
>>>> +      - const: secondary-mi2s
>>>> +      - const: secondary-mclk
>>>> +      - const: tertiary-mi2s
>>>> +      - const: tertiary-mclk
>>>> +      - const: quaternary-mi2s
>>>> +      - const: quaternary-mclk
>>>> +      - const: quinary-mi2s
>>>> +      - const: quinary-mclk
>>>> +      - const: senary-mi2s
>>>> +      - const: senary-mclk
>>>> +
>>>
>>> I don't this is correct way to handling bitclk and mclks for I2S, these
>>> are normally handled as part of snd_soc_dai_set_sysclk() transparently
>>> without need of any device tree description.
>>>
>>> Also doing this way is an issue as this is going to break existing Elite
>>> based platforms, and the device description should not change across
>>> these both audio firmwares.
>>
>> This is only for AudioReach platforms, on those platforms the
>> clocks are registered in DT and are not accessible by the card.
>>
> Clocks will be acessable via snd_soc_dai_set_sysclk ->
> q6prm_set_lpass_clock once set_sysclk support is added to q6apm-lpass
> i2s dai ops.
> 
> 
>> Device description is obviously different for the AudioReach platforms.
> 
> Why should it be different, its same device.
> We have platforms that use both Elite and Audioreach.

I'm perfectly aware of that, it's the case for sc7280/qcm6490. And I agree
the card bindings is the same, but it doesn't mean the DSP elements are the
same and uses in the same manner.

So let's forget the bindings and forget those clocks entries, and imagine
I'll implement those _sys_sysclk calls like for the Elite platforms.
This means I'll bypass the clock framework by directly setting the PRM
clocks, this is clearly a layer violation.

Neil

> 
> --srini
>>
>> Neil
>>
>>>
>>> thanks,
>>> Srini
>>>
>>>>    patternProperties:
>>>>      ".*-dai-link$":
>>>>        description:
>>>>
>>>
>>
> 

Re: [PATCH RFC v2 2/6] ASoC: dt-bindings: qcom,sm8250: Add clocks properties for I2S
Posted by Srinivas Kandagatla 1 month, 4 weeks ago

On 10/9/25 3:25 PM, Neil Armstrong wrote:
> On 10/9/25 16:06, Srinivas Kandagatla wrote:
>>
>>
>> On 10/9/25 3:03 PM, Neil Armstrong wrote:
>>> On 10/9/25 15:36, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 10/8/25 7:56 PM, Neil Armstrong wrote:
>>>>> In order to describe the block and master clock of each I2S bus, add
>>>>> the first 5 I2S busses clock entries.
>>>>>
>>>>> The names (primary, secondary, tertiarty, quaternary, quinary, senary)
>>>>> uses the LPASS clock naming which were used for a long time on
>>>>> Qualcomm
>>>>> LPASS firmware interfaces.
>>>>>
>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>>> ---
>>>>>    .../devicetree/bindings/sound/qcom,sm8250.yaml      | 21 ++++++++++
>>>>> +++++++++++
>>>>>    1 file changed, 21 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>> b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>> index
>>>>> 8ac91625dce5ccba5c5f31748c36296b12fac1a6..d1420d138b7ed8152aa53769c4d495e1674275e6 100644
>>>>> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>> @@ -64,6 +64,27 @@ properties:
>>>>>        $ref: /schemas/types.yaml#/definitions/string
>>>>>        description: User visible long sound card name
>>>>>    +  clocks:
>>>>> +    minItems: 2
>>>>> +    maxItems: 12
>>>>> +
>>>>> +  clock-names:
>>>>> +    minItems: 2
>>>>> +    items:
>>>>> +      # mclk is the I2S Master Clock, mi2s the I2S Bit Clock
>>>>> +      - const: primary-mi2s
>>>>> +      - const: primary-mclk
>>>>> +      - const: secondary-mi2s
>>>>> +      - const: secondary-mclk
>>>>> +      - const: tertiary-mi2s
>>>>> +      - const: tertiary-mclk
>>>>> +      - const: quaternary-mi2s
>>>>> +      - const: quaternary-mclk
>>>>> +      - const: quinary-mi2s
>>>>> +      - const: quinary-mclk
>>>>> +      - const: senary-mi2s
>>>>> +      - const: senary-mclk
>>>>> +
>>>>
>>>> I don't this is correct way to handling bitclk and mclks for I2S, these
>>>> are normally handled as part of snd_soc_dai_set_sysclk() transparently
>>>> without need of any device tree description.
>>>>
>>>> Also doing this way is an issue as this is going to break existing
>>>> Elite
>>>> based platforms, and the device description should not change across
>>>> these both audio firmwares.
>>>
>>> This is only for AudioReach platforms, on those platforms the
>>> clocks are registered in DT and are not accessible by the card.
>>>
>> Clocks will be acessable via snd_soc_dai_set_sysclk ->
>> q6prm_set_lpass_clock once set_sysclk support is added to q6apm-lpass
>> i2s dai ops.
>>
>>
>>> Device description is obviously different for the AudioReach platforms.
>>
>> Why should it be different, its same device.
>> We have platforms that use both Elite and Audioreach.
> 
> I'm perfectly aware of that, it's the case for sc7280/qcm6490. And I agree
> the card bindings is the same, but it doesn't mean the DSP elements are the
> same and uses in the same manner.
> 
> So let's forget the bindings and forget those clocks entries, and imagine
> I'll implement those _sys_sysclk calls like for the Elite platforms.
> This means I'll bypass the clock framework by directly setting the PRM
> clocks, this is clearly a layer violation.

You can claim clocks in the dsp layer (q6apm-lpass-dais) instead of
claiming it in machine layer, it does not necessarily have to bypass the
clk framework.

--srini
> 
> Neil
> 
>>
>> --srini
>>>
>>> Neil
>>>
>>>>
>>>> thanks,
>>>> Srini
>>>>
>>>>>    patternProperties:
>>>>>      ".*-dai-link$":
>>>>>        description:
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH RFC v2 2/6] ASoC: dt-bindings: qcom,sm8250: Add clocks properties for I2S
Posted by Neil Armstrong 1 month, 4 weeks ago
On 10/9/25 16:29, Srinivas Kandagatla wrote:
> 
> 
> On 10/9/25 3:25 PM, Neil Armstrong wrote:
>> On 10/9/25 16:06, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 10/9/25 3:03 PM, Neil Armstrong wrote:
>>>> On 10/9/25 15:36, Srinivas Kandagatla wrote:
>>>>>
>>>>>
>>>>> On 10/8/25 7:56 PM, Neil Armstrong wrote:
>>>>>> In order to describe the block and master clock of each I2S bus, add
>>>>>> the first 5 I2S busses clock entries.
>>>>>>
>>>>>> The names (primary, secondary, tertiarty, quaternary, quinary, senary)
>>>>>> uses the LPASS clock naming which were used for a long time on
>>>>>> Qualcomm
>>>>>> LPASS firmware interfaces.
>>>>>>
>>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>> ---
>>>>>>     .../devicetree/bindings/sound/qcom,sm8250.yaml      | 21 ++++++++++
>>>>>> +++++++++++
>>>>>>     1 file changed, 21 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>>> b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>>> index
>>>>>> 8ac91625dce5ccba5c5f31748c36296b12fac1a6..d1420d138b7ed8152aa53769c4d495e1674275e6 100644
>>>>>> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>>> @@ -64,6 +64,27 @@ properties:
>>>>>>         $ref: /schemas/types.yaml#/definitions/string
>>>>>>         description: User visible long sound card name
>>>>>>     +  clocks:
>>>>>> +    minItems: 2
>>>>>> +    maxItems: 12
>>>>>> +
>>>>>> +  clock-names:
>>>>>> +    minItems: 2
>>>>>> +    items:
>>>>>> +      # mclk is the I2S Master Clock, mi2s the I2S Bit Clock
>>>>>> +      - const: primary-mi2s
>>>>>> +      - const: primary-mclk
>>>>>> +      - const: secondary-mi2s
>>>>>> +      - const: secondary-mclk
>>>>>> +      - const: tertiary-mi2s
>>>>>> +      - const: tertiary-mclk
>>>>>> +      - const: quaternary-mi2s
>>>>>> +      - const: quaternary-mclk
>>>>>> +      - const: quinary-mi2s
>>>>>> +      - const: quinary-mclk
>>>>>> +      - const: senary-mi2s
>>>>>> +      - const: senary-mclk
>>>>>> +
>>>>>
>>>>> I don't this is correct way to handling bitclk and mclks for I2S, these
>>>>> are normally handled as part of snd_soc_dai_set_sysclk() transparently
>>>>> without need of any device tree description.
>>>>>
>>>>> Also doing this way is an issue as this is going to break existing
>>>>> Elite
>>>>> based platforms, and the device description should not change across
>>>>> these both audio firmwares.
>>>>
>>>> This is only for AudioReach platforms, on those platforms the
>>>> clocks are registered in DT and are not accessible by the card.
>>>>
>>> Clocks will be acessable via snd_soc_dai_set_sysclk ->
>>> q6prm_set_lpass_clock once set_sysclk support is added to q6apm-lpass
>>> i2s dai ops.
>>>
>>>
>>>> Device description is obviously different for the AudioReach platforms.
>>>
>>> Why should it be different, its same device.
>>> We have platforms that use both Elite and Audioreach.
>>
>> I'm perfectly aware of that, it's the case for sc7280/qcm6490. And I agree
>> the card bindings is the same, but it doesn't mean the DSP elements are the
>> same and uses in the same manner.
>>
>> So let's forget the bindings and forget those clocks entries, and imagine
>> I'll implement those _sys_sysclk calls like for the Elite platforms.
>> This means I'll bypass the clock framework by directly setting the PRM
>> clocks, this is clearly a layer violation.
> 
> You can claim clocks in the dsp layer (q6apm-lpass-dais) instead of
> claiming it in machine layer, it does not necessarily have to bypass the
> clk framework.

The current q6afe implementation totally bypasses the clock framework:

static int q6afe_set_lpass_clock_v2(struct q6afe_port *port,
				 struct afe_clk_set *cfg)
{
	return q6afe_port_set_param(port, cfg, AFE_PARAM_ID_CLOCK_SET,
				    AFE_MODULE_CLOCK_SET, sizeof(*cfg));
}

I have no time right now to implement all that for q6apm & q6prm in the
way you propose, so I'll probably not send a new version.

Neil

> 
> --srini
>>
>> Neil
>>
>>>
>>> --srini
>>>>
>>>> Neil
>>>>
>>>>>
>>>>> thanks,
>>>>> Srini
>>>>>
>>>>>>     patternProperties:
>>>>>>       ".*-dai-link$":
>>>>>>         description:
>>>>>>
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH RFC v2 2/6] ASoC: dt-bindings: qcom,sm8250: Add clocks properties for I2S
Posted by Srinivas Kandagatla 1 month, 4 weeks ago

On 10/9/25 4:30 PM, Neil Armstrong wrote:
> On 10/9/25 16:29, Srinivas Kandagatla wrote:
>>
>>
>> On 10/9/25 3:25 PM, Neil Armstrong wrote:
>>> On 10/9/25 16:06, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 10/9/25 3:03 PM, Neil Armstrong wrote:
>>>>> On 10/9/25 15:36, Srinivas Kandagatla wrote:
>>>>>>
>>>>>>
>>>>>> On 10/8/25 7:56 PM, Neil Armstrong wrote:
>>>>>>> In order to describe the block and master clock of each I2S bus, add
>>>>>>> the first 5 I2S busses clock entries.
>>>>>>>
>>>>>>> The names (primary, secondary, tertiarty, quaternary, quinary,
>>>>>>> senary)
>>>>>>> uses the LPASS clock naming which were used for a long time on
>>>>>>> Qualcomm
>>>>>>> LPASS firmware interfaces.
>>>>>>>
>>>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>>> ---
>>>>>>>     .../devicetree/bindings/sound/qcom,sm8250.yaml      | 21 ++++
>>>>>>> ++++++
>>>>>>> +++++++++++
>>>>>>>     1 file changed, 21 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/sound/
>>>>>>> qcom,sm8250.yaml
>>>>>>> b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>>>> index
>>>>>>> 8ac91625dce5ccba5c5f31748c36296b12fac1a6..d1420d138b7ed8152aa53769c4d495e1674275e6 100644
>>>>>>> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>>>> @@ -64,6 +64,27 @@ properties:
>>>>>>>         $ref: /schemas/types.yaml#/definitions/string
>>>>>>>         description: User visible long sound card name
>>>>>>>     +  clocks:
>>>>>>> +    minItems: 2
>>>>>>> +    maxItems: 12
>>>>>>> +
>>>>>>> +  clock-names:
>>>>>>> +    minItems: 2
>>>>>>> +    items:
>>>>>>> +      # mclk is the I2S Master Clock, mi2s the I2S Bit Clock
>>>>>>> +      - const: primary-mi2s
>>>>>>> +      - const: primary-mclk
>>>>>>> +      - const: secondary-mi2s
>>>>>>> +      - const: secondary-mclk
>>>>>>> +      - const: tertiary-mi2s
>>>>>>> +      - const: tertiary-mclk
>>>>>>> +      - const: quaternary-mi2s
>>>>>>> +      - const: quaternary-mclk
>>>>>>> +      - const: quinary-mi2s
>>>>>>> +      - const: quinary-mclk
>>>>>>> +      - const: senary-mi2s
>>>>>>> +      - const: senary-mclk
>>>>>>> +
>>>>>>
>>>>>> I don't this is correct way to handling bitclk and mclks for I2S,
>>>>>> these
>>>>>> are normally handled as part of snd_soc_dai_set_sysclk()
>>>>>> transparently
>>>>>> without need of any device tree description.
>>>>>>
>>>>>> Also doing this way is an issue as this is going to break existing
>>>>>> Elite
>>>>>> based platforms, and the device description should not change across
>>>>>> these both audio firmwares.
>>>>>
>>>>> This is only for AudioReach platforms, on those platforms the
>>>>> clocks are registered in DT and are not accessible by the card.
>>>>>
>>>> Clocks will be acessable via snd_soc_dai_set_sysclk ->
>>>> q6prm_set_lpass_clock once set_sysclk support is added to q6apm-lpass
>>>> i2s dai ops.
>>>>
>>>>
>>>>> Device description is obviously different for the AudioReach
>>>>> platforms.
>>>>
>>>> Why should it be different, its same device.
>>>> We have platforms that use both Elite and Audioreach.
>>>
>>> I'm perfectly aware of that, it's the case for sc7280/qcm6490. And I
>>> agree
>>> the card bindings is the same, but it doesn't mean the DSP elements
>>> are the

> }
> 
> I have no time right now to implement all that for q6apm & q6prm in the
> way you propose, so I'll probably not send a new version.

Or Update the codec driver to handle the mclk, instead of putting this
in machine driver.
BIT clks are normally automatically setup by DSP based on interface
index, type and pcm params.

--srini

> 
> Neil
> 
>>
>> --srini
>>>
>>> Neil
>>>
>>>>
>>>> --srini
>>>>>
>>>>> Neil
>>>>>
>>>>>>
>>>>>> thanks,
>>>>>> Srini
>>>>>>
>>>>>>>     patternProperties:
>>>>>>>       ".*-dai-link$":
>>>>>>>         description:
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH RFC v2 2/6] ASoC: dt-bindings: qcom,sm8250: Add clocks properties for I2S
Posted by Srinivas Kandagatla 1 month, 4 weeks ago

On 10/9/25 4:30 PM, Neil Armstrong wrote:
> On 10/9/25 16:29, Srinivas Kandagatla wrote:
>>
>>
>> On 10/9/25 3:25 PM, Neil Armstrong wrote:
>>> On 10/9/25 16:06, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 10/9/25 3:03 PM, Neil Armstrong wrote:
>>>>> On 10/9/25 15:36, Srinivas Kandagatla wrote:
>>>>>>
>>>>>>
>>>>>> On 10/8/25 7:56 PM, Neil Armstrong wrote:
>>>>>>> In order to describe the block and master clock of each I2S bus, add
>>>>>>> the first 5 I2S busses clock entries.
>>>>>>>
>>>>>>> The names (primary, secondary, tertiarty, quaternary, quinary,
>>>>>>> senary)
>>>>>>> uses the LPASS clock naming which were used for a long time on
>>>>>>> Qualcomm
>>>>>>> LPASS firmware interfaces.
>>>>>>>
>>>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>>> ---
>>>>>>>     .../devicetree/bindings/sound/qcom,sm8250.yaml      | 21 ++++
>>>>>>> ++++++
>>>>>>> +++++++++++
>>>>>>>     1 file changed, 21 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/sound/
>>>>>>> qcom,sm8250.yaml
>>>>>>> b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>>>> index
>>>>>>> 8ac91625dce5ccba5c5f31748c36296b12fac1a6..d1420d138b7ed8152aa53769c4d495e1674275e6 100644
>>>>>>> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
>>>>>>> @@ -64,6 +64,27 @@ properties:
>>>>>>>         $ref: /schemas/types.yaml#/definitions/string
>>>>>>>         description: User visible long sound card name
>>>>>>>     +  clocks:
>>>>>>> +    minItems: 2
>>>>>>> +    maxItems: 12
>>>>>>> +
>>>>>>> +  clock-names:
>>>>>>> +    minItems: 2
>>>>>>> +    items:
>>>>>>> +      # mclk is the I2S Master Clock, mi2s the I2S Bit Clock
>>>>>>> +      - const: primary-mi2s
>>>>>>> +      - const: primary-mclk
>>>>>>> +      - const: secondary-mi2s
>>>>>>> +      - const: secondary-mclk
>>>>>>> +      - const: tertiary-mi2s
>>>>>>> +      - const: tertiary-mclk
>>>>>>> +      - const: quaternary-mi2s
>>>>>>> +      - const: quaternary-mclk
>>>>>>> +      - const: quinary-mi2s
>>>>>>> +      - const: quinary-mclk
>>>>>>> +      - const: senary-mi2s
>>>>>>> +      - const: senary-mclk
>>>>>>> +
>>>>>>
>>>>>> I don't this is correct way to handling bitclk and mclks for I2S,
>>>>>> these
>>>>>> are normally handled as part of snd_soc_dai_set_sysclk()
>>>>>> transparently
>>>>>> without need of any device tree description.
>>>>>>
>>>>>> Also doing this way is an issue as this is going to break existing
>>>>>> Elite
>>>>>> based platforms, and the device description should not change across
>>>>>> these both audio firmwares.
>>>>>
>>>>> This is only for AudioReach platforms, on those platforms the
>>>>> clocks are registered in DT and are not accessible by the card.
>>>>>
>>>> Clocks will be acessable via snd_soc_dai_set_sysclk ->
>>>> q6prm_set_lpass_clock once set_sysclk support is added to q6apm-lpass
>>>> i2s dai ops.
>>>>
>>>>
>>>>> Device description is obviously different for the AudioReach
>>>>> platforms.
>>>>
>>>> Why should it be different, its same device.
>>>> We have platforms that use both Elite and Audioreach.
>>>
>>> I'm perfectly aware of that, it's the case for sc7280/qcm6490. And I
>>> agree
>>> the card bindings is the same, but it doesn't mean the DSP elements
>>> are the
>>> same and uses in the same manner.
>>>
>>> So let's forget the bindings and forget those clocks entries, and
>>> imagine
>>> I'll implement those _sys_sysclk calls like for the Elite platforms.
>>> This means I'll bypass the clock framework by directly setting the PRM
>>> clocks, this is clearly a layer violation.
>>
>> You can claim clocks in the dsp layer (q6apm-lpass-dais) instead of
>> claiming it in machine layer, it does not necessarily have to bypass the
>> clk framework.
> 
> The current q6afe implementation totally bypasses the clock framework:
> 
> static int q6afe_set_lpass_clock_v2(struct q6afe_port *port,
>                  struct afe_clk_set *cfg)
> {
>     return q6afe_port_set_param(port, cfg, AFE_PARAM_ID_CLOCK_SET,
>                     AFE_MODULE_CLOCK_SET, sizeof(*cfg));
> }
> 
> I have no time right now to implement all that for q6apm & q6prm in the
> way you propose, so I'll probably not send a new version.

This is not really scalable, you are suggesting that we add this for
every board, when you can do it more generically in the dsp code using
set_sysclk callback.



--srini
> 
> Neil
> 
>>
>> --srini
>>>
>>> Neil
>>>
>>>>
>>>> --srini
>>>>>
>>>>> Neil
>>>>>
>>>>>>
>>>>>> thanks,
>>>>>> Srini
>>>>>>
>>>>>>>     patternProperties:
>>>>>>>       ".*-dai-link$":
>>>>>>>         description:
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>