| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4a6f7c18-82d1-499c-af9c-9681e16a0db6@kernel.org>
Date: Thu, 13 Nov 2025 17:59:16 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: "Aiqun(Maria) Yu" <aiqun.yu@....qualcomm.com>,
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 13/11/2025 11:03, 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].
> "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
There is no MFD. Don't use that term in context of supporting a change.
But regardless, this documents only random regs.
> binding to document the whole tcsr block which is not belonged to tcsr
> clock controller.
I don't understand whether you claim this patch as "this suggestion" or
something else.
Best regards,
Krzysztof
Powered by blists - more mailing lists