Re: [PATCH v4 08/21] dt-bindings: reserved-memory: Add qcom,ramoops binding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 7/3/2023 12:50 PM, Krzysztof Kozlowski wrote:
On Mon, 3 Jul 2023 at 08:22, Mukesh Ojha <quic_mojha@xxxxxxxxxxx> 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.

Why can't ramoops binding have one more feature where it can add a flag *dynamic* to indicate the regions are dynamic and it is for platforms
where there is another entity 'minidump' who is interested in these
regions.


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.

I had attempted it here,

https://lore.kernel.org/lkml/1683133352-10046-10-git-send-email-quic_mojha@xxxxxxxxxxx/

but got comments related to hard coding and some in favor of having
the same set of properties what ramoops has/provides

https://lore.kernel.org/lkml/e25723bf-be85-b458-a84c-1a45392683bb@xxxxxxxxx/

https://lore.kernel.org/lkml/202305161347.80204C1A0E@keescook/




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.

ramoops already support module params and a way to pass these parameters
from bootargs but it also need to know the hard-codes addresses, so, doing something in sysfs will be again duplication with ramoops driver..

If this can be accommodated under ramoops, this will be very small change, like this

https://lore.kernel.org/lkml/20230622005213.458236-1-isaacmanjarres@xxxxxxxxxx/

-- Mukesh

Best regards,
Krzysztof



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux