diff mbox series

[v6,13/52] dt-bindings: memory: tegra124: emc: Document new interconnect property

Message ID 20201025221735.3062-14-digetx@gmail.com
State Not Applicable
Headers show
Series Introduce memory interconnect for NVIDIA Tegra SoCs | expand

Checks

Context Check Description
robh/dt-meta-schema fail build log
robh/checkpatch success

Commit Message

Dmitry Osipenko Oct. 25, 2020, 10:16 p.m. UTC
External memory controller is interconnected with memory controller and
with external memory. Document new interconnect property which turns
External Memory Controller into interconnect provider.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 .../bindings/memory-controllers/nvidia,tegra124-emc.yaml   | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Rob Herring Oct. 26, 2020, 12:51 p.m. UTC | #1
On Mon, 26 Oct 2020 01:16:56 +0300, Dmitry Osipenko wrote:
> External memory controller is interconnected with memory controller and
> with external memory. Document new interconnect property which turns
> External Memory Controller into interconnect provider.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  .../bindings/memory-controllers/nvidia,tegra124-emc.yaml   | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Krzysztof Kozlowski Oct. 27, 2020, 10:25 a.m. UTC | #2
On Mon, Oct 26, 2020 at 01:16:56AM +0300, Dmitry Osipenko wrote:
> External memory controller is interconnected with memory controller and
> with external memory. Document new interconnect property which turns
> External Memory Controller into interconnect provider.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  .../bindings/memory-controllers/nvidia,tegra124-emc.yaml   | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
> index 278549f9e051..ac00832ceac1 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
> @@ -29,6 +29,9 @@ properties:
>      items:
>        - const: emc
>  
> +  "#interconnect-cells":
> +    const: 0
> +
>    nvidia,memory-controller:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description:
> @@ -327,6 +330,7 @@ required:
>    - clocks
>    - clock-names
>    - nvidia,memory-controller
> +  - "#interconnect-cells"

Another required property, what about all existing users of this binding?

>  
>  additionalProperties: false
>  
> @@ -345,6 +349,7 @@ examples:
>  
>          #iommu-cells = <1>;
>          #reset-cells = <1>;
> +        #interconnect-cells = <1>;

You meant '0'?

Best regards,
Krzysztof
Dmitry Osipenko Oct. 27, 2020, 7:19 p.m. UTC | #3
27.10.2020 13:25, Krzysztof Kozlowski пишет:
> On Mon, Oct 26, 2020 at 01:16:56AM +0300, Dmitry Osipenko wrote:
>> External memory controller is interconnected with memory controller and
>> with external memory. Document new interconnect property which turns
>> External Memory Controller into interconnect provider.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  .../bindings/memory-controllers/nvidia,tegra124-emc.yaml   | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
>> index 278549f9e051..ac00832ceac1 100644
>> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
>> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
>> @@ -29,6 +29,9 @@ properties:
>>      items:
>>        - const: emc
>>  
>> +  "#interconnect-cells":
>> +    const: 0
>> +
>>    nvidia,memory-controller:
>>      $ref: /schemas/types.yaml#/definitions/phandle
>>      description:
>> @@ -327,6 +330,7 @@ required:
>>    - clocks
>>    - clock-names
>>    - nvidia,memory-controller
>> +  - "#interconnect-cells"
> 
> Another required property, what about all existing users of this binding?

EMC/devfreq drivers check presence of the new properties and ask users
to upgrade the DT. The kernel will continue to work fine using older
DTBs, but devfreq driver won't load.

>>  additionalProperties: false
>>  
>> @@ -345,6 +349,7 @@ examples:
>>  
>>          #iommu-cells = <1>;
>>          #reset-cells = <1>;
>> +        #interconnect-cells = <1>;
> 
> You meant '0'?

'1' is for the "mc" node in the example (not "emc" node).

Anyways, I'll move this hunk to the previous patch in order to fix the
kernel bot warning.
Krzysztof Kozlowski Oct. 27, 2020, 7:48 p.m. UTC | #4
On Tue, Oct 27, 2020 at 10:19:28PM +0300, Dmitry Osipenko wrote:
> 27.10.2020 13:25, Krzysztof Kozlowski пишет:
> > On Mon, Oct 26, 2020 at 01:16:56AM +0300, Dmitry Osipenko wrote:
> >> External memory controller is interconnected with memory controller and
> >> with external memory. Document new interconnect property which turns
> >> External Memory Controller into interconnect provider.
> >>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---
> >>  .../bindings/memory-controllers/nvidia,tegra124-emc.yaml   | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
> >> index 278549f9e051..ac00832ceac1 100644
> >> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
> >> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
> >> @@ -29,6 +29,9 @@ properties:
> >>      items:
> >>        - const: emc
> >>  
> >> +  "#interconnect-cells":
> >> +    const: 0
> >> +
> >>    nvidia,memory-controller:
> >>      $ref: /schemas/types.yaml#/definitions/phandle
> >>      description:
> >> @@ -327,6 +330,7 @@ required:
> >>    - clocks
> >>    - clock-names
> >>    - nvidia,memory-controller
> >> +  - "#interconnect-cells"
> > 
> > Another required property, what about all existing users of this binding?
> 
> EMC/devfreq drivers check presence of the new properties and ask users
> to upgrade the DT. The kernel will continue to work fine using older
> DTBs, but devfreq driver won't load.

If the devfreq was working fine before (with these older DTBs and older
kernel) then you break the feature.

If devfreq was not working or was not stable enough, then nothing is
broken so such change is accepted.

Which one is then?

