public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
Cc: Srinivas Kandagatla <srini@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	linux-sound@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
Subject: Re: [PATCH v1 1/4] ASoC: dt-bindings: qcom,q6apm-lpass-dais: Document DAI subnode
Date: Thu, 26 Mar 2026 11:10:49 +0100	[thread overview]
Message-ID: <376ad62e-e6b5-438b-a23a-0988b6489e5c@kernel.org> (raw)
In-Reply-To: <138dee9e-56e7-43a0-bcbd-0e4db4d5b5af@oss.qualcomm.com>

On 26/03/2026 10:57, Mohammad Rafi Shaik wrote:
> 
> 
> On 3/17/2026 12:41 PM, Krzysztof Kozlowski wrote:
>> On 17/03/2026 06:27, Mohammad Rafi Shaik wrote:
>>>
>>>
>>> On 3/10/2026 3:25 PM, Krzysztof Kozlowski wrote:
>>>> On Mon, Mar 09, 2026 at 04:42:57PM +0530, Mohammad Rafi Shaik wrote:
>>>>> Extend the qcom,q6apm-lpass-dais device tree binding to explicitly
>>>>> describe Digital Audio Interface (DAI) child nodes.
>>>>>
>>>>> Add #address-cells and #size-cells to allow representation of multiple
>>>>> DAI instances as child nodes, and define a dai@<id> pattern to document
>>>>> per-DAI properties such as the interface ID and associated clocks.
>>>>>
>>>>> Qualcomm platforms like talos integrate third-party audio codecs or use
>>>>> different external audio paths. These designs often require additional
>>>>> configuration such as explicit MI2S MCLK settings for audio to work.
>>>>>
>>>>> Co-developed-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
>>>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
>>>>> Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
>>>>> ---
>>>>>    .../bindings/sound/qcom,q6apm-lpass-dais.yaml | 41 ++++++++++++++++++-
>>>>>    1 file changed, 40 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm-lpass-dais.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm-lpass-dais.yaml
>>>>> index 2fb95544d..1d770cbcb 100644
>>>>> --- a/Documentation/devicetree/bindings/sound/qcom,q6apm-lpass-dais.yaml
>>>>> +++ b/Documentation/devicetree/bindings/sound/qcom,q6apm-lpass-dais.yaml
>>>>> @@ -21,6 +21,34 @@ properties:
>>>>>      '#sound-dai-cells':
>>>>>        const: 1
>>>>>    
>>>>> +  '#address-cells':
>>>>> +    const: 1
>>>>> +
>>>>> +  '#size-cells':
>>>>> +    const: 0
>>>>> +
>>>>> +# Digital Audio Interfaces
>>>>> +patternProperties:
>>>>> +  '^dai@[0-9]+$':
>>>>> +    type: object
>>>>> +    description:
>>>>> +      Q6DSP Digital Audio Interfaces.
>>>>> +
>>>>> +    properties:
>>>>> +      reg:
>>>>> +        description:
>>>>> +          Digital Audio Interface ID
>>>>> +
>>>>> +      clocks:
>>>>> +        minItems: 1
>>>>> +        maxItems: 3
>>>>> +
>>>>> +      clock-names:
>>>>> +        minItems: 1
>>>>> +        maxItems: 3
>>>>
>>>> No, this is just way too generic. There is no such syntax in the kernel
>>>> and this should stop you right there. You are not allowed to add your
>>>> own style.
>>>>
>>>> I don't think DAI is here a separate device needing its own resources
>>>> expressed in DT. This is still part of ADSP so you just described in DT
>>>> internal routing between two services on ADSP.
>>>>
>>>
>>> Thanks for reviewing.
>>>
>>> I’d like to clarify that this is not intended to model the DAI as a
>>> separate physical device or to describe internal ADSP routing.
>>
>> If you do not want to represent the physical device, then I don't think
>> it should be represented at all.
>>
>>>
>>> Requirement is to allow the kernel to send clock‑voting requests to the
>>> ADSP. LPASS MCLK routing is not enabled by default on the ADSP, so the
>>> kernel must explicitly request the ADSP to enable the relevant LPASS
>>> MCLKs, which is a real hardware control requirement.
>>>
>>> These clocks are LPASS‑owned, and driving them via a third‑party codec
>>> is not appropriate. The intent of adding clock capabilities at the DAI
>>> level is to allow the kernel to associate LPASS clock votes with a
>>> specific DAI instance during stream activity.
>>>
>>> While the DAI itself is not a physical device, some DT representation is
>>> required to describe per‑DAI LPASS clock requirements.
>>
>> DT's purpose is not to describe software constructs, thus DT is not the
>> answer to your requirement of mapping clocks to specific DAI needs.
>> Every person adding software properties made "some DT representation is
>> required" claim.
>>
>>>
>>> I’m open to considering alternative representations, but removing this
>>> entirely would leave no generic way for the kernel to handle correct
>>> LPASS MCLK voting.
>>
>> I imagine that, since this is software construct, the software knows
>> which DAI needs which clock. Clocks are strictly defined, thus driver
>> should handle all this.
>>
> 
> No, the MCLK connection is not fixed to a specific DAI.
> 
> The LPASS MCLKs
> LPASS_CLK_ID_MCLK_1 … LPASS_CLK_ID_MCLK_5
> 
> are hard‑wired connection, each physically routed to an external codec 
> on the board.
> 
> Because of this, the clock that must be voted depends purely on the 
> hardware wiring, not on which DAI (Primary/Secondary/Tertiary/Quaternary 
> MI2S) is used.

