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

[Neil Armstrong <neil.armstrong@linaro.org>]

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:
>>>>
>>>
>>
>