Re: [PATCH 1/4] dt-bindings: memory: Factor out common properties of LPDDR bindings

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 01/09/2022 03:09, Julius Werner wrote:
>>> +properties:
>>> +  manufacturer-id:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description:
>>> +      Manufacturer ID read from Mode Register 5.
>>
>> Are you sure that register numbers (here 5, 6-7-8 later) are the same
>> between LPDDR 2-5? The description should match the broadest case and
>> specific schema can narrow or correct it.
> 
> Yes, the new LPDDR versions only ever seem to add new mode registers,
> but the meaning of 5, 6 and 7 have always stayed the same. (For 8, the
> bit fields have remained the same but the mapping of bit patterns to
> values has changed.)
> 
>>> This also un-deprecates the manufacturer ID property for LPDDR3 (and
>>> introduces it to LPDDR2), since it was found that having this
>>> information available in a separate property can be useful in some
>>> cases.
>>
>> Why do you need to un-deprecate them if you have this information in
>> compatible? This was not exactly the previous consensus. My statement
>> was ok for un-deprecating if you cannot derive them from compatible. Now
>> you can. This should be the same as USB device schema.
> 
> Okay. I think there is value in having these as separate properties,
> because it makes them much easier to read and use. 

Storing same value in multiple places is duplication and maintenance
effort. Does not make anything easier.


> If we don't have
> them we always need to do all this string parsing first, and if
> systems allow both kinds of compatible strings (auto-generated and
> static) they'll need to be able to distinguish and handle both of
> those when parsing... I think it would be much less friction if each
> datum of interest could directly be read out of a property. I think
> having a bit of redundancy is fine and common in device tree bindings

No, it's not common.

> (e.g. most properties for most devices are really implied by the
> compatible string because that same type of device is always built in
> the same way, but that doesn't mean it's not useful to list them
> separately for ease-of-access). But I can remove them if you disagree.

Just like we do not have them for USB, I don't really see the reason to
have them for memory.

Best regards,
Krzysztof