Re: [PATCH v3 1/2] dt-bindings: sram: Document samsung,exynos850-acpm-sram

[Sam Protsenko]

On Fri, Sep 11, 2026 at 11:20 AM Alexey Klimov <alexey.klimov@xxxxxxxxxx> wrote:
>
> On Fri Sep 11, 2026 at 1:42 AM BST, Sam Protsenko wrote:
> > On Mon, Jul 27, 2026 at 8:23 AM Alexey Klimov <alexey.klimov@xxxxxxxxxx> wrote:
> >>
> >> Document "samsung,exynos850-acpm-sram" which represents an SRAM region used
> >> for ACPM communication on Samsung-based SoCs.
> >>
> >> The SRAM is consumed as a whole, is not split up using child nodes and
> >> properties related to this are not required.
> >>
> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxxxxx>
> >> Signed-off-by: Alexey Klimov <alexey.klimov@xxxxxxxxxx>
> >> ---
> >>  Documentation/devicetree/bindings/sram/sram.yaml | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/sram/sram.yaml b/Documentation/devicetree/bindings/sram/sram.yaml
> >> index b83fc6831947..b9c4d4f3199b 100644
> >> --- a/Documentation/devicetree/bindings/sram/sram.yaml
> >> +++ b/Documentation/devicetree/bindings/sram/sram.yaml
> >> @@ -43,6 +43,7 @@ properties:
> >>          - qcom,rpm-msg-ram
> >>          - qcom,shikra-imem
> >>          - rockchip,rk3288-pmu-sram
> >> +        - samsung,exynos850-acpm-sram
> >
> > Is there a reason we can't just use "mmio-sram" for Exynos850 ACPM
> > needs? I can see it's used in the gs101 device tree like this. If
> > there is a reason for this, I think it might be a good idea to
> > document it in the commit message (probably in both patches).
>
> But isn't it already in this commit message?
> "The SRAM is consumed as a whole, is not split up using child nodes and
> properties related to this are not required."
>
> Also: https://lore.kernel.org/all/5797be19-8886-45a2-bfce-eb7d5f0df6c2@xxxxxxxxxx/
>
> Do you want it to be re-phrased smth like this:
> "OSPM ACPM should parse the SRAM and discover essential data (e.g. queue
> offsets) for establishing ACPM communication. At this point it is not
> expected for any software entities apart ACPM to use SRAM so the SRAM is
> consumed as a whole, is not split up using child nodes and properties
> related to this are not required."
>
> or
>
> "The OSPM/ACPM IPC should parse the SRAM at runtime to find configuration
> data (e.g., queue offsets). Because this SRAM region is dedicated
> exclusively to ACPM IPC communication and is not shared with other
> software entities, it is represented as a monolithic SRAM block without
> requiring sub-node partitioning."
>
> ?
>

I was thinking about something more like this (in addition to your
existing commit message):

    "mmio-sram compatible can't be used for ACPM purposes because it
requires child nodes to be declared, which are not needed and make no
sense in case of ACPM."

basically to answer this question directly -- at least if I understood
correctly what was discussed with Krzysztof [1]. Other additions you
mentioned might be valuable too. Anyways, that's just a suggestion, I
figured if you're going to submit the next patch series version, might
as well provide more context in the commit message, but no strong
opinion here. Also, feel free to add:

Reviewed-by: Sam Protsenko <semen.protsenko@xxxxxxxxxx>

> I hope if there will be other software entities then we will be able to
> fix it later.
>
> >Also,
> > should "mmio-sram" be added as a backup option (after comma) in your
> > [PATCH 2/2]?
>
> Sure, I can add "mmio-sram".
> The compatible = "rockchip,rk3288-pmu-sram", "mmio-sram"
> and compatible = "qcom,rpm-msg-ram", "mmio-sram" are used in such way.
>

On the second thought (after reading your discussion with Krzysztof
[1]) that probably is not a good idea, kinda defies the point, because
for "mmio-sram" you'll have to provide more stuff in the node, right?
So I think it's better to keep only:

    compatible = "samsung,exynos850-acpm-sram";

in your [PATCH 2/2].

[1] https://lkml.org/lkml/2026/4/14/523

> Best regards,
> Alexey