Re: [PATCH v1 1/2] dt-bindings: memory: tegra20: emc: Document optional LPDDR properties

[Dmitry Osipenko]

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!