diff mbox series

[v3,5/8] dt-bindings: memory: lpddr3: deprecate manufacturer ID

Message ID 20220206135807.211767-6-krzysztof.kozlowski@canonical.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series dt-bindings: memory: convert to dtschema | expand

Commit Message

Krzysztof Kozlowski Feb. 6, 2022, 1:58 p.m. UTC
The memory manufacturer should be described in vendor part of
compatible, so there is no need to duplicate it in a separate property.
Similarly is done in LPDDR2 bindings.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 .../bindings/memory-controllers/ddr/jedec,lpddr3.yaml         | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Alim Akhtar Feb. 7, 2022, 4:14 a.m. UTC | #1
Hi Krzysztof

>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>Sent: Sunday, February 6, 2022 7:28 PM
>To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>; Rob Herring
><robh+dt@kernel.org>; Lukasz Luba <lukasz.luba@arm.com>; Alim Akhtar
><alim.akhtar@samsung.com>; Dmitry Osipenko <digetx@gmail.com>; linux-
>kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-
>pm@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-arm-
>kernel@lists.infradead.org
>Subject: [PATCH v3 5/8] dt-bindings: memory: lpddr3: deprecate
>manufacturer ID
>
>The memory manufacturer should be described in vendor part of compatible,
>so there is no need to duplicate it in a separate property.
>Similarly is done in LPDDR2 bindings.
>
>Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>---
> .../bindings/memory-controllers/ddr/jedec,lpddr3.yaml         | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/Documentation/devicetree/bindings/memory-
>controllers/ddr/jedec,lpddr3.yaml
>b/Documentation/devicetree/bindings/memory-
>controllers/ddr/jedec,lpddr3.yaml
>index d6787b5190ee..3bcba15098ea 100644
>--- a/Documentation/devicetree/bindings/memory-
>controllers/ddr/jedec,lpddr3.yaml
>+++ b/Documentation/devicetree/bindings/memory-
>controllers/ddr/jedec,lpd
>+++ dr3.yaml
>@@ -40,7 +40,9 @@ properties:
>   manufacturer-id:
>     $ref: /schemas/types.yaml#/definitions/uint32
>     description: |
>-      Manufacturer ID value read from Mode Register 5.
>+      Manufacturer ID value read from Mode Register 5.  The property is
>+      deprecated, manufacturer should be derived from the compatible.
>+    deprecated: true
>

Shouldn't it be the other way? As DT describes hardware and MR5 does contain
the Manufacturer ID, 
so getting Manufacturer ID from MR5 makes aligned to hardware description.


>   revision-id:
>     $ref: /schemas/types.yaml#/definitions/uint32-array
>--
>2.32.0
Krzysztof Kozlowski Feb. 7, 2022, 8:56 a.m. UTC | #2
On 07/02/2022 05:14, Alim Akhtar wrote:
> Hi Krzysztof
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>> Sent: Sunday, February 6, 2022 7:28 PM
>> To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>; Rob Herring
>> <robh+dt@kernel.org>; Lukasz Luba <lukasz.luba@arm.com>; Alim Akhtar
>> <alim.akhtar@samsung.com>; Dmitry Osipenko <digetx@gmail.com>; linux-
>> kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-
>> pm@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org
>> Subject: [PATCH v3 5/8] dt-bindings: memory: lpddr3: deprecate
>> manufacturer ID
>>
>> The memory manufacturer should be described in vendor part of compatible,
>> so there is no need to duplicate it in a separate property.
>> Similarly is done in LPDDR2 bindings.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>> ---
>> .../bindings/memory-controllers/ddr/jedec,lpddr3.yaml         | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/memory-
>> controllers/ddr/jedec,lpddr3.yaml
>> b/Documentation/devicetree/bindings/memory-
>> controllers/ddr/jedec,lpddr3.yaml
>> index d6787b5190ee..3bcba15098ea 100644
>> --- a/Documentation/devicetree/bindings/memory-
>> controllers/ddr/jedec,lpddr3.yaml
>> +++ b/Documentation/devicetree/bindings/memory-
>> controllers/ddr/jedec,lpd
>> +++ dr3.yaml
>> @@ -40,7 +40,9 @@ properties:
>>   manufacturer-id:
>>     $ref: /schemas/types.yaml#/definitions/uint32
>>     description: |
>> -      Manufacturer ID value read from Mode Register 5.
>> +      Manufacturer ID value read from Mode Register 5.  The property is
>> +      deprecated, manufacturer should be derived from the compatible.
>> +    deprecated: true
>>
> 
> Shouldn't it be the other way? As DT describes hardware and MR5 does contain
> the Manufacturer ID, 
> so getting Manufacturer ID from MR5 makes aligned to hardware description.

