Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <1687955688-20809-1-git-send-email-quic_mojha@quicinc.com>
 <1687955688-20809-9-git-send-email-quic_mojha@quicinc.com>
 <CAL_JsqJ_TTnGjjB2d8_FKHpWBRG5GHLoWnabCKjsdeZ4QFdNEg@mail.gmail.com>
 <cacbbb02-732e-076e-50bf-292d20a4d722@quicinc.com> <58a26b9e-a48d-d567-c310-193a2c52521e@linaro.org>
 <5447f9f8-55b4-8bed-66a6-1c9d62b02c79@quicinc.com>
In-Reply-To: <5447f9f8-55b4-8bed-66a6-1c9d62b02c79@quicinc.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Date:   Mon, 3 Jul 2023 09:20:35 +0200
Message-ID: <CAGE=qrq0CuO4J-6yC=YZ4xjL67o9QTqpei0ovX-X_8MLVeEH6g@mail.gmail.com>
Subject: Re: [PATCH v4 08/21] dt-bindings: reserved-memory: Add qcom,ramoops binding
To:     Mukesh Ojha <quic_mojha@quicinc.com>
Cc:     Rob Herring <robh+dt@kernel.org>, corbet@lwn.net,
        agross@kernel.org, andersson@kernel.org, konrad.dybcio@linaro.org,
        conor+dt@kernel.org, keescook@chromium.org, tony.luck@intel.com,
        gpiccoli@igalia.com, mathieu.poirier@linaro.org,
        catalin.marinas@arm.com, will@kernel.org, linus.walleij@linaro.org,
        andy.shevchenko@gmail.com, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-hardening@vger.kernel.org,
        linux-remoteproc@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, 3 Jul 2023 at 08:22, Mukesh Ojha <quic_mojha@quicinc.com> wrote:
> On 7/2/2023 1:42 PM, Krzysztof Kozlowski wrote:
> >>> The big difference is if firmware is not deciding where this log
> >>> lives, then it doesn't need to be in DT. How does anything except the
> >>> kernel that allocates the log find the logs?
> >>
> >> Yes, you are correct, firmware is not deciding where the logs lives
> >> instead here, Kernel has reserved the region where the ramoops region
> >> lives and later with the minidump registration where, physical
> >> address/size/virtual address(for parsing) are passed and that is how
> >> firmware is able to know and dump those region before triggering system
> >> reset.
> >
> > Your explanation does not justify storing all this in DT. Kernel can
> > allocate any memory it wishes, store there logs and pass the address to
> > the firmware. That's it, no need for DT.
>
> If you go through the driver, you will know that what it does, is

We talk about bindings and I should not be forced to look at the
driver to be able to understand them. Bindings should stand on their
own.

> just create platform device for actual ramoops driver to probe and to

Not really justification for Devicetree anyway. Whatever your driver
is doing, is driver's business, not bindings.

> provide this it needs exact set of parameters of input what original
> ramoops DT provides, we need to keep it in DT as maintaining this in
> driver will not scale well with different size/parameter size
> requirement for different targets.

Really? Why? I don't see a problem in scaling. At all.

>
> >
> >>
> >> A part of this registration code you can find in 11/21
> >>
> >>> I'm pretty sure I already said all this before.
> >>
> >> Yes, you said this before but that's the reason i came up with vendor
> >> ramoops instead of changing traditional ramoops binding.
> >
> > That's unexpected conclusion. Adding more bindings is not the answer to
> > comment that it should not be in the DTS in the first place.
>
> Please suggest, what is the other way being above text as requirement..

I do not see any requirement for us there. Forcing me to figure out
how to add non-hardware property to DT is not the way to convince
reviewers. But if you insist - we have ABI for this, called sysfs. If
it is debugging feature, then debugfs.

Best regards,
Krzysztof