[PATCH v2 1/3] powerpc/85xx: Add SEC6.0 device tree

[Liu Po-B43644]

>  -----Original Message-----
>  From: Wood Scott-B07421
>  Sent: Saturday, July 27, 2013 5:55 AM
>  To: Liu Po-B43644
>  Cc: linuxppc-dev at ozlabs.org; galak at kernel.crashing.org; Fleming Andy-
>  AFLEMING; Hu Mingkai-B21284; Liu Po-B43644
>  Subject: Re: [PATCH v2 1/3] powerpc/85xx: Add SEC6.0 device tree
>  
>  On 07/25/2013 09:41:17 PM, Po Liu wrote:
>  > +=====================================================================
>  > +Job Ring (JR) Node
>  > +
>  > +    Child of the crypto node defines data processing interface to
>  > SEC 6
>  > +    across the peripheral bus for purposes of processing
>  > +    cryptographic descriptors. The specified address
>  > +    range can be made visible to one (or more) cores.
>  > +    The interrupt defined for this node is controlled within
>  > +    the address range of this node.
>  > +
>  > +  - compatible
>  > +      Usage: required
>  > +      Value type: <string>
>  > +      Definition: Must include "fsl,sec-v6.0-job-ring", if it is
>  > +      back compatible with old version, better add them all.
>  
>  Please don't use colloquialisms such as "[you'd] better do this" in a
>  formal specification.
>  
>  Just say 'Must include "fsl,sec-v6.0-job-ring"' and leave it at that,
>  like the other bindings do.
Ok, I'll remove redundant words.
>  
>  > +===================================================================
>  > +Full Example
>  > +
>  > +Since some chips may embeded with more than one SEC 6, we abstract
>  > +all the same properties into one file qoriq-sec6.0-0.dtsi. Each chip
>  > +want to binding the node could simply include it in its own device
>  > +node tree. Below is full example in C293PCIE:
>  
>  Replace this with:
>  
>  Since some chips may contain more than one SEC, the dtsi contains only
>  the node contents, not the node itself.  A chip using the SEC should
>  include the dtsi inside each SEC node.  Example:
>  
>  > +In qoriq-sec6.0-0.dtsi:
>  > +
>  > +	compatible = "fsl,sec-v6.0";
>  > +	fsl,sec-era = <6>;
>  > +	#address-cells = <1>;
>  > +	#size-cells = <1>;
>  > +
>  > +	jr at 1000 {
>  > +		compatible = "fsl,sec-v6.0-job-ring",
>  > +			     "fsl,sec-v5.2-job-ring",
>  > +			     "fsl,sec-v5.0-job-ring",
>  > +			     "fsl,sec-v4.4-job-ring",
>  > +			     "fsl,sec-v4.0-job-ring";
>  > +		reg	   = <0x1000 0x1000>;
>  > +	};
>  > +
>  > +	jr at 2000 {
>  > +		compatible = "fsl,sec-v6.0-job-ring",
>  > +			     "fsl,sec-v5.2-job-ring",
>  > +			     "fsl,sec-v5.0-job-ring",
>  > +			     "fsl,sec-v4.4-job-ring",
>  > +			     "fsl,sec-v4.0-job-ring";
>  > +		reg	   = <0x2000 0x1000>;
>  > +	};
>  > +
>  > +In the C293 device tree, we add the include of public property:
>  > +
>  > +crypto at a0000 {
>  > +/include/ "qoriq-sec6.0-0.dtsi"
>  > +	};
>  
>  Whitespace
>  
>  > +
>  > +	crypto at a0000 {
>  > +		reg = <0xa0000 0x20000>;
>  > +		ranges = <0x0 0xa0000 0x20000>;
>  > +
>  > +		jr at 1000{
>  > +			interrupts = <49 2 0 0>;
>  > +		};
>  > +		jr at 2000{
>  > +			interrupts = <50 2 0 0>;
>  > +		};
>  > +	};
>  
>  You could combine the above like this:
>  
>  	crypto at a0000 {
>  		reg = <0xa0000 0x20000>;
>  		ranges = <0 0xa0000 0x20000>;
>  
>  		/include/ "qoriq-sec6.0-0.dtsi"
>  
>  		jr at 1000 {
>  			interrupts = <49 2 0 0>;
>  		};
>  
>  		jr at 2000 {
>  			interrupts = <50 2 0 0>;
>  		};
>  	};
>  
>  Why is it "qoriq-sec6.0-0.dtsi" and not "qoriq-sec6.0-dtsi"?
Ok, I'll change to qoriq-sec6.0.dtsi
>  
>  -Scott