On 03/07/2023 17:55, Mukesh Ojha wrote: > > > 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. Because we do not define dynamic stuff in Devicetree. Dynamic means defined by SW or runtime configurable. It is against the entire idea of Devicetree which is for non-discoverable hardware. > >> >>> 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/ Then you were tricked. I don't get why someone else suggests that non-hardware property should be part of Devicetree, but anyway it's the call of Devicetree binding maintainers, not someone else. DT is not dumping ground for all the system configuration variables. >> >>> >>>> >>>>> >>>>> 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.. Why do you need hard-coded addresses? > > 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/ That's also funny patch - missing bindings updated, missing CC DT maintainers. Best regards, Krzysztof
