Hi Rob, On 4/25/19 9:51 PM, Rob Herring wrote: > On Fri, Apr 19, 2019 at 04:19:22PM +0200, Lukasz Luba wrote: >> The device tree bindings for LPDDR3 SDRAM memories. >> >> For specifying the AC timing parameters of the memory device >> the 'lpddr3' binding uses binding 'lpddr2-timings'. >> >> Signed-off-by: Lukasz Luba <l.luba@xxxxxxxxxxxxxxxxxxx> >> --- >> .../devicetree/bindings/lpddr3/lpddr3-timings.txt | 57 +++++++++++++ >> .../devicetree/bindings/lpddr3/lpddr3.txt | 93 ++++++++++++++++++++++ > > Please rename the lpddr2 directory to 'ddr' and add these to it. OK, I will rename it in the nex patch set. > > Maybe whatever properties are common should be put in a common doc. There are maybe a few common properties, but I would not dare to merge lpddr2 and lpddr3 before consulting it with TI engineers who made LPDDR2 support. Could we work on a common file after the patch set got merged? > >> 2 files changed, 150 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/lpddr3/lpddr3-timings.txt >> create mode 100644 Documentation/devicetree/bindings/lpddr3/lpddr3.txt >> >> diff --git a/Documentation/devicetree/bindings/lpddr3/lpddr3-timings.txt b/Documentation/devicetree/bindings/lpddr3/lpddr3-timings.txt >> new file mode 100644 >> index 0000000..ebf3e00 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/lpddr3/lpddr3-timings.txt >> @@ -0,0 +1,57 @@ >> +* AC timing parameters of LPDDR3 memories for a given speed-bin. >> +* The structures are based on LPDDR2 and extended where needed. >> + >> +Required properties: >> +- compatible : Should be "jedec,lpddr3-timings" >> +- min-freq : minimum DDR clock frequency for the speed-bin. Type is <u32> >> +- max-freq : maximum DDR clock frequency for the speed-bin. Type is <u32> >> + >> +Optional properties: >> + >> +The following properties represent AC timing parameters from the memory >> +data-sheet of the device for a given speed-bin. All these properties are >> +of type <u32> and the default unit is ps (pico seconds). >> +- tRFC >> +- tRRD >> +- tRPab >> +- tRPpb >> +- tRCD >> +- tRC >> +- tRAS >> +- tWTR >> +- tWR >> +- tRTP >> +- tW2W-C2C >> +- tR2R-C2C >> +- tFAW >> +- tXSR >> +- tXP >> +- tCKE >> +- tCKESR >> +- tMRD >> + >> +Example: >> + >> +timings_samsung_K3QF2F20DB_800mhz: lpddr3-timings@0 { > > Since the lpddr2 version was written, we've gotten stricter about > allowing unit-address without reg property. Perhaps 'reg' should be the > max-freq instead. OK, so I will rename 'max-freq' to 'reg' and add a comment with: '/* workaround: it shows max-freq */ Does it make sense? > >> + compatible = "jedec,lpddr3-timings"; >> + min-freq = <100000000>; >> + max-freq = <800000000>; >> + tRFC = <65000>; >> + tRRD = <6000>; >> + tRPab = <12000>; >> + tRPpb = <12000>; >> + tRCD = <10000>; >> + tRC = <33750>; >> + tRAS = <23000>; >> + tWTR = <3750>; >> + tWR = <7500>; >> + tRTP = <3750>; >> + tW2W-C2C = <0>; >> + tR2R-C2C = <0>; >> + tFAW = <25000>; >> + tXSR = <70000>; >> + tXP = <3750>; >> + tCKE = <3750>; >> + tCKESR = <3750>; >> + tMRD = <7000>; >> +}; >> diff --git a/Documentation/devicetree/bindings/lpddr3/lpddr3.txt b/Documentation/devicetree/bindings/lpddr3/lpddr3.txt >> new file mode 100644 >> index 0000000..fc7875c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/lpddr3/lpddr3.txt >> @@ -0,0 +1,93 @@ >> +* LPDDR3 SDRAM memories compliant to JEDEC JESD209-2 > > That's an LPDDR2 spec. Right, should be JESD209-3C. Thank you for the review. Regards, Lukasz > >> + >> +Required properties: >> +- compatible : Should be - "jedec,lpddr3" >> +- density : <u32> representing density in Mb (Mega bits) >> +- io-width : <u32> representing bus width. Possible values are 8, 16, 32, 64 >> + >> +Optional properties: >> + >> +The following optional properties represent the minimum value of some AC >> +timing parameters of the DDR device in terms of number of clock cycles. >> +These values shall be obtained from the device data-sheet. >> +- tRFC-min-tck >> +- tRRD-min-tck >> +- tRPab-min-tck >> +- tRPpb-min-tck >> +- tRCD-min-tck >> +- tRC-min-tck >> +- tRAS-min-tck >> +- tWTR-min-tck >> +- tWR-min-tck >> +- tRTP-min-tck >> +- tW2W-C2C-min-tck >> +- tR2R-C2C-min-tck >> +- tWL-min-tck >> +- tDQSCK-min-tck >> +- tRL-min-tck >> +- tFAW-min-tck >> +- tXSR-min-tck >> +- tXP-min-tck >> +- tCKE-min-tck >> +- tCKESR-min-tck >> +- tMRD-min-tck >> + >> +Child nodes: >> +- The lpddr3 node may have one or more child nodes of type "lpddr3-timings". >> + "lpddr3-timings" provides AC timing parameters of the device for >> + a given speed-bin. Please see Documentation/devicetree/ >> + bindings/lpddr3/lpddr3-timings.txt for more information on "lpddr3-timings" >> + >> +Example: >> + >> +samsung_K3QF2F20DB: lpddr3 { >> + compatible = "Samsung,K3QF2F20DB","jedec,lpddr3"; >> + density = <16384>; >> + io-width = <32>; >> + >> + tRFC-min-tck = <17>; >> + tRRD-min-tck = <2>; >> + tRPab-min-tck = <2>; >> + tRPpb-min-tck = <2>; >> + tRCD-min-tck = <3>; >> + tRC-min-tck = <6>; >> + tRAS-min-tck = <5>; >> + tWTR-min-tck = <2>; >> + tWR-min-tck = <7>; >> + tRTP-min-tck = <2>; >> + tW2W-C2C-min-tck = <0>; >> + tR2R-C2C-min-tck = <0>; >> + tWL-min-tck = <8>; >> + tDQSCK-min-tck = <5>; >> + tRL-min-tck = <14>; >> + tFAW-min-tck = <5>; >> + tXSR-min-tck = <12>; >> + tXP-min-tck = <2>; >> + tCKE-min-tck = <2>; >> + tCKESR-min-tck = <2>; >> + tMRD-min-tck = <5>; >> + >> + timings_samsung_K3QF2F20DB_800mhz: lpddr3-timings@0 { >> + compatible = "jedec,lpddr3-timings"; >> + min-freq = <100000000>; >> + max-freq = <800000000>; >> + tRFC = <65000>; >> + tRRD = <6000>; >> + tRPab = <12000>; >> + tRPpb = <12000>; >> + tRCD = <10000>; >> + tRC = <33750>; >> + tRAS = <23000>; >> + tWTR = <3750>; >> + tWR = <7500>; >> + tRTP = <3750>; >> + tW2W-C2C = <0>; >> + tR2R-C2C = <0>; >> + tFAW = <25000>; >> + tXSR = <70000>; >> + tXP = <3750>; >> + tCKE = <3750>; >> + tCKESR = <3750>; >> + tMRD = <7000>; >> + }; >> +} >> -- >> 2.7.4 >> > >