If they are routed to external codecs, then they are already present in
these nodes and duplicating them here is not necessary.

> 
> In other words, DAI ↔ MCLK is not a fixed mapping.
> 
> Examples:
> On Talos‑EVK, the speaker is connected via Primary MI2S, but the 
> corresponding MCLK line wired to the codec is LPASS_CLK_ID_MCLK_2.
> 
> On Kodiak, the customer connected an SGTL5000 codec via Quaternary MI2S, 
> yet the required MCLK is still LPASS_CLK_ID_MCLK_2.
> 
> Instead, the kernel must vote for the MCLK that is physically connected 
> to the external codec on that specific board.

No, the external codec driver must vote for that MCLK.

Best regards,
Krzysztof

  reply	other threads:[~2026-03-26 10:10 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-09 11:12 [PATCH v1 0/4] ASoC: qcom: qdsp6: Add MI2S clock control Mohammad Rafi Shaik
2026-03-09 11:12 ` [PATCH v1 1/4] ASoC: dt-bindings: qcom,q6apm-lpass-dais: Document DAI subnode Mohammad Rafi Shaik
2026-03-09 19:55   ` Dmitry Baryshkov
2026-03-10  9:41     ` Mohammad Rafi Shaik
2026-03-10  9:55   ` Krzysztof Kozlowski
2026-03-17  5:27     ` Mohammad Rafi Shaik
2026-03-17  7:11       ` Krzysztof Kozlowski
2026-03-26  9:57         ` Mohammad Rafi Shaik
2026-03-26 10:10           ` Krzysztof Kozlowski [this message]
2026-03-27 13:16             ` Mohammad Rafi Shaik
2026-03-27 13:23               ` Krzysztof Kozlowski
2026-03-09 11:12 ` [PATCH v1 2/4] ASoC: qcom: qdsp6: q6prm: add the missing LPASS MCLK clock IDs Mohammad Rafi Shaik
2026-03-09 14:07   ` Srinivas Kandagatla
2026-03-10  9:30     ` Mohammad Rafi Shaik
2026-03-09 11:12 ` [PATCH v1 3/4] ASoC: qcom: q6apm-lpass-dais: Add MI2S clock control Mohammad Rafi Shaik
2026-03-11 18:25   ` kernel test robot
2026-03-09 11:13 ` [PATCH v1 4/4] ASoC: qcom: sc8280xp: ASoC: qcom: sc8280xp: enhance machine driver for board-specific config Mohammad Rafi Shaik
2026-03-10 11:44   ` Neil Armstrong
2026-03-17  5:35     ` Mohammad Rafi Shaik
2026-03-10 14:02 ` [PATCH v1 0/4] ASoC: qcom: qdsp6: Add MI2S clock control Neil Armstrong
2026-03-17  5:48   ` Mohammad Rafi Shaik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=376ad62e-e6b5-438b-a23a-0988b6489e5c@kernel.org \
    --to=krzk@kernel.org \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=mohammad.rafi.shaik@oss.qualcomm.com \
    --cc=perex@perex.cz \
    --cc=robh@kernel.org \
    --cc=srini@kernel.org \
    --cc=srinivas.kandagatla@oss.qualcomm.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox