Message-ID: <bf706156-1413-42cb-a463-803063c347fc@oss.qualcomm.com>
Date: Thu, 13 Nov 2025 18:03:33 +0800
From: "Aiqun(Maria) Yu" <aiqun.yu@....qualcomm.com>
To: Bjorn Andersson <andersson@...nel.org>
Cc: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Jingyi Wang <jingyi.wang@....qualcomm.com>, Lee Jones <lee@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        tingwei.zhang@....qualcomm.com, trilok.soni@....qualcomm.com,
        yijie.yang@....qualcomm.com
Subject: Re: [PATCH] dt-bindings: mfd: qcom,tcsr: Add compatible for Kaanapali

On 11/12/2025 12:05 AM, Bjorn Andersson wrote:
> On Tue, Nov 11, 2025 at 08:27:17PM +0800, Aiqun(Maria) Yu wrote:
>> On 11/7/2025 12:24 AM, Konrad Dybcio wrote:
>>> On 11/6/25 11:16 AM, Aiqun(Maria) Yu wrote:
>>>> On 11/6/2025 5:06 AM, Bjorn Andersson wrote:
>>>>> On Tue, Nov 04, 2025 at 01:35:01PM +0800, Jingyi Wang wrote:
>>>>>>
>>>>>>
>>>>>> On 11/4/2025 12:02 PM, Bjorn Andersson wrote:
>>>>>>> On Tue, Nov 04, 2025 at 11:34:25AM +0800, Aiqun(Maria) Yu wrote:
>>>>>>>> On 9/25/2025 7:23 AM, Jingyi Wang wrote:
>>>>>>>>> Document the qcom,tcsr-kaanapali compatible, tcsr will provide various
>>>>>>>>> control and status functions for their peripherals.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Jingyi Wang <jingyi.wang@....qualcomm.com>
>>>>>>>>> ---
>>>>>>>>>  Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml | 1 +
>>>>>>>>>  1 file changed, 1 insertion(+)
>>>>>>>>>
>>>>>>>>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml b/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml
>>>>>>>>> index 14ae3f00ef7e..ae55b0a70766 100644
>>>>>>>>> --- a/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml
>>>>>>>>> +++ b/Documentation/devicetree/bindings/mfd/qcom,tcsr.yaml
>>>>>>>>> @@ -48,6 +48,7 @@ properties:
>>>>>>>>>            - qcom,tcsr-ipq8064
>>>>>>>>>            - qcom,tcsr-ipq8074
>>>>>>>>>            - qcom,tcsr-ipq9574
>>>>>>>>> +          - qcom,tcsr-kaanapali
>>>>>>>>
>>>>>>>> It looks good to me. Glymur didn't have this functionality verified yet.
>>>>>>>
>>>>>>> You spelled Reviewed-by: Aiqun Yu <..> wrong.
>>>>>>>
>>>>>>>> Remind for review.
>>>>>>>
>>>>>>> No need for that, reviewers will review when they have time.
>>>>>>>
>>>>>>>>
>>>>>>
>>>>>> Hi Bjorn,
>>>>>>
>>>>>>>
>>>>>>> But that said, most modern additions to this binding follow the common
>>>>>>> format of qcom,<soc>-<block>.
>>>>>>>
>>>>>>> So I would prefer this to be qcom,kaanapali-tcsr.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Bjorn
>>>>>>>
>>>>>>
>>>>>> qcom,tcsr-kaanapali is used to distinguish with binding for GCC:
>>>>>> https://lore.kernel.org/all/20251030-gcc_kaanapali-v2-v2-2-a774a587af6f@oss.qualcomm.com/
>>>>>>
>>>>>
>>>>> So, qcom,kaanapali-tcsr is the clock controller region of TCSR and
>>>>> qcom,tcsr-kaanapali is the non-clock controller region of TCSR?
>>>>>
>>>>> Sorry for not understanding that earlier, but this doesn't work for me.
>>>>>
>>>>> It's a bit of a lie that TCSR_MUTEX is a separate node in devicetree,
>>>>> but it's always an nice chunk of 256K in the beginning (or end in some
>>>>> cases?) of TCSR. But for the rest, there should be a single tcsr node in
>>>>> DeviceTree and that one node should be a syscon and a clock controller.
>>>>
>>>> I've been dive deeply on this tcsr block. And actually the tcsr clock
>>>> controller part is a very small trunk size(0x1c) of the tcsr block. And
>>>> this block have contain other multiple purposed sys registers. So maybe
>>>> we can have a more discussion on how to have device tree node describe
>>>> this situation? It is not straight forward that to have a non-tcsrcc
>>>> related area being described in tcsrcc.
>>>>
>>>> What about option 1 (tcsr_mutex + tcsr_dload_syscon + tcsrcc):>> tcsr_mutex: hwlock@...0000 {
>>>> 	compatible = "qcom,tcsr-mutex";
>>>> 	reg = <0x0 0x01f40000 0x0 0x20000>;
>>>> 	#hwlock-cells = <1>;
>>>> };
>>>>
>>>> tcsr_dload: syscon@...0000 {
>>>> 	compatible = "qcom,tcsr-kaanapali", "syscon";
>>>> 	reg = <0x0 0x1fc0000 0x0 0x30000>;
>>>> };
>>>>
>>>> tcsrcc: clock-controller@...5044 {
>>>> 	compatible = "qcom,kaanapali-tcsr", "syscon";
>>
>> Remove "syscon" here. Not need for tcsrcc fallback to "syscon".
>>
>>>> 	reg = <0x0 0x01fd5044 0x0 0x1c>;
>>>> ...
>>>> };
>>>>
>>>> What about option 2 (tcsr whole block + tcsr_mutex  + tcsrcc):
>>>>
>>>> tcsr: syscon@...0000 {
>>>> 	compatible = "qcom,tcsr-kaanapali", "syscon";
>>>> 	reg = <0x0 0x1f40000 0x0 0xC0000>; //align with the whole hardware
>>>> block design.
>>>> };
>>>>
>>>> tcsr_mutex: hwlock@...0000 {
>>>> 	compatible = "qcom,tcsr-mutex";
>>>> 	reg = <0x0 0x01f40000 0x0 0x20000>;
>>>> 	#hwlock-cells = <1>;
>>>> };
>>>>
>>>> tcsrcc: clock-controller@...5044 {
>>>> 	compatible = "qcom,kaanapali-tcsr", "syscon";
>>
>> Same here, don't need to have "syscon" here.
>>
>>>> 	reg = <0x0 0x01fd5044 0x0 0x1c>;
>>>> ...
>>>> };
>>>
>>> Is there anything wrong with what we have done for x1e80100?
>>> ______________________
>>> |             |       |
>>> | TCSR_MUTEX  | mutex |
>>> |_____________|_______|
>>> |	      |       |
>>> | RANDOM_REGS |       |
>>> |_____________|       |
>>> |	      |       |
>>> | TCSR_CLKS   | tcsr  |
>>> |_____________|       |
>>> |	      |       |
>>> | RANDOM_REGS |       |
>>> |_____________|_______|
>>>
>>
>> Second you! We can firstly have a option selected for kaanapali, and
>> then other platform can be followed or fixed afterwards.
>>
>> Here suggest to have option 2 which is remove "syscon" from tcsr clocks,
>> and only add the whole "syscon" to "tcsr" whole block.
>>
> 
> I think you misunderstood Konrad, or perhaps I misunderstand you.

