| 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: <zi6jlw35clxwxtlshhjazbotv2wkfrbv3kkzgb76igirycxzak@3nujn2mvhjbh>
Date: Thu, 13 Nov 2025 12:01:37 -0600
From: Bjorn Andersson <andersson@...nel.org>
To: "Aiqun(Maria) Yu" <aiqun.yu@....qualcomm.com>
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 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.
>
Yes, the TCSR clocks are controlled in the TCSR block, but there's no
"TCSR clocks"-IP-block in the memory map, it's just some registers in
the TCSR block.
And technically TCSR_MUTEX isn't a block of its own either, but so far
it's always been a clearly defined region of TCSR, either in the
beginning or end. (Might even be a separate block in the register map on
some older target, iirc)
> >
> > 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.
>
Compare with GCC, the DeviceTree isn't saying "address X is a clock,
address Y is a reset, address Z is a GDSC...".
The DeviceTree says "at address A we have the Kaanapali GCC IP block",
and we can refer to it from other nodes as a clock controller, a
reset-controller, and a power-domain provider.
Same here, the DeviceTree should say that at 0x01fc0000 there is a
Kaanapali TCSR block, which is a "syscon", a clock controller and a
reset controller.
We don't know which operating system (or other implementation - e.g.
U-boot) will consume this information, and we don't know what frameworks
or driver model is implemented, so we need to describe things without
thinking of a particular driver model.
Regards,
Bjorn
Powered by blists - more mailing lists