The code/driver can read MR5 and report it to user-space in case for
example we have a device compatible with different vendor and same
compatible is used. So compatible is re-used but we want real
manufacturer ID from the hardware.

But storing a fixed MR5 value in DT does not fit here. If someone takes
effort to encode manufacturer ID in the DTS, then he/she should take
effort to actually document the compatible.

Basically having MR5 in DT is equal to having a compatible in DTS. I
prefer the latter - simpler, less properties, using existing property
from DT spec instead of custom one.

Best regards,
Krzysztof
Alim Akhtar Feb. 7, 2022, 11:24 a.m. UTC | #3
>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>Sent: Monday, February 7, 2022 2:27 PM
>To: Alim Akhtar <alim.akhtar@samsung.com>; 'Rob Herring'
><robh+dt@kernel.org>; 'Lukasz Luba' <lukasz.luba@arm.com>; 'Dmitry
>Osipenko' <digetx@gmail.com>; linux-kernel@vger.kernel.org;
>devicetree@vger.kernel.org; linux-pm@vger.kernel.org; linux-samsung-
>soc@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>Subject: Re: [PATCH v3 5/8] dt-bindings: memory: lpddr3: deprecate
>manufacturer ID
>
>On 07/02/2022 05:14, Alim Akhtar wrote:
>> Hi Krzysztof
>>
>>> -----Original Message-----
>>> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>>> Sent: Sunday, February 6, 2022 7:28 PM
>>> To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>; Rob
>>> Herring <robh+dt@kernel.org>; Lukasz Luba <lukasz.luba@arm.com>; Alim
>>> Akhtar <alim.akhtar@samsung.com>; Dmitry Osipenko
><digetx@gmail.com>;
>>> linux- kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-
>>> pm@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-arm-
>>> kernel@lists.infradead.org
>>> Subject: [PATCH v3 5/8] dt-bindings: memory: lpddr3: deprecate
>>> manufacturer ID
>>>
>>> The memory manufacturer should be described in vendor part of
>>> compatible, so there is no need to duplicate it in a separate property.
>>> Similarly is done in LPDDR2 bindings.
>>>
>>> Signed-off-by: Krzysztof Kozlowski
>>> <krzysztof.kozlowski@canonical.com>
>>> ---
>>> .../bindings/memory-controllers/ddr/jedec,lpddr3.yaml         | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/memory-
>>> controllers/ddr/jedec,lpddr3.yaml
>>> b/Documentation/devicetree/bindings/memory-
>>> controllers/ddr/jedec,lpddr3.yaml
>>> index d6787b5190ee..3bcba15098ea 100644
>>> --- a/Documentation/devicetree/bindings/memory-
>>> controllers/ddr/jedec,lpddr3.yaml
>>> +++ b/Documentation/devicetree/bindings/memory-
>>> controllers/ddr/jedec,lpd
>>> +++ dr3.yaml
>>> @@ -40,7 +40,9 @@ properties:
>>>   manufacturer-id:
>>>     $ref: /schemas/types.yaml#/definitions/uint32
>>>     description: |
>>> -      Manufacturer ID value read from Mode Register 5.
>>> +      Manufacturer ID value read from Mode Register 5.  The property is
>>> +      deprecated, manufacturer should be derived from the compatible.
>>> +    deprecated: true
>>>
>>
>> Shouldn't it be the other way? As DT describes hardware and MR5 does
>> contain the Manufacturer ID, so getting Manufacturer ID from MR5 makes
>> aligned to hardware description.
>
>The code/driver can read MR5 and report it to user-space in case for example
>we have a device compatible with different vendor and same compatible is
>used. So compatible is re-used but we want real manufacturer ID from the
>hardware.
>
>But storing a fixed MR5 value in DT does not fit here. If someone takes effort
>to encode manufacturer ID in the DTS, then he/she should take effort to
>actually document the compatible.
>
>Basically having MR5 in DT is equal to having a compatible in DTS. I prefer the
>latter - simpler, less properties, using existing property from DT spec instead
>of custom one.
>
Ok, so there are two part of it, first one to get the compatible and bind the device 
and second one to get the manufacturer ID from MR5 for say user application.
Second one can still be handled at driver side, so

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>


>Best regards,
>Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml
index d6787b5190ee..3bcba15098ea 100644
--- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml
@@ -40,7 +40,9 @@  properties:
   manufacturer-id:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: |
-      Manufacturer ID value read from Mode Register 5.
+      Manufacturer ID value read from Mode Register 5.  The property is
+      deprecated, manufacturer should be derived from the compatible.
+    deprecated: true
 
   revision-id:
     $ref: /schemas/types.yaml#/definitions/uint32-array