Maybe let Konrad help to explain more here. I thought the chart below is
very clearly indicate the tcsr_clks is only part of the tcsr block.

> 
> This is what we have for Hamoa:
> 
> tcsr_mutex: hwlock@...0000 {
>         compatible = "qcom,tcsr-mutex";
>         reg = <0 0x01f40000 0 0x20000>;
>         #hwlock-cells = <1>;
> };
> 
> tcsr: clock-controller@...0000 {


It is not a clock-controller start from 0x1fc0000.

>         compatible = "qcom,x1e80100-tcsr", "syscon";
>         reg = <0 0x01fc0000 0 0x30000>;

This is what we have a discussion initialized here:
"qcom,<platform>-tcsr" is only a tcsr clock controller binder, reference
from [1].
"qcom,tcsr-<platform>" is a common tcsr block. reference current binding
patch.

For below hardware block information, is it really a recommendation to
combine the tscr block with tcsr clock controller all together?
______________________
|             |       |
| TCSR_MUTEX  | mutex |
|_____________|_______|
|	      |       |
| RANDOM_REGS |       |
|_____________|       |
|	      |       |
| TCSR_CLKS   | tcsr  |
|_____________|       |
|	      |       |
| RANDOM_REGS |       |
|_____________|_______|

[1]https://lore.kernel.org/all/20251030-gcc_kaanapali-v2-v2-2-a774a587af6f@oss.qualcomm.com/


>         clocks = <&rpmhcc RPMH_CXO_CLK>;
>         #clock-cells = <1>;
>         #reset-cells = <1>;
> };
> 
> This is exactly what I suggested above and Konrad is asking you why
> this doesn't work for Kaanapali. The addresses are even the same, what
> is the problem?

The problem is the current patchset document a separate tcsr block as a
mfd. While the suggestion here is to use the tcsr clock controller
binding to document the whole tcsr block which is not belonged to tcsr
clock controller.

> 
> Regards,
> Bjorn
> 
>>>
>>> 8750 was different because someone decided to stick the "TCSR clocks"
>>> into the TLMM address space, but it was a one-off
>>>
>>> Konrad
>>
>>
>> -- 
>> Thx and BRs,
>> Aiqun(Maria) Yu


-- 
Thx and BRs,
Aiqun(Maria) Yu

Powered by blists - more mailing lists