Re: [PATCH v3 3/3] dt-bindings: pmem-region: Document memory-region

[Rob Herring]

On Sun, Feb 23, 2020 at 06:10:29PM -0800, Alistair Delva wrote:
> From: Kenny Root <kroot@xxxxxxxxxx>
> 
> Add documentation and example for memory-region in pmem.
> 
> Signed-off-by: Kenny Root <kroot@xxxxxxxxxx>
> Signed-off-by: Alistair Delva <adelva@xxxxxxxxxx>
> Cc: "Oliver O'Halloran" <oohall@xxxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
> Cc: Vishal Verma <vishal.l.verma@xxxxxxxxx>
> Cc: Dave Jiang <dave.jiang@xxxxxxxxx>
> Cc: Ira Weiny <ira.weiny@xxxxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx
> Cc: linux-nvdimm@xxxxxxxxxxxx
> Cc: kernel-team@xxxxxxxxxxx
> ---
> [v3: adelva: remove duplicate "From:"]
>  .../devicetree/bindings/pmem/pmem-region.txt  | 29 +++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pmem/pmem-region.txt b/Documentation/devicetree/bindings/pmem/pmem-region.txt
> index 5cfa4f016a00..0ec87bd034e0 100644
> --- a/Documentation/devicetree/bindings/pmem/pmem-region.txt
> +++ b/Documentation/devicetree/bindings/pmem/pmem-region.txt
> @@ -29,6 +29,18 @@ Required properties:
>  		in a separate device node. Having multiple address ranges in a
>  		node implies no special relationship between the two ranges.
>  
> +		This property may be replaced or supplemented with a
> +		memory-region property. Only one of reg or memory-region
> +		properties is required.
> +
> +	- memory-region:
> +		Reference to the reserved memory node. The reserved memory
> +		node should be defined as per the bindings in
> +		reserved-memory.txt

Though we've never enforced it, but /reserved-memory should be within 
the bounds of /memory node(s). Is that the intent here? If so, how does 
that work? Wouldn't all the memory be persistent then? Or some other 
system processor is preserving the contents?

> +
> +		This property may be replaced or supplemented with a reg
> +		property. Only one of reg or memory-region is required.
> +
>  Optional properties:
>  	- Any relevant NUMA assocativity properties for the target platform.
>  
> @@ -63,3 +75,20 @@ Examples:
>  		volatile;
>  	};
>  
> +
> +	/*
> +	 * This example uses a reserved-memory entry instead of
> +	 * specifying the memory region directly in the node.
> +	 */
> +
> +	reserved-memory {
> +		pmem_1: pmem@5000 {
> +			no-map;

Just add 'compatible = "pmem-region";' here and be done with it. Why add 
a layer of indirection?

> +			reg = <0x00005000 0x00001000>;
> +		};
> +	};
> +
> +	pmem@1 {

No 'reg', so shouldn't have a unit-address here.

> +		compatible = "pmem-region";
> +		memory-region = <&pmem_1>;
> +	};
> -- 
> 2.25.0.265.gbab2e86ba0-goog
>