Re: [PATCH] arm64: dts: exynos850: Add SRAM node

[Tudor Ambarus <tudor.ambarus@linaro.org>]

Hi!

On 4/14/26 12:08 PM, Krzysztof Kozlowski wrote:
> On 14/04/2026 11:00, Alexey Klimov wrote:
>> On Mon Apr 13, 2026 at 4:23 PM BST, Krzysztof Kozlowski wrote:
>>> On 13/04/2026 16:52, Alexey Klimov wrote:
>>>> SRAM is used by the ACPM protocol to retrieve the ACPM channels
>>>> information and configuration data. Add the SRAM node.
>>>>
>>>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>>>> ---
>>>>  arch/arm64/boot/dts/exynos/exynos850.dtsi | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/exynos/exynos850.dtsi b/arch/arm64/boot/dts/exynos/exynos850.dtsi
>>>> index cb55015c8dce..cf4a6168846c 100644
>>>> --- a/arch/arm64/boot/dts/exynos/exynos850.dtsi
>>>> +++ b/arch/arm64/boot/dts/exynos/exynos850.dtsi
>>>> @@ -910,6 +910,14 @@ spi_2: spi@11d20000 {
>>>>  			};
>>>>  		};
>>>>  	};
>>>> +
>>>> +	apm_sram: sram@2039000 {
>>>> +		compatible = "mmio-sram";
>>>> +		reg = <0x0 0x2039000 0x40000>;
>>>> +		#address-cells = <1>;
>>>> +		#size-cells = <1>;
>>>> +		ranges = <0x0 0x0 0x2039000 0x40000>;
>>>
>>> You miss here children.
>>
>> Thank you! I guess I should convert it to smth like this:
>>
>> apm_sram: sram@2039000 {
>> 		compatible = "mmio-sram";
>> 		reg = <0x0 0x2039000 0x40000>;
>> 		ranges = <0x0 0x0 0x2039000 0x40000>;
>> 		#address-cells = <1>;
>> 		#size-cells = <1>;
>>
>> 		acpm_sram_region: sram-section@0 {
>> 			reg = <0x0 0x40000>;
> 
> This covers entire block, so feels pointless. Maybe requirement of
> children should be dropped. What's the point of having children? Why
> does the driver need them?
> 
>> 		};
>> 	};
>>
>> And then later reference shmem = &acpm_sram_region from acpm node.
>>
>>> Also, 'ranges' should be after 'reg'.
>>
>> Thanks, will fix this.
>>
>> FWIW this commit is a copy of commit 48e7821b26904
>> https://lore.kernel.org/r/20250207-gs101-acpm-dt-v4-1-230ba8663a2d@linaro.org
> 
> 
> Huh, we should fix that one as well.
> 

On gs101, likely on e850 too, ACPM parses the SRAM and discovers at runtime where
the TX/RX queue offsets are in SRAM. So we can't define static partitions in DT.
I remember that I thought about extending the SRAM driver to add dynamic
partitions (clients to request the mmio-sram driver to create partitions at
runtime), but it was just ACPM that's using SRAM, so I didn't need it.

Cheers,
ta