| 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: <x4rzktpij4ggnbvnuyoli65gugymli5acrmrlovglttpakaauw@o3vu23bjedul>
Date: Wed, 29 Oct 2025 10:37:37 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: "Aiqun(Maria) Yu" <aiqun.yu@....qualcomm.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
Jingyi Wang <jingyi.wang@....qualcomm.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Robert Marko <robimarko@...il.com>,
Das Srinagesh <quic_gurus@...cinc.com>, tingwei.zhang@....qualcomm.com, trilok.soni@....qualcomm.com,
yijie.yang@....qualcomm.com, linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Eugen Hristev <eugen.hristev@...aro.org>
Subject: Re: [PATCH v2 1/4] dt-bindings: soc: qcom: Add qcom,kaanapali-imem
compatible
On Wed, Oct 29, 2025 at 07:47:11PM +0800, Aiqun(Maria) Yu wrote:
> On 10/28/2025 2:44 AM, Bjorn Andersson wrote:
> > On Thu, Oct 23, 2025 at 03:06:00AM +0300, Dmitry Baryshkov wrote:
> >> On Wed, Oct 22, 2025 at 05:42:58PM -0500, Bjorn Andersson wrote:
> >>> On Wed, Oct 22, 2025 at 12:34:58PM +0300, Dmitry Baryshkov wrote:
> >>>> On Wed, Oct 22, 2025 at 05:05:30PM +0800, Jingyi Wang wrote:
> >>>>>
> >>>>>
> >>>>> On 10/22/2025 4:49 PM, Dmitry Baryshkov wrote:
> >>>>>> On Wed, Oct 22, 2025 at 12:28:41AM -0700, Jingyi Wang wrote:
> >>>>>>> Document qcom,kaanapali-imem compatible.
> >>>>>>>
> >>>>>>> Reviewed-by: Eugen Hristev <eugen.hristev@...aro.org>
> >>>>>>> Signed-off-by: Jingyi Wang <jingyi.wang@....qualcomm.com>
> >>>>>>> ---
> >>>>>>> Documentation/devicetree/bindings/sram/qcom,imem.yaml | 1 +
> >>>>>>> 1 file changed, 1 insertion(+)
> >>>>>>>
> >>>>>>> diff --git a/Documentation/devicetree/bindings/sram/qcom,imem.yaml b/Documentation/devicetree/bindings/sram/qcom,imem.yaml
> >>>>>>> index 6a627c57ae2f..1e29a8ff287f 100644
> >>>>>>> --- a/Documentation/devicetree/bindings/sram/qcom,imem.yaml
> >>>>>>> +++ b/Documentation/devicetree/bindings/sram/qcom,imem.yaml
> >>>>>>> @@ -19,6 +19,7 @@ properties:
> >>>>>>> - enum:
> >>>>>>> - qcom,apq8064-imem
> >>>>>>> - qcom,ipq5424-imem
> >>>>>>> + - qcom,kaanapali-imem
> >>>>>>
> >>>>>> Can you use mmio-sram instead?
> >>>>>>
> >>>>>
> >>>>> Here is the node:
> >>>>>
> >>>>> sram@...80000 {
> >>>>> compatible = "qcom,kaanapali-imem", "syscon", "simple-mfd";
> >>>>> reg = <0x0 0x14680000 0x0 0x1000>;
> >>>>> ranges = <0 0 0x14680000 0x1000>;
> >>>>>
> >>>>> #address-cells = <1>;
> >>>>> #size-cells = <1>;
> >>>>>
> >>>>> pil-reloc@94c {
> >>>>> compatible = "qcom,pil-reloc-info";
> >>>>> reg = <0x94c 0xc8>;
> >>>>> };
> >>>>> };
> >>>>>
> >>>>> other qualcomm are also using imem, could you please give more details on why
> >>>>> we should use mmio-sram here?
> >>>>
> >>>> https://lore.kernel.org/linux-arm-msm/e4c5ecc3-fd97-4b13-a057-bb1a3b7f9207@kernel.org/
> >>>>
> >>>
> >>> I considered exactly this when I wrote the binding back then...
> >>>
> >>> But the binding defines mmio-sram as "Simple IO memory regions to be
> >>> managed by the genalloc API." and the Linux sram driver follows that and
> >>> registers a gen_pool across the sram memory region.
> >>>
> >>> I believe IMEM is SRAM (it's at least not registers), but its memory
> >>> layout is fixed, so it's not a pool in any form.
> >>>
> >>>
> >>> What Krzysztof says makes sense, but rather than just throwing a yak at
> >>> Jingyi, it would be nice if you provided some guidance on how you would
> >>> like to see this turn out.
> >>
> >> I tested, pretty same approach seems to work:
> >>
> >
> > Now you're shaving at random ;)
> >
> >> sram@...80000 {
> >> compatible = "mmio-sram";
> >
> > You can put "pil-reloc-sram" wherever, because it will perform a
> > of_find_compatible_node() to dig up some node with the compatible
> > "qcom,pil-reloc-info" .
> >
> > In other words, this line created a genpool for something that really
> > isn't a genpool, but luckily that didn't have any side effects.
> >
> >
> > There are however other users of IMEM, such as the "reboot-mode", which
> > relies on the "sram" device probing child devices, and is implemented by
> > "syscon-reboot-mode".
> >
> > Perhaps the solution is to not support any new users of that?
> >
> >
> > But no matter what, the definition "Simple IO memory regions to be
> > managed by the genalloc API" will never be true for IMEM.
> >
> > And as this isn't a syscon, simple-mfd, or mmio-sram...how about making
> > the fallback "qcom,imem" (in this same binding) and omitting any
> > implementation until we need one)?
>
>
> Totally agree. We can remove the "syscon" and "simple-mfd" compatibles
> for Kaanapali.
> For Kaanapali, the reboot reason does not rely on imem at all—it uses
> nvmem cells instead.
> Previously, the syscon-reboot-mode required "syscon" and "simple-mfd"
> compatibles for older targets like APQ8064, which used imem as the
> reboot mode solution.
>
And there's
https://lore.kernel.org/lkml/20250527-topic-ipa_imem-v2-0-6d1aad91b841@oss.qualcomm.com/
which Konrad pointed out, which would also work with this model
(qcom,imem fallback but no implementation).
This also does leave the door open for a future qcom,imem
implementation, if we need to associate some logic to the imem device
itself.
Regards,
Bjorn
>
> >
> > Regards,
> > Bjorn
> >
> >> reg = <0x0 0x14680000 0x0 0x1000>;
> >> ranges = <0 0 0x14680000 0x1000>;
> >>
> >> #address-cells = <1>;
> >> #size-cells = <1>;
> >>
> >> pil-reloc-sram@94c {
> >> compatible = "qcom,pil-reloc-info";
> >> reg = <0x94c 0xc8>;
> >> };
> >> };
> >>
> >>
> >> --
> >> With best wishes
> >> Dmitry
>
>
> --
> Thx and BRs,
> Aiqun(Maria) Yu
Powered by blists - more mailing lists