Re: [PATCH v2 02/19] PM / devfreq: exynos: Add documentation for generic exynos bus frequency driver

[Chanwoo Choi <cw00.choi@samsung.com>]

Hi,

On 2015년 12월 10일 09:39, Krzysztof Kozlowski wrote:
> On 09.12.2015 13:07, Chanwoo Choi wrote:
>> This patch adds the documentation for generic exynos bus frequency
>> driver.
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  .../devicetree/bindings/devfreq/exynos-bus.txt     | 94 ++++++++++++++++++++++
>>  1 file changed, 94 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>>
>> diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>> new file mode 100644
>> index 000000000000..54a1f9c46c88
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>> @@ -0,0 +1,94 @@
>> +* Generic Exynos Bus frequency device
>> +
>> +The Samsung Exynos SoC have many buses for data transfer between DRAM
>> +and sub-blocks in SoC. Almost Exynos SoC have the common architecture
>> +for buses. Generally, the each bus of Exynos SoC includes the source clock
>> +and power line and then is able to change the clock according to the usage
>> +of each buses on runtime. When gathering the usage of each buses on runtime,
>> +thie driver uses the PPMU (Platform Performance Monitoring Unit) which
> 
> s/thie/the/

OK.

> 
>> +is able to measure the current load of sub-blocks.
>> +
>> +There are a little different composition among Exynos SoC because each Exynos
>> +SoC has the different sub-blocks. So, this difference should be specified
>> +in devicetree file instead of each device driver. In result, this driver
>> +is able to support the bus frequency for all Exynos SoCs.
>> +
>> +Required properties for bus device:
>> +- compatible: Should be "samsung,exynos-bus".
>> +- clock-names : the name of clock used by the bus, "bus".
>> +- clocks : phandles for clock specified in "clock-names" property.
>> +- #clock-cells: should be 1.
> 
> This is a clock consumer, right? So the clock-cells is not valid here.

You're right. I'll remove '#clock-cells'.

> 
>> +- operating-points-v2: the OPP table including frequency/voltage information
>> +  to support DVFS (Dynamic Voltage/Frequency Scaling) feature.
>> +- vdd-supply: the regulator to provide the buses with the voltage.
>> +- devfreq-events: the devfreq-event device to monitor the curret utilization
> 
> s/curret/current/
> 
>> +  of buses.
>> +
>> +Optional properties for bus device:
>> +- exynos,saturation-ratio: the percentage value which is used to calibrate
>> +                   the performance count againt total cycle count.
> 
> s/againt/against/

OK.

> 
>> +
>> +Example1:
>> +	Show the AXI buses of Exynos3250 SoC. Exynos3250 divides the buses to
>> +	power line (regulator). The MIF (Memory Interface) AXI bus is used to
>> +	transfer data between DRAM and CPU and uses the VDD_MIF regualtor.
>> +
>> +	- power line(VDD_MIF) --> bus for DMC (Dynamic Memory Controller) block
>> +
>> +	- MIF bus's frequency/voltage table
>> +	-----------------------
>> +	|Lv| Freq   | Voltage |
>> +	-----------------------
>> +	|L1| 50000  |800000   |
>> +	|L2| 100000 |800000   |
>> +	|L3| 134000 |800000   |
>> +	|L4| 200000 |800000   |
>> +	|L5| 400000 |875000   |
>> +	-----------------------
>> +
>> +Example2 :
>> +	The bus of DMC (Dynamic Memory Controller) block in exynos3250.dtsi
>> +	are listed below:
> 
> s/are/is/ (one bus is listed)

OK.

> 
>> +
>> +	bus_dmc: bus_dmc {
>> +		compatible = "samsung,exynos-bus";
>> +		clocks = <&cmu_dmc CLK_DIV_DMC>;
>> +		clock-names = "bus";
>> +		operating-points-v2 = <&bus_dmc_opp_table>;
>> +		status = "disabled";
>> +	};
>> +
>> +	bus_dmc_opp_table: opp_table0 {
>> +		compatible = "operating-points-v2";
>> +		opp-shared;
>> +
>> +		opp00 {
> 
> Maybe use convention with frequency, like:
> 	opp@50000000
> This also used in opp.txt examples.

In the Documentations/devicetree/bindings/opp/opp.txt,
the example uses the 'opp@0x'. I check the opp.txt of Linux 4.4-rc4.

> 
> 
>> +			opp-hz = /bits/ 64 <50000000>;
>> +			opp-microvolt = <800000>;
>> +		};
>> +		opp01 {
>> +			opp-hz = /bits/ 64 <100000000>;
>> +			opp-microvolt = <800000>;
>> +		};
>> +		opp02 {
>> +			opp-hz = /bits/ 64 <134000000>;
>> +			opp-microvolt = <800000>;
>> +		};
>> +		opp03 {
>> +			opp-hz = /bits/ 64 <200000000>;
>> +			opp-microvolt = <800000>;
>> +		};
>> +		opp04 {
>> +			opp-hz = /bits/ 64 <400000000>;
>> +			opp-microvolt = <875000>;
>> +		};
>> +	};
>> +
>> +	Usage case to handle the frequency and voltage of bus on runtime
>> +	in exynos3250-rinato.dts are listed below:
> 
> s/are/is/

OK.

> 
>> +
>> +	&bus_dmc {
>> +		devfreq-events = <&ppmu_dmc0_3>, <&ppmu_dmc1_3>;
>> +		vdd-supply = <&buck1_reg>;	/* VDD_MIF */
>> +		status = "okay";
>> +	};

Thanks for your review.

Regards,
Chanwoo Choi