Message ID | 20210929200305.4245-2-digetx@gmail.com |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | tegra20-emc: Identify memory chip by LPDDR configuration | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success | |
robh/dt-meta-schema | success | |
robh/dtbs-check | success |
On 29/09/2021 22:03, Dmitry Osipenko wrote: > Some Tegra20 boards don't use RAM code for the memory chip identification > and the identity information should read out from LPDDR chip in this case. > Document new optional generic LPDDR properties that will be used for the > memory chip identification if RAM code isn't provided. Please mention how they are going to be used. Naively I would assume that these new properties describe the RAM you have. However it seems you do not use them to configure the device but to compare with the device. Why do you need them? > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > .../nvidia,tegra20-emc.yaml | 43 +++++++++++++++++-- > 1 file changed, 40 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml > index cac6842dc8f1..6d01b1bf6304 100644 > --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml > +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml > @@ -158,6 +158,46 @@ patternProperties: > description: > Value of RAM_CODE this timing set is used for. > > + jedec,lpddr-manufacturer-id: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Unique manufacturer ID of SDRAM chip this timing set is used for. > + See MR5 description in JEDEC LPDDR2 specification (JESD209-2). > + > + jedec,lpddr-revision-id1: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Revision 1 value of SDRAM chip this timing set is used for. > + See MR6 description in chip vendor specification. > + > + jedec,lpddr-revision-id2: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Revision 2 value of SDRAM chip this timing set is used for. > + See MR7 description in chip vendor specification. > + > + jedec,lpddr-density-mbits: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Density in megabits of SDRAM chip this timing set is used for. > + See MR8 description in JEDEC LPDDR2 specification. > + > + jedec,lpddr-io-width-bits: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + IO bus width in bits of SDRAM chip this timing set is used for. > + See MR8 description in JEDEC LPDDR2 specification. > + > + jedec,lpddr-type: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + LPDDR type which corresponds to a number of words SDRAM pre-fetches > + per column request that this timing set is used for. > + See MR8 description in JEDEC LPDDR2 specification. > + enum: > + - 4 # S4 (4 words prefetch architecture) > + - 2 # S2 (2 words prefetch architecture) I think instead you should use generic lpddr{2,3} bindings - have a separate node and reference it via a phandle. Best regards, Krzysztof
30.09.2021 09:54, Krzysztof Kozlowski пишет: > On 29/09/2021 22:03, Dmitry Osipenko wrote: >> Some Tegra20 boards don't use RAM code for the memory chip identification >> and the identity information should read out from LPDDR chip in this case. >> Document new optional generic LPDDR properties that will be used for the >> memory chip identification if RAM code isn't provided. > > Please mention how they are going to be used. Naively I would assume > that these new properties describe the RAM you have. However it seems > you do not use them to configure the device but to compare with the > device. Why do you need them? Yes, the properties describe hardware configuration of external DRAM chip. This information is read-only and it's actually used for configuring SoC memory controller. This MC configuration is already pre-configured by bootloader and partially it shouldn't be ever touched by software. Kernel driver needs to reconfigure only a part of hardware on memory freq changes. The memory timing data is tuned for a specific DRAM chip and board, it doesn't include info which identifies the chip. So we need to read out DRAM config from hardware and find the matching timing in a device-tree by comparing the chip-unique properties. Note that only LPDDR chips have that chip-identity info. Regular DDR chips require SPD or other means, like NVMEM in case of Tegra. I'll extend the commit message. ... >> + - 4 # S4 (4 words prefetch architecture) >> + - 2 # S2 (2 words prefetch architecture) > > I think instead you should use generic lpddr{2,3} bindings - have a > separate node and reference it via a phandle. It indeed shouldn't be a problem to create lpddr binding and move these props there. Extra phandle shouldn't be needed, should be fine to keep these new DRAM properties within the chip-descriptor nodes that we already have in tegra device-trees. We'll only need to $ref the lpddr binding for the descriptor node in the binding. I.e. to make it similar to regulator bindings where there is generic regulator.yaml + hw-specific properties. I'll try to implement this in v2, thanks!
diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml index cac6842dc8f1..6d01b1bf6304 100644 --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml @@ -158,6 +158,46 @@ patternProperties: description: Value of RAM_CODE this timing set is used for. + jedec,lpddr-manufacturer-id: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Unique manufacturer ID of SDRAM chip this timing set is used for. + See MR5 description in JEDEC LPDDR2 specification (JESD209-2). + + jedec,lpddr-revision-id1: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Revision 1 value of SDRAM chip this timing set is used for. + See MR6 description in chip vendor specification. + + jedec,lpddr-revision-id2: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Revision 2 value of SDRAM chip this timing set is used for. + See MR7 description in chip vendor specification. + + jedec,lpddr-density-mbits: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Density in megabits of SDRAM chip this timing set is used for. + See MR8 description in JEDEC LPDDR2 specification. + + jedec,lpddr-io-width-bits: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + IO bus width in bits of SDRAM chip this timing set is used for. + See MR8 description in JEDEC LPDDR2 specification. + + jedec,lpddr-type: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + LPDDR type which corresponds to a number of words SDRAM pre-fetches + per column request that this timing set is used for. + See MR8 description in JEDEC LPDDR2 specification. + enum: + - 4 # S4 (4 words prefetch architecture) + - 2 # S2 (2 words prefetch architecture) + "#address-cells": const: 1 @@ -168,9 +208,6 @@ patternProperties: "^emc-table@[0-9]+$": $ref: "#/$defs/emc-table" - required: - - nvidia,ram-code - additionalProperties: false required:
Some Tegra20 boards don't use RAM code for the memory chip identification and the identity information should read out from LPDDR chip in this case. Document new optional generic LPDDR properties that will be used for the memory chip identification if RAM code isn't provided. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- .../nvidia,tegra20-emc.yaml | 43 +++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-)