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, 2 Sep 2022 15:24:53 -0500
From:   Rob Herring <robh@kernel.org>
To:     Julius Werner <jwerner@chromium.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Dmitry Osipenko <digetx@gmail.com>,
        Doug Anderson <dianders@chromium.org>,
        Jian-Jia Su <jjsu@google.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/4] dt-bindings: memory: Add jedec,lpddr4 and
 jedec,lpddr5 bindings
Message-ID: <20220902202453.GA338977-robh@kernel.org>
References: <20220831013359.1807905-1-jwerner@chromium.org>
 <20220831013359.1807905-4-jwerner@chromium.org>
 <983c1224-8174-3534-a276-d1ab1f9968a4@linaro.org>
 <CAODwPW_70kdn4XTCs_vhbWwjEXS8E8zC9MTa6-szb5SayvcSag@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAODwPW_70kdn4XTCs_vhbWwjEXS8E8zC9MTa6-szb5SayvcSag@mail.gmail.com>
Precedence: bulk

On Wed, Aug 31, 2022 at 06:10:51PM -0700, Julius Werner wrote:
> > > diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
> > > index 0c7d2feafd77c8..e1182e75ca1a3f 100644
> > > --- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
> > > +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
> > > @@ -53,9 +53,13 @@ properties:
> > >        - 512
> > >        - 1024
> > >        - 2048
> > > +      - 3072
> > >        - 4096
> > > +      - 6144
> > >        - 8192
> > > +      - 12288
> > >        - 16384
> > > +      - 24576
> > >        - 32768
> >
> > Either you limit now LPDDR2 and LPDDR3 to old values or instead add this
> > bigger list to LPDDR4 and LPDDR5 (if it works that way).
> 
> The problem is that each spec has its own set of valid values, e.g.
> LPDDR3 only defines 4GB, 8GB, 16GB and 32GB, and then LPDDR4 inserted
> the 6GB, 12GB and 24GB options in between there. I don't think there's
> a way to exactly describe the valid values for each version without
> having a whole separate enum list for each. Do you think checking for
> that is important enough to be worth having all that extra duplication
> between the schemas? I don't think it adds that much (e.g. a value for
> an individual memory part can still be wrong even if it is one of the
> valid values for that type, so how much use is this validation
> anyway?), but I can split it out if you want to.

I tend to agree with you that it isn't worth the complexity.


> > > +  serial-id:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +    description:
> > > +      Serial IDs read from Mode Registers 47 through 54. One byte per uint32
> > > +      cell (i.e. <MR47 MR48 MR49 MR50 MR51 MR52 MR53 MR54>).
> > > +    minItems: 8
> >
> > No need for minItems.
> 
> Can you explain why? I'm okay with taking these out, but it is a real
> constraint so I'm not sure why we shouldn't be describing it here?
> (The serial ID always has exactly 8 bytes, an ID with less than 8
> would not be valid and probably a typo.)

Because if minItems is not specified, then it defaults to same as 
maxItems value. This is a departure from json-schema, but we almost 
always need a fixed number here and I didn't want to be specifying 
minItems/maxItems everywhere. We really need a 'numItems' or something.

Rob