Re: [PATCH v6] pstore/ram: Add ramoops support for the FlattenedDevice Tree.

From: Marco Stornelli
Date: Sat Sep 08 2012 - 03:31:06 EST

Il 08/09/2012 07:29, Anton Vorontsov ha scritto:
On Fri, Sep 07, 2012 at 11:29:36AM -0700, Bryan Freed wrote:
When called with a non-zero of_node, fill out a new ramoops_platform_data
with data from the specified Flattened Device Tree node.
Update ramoops documentation with the new FDT interface.
Update devicetree/binding documentation with pstore/ramoops.txt.

Thanks for your work, Bryan! There were a few issues, I fixed
them myself but I need your confirmation if you're OK w/ all
the changes.

First of, the Signed-off-by tag is missing, but I see it in v5.
I also see that v5 had an Ack from Kees Cook, you did not preserve it,
but generally it's a good idea to do so (if vX -> v(X+1) changes
aren't drastic).

+Optional properties:
+- record-size: Specifies the size of each record in preserved memory.
+- console-size: Specifies the size of the console log.
+- ftrace-size: Specifies the size of the ftrace log.
+- ecc-size: Specifies the size of each record's ECC buffer.
+- dump-oops: Specifies to dump oops in addition to panics.

Personally, I don't see how this fits into device tree. It doesn't
describe hardware, instead it's more a configuration stuff, which
usually we try to not put into the device tree.

It would be better to have a sane defaults in ramoops, instead of
introducing more "virtual" stuff in the device tree. That is, feel
free to change defaults if they seem to be not enough for most your

The only property that I see which is truly hardware-specific is
ecc-size. E.g. if we're pretty sure that a specific hw does not
need ecc, it's OK to avoid it. And some hw setups might require
bigger ECC sizes, so it's OK to have it in the device-tree.

static struct ramoops_platform_data * __init
of_ramoops_platform_data(struct device *dev)

You call this function from __devinit section, so using __init
is an error.

WARNING: fs/pstore/ramoops.o(.devinit.text+0x37): Section mismatch in reference from the function ramoops_probe() to the function .init.text:of_ramoops_platform_data()
The function __devinit ramoops_probe() references

I changed this to __devnit.

+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (pdata == NULL)

I wonder why people prefer to not write !pdata, which is more
natural when reading the code.. :-)

I think it's the same for sizeof, it's much more readable
sizeof(struct ramoops_platform_data).

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at