04.10.2021 10:42, Krzysztof Kozlowski пишет: > On 03/10/2021 03:32, Dmitry Osipenko wrote: >> Add binding for standard LPDDR2 memory chip properties. >> >> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx> >> --- >> .../memory-controllers/jedec,lpddr2.yaml | 80 +++++++++++++++++++ >> include/dt-bindings/memory/lpddr2.h | 25 ++++++ > > Hi Dmitry, > > Thanks for doing this. I think I should be slightly more descriptive in > my previous comment. What I meant, is to use existing DDR bindings > (which might include or require converting them to YAML): > Documentation/devicetree/bindings/ddr/ > > The bindings are already used: > arch/arm/boot/dts/elpida_ecb240abacn.dtsi > arch/arm/boot/dts/exynos5422-odroid-core.dtsi > drivers/memory/samsung/exynos5422-dmc.c Thanks! I missed that there is ddr/ subdir. > You can remove the Documentation/devicetree/bindings/ddr/lpddr2.txt > after full conversion, so also including AC timings and AC timing > parameters. The timing parameters could be a separate YAML, if you want > to convert everything. You can also skip it, because it is not necessary > for your work. > > > Rob, > Any advice from your side where to put LPDDR2 dtschema bindings? The > existing location was bindings/ddr/ but maybe this should be part of > memory-controllers (although it is not actually a controller but rather > used by the controller)? +1 for having it inside of memory-controllers/ >> 2 files changed, 105 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml >> create mode 100644 include/dt-bindings/memory/lpddr2.h >> >> diff --git a/Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml b/Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml >> new file mode 100644 >> index 000000000000..ef227eba1e4a >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml >> @@ -0,0 +1,80 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/memory-controllers/jedec,lpddr2.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: JEDEC LPDDR2 SDRAM >> + >> +maintainers: >> + - Krzysztof Kozlowski <krzk@xxxxxxxxxx> >> + >> +properties: > > You need compatible (see lpddr2.txt) > >> + jedec,lpddr2-manufacturer-id: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + maximum: 255 >> + description: | >> + Unique manufacturer ID of SDRAM chip. See MR5 description in JESD209-2. > > Plus: > "See include/dt-bindings/memory/lpddr2.h for known manufactured IDs." > > However I wonder whether we need it. It should be taken from the vendor > part of compatible. It shouldn't be needed if compatible is used. >> + >> + jedec,lpddr2-revision-id1: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + maximum: 255 >> + description: | >> + Revision 1 value of SDRAM chip. >> + See MR6 description in chip vendor specification. >> + >> + jedec,lpddr2-revision-id2: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + maximum: 255 >> + description: | >> + Revision 2 value of SDRAM chip. >> + See MR7 description in chip vendor specification. >> + >> + jedec,lpddr2-density-mbits: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: | >> + Density in megabits of SDRAM chip. See MR8 description in JESD209-2. >> + enum: >> + - 64 >> + - 128 >> + - 256 >> + - 512 >> + - 1024 >> + - 2048 >> + - 4096 >> + - 8192 >> + - 16384 >> + - 32768 >> + >> + jedec,lpddr2-io-width-bits: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: | >> + IO bus width in bits of SDRAM chip. See MR8 description in JESD209-2. >> + enum: >> + - 32 >> + - 16 >> + - 8 >> + >> + jedec,lpddr2-type: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: | >> + LPDDR type which corresponds to a number of words SDRAM pre-fetches >> + per column request. See MR8 description in JESD209-2. >> + enum: >> + - 0 # S4 (4 words prefetch architecture) >> + - 1 # S2 (2 words prefetch architecture) >> + - 2 # NVM (Non-volatile memory) > > Type should not be needed but instead taken from compatible. Unless Rob > has here preference and maybe change the DDR bindings? > > requiredProperties for compatible, density, io-width. Alright
