linux-kernel - Re: [PATCH v2 1/4] dt-bindings: soc: qcom: Add qcom,kaanapali-imem compatible

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a7cdd2b3-6097-4a8c-a639-af974292cc8b@oss.qualcomm.com>
Date: Wed, 29 Oct 2025 19:47:11 +0800
From: "Aiqun(Maria) Yu" <aiqun.yu@....qualcomm.com>
To: Bjorn Andersson <andersson@...nel.org>,
        Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: 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 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.


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