Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20211006224659.21434-1-digetx@gmail.com> <20211006224659.21434-4-digetx@gmail.com>
 <YWimShkkxkR+bQLK@robh.at.kernel.org> <CAODwPW-2othAhTC9a_joBdHScA86iqs-FkZJZDrK5LxrJm8F8Q@mail.gmail.com>
 <d69f99ed-75a4-9b8c-addb-3eaa2b2be6a6@canonical.com> <CAODwPW-AMqQ+ZbnvUTuxP5Bbxdcy08GSXnhcLC+V6BW=OPYaqg@mail.gmail.com>
 <6568fd31-113f-1581-4eff-45a4a1eb4e5d@canonical.com> <c33c9de2-ff0f-0ba7-b424-728d316dcca2@gmail.com>
In-Reply-To: <c33c9de2-ff0f-0ba7-b424-728d316dcca2@gmail.com>
From:   Julius Werner <jwerner@chromium.org>
Date:   Wed, 9 Feb 2022 16:32:25 -0800
Message-ID: <CAODwPW-VfbvWs3OVON1m=WWFXN2V86o9jqurVDShpfTrSn=Mkw@mail.gmail.com>
Subject: Re: [PATCH v5 3/9] dt-bindings: memory: lpddr2: Add revision-id properties
To:     Dmitry Osipenko <digetx@gmail.com>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        Julius Werner <jwerner@chromium.org>,
        Rob Herring <robh@kernel.org>, linux-tegra@vger.kernel.org,
        Rob Herring <robh+dt@kernel.org>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        devicetree@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
        Thierry Reding <thierry.reding@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

> I don't mind, but I also don't see where the revision-id property of
> LPDDR3 is used at all. I can't find any device-tree with LPDDR3
> revision-id and don't see it being used in the code either. Maybe it's
> the LPDDR3 binding that needs to be changed?

We are using the revision ID in userspace (read through
/proc/device-tree) for runtime memory identification. We don't have a
kernel driver bound to it. Our boot firmware is inserting this value
at runtime into the FDT (that's basically the reason we have this, our
firmware auto-detects memory during boot and we use the FDT to report
what it found to userspace), that's why you can't find it anywhere in
the static device trees in boot/dts/.

> I made each LPDDR2 revision-id property to correspond to a dedicated MR
> of LPDDR, which feels okay to me to since it matches h/w.

I'm not super married to my solution, so if that makes things easier
we can standardize on the two-property version as well. I mostly
designed it my way because I thought we may one day also want to do
something like this for the 8-byte LPDDR5 serial-id, and then it would
get kinda cumbersome to have serial-id1 through serial-id8 all as
separate properties. But that's also a bridge we can cross when we get
there.

My use case is in a position where we could still change this now
without requiring backwards-compatibility. Krzysztof, would you be
okay if I instead changed the "jedec,lpddr3" to the same thing
"jedec,lpddr2" does -- seeing as the original patch was from me, my
use case could handle the switch, there has never been any actual
kernel code using the property, and it seems very unlikely that anyone
else has silently started using the same thing in the time it's been
in the tree? Or do we also need to go the official deprecation route
for that?