Re: [PATCH] dt-bindings: mfd: qcom,tcsr: Add compatible for Kaanapali

[Bjorn Andersson]

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@xxxxxxxxxxxxxxxx>
> >>>>>>> ---
> >>>>>>>  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@xxxxxxxxxxxxxxxx/
> >>>>
> >>>
> >>> 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@1f40000 {
> >> 	compatible = "qcom,tcsr-mutex";
> >> 	reg = <0x0 0x01f40000 0x0 0x20000>;
> >> 	#hwlock-cells = <1>;
> >> };
> >>
> >> tcsr_dload: syscon@1fc0000 {
> >> 	compatible = "qcom,tcsr-kaanapali", "syscon";
> >> 	reg = <0x0 0x1fc0000 0x0 0x30000>;
> >> };
> >>
> >> tcsrcc: clock-controller@1fd5044 {
> >> 	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@1f40000 {
> >> 	compatible = "qcom,tcsr-kaanapali", "syscon";
> >> 	reg = <0x0 0x1f40000 0x0 0xC0000>; //align with the whole hardware
> >> block design.
> >> };
> >>
> >> tcsr_mutex: hwlock@1f40000 {
> >> 	compatible = "qcom,tcsr-mutex";
> >> 	reg = <0x0 0x01f40000 0x0 0x20000>;
> >> 	#hwlock-cells = <1>;
> >> };
> >>
> >> tcsrcc: clock-controller@1fd5044 {
> >> 	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.

This is what we have for Hamoa:

tcsr_mutex: hwlock@1f40000 {
        compatible = "qcom,tcsr-mutex";
        reg = <0 0x01f40000 0 0x20000>;
        #hwlock-cells = <1>;
};

tcsr: clock-controller@1fc0000 {
        compatible = "qcom,x1e80100-tcsr", "syscon";
        reg = <0 0x01fc0000 0 0x30000>;
        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?

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