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: <86b9c6d6-e8e5-7f6d-0970-460baf9b6fcc@linaro.org>
Date:   Wed, 27 Jul 2022 10:47:10 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [RFC] Correct memory layout reporting for "jedec,lpddr2" and
 related bindings
Content-Language: en-US
To:     Julius Werner <jwerner@chromium.org>
Cc:     Krzysztof Kozlowski <krzk+dt@kernel.org>,
        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>
 <3bb0ffa0-8091-0848-66af-180a41a68bf7@linaro.org>
 <CAODwPW89xZQZiZdQNt6+CcRjz=nbEAAFH0h_dBFSE5v3aFU4rQ@mail.gmail.com>
 <8f51aed8-956b-ac09-3baf-2b4572db1352@linaro.org>
 <CAODwPW9MvYJo8QbKOoVcUAKJ8Hxon2MCv_H5qpv=yaSTLLc+ug@mail.gmail.com>
 <628a7302-1409-81f7-f72b-6b1645df9225@linaro.org>
 <CAODwPW-4i+idH8Nz6=EmNUXYWgWkoOHs3wOZ7BbrH5GwGDZ1Ww@mail.gmail.com>
 <1f3189ef-7d3f-27b3-a691-b9649090b650@linaro.org>
 <CAODwPW-GDkfyFaNSnEngpSfz8LSXRetu+xwp3QrFHP1rH1O06w@mail.gmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <CAODwPW-GDkfyFaNSnEngpSfz8LSXRetu+xwp3QrFHP1rH1O06w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 21/07/2022 01:42, Julius Werner wrote:
> Sorry, got distracted from this for a bit. Sounds like we were pretty
> much on the same page about how the updated binding should look like
> here, the remaining question was just about the compatible string.
> 
>>>> Yes, we can. You still would need to generate the compatible according
>>>> to the current bindings. Whether we can change it I am not sure. I think
>>>> it depends how much customization is possible per vendor, according to
>>>> JEDEC spec. If we never ever have to identify specific part, because
>>>> JEDEC spec and registers tell us everything, then we could skip it,
>>>> similarly to lpddr2 and jedec,spi-nor.
>>>
>>> Shouldn't that be decided per use case? In general LPDDR is a pretty
>>> rigid set of standards and memory controllers are generally compatible
>>> with any vendor without hardcoding vendor-specific behavior, so I
>>> don't anticipate that this would be likely (particularly since there
>>> is no "real" kernel device driver that needs to initialize the full
>>> memory controller, after all, these bindings are mostly
>>> informational).
>>
>> If decided per use case understood as "decided depending how to use the
>> bindings" then answer is rather not. For example Linux implementation is
>> usually not the best argument to shape the bindings and usually to such
>> arguments answer is: "implementation does not matter".
>>
>> If by "use case" you mean actual hardware or specification
>> characteristics, then yes, of course. This is why I wrote "it depends".
> 
> By "use case" I mean our particular platform and firmware requirements
> -- or rather, the realities of building devices with widely
> multi-sourced LPDDR parts. One cannot efficiently build firmware that
> can pass an exact vendor-and-part-specific compatible string to Linux
> for this binding for every single LPDDR part used on such a platform.

Why cannot? You want to pass them as numerical values which directly map
to vendor ID and some part, don't they?

> And I don't see why that should be needed, either... that's kinda the
> point of having an interoperability standard, after all, that you can
> just assume the devices all work according to the same spec and don't
> need to hardcode details about each specific instance.

If we talk about standard, then DT purpose is not for autodetectable
pieces. These values are autodetectable, so such properties should not
be encoded in DT.

> 
> The existing bindings seem to have clearly been designed for platforms
> that only ever use a single specific LPDDR part, and in those cases
> these issues don't come up, so I assume this choice had just been made
> without much thought. 

True.

> But now I would like to reuse them on platforms
> where this is a problem, and that's why I would like to either expand
> or change the binding to (also) allow a generic compatible string to
> solve it. (Whether that means moving all platforms to only use this
> generic compatible string, or allowing it side-by-side with the
> existing part-specific compatible strings is up to you -- I don't want
> to preclude other platforms from doing what they prefer, I just want
> to make sure there's some form of workable solution for my platform.
> But if you would prefer this to be an all-or-nothing thing, we could
> change everything over to a generic compatible string too if you
> want.)
> 
>>> Of course there may always be mistakes and broken
>>> devices that need custom handling, and if someone has a platform with
>>> such a case I of course don't want to preclude them from tying special
>>> behavior to a custom compatible string. But why would that mean we
>>> need to make this mandatory for all platforms even if it's not
>>> relevant (and not practically feasible) for them? Why not allow both?
>>
>> Depends. If generic compatible is not enough (works only for your case)
>> then it should have never been added alone.
> 
> No, I don't think it would work only for my case -- in fact I think
> it's pretty unlikely that we'll ever find a case where this doesn't
> work. LPDDR is a very rigid standard that generally needs to be able
> to interoperate (at the memory controller and firmware training code
> level) without any vendor-specific software quirks. I was just trying
> to say that I wouldn't want to stop anybody from adding
> vendor-specific quirks for a platform if they really needed to -- but
> I don't know of any such case in practice and I doubt it actually
> exists. The few existing uses of this binding don't use the compatible
> string for anything more than to parse out the manufacturer ID, which
> I think would make much more sense to model as a property.


Best regards,
Krzysztof