> 
> >>  additionalProperties: false
> >>  
> >> @@ -345,6 +349,7 @@ examples:
> >>  
> >>          #iommu-cells = <1>;
> >>          #reset-cells = <1>;
> >> +        #interconnect-cells = <1>;
> > 
> > You meant '0'?
> 
> '1' is for the "mc" node in the example (not "emc" node).
> 
> Anyways, I'll move this hunk to the previous patch in order to fix the
> kernel bot warning.

Right, thanks.

Best regards,
Krzysztof
Dmitry Osipenko Oct. 27, 2020, 8:16 p.m. UTC | #5
27.10.2020 22:48, Krzysztof Kozlowski пишет:
> On Tue, Oct 27, 2020 at 10:19:28PM +0300, Dmitry Osipenko wrote:
>> 27.10.2020 13:25, Krzysztof Kozlowski пишет:
>>> On Mon, Oct 26, 2020 at 01:16:56AM +0300, Dmitry Osipenko wrote:
>>>> External memory controller is interconnected with memory controller and
>>>> with external memory. Document new interconnect property which turns
>>>> External Memory Controller into interconnect provider.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>>  .../bindings/memory-controllers/nvidia,tegra124-emc.yaml   | 7 +++++++
>>>>  1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
>>>> index 278549f9e051..ac00832ceac1 100644
>>>> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
>>>> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
>>>> @@ -29,6 +29,9 @@ properties:
>>>>      items:
>>>>        - const: emc
>>>>  
>>>> +  "#interconnect-cells":
>>>> +    const: 0
>>>> +
>>>>    nvidia,memory-controller:
>>>>      $ref: /schemas/types.yaml#/definitions/phandle
>>>>      description:
>>>> @@ -327,6 +330,7 @@ required:
>>>>    - clocks
>>>>    - clock-names
>>>>    - nvidia,memory-controller
>>>> +  - "#interconnect-cells"
>>>
>>> Another required property, what about all existing users of this binding?
>>
>> EMC/devfreq drivers check presence of the new properties and ask users
>> to upgrade the DT. The kernel will continue to work fine using older
>> DTBs, but devfreq driver won't load.
> 
> If the devfreq was working fine before (with these older DTBs and older
> kernel) then you break the feature.
> 
> If devfreq was not working or was not stable enough, then nothing is
> broken so such change is accepted.
> 
> Which one is then?

Definitely the latter. The current devfreq works okay'ish, but we rely
on hardware to recover from temporal FIFO underflows and it's a
user-visible problem which this series addresses.
Krzysztof Kozlowski Oct. 28, 2020, 7:27 p.m. UTC | #6
On Tue, Oct 27, 2020 at 11:16:29PM +0300, Dmitry Osipenko wrote:
> 27.10.2020 22:48, Krzysztof Kozlowski пишет:
> > On Tue, Oct 27, 2020 at 10:19:28PM +0300, Dmitry Osipenko wrote:
> >> 27.10.2020 13:25, Krzysztof Kozlowski пишет:
> >>> On Mon, Oct 26, 2020 at 01:16:56AM +0300, Dmitry Osipenko wrote:
> >>>> External memory controller is interconnected with memory controller and
> >>>> with external memory. Document new interconnect property which turns
> >>>> External Memory Controller into interconnect provider.
> >>>>
> >>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >>>> ---
> >>>>  .../bindings/memory-controllers/nvidia,tegra124-emc.yaml   | 7 +++++++
> >>>>  1 file changed, 7 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
> >>>> index 278549f9e051..ac00832ceac1 100644
> >>>> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
> >>>> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
> >>>> @@ -29,6 +29,9 @@ properties:
> >>>>      items:
> >>>>        - const: emc
> >>>>  
> >>>> +  "#interconnect-cells":
> >>>> +    const: 0
> >>>> +
> >>>>    nvidia,memory-controller:
> >>>>      $ref: /schemas/types.yaml#/definitions/phandle
> >>>>      description:
> >>>> @@ -327,6 +330,7 @@ required:
> >>>>    - clocks
> >>>>    - clock-names
> >>>>    - nvidia,memory-controller
> >>>> +  - "#interconnect-cells"
> >>>
> >>> Another required property, what about all existing users of this binding?
> >>
> >> EMC/devfreq drivers check presence of the new properties and ask users
> >> to upgrade the DT. The kernel will continue to work fine using older
> >> DTBs, but devfreq driver won't load.
> > 
> > If the devfreq was working fine before (with these older DTBs and older
> > kernel) then you break the feature.
> > 
> > If devfreq was not working or was not stable enough, then nothing is
> > broken so such change is accepted.
> > 
> > Which one is then?
> 
> Definitely the latter. The current devfreq works okay'ish, but we rely
> on hardware to recover from temporal FIFO underflows and it's a
> user-visible problem which this series addresses.

I understand. Fine with me, thanks for explanation.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
index 278549f9e051..ac00832ceac1 100644
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml
@@ -29,6 +29,9 @@  properties:
     items:
       - const: emc
 
+  "#interconnect-cells":
+    const: 0
+
   nvidia,memory-controller:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
@@ -327,6 +330,7 @@  required:
   - clocks
   - clock-names
   - nvidia,memory-controller
+  - "#interconnect-cells"
 
 additionalProperties: false
 
@@ -345,6 +349,7 @@  examples:
 
         #iommu-cells = <1>;
         #reset-cells = <1>;
+        #interconnect-cells = <1>;
     };
 
     external-memory-controller@7001b000 {
@@ -355,6 +360,8 @@  examples:
 
         nvidia,memory-controller = <&mc>;
 
+        #interconnect-cells = <0>;
+
         emc-timings-0 {
             nvidia,ram-code = <3>;