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;
Date:   Fri, 11 Feb 2022 02:17:39 +0300
From:   Dmitry Osipenko <digetx@gmail.com>
To:     Julius Werner <jwerner@chromium.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        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>
Subject: Re: [PATCH v5 3/9] dt-bindings: memory: lpddr2: Add revision-id
 properties
Message-ID: <20220211021739.14f3614c@dimatab>
In-Reply-To: <CAODwPW-VfbvWs3OVON1m=WWFXN2V86o9jqurVDShpfTrSn=Mkw@mail.gmail.com>
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>
        <CAODwPW-VfbvWs3OVON1m=WWFXN2V86o9jqurVDShpfTrSn=Mkw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

=D0=92 Wed, 9 Feb 2022 16:32:25 -0800
Julius Werner <jwerner@chromium.org> =D0=BF=D0=B8=D1=88=D0=B5=D1=82:

> > 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? =20
>=20
> 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/.

Thank you for the clarification. Which device is that and why userspace
needs to know so much about memory?

> > 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. =20
>=20
> 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.
>=20
> 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?

If you're going to use multiple cells for other properties, then indeed
will be better to keep it consistent.