Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <4afdd1a2-6670-35aa-da70-7246ed96ef60@linaro.org>
Date:   Fri, 17 Jun 2022 19:17:00 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [RFC] Correct memory layout reporting for "jedec,lpddr2" and
 related bindings
Content-Language: en-US
To:     Julius Werner <jwerner@chromium.org>,
        Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc:     Dmitry Osipenko <digetx@gmail.com>, Jian-Jia Su <jjsu@google.com>,
        Doug Anderson <dianders@chromium.org>,
        Rob Herring <robh+dt@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        Nikola Milosavljevic <mnidza@outlook.com>
References: <CAODwPW9E8wWwxbYKyf4_-JFb4F-JSmLR3qOF_iudjX0f9ndF0A@mail.gmail.com>
 <CAODwPW8fiFSNehZbZDdR9kjHxohLGiyE7edU=Opy0xV_P8JbEQ@mail.gmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <CAODwPW8fiFSNehZbZDdR9kjHxohLGiyE7edU=Opy0xV_P8JbEQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 14/06/2022 19:28, Julius Werner wrote:
> Sorry, wrong email for Krzysztof.

You need to base your upstream work on upstream tree. My email was
changed like three months ago...

> 
> On Tue, Jun 14, 2022 at 7:25 PM Julius Werner <jwerner@chromium.org> wrote:
>>
>> We (Chromium OS) have been trying to find a way to pass LPDDR memory
>> chip information that is available to the firmware through the FDT
>> (mostly for userspace informational purposes, for now). We have been
>> using and expanding the existing "jedec,lpddr2" and "jedec,lpddr3"
>> bindings for this (e.g. [1]). The goal is to be able to identify the
>> memory layout of the system (how the parts look like, how they're tied
>> together, how much capacity there is in total) as accurately as
>> possible from software-probed values.
>>
>> The existing bindings contain the fields "io-width" and "density",
>> which is terminology directly matching what an LPDDR chip can report
>> to firmware through the "Mode Register" interface, specifically MR8.
>> (The LPDDR specs describing this are not public, but you can see the
>> register definitions in most LPDDR chip datasheets that can be
>> randomly found online, e.g. [2] page 37.) The code in
>> drivers/memory/of_memory.c also suggests that these are supposed to
>> directly correspond to the MR8 values read from the chip, since when
>> of_lpddr2_get_info() copies the device tree values into struct
>> lpddr2_info, it encodes them in a format that directly matches the
>> mode register bit field patterns.
>>
>> The problem with this is that each individual LPDDR chip has its own
>> set of mode registers (per rank) that only describe the density of
>> that particular chip (rank). The host memory controller may have
>> multiple channels (each of which is basically an entirely separate set
>> of physical LPDDR pins on the board), a single channel may be
>> connected to multiple LPDDR chips (e.g. if the memory controller has
>> an outgoing 32-bit channel, that channel could be tied to two 16-bit
>> LPDDR chips by tying the low 16 bits to one and the high 16 bits to
>> the other), and then each of those chips may offer multiple
>> independent ranks (which rank is being accessed at a given time is
>> controlled by a separate chip select pin).
>>
>> So if we just have one "io-width" and one "density" field in the FDT,
>> there's no way to figure out how much memory there's actually
>> connected in total, because that only describes a single LPDDR chip.


Isn't the memory node used for that purpose - to identify how much
memory you have in total?

>> Worse, there may be chips where different ranks have different
>> densities (e.g. a 6GB dual-rank chip with one 4GB and one 2GB rank),
>> and different channels could theoretically be connected to chips of
>> completely different manufacturers.

>>
>> We need to be able to report the information that's currently encoded
>> in the "jedec,lpddr2" binding separately for each channel+rank
>> combination, and we need to be able to tell how many LPDDR chips are
>> combined under a single memory channel. For the former, I'd suggest
>> creating a separate FDT node for each channel, and then creating
>> subnodes under those for each rank that implement the binding. For the
>> latter, I would suggest adding a new property "channel-io-width" which

No, because io-width is a standard property, so it should be used
instead. It could be defined in channel node.

I'll think later, although it would be easier if you just bounce the
message to me, not forward. Much easier to read...


Best regards,
Krzysztof