| 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: <19ee7cf6-8b92-4221-b450-75b3e3d66e8b@oss.qualcomm.com>
Date: Tue, 4 Nov 2025 13:51:46 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Bjorn Andersson <andersson@...nel.org>,
"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 10/29/25 4:37 PM, Bjorn Andersson wrote:
> 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).
Hm sorry I skipped this thread and started repeating similar points
in v3.
Ultimately I don't really care either way (mmio-sram vs generic node
acted upon by different drivers), but I do care about closing this
discussion..
Konrad
Powered by blists - more mailing lists