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

[Rob Herring <robh@kernel.org>]

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