| Message ID | 20201025221735.3062-9-digetx@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Introduce memory interconnect for NVIDIA Tegra SoCs | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Mon, Oct 26, 2020 at 01:16:51AM +0300, Dmitry Osipenko wrote: > External Memory Controller can gather various hardware statistics that > are intended to be used for debugging purposes and for dynamic frequency > scaling of memory bus. > > Document the new mfd-simple compatible and EMC statistics sub-device. > The subdev contains EMC DFS OPP table and interconnect paths to be used > for dynamic scaling of system's memory bandwidth based on EMC utilization > statistics. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > .../memory-controllers/nvidia,tegra20-emc.txt | 43 +++++++++++++++++-- > 1 file changed, 40 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt > index 8d09b228ac42..382aabcd6952 100644 > --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt > +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt > @@ -4,7 +4,7 @@ Properties: > - name : Should be emc > - #address-cells : Should be 1 > - #size-cells : Should be 0 > -- compatible : Should contain "nvidia,tegra20-emc". > +- compatible : Should contain "nvidia,tegra20-emc" and "simple-mfd". Changing a compatible match is another break of ABI, unless it is not really required. It's unclear to me from the contents of the patch. > - reg : Offset and length of the register set for the device > - nvidia,use-ram-code : If present, the sub-nodes will be addressed > and chosen using the ramcode board selector. If omitted, only one > @@ -17,7 +17,8 @@ Properties: > - core-supply: Phandle of voltage regulator of the SoC "core" power domain. > - operating-points-v2: See ../bindings/opp/opp.txt for details. > > -Child device nodes describe the memory settings for different configurations and clock rates. > +Child device nodes describe the memory settings for different configurations and clock rates, > +as well as EMC activity statistics collection sub-device. > > Example: > > @@ -31,17 +32,34 @@ Example: > ... > }; > > + emc_bw_dfs_opp_table: emc_opp_table1 { Hyphens for node name. Best regards, Krzysztof
On Mon, Oct 26, 2020 at 01:16:51AM +0300, Dmitry Osipenko wrote: > External Memory Controller can gather various hardware statistics that > are intended to be used for debugging purposes and for dynamic frequency > scaling of memory bus. > > Document the new mfd-simple compatible and EMC statistics sub-device. > The subdev contains EMC DFS OPP table and interconnect paths to be used > for dynamic scaling of system's memory bandwidth based on EMC utilization > statistics. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > .../memory-controllers/nvidia,tegra20-emc.txt | 43 +++++++++++++++++-- > 1 file changed, 40 insertions(+), 3 deletions(-) Why does this have to be modelled as a separate device? Isn't this just using a couple of registers out of the EMC register range? If so, this would better just be integrated into the parent node and implemented as part of the EMC driver. No need to further complicate things by adding a dummy child. Thierry
27.10.2020 12:02, Krzysztof Kozlowski пишет: >> @@ -31,17 +32,34 @@ Example: >> ... >> }; >> >> + emc_bw_dfs_opp_table: emc_opp_table1 { > Hyphens for node name. We already use underscores for the Tegra CPU OPP table. https://elixir.bootlin.com/linux/v5.10-rc1/source/arch/arm/boot/dts/tegra20-cpu-opp.dtsi#L4 What makes you think that hyphens will be a better choice? Is it a documented naming convention?
27.10.2020 16:22, Thierry Reding пишет: > On Mon, Oct 26, 2020 at 01:16:51AM +0300, Dmitry Osipenko wrote: >> External Memory Controller can gather various hardware statistics that >> are intended to be used for debugging purposes and for dynamic frequency >> scaling of memory bus. >> >> Document the new mfd-simple compatible and EMC statistics sub-device. >> The subdev contains EMC DFS OPP table and interconnect paths to be used >> for dynamic scaling of system's memory bandwidth based on EMC utilization >> statistics. >> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> .../memory-controllers/nvidia,tegra20-emc.txt | 43 +++++++++++++++++-- >> 1 file changed, 40 insertions(+), 3 deletions(-) > > Why does this have to be modelled as a separate device? Isn't this just > using a couple of registers out of the EMC register range? If so, this > would better just be integrated into the parent node and implemented as > part of the EMC driver. No need to further complicate things by adding > a dummy child. Maybe true, I'll take a closer look.
On Tue, Oct 27, 2020 at 10:22:19PM +0300, Dmitry Osipenko wrote: > 27.10.2020 12:02, Krzysztof Kozlowski пишет: > >> @@ -31,17 +32,34 @@ Example: > >> ... > >> }; > >> > >> + emc_bw_dfs_opp_table: emc_opp_table1 { > > Hyphens for node name. > > We already use underscores for the Tegra CPU OPP table. > > https://elixir.bootlin.com/linux/v5.10-rc1/source/arch/arm/boot/dts/tegra20-cpu-opp.dtsi#L4 > > What makes you think that hyphens will be a better choice? Is it a > documented naming convention? Unfortunately that's the source of confusion also for me because Devicetree spec mentions both of them (and does not specify preferences). The choice of dashes/hyphens comes now explicitly from all dtschema files. Previously, the documentation were emails from Rob. :) Best regards, Krzysztof
27.10.2020 22:44, Krzysztof Kozlowski пишет: > On Tue, Oct 27, 2020 at 10:22:19PM +0300, Dmitry Osipenko wrote: >> 27.10.2020 12:02, Krzysztof Kozlowski пишет: >>>> @@ -31,17 +32,34 @@ Example: >>>> ... >>>> }; >>>> >>>> + emc_bw_dfs_opp_table: emc_opp_table1 { >>> Hyphens for node name. >> >> We already use underscores for the Tegra CPU OPP table. >> >> https://elixir.bootlin.com/linux/v5.10-rc1/source/arch/arm/boot/dts/tegra20-cpu-opp.dtsi#L4 >> >> What makes you think that hyphens will be a better choice? Is it a >> documented naming convention? > > Unfortunately that's the source of confusion also for me because > Devicetree spec mentions both of them (and does not specify preferences). > > The choice of dashes/hyphens comes now explicitly from all dtschema > files. Previously, the documentation were emails from Rob. :) Okay, I'll change it in v7. So far I haven't seen warnings about it from the schema-checker.
On Tue, Oct 27, 2020 at 11:18:34PM +0300, Dmitry Osipenko wrote: > 27.10.2020 22:44, Krzysztof Kozlowski пишет: > > On Tue, Oct 27, 2020 at 10:22:19PM +0300, Dmitry Osipenko wrote: > >> 27.10.2020 12:02, Krzysztof Kozlowski пишет: > >>>> @@ -31,17 +32,34 @@ Example: > >>>> ... > >>>> }; > >>>> > >>>> + emc_bw_dfs_opp_table: emc_opp_table1 { > >>> Hyphens for node name. > >> > >> We already use underscores for the Tegra CPU OPP table. > >> > >> https://elixir.bootlin.com/linux/v5.10-rc1/source/arch/arm/boot/dts/tegra20-cpu-opp.dtsi#L4 > >> > >> What makes you think that hyphens will be a better choice? Is it a > >> documented naming convention? > > > > Unfortunately that's the source of confusion also for me because > > Devicetree spec mentions both of them (and does not specify preferences). > > > > The choice of dashes/hyphens comes now explicitly from all dtschema > > files. Previously, the documentation were emails from Rob. :) > > Okay, I'll change it in v7. So far I haven't seen warnings about it from > the schema-checker. dtc with W=2 will warn. The bigger issue is the name should be generic. Rob
On Mon, Oct 26, 2020 at 01:16:51AM +0300, Dmitry Osipenko wrote: > External Memory Controller can gather various hardware statistics that > are intended to be used for debugging purposes and for dynamic frequency > scaling of memory bus. > > Document the new mfd-simple compatible and EMC statistics sub-device. It's simple-mfd. That should only be used if the child has no dependencies on the parent node (and driver). > The subdev contains EMC DFS OPP table and interconnect paths to be used > for dynamic scaling of system's memory bandwidth based on EMC utilization > statistics. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > .../memory-controllers/nvidia,tegra20-emc.txt | 43 +++++++++++++++++-- > 1 file changed, 40 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt > index 8d09b228ac42..382aabcd6952 100644 > --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt > +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt > @@ -4,7 +4,7 @@ Properties: > - name : Should be emc > - #address-cells : Should be 1 > - #size-cells : Should be 0 > -- compatible : Should contain "nvidia,tegra20-emc". > +- compatible : Should contain "nvidia,tegra20-emc" and "simple-mfd". > - reg : Offset and length of the register set for the device > - nvidia,use-ram-code : If present, the sub-nodes will be addressed > and chosen using the ramcode board selector. If omitted, only one > @@ -17,7 +17,8 @@ Properties: > - core-supply: Phandle of voltage regulator of the SoC "core" power domain. > - operating-points-v2: See ../bindings/opp/opp.txt for details. > > -Child device nodes describe the memory settings for different configurations and clock rates. > +Child device nodes describe the memory settings for different configurations and clock rates, > +as well as EMC activity statistics collection sub-device. > > Example: > > @@ -31,17 +32,34 @@ Example: > ... > }; > > + emc_bw_dfs_opp_table: emc_opp_table1 { > + compatible = "operating-points-v2"; > + > + opp@36000000 { > + opp-hz = /bits/ 64 <36000000>; > + opp-peak-kBps = <144000>; > + }; > + ... > + }; > + > memory-controller@7000f400 { > #address-cells = < 1 >; > #size-cells = < 0 >; > #interconnect-cells = < 0 >; > - compatible = "nvidia,tegra20-emc"; > + compatible = "nvidia,tegra20-emc", "simple-mfd"; > reg = <0x7000f400 0x400>; > interrupts = <0 78 0x04>; > clocks = <&tegra_car TEGRA20_CLK_EMC>; > nvidia,memory-controller = <&mc>; > core-supply = <&core_vdd_reg>; > operating-points-v2 = <&emc_icc_dvfs_opp_table>; > + > + emc-stats { > + compatible = "nvidia,tegra20-emc-statistics"; > + operating-points-v2 = <&emc_bw_dfs_opp_table>; > + interconnects = <&mc TEGRA20_MC_MPCORER &emc>; > + interconnect-names = "cpu"; > + }; > } > > > @@ -120,3 +138,22 @@ Properties: > 0 0 0 0 0 0 0 0 0 0 0 0 0 0 > 0 0 0 0 >; > }; > + > + > + > +Embedded Memory Controller statistics gathering sub-device > + > +EMC statistics subdev gathers information about hardware utilization > +which is intended to be used for debugging purposes and for dynamic > +frequency scaling based on the collected stats. > + > +Properties: > +- name : Should be emc-stats. > +- compatible : Should contain "nvidia,tegra20-emc-statistics". > +- operating-points-v2: See ../bindings/opp/opp.txt for details. > +- interconnects: Should contain entries for memory clients sitting on > + MC->EMC memory interconnect path. > +- interconnect-names: Should include name of the interconnect path for each > + interconnect entry. Consult TRM documentation for > + information about available memory clients, see MEMORY > + CONTROLLER section. > -- > 2.27.0 >
28.10.2020 18:26, Rob Herring пишет: > On Tue, Oct 27, 2020 at 11:18:34PM +0300, Dmitry Osipenko wrote: >> 27.10.2020 22:44, Krzysztof Kozlowski пишет: >>> On Tue, Oct 27, 2020 at 10:22:19PM +0300, Dmitry Osipenko wrote: >>>> 27.10.2020 12:02, Krzysztof Kozlowski пишет: >>>>>> @@ -31,17 +32,34 @@ Example: >>>>>> ... >>>>>> }; >>>>>> >>>>>> + emc_bw_dfs_opp_table: emc_opp_table1 { >>>>> Hyphens for node name. >>>> >>>> We already use underscores for the Tegra CPU OPP table. >>>> >>>> https://elixir.bootlin.com/linux/v5.10-rc1/source/arch/arm/boot/dts/tegra20-cpu-opp.dtsi#L4 >>>> >>>> What makes you think that hyphens will be a better choice? Is it a >>>> documented naming convention? >>> >>> Unfortunately that's the source of confusion also for me because >>> Devicetree spec mentions both of them (and does not specify preferences). >>> >>> The choice of dashes/hyphens comes now explicitly from all dtschema >>> files. Previously, the documentation were emails from Rob. :) >> >> Okay, I'll change it in v7. So far I haven't seen warnings about it from >> the schema-checker. > > dtc with W=2 will warn. > > The bigger issue is the name should be generic. Indeed, thanks. I'll correct the name.
diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt index 8d09b228ac42..382aabcd6952 100644 --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt @@ -4,7 +4,7 @@ Properties: - name : Should be emc - #address-cells : Should be 1 - #size-cells : Should be 0 -- compatible : Should contain "nvidia,tegra20-emc". +- compatible : Should contain "nvidia,tegra20-emc" and "simple-mfd". - reg : Offset and length of the register set for the device - nvidia,use-ram-code : If present, the sub-nodes will be addressed and chosen using the ramcode board selector. If omitted, only one @@ -17,7 +17,8 @@ Properties: - core-supply: Phandle of voltage regulator of the SoC "core" power domain. - operating-points-v2: See ../bindings/opp/opp.txt for details. -Child device nodes describe the memory settings for different configurations and clock rates. +Child device nodes describe the memory settings for different configurations and clock rates, +as well as EMC activity statistics collection sub-device. Example: @@ -31,17 +32,34 @@ Example: ... }; + emc_bw_dfs_opp_table: emc_opp_table1 { + compatible = "operating-points-v2"; + + opp@36000000 { + opp-hz = /bits/ 64 <36000000>; + opp-peak-kBps = <144000>; + }; + ... + }; + memory-controller@7000f400 { #address-cells = < 1 >; #size-cells = < 0 >; #interconnect-cells = < 0 >; - compatible = "nvidia,tegra20-emc"; + compatible = "nvidia,tegra20-emc", "simple-mfd"; reg = <0x7000f400 0x400>; interrupts = <0 78 0x04>; clocks = <&tegra_car TEGRA20_CLK_EMC>; nvidia,memory-controller = <&mc>; core-supply = <&core_vdd_reg>; operating-points-v2 = <&emc_icc_dvfs_opp_table>; + + emc-stats { + compatible = "nvidia,tegra20-emc-statistics"; + operating-points-v2 = <&emc_bw_dfs_opp_table>; + interconnects = <&mc TEGRA20_MC_MPCORER &emc>; + interconnect-names = "cpu"; + }; } @@ -120,3 +138,22 @@ Properties: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 >; }; + + + +Embedded Memory Controller statistics gathering sub-device + +EMC statistics subdev gathers information about hardware utilization +which is intended to be used for debugging purposes and for dynamic +frequency scaling based on the collected stats. + +Properties: +- name : Should be emc-stats. +- compatible : Should contain "nvidia,tegra20-emc-statistics". +- operating-points-v2: See ../bindings/opp/opp.txt for details. +- interconnects: Should contain entries for memory clients sitting on + MC->EMC memory interconnect path. +- interconnect-names: Should include name of the interconnect path for each + interconnect entry. Consult TRM documentation for + information about available memory clients, see MEMORY + CONTROLLER section.
External Memory Controller can gather various hardware statistics that are intended to be used for debugging purposes and for dynamic frequency scaling of memory bus. Document the new mfd-simple compatible and EMC statistics sub-device. The subdev contains EMC DFS OPP table and interconnect paths to be used for dynamic scaling of system's memory bandwidth based on EMC utilization statistics. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- .../memory-controllers/nvidia,tegra20-emc.txt | 43 +++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-)