linux-kernel - Re: [PATCH 3/4] dt-bindings: memory: Add jedec,lpddr4 and jedec,lpddr5 bindings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220902202453.GA338977-robh@kernel.org>
Date:   Fri, 2 Sep 2022 15:24:53 -0500
From:   Rob Herring <robh@...nel.org>
To:     Julius Werner <jwerner@...omium.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Dmitry Osipenko <digetx@...il.com>,
        Doug Anderson <dianders@...omium.org>,
        Jian-Jia Su <jjsu@...gle.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/4] dt-bindings: memory: Add jedec,lpddr4 and
 jedec,lpddr5 bindings

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