On Thu, Feb 14, 2019 at 7:06 PM liaoweixiong
[off-list ref] wrote:

On 2019-02-14 04:30, Rob Herring wrote:

quoted

On Wed, Feb 13, 2019 at 7:51 AM liaoweixiong
[off-list ref] wrote:

quoted

On 2019-01-31 00:07, Rob Herring wrote:> On Wed, Jan 23, 2019 at
08:05:13PM +0800, liaoweixiong wrote:

quoted

quoted

Create DT binding document for blkoops.

Signed-off-by: liaoweixiong <redacted>
---
 .../devicetree/bindings/pstore-block/blkoops.txt   | 32 ++++++++++++++++++++++

/bindings/pstore/...

I wouldn't call it blkoops either. I believe ramoops is called that to
maintain compatibility keeping the same kernel module name that
preceeded pstore.

Fixed.

In addition, I don't known whether should we move
ramreserved-memory/ooos.txt to /bindings/pstore. This is for maintainer
to decide, and do it on other patch.

quoted

quoted

 MAINTAINERS                                        |  1 +
 2 files changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pstore-block/blkoops.txt

diff --git a/Documentation/devicetree/bindings/pstore-block/blkoops.txt b/Documentation/devicetree/bindings/pstore-block/blkoops.txt
new file mode 100644
index 0000000..a25835b
--- /dev/null
+++ b/Documentation/devicetree/bindings/pstore-block/blkoops.txt

@@ -0,0 +1,32 @@
+Blkoops oops logger
+===================
+
+Blkoops provides a block partition for oops, excluding panics now, so they can
+be recovered after a reboot.
+
+Any space of block partition will be used for a circular buffer of oops records.
+These records have a configurable size, with a size of 0 indicating that they
+should be disabled.
+
+"partition-size" and at least one of "dmesg-size" or "pmsg-size" must be set
+non-zero, but are otherwise optional as listed below.
+
+Blkoops will take value from Kconfig if device tree do not set, but settings
+from module parameters can also overwrite them.

That's all kernel details not relevant to the binidng.

Deleted.

quoted

quoted

+
+Required properties:
+
+- compatible: must be "blkoops".
+
+- partition-size: size in kbytes, must be a multiple of 4.

This seems unnecessary given a partition has a known size.

partition-size is necessary for psotre/blk. User should tell pstore/blk
how large space can it use.

The partition table says how big a partition is. If you only want to
use part of it, then make the partition smaller or use a file system.
This is a solved problem, so we don't need a new way in DT to handle
this.

You are right, partition size is known and pstore/blk can get it
automatically from specified block device. I will try to do it on next
version patch.
But i prefer to rename partition-size to total-size and move it to
optional properties. Total-size means how big space pstore/blk can use.
It is not only about partition size as pstore/blk can use ram if no
partition specified.
So, I will process as follow logic:
1. If specify total size, use total size.
2. If no total size but specify partition, get size from partition.

You haven't given any reason why we need to support this.

quoted

quoted

quoted

quoted

+Optional properties:
+
+- partition-path: strings must begin with "/dev", tell blkoops which partition
+  it can used. If it is not set, blkoops will drop all data when reboot.

No. '/dev/...' is a Linux thing and doesn't belong in DT.

You should define a partition UUID and/or label and the kernel can find
the right partition to use.

pstore/blk do general read/write by filp_open/kernel_read/kernel_write,
which need device path.
In addition, i have no idea how to use UUID and/or label to do general
read/write on kernel layer, can you give me a tip?

The kernel can mount a filesystem by label or UUID though I think
those are filesystem UUID and label, not partition UUID and label. But
certainly bootloaders find the EFI system partition by UUID.

As your words, those are file system UUID and label, not partition
UUID/label.

Actually, looking at do_mounts.c, it is the partition UUID and label.
See PARTUUID and PARTLABEL.

Pstore is a file system, there is no other file system any
more for specified partition. Pstore/blk can't get specified partition
by UUID or label.

As far as i know, block device manager on user space scans each block
device and matches file system table to get UUID and label. Not even all
file systems have UUID/label.

Yes, userspace has more capabilities for mounting.

MTD device may has label, but it is not suitable for all block device.

MTD is special...

How if i cancel the prefix requirement for /dev and add it to the codes?
Then rename partition-path to block-device, by this, DT property may be
"mmcblk0p10" or "sda6" .

There is simply no way we are putting Linux device names into DT.
Besides just being Linux specific, the device names are not guaranteed
to be stable.

Rob