diff mbox series

[v6,08/52] dt-bindings: memory: tegra20: emc: Document mfd-simple compatible and statistics sub-device

Message ID 20201025221735.3062-9-digetx@gmail.com
State Changes Requested
Headers show
Series Introduce memory interconnect for NVIDIA Tegra SoCs | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Dmitry Osipenko Oct. 25, 2020, 10:16 p.m. UTC
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(-)

Comments

Krzysztof Kozlowski Oct. 27, 2020, 9:02 a.m. UTC | #1
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
Thierry Reding Oct. 27, 2020, 1:22 p.m. UTC | #2
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
Dmitry Osipenko Oct. 27, 2020, 7:22 p.m. UTC | #3
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?
Dmitry Osipenko Oct. 27, 2020, 7:23 p.m. UTC | #4
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.
Krzysztof Kozlowski Oct. 27, 2020, 7:44 p.m. UTC | #5
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
Dmitry Osipenko Oct. 27, 2020, 8:18 p.m. UTC | #6
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.
Rob Herring Oct. 28, 2020, 3:26 p.m. UTC | #7
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
Rob Herring Oct. 28, 2020, 3:28 p.m. UTC | #8
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
>
Dmitry Osipenko Oct. 31, 2020, 7:53 p.m. UTC | #9
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 mbox series

Patch

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.