Message-ID: <we7qsu2aivdnqttvxzveiryp2axy3dxu4dw7s43xmytxnaifzt@ajyt5y4cqwy5>
Date: Thu, 13 Nov 2025 14:25:49 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: "Aiqun(Maria) Yu" <aiqun.yu@....qualcomm.com>
Cc: Bjorn Andersson <andersson@...nel.org>,
        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 Thu, Nov 13, 2025 at 06:03:33PM +0800, Aiqun(Maria) Yu wrote:
> 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].

SoC-tcsrcc? Make it more obvious, please.

> "qcom,tcsr-<platform>" is a common tcsr block. reference current binding
> patch.

SoC-tcsr, please, if it didn't land yet.

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

What prevents us from using TCSR as an MFD and describing hwmutex and
TCSRCC as subdevices?

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

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists