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

[Aiqun(Maria) Yu]

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.

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