Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Date:   Mon, 9 Dec 2019 14:41:00 +0000
From:   ppvk@codeaurora.org
To:     Georgi Djakov <georgi.djakov@linaro.org>
Cc:     adrian.hunter@intel.com, robh+dt@kernel.org,
        ulf.hansson@linaro.org, asutoshd@codeaurora.org,
        vbadigan@codeaurora.org, stummala@codeaurora.org,
        sayalil@codeaurora.org, rampraka@codeaurora.org, sboyd@kernel.org,
        linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        agross@kernel.org, linux-mmc-owner@vger.kernel.org
Subject: Re: [RFC-v2 1/2] dt-bindings: mmc: sdhci-msm: Add Bus BW vote
 supported strings
In-Reply-To: <8d7f2a2f-1312-8983-4e58-80dc6939154c@linaro.org>
References: <1573220319-4287-1-git-send-email-ppvk@codeaurora.org>
 <1573220319-4287-2-git-send-email-ppvk@codeaurora.org>
 <8d7f2a2f-1312-8983-4e58-80dc6939154c@linaro.org>
Message-ID: <0101016eeb1b3894-fc246837-27f4-447b-b37a-6bc4dd1893dc-000000@us-west-2.amazonses.com>
User-Agent: Roundcube Webmail/1.3.9
Feedback-ID: 1.us-west-2.CZuq2qbDmUIuT3qdvXlRHZZCpfZqZ4GtG9v3VKgRyF0=:AmazonSES
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2019-11-13 18:46, Georgi Djakov wrote:
> Hi Pradeep,
> 
> Thanks for the patch!
> 
> On 8.11.19 г. 15:38 ч., Pradeep P V K wrote:
>> Add Bus bandwidth voting supported strings for qcom-sdhci controller.
>> 
>> Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
>> ---
>>  .../devicetree/bindings/mmc/sdhci-msm.txt          | 32 
>> ++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt 
>> b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> index da4edb1..22fb140 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> @@ -39,6 +39,25 @@ Required properties:
>>  	"cal"	- reference clock for RCLK delay calibration (optional)
>>  	"sleep"	- sleep clock for RCLK delay calibration (optional)
>> 
>> +Optional Properties:
>> +* Following bus parameters are required for bus bw voting:
> 
> "bus bw voting" sounds a bit vague to me. I would say instead:
> 
> The following DT properties are required for interconnect bandwidth 
> scaling.
> 
  Thanks for the review. I will address this in my next patch set.

>> +- interconnects: Pairs of phandles and interconnect provider 
>> specifier
>> +		 to denote the edge source and destination ports of
>> +		 the interconnect path. Please refer to
>> +		 Documentation/devicetree/bindings/interconnect/
>> +		 for more details.
>> +- interconnect-names: List of interconnect path name strings sorted 
>> in the same
>> +		order as the interconnects property. Consumers drivers will use
>> +		interconnect-names to match interconnect paths with interconnect
>> +		specifiers. Please refer to Documentation/devicetree/bindings/
>> +		interconnect/ for more details.
> 
> Please describe here what there are two paths are for sdhc and how they 
> are
> expected to be named. You already responded to this question, so please 
> include
> this information here.
> Hint: Refer to the Documentation for how we described it for other 
> subsystems.
> 
ok. I will address in my next patch set.

>> +- msm-bus,name: string describing the bus path
>> +- msm-bus,num-cases: number of configurations in which sdhc can 
>> operate in
>> +- msm-bus,num-paths: number of paths to vote for
>> +- msm-bus,vectors-KBps: Takes a tuple <ib ab>, <ib ab> (2 tuples for 
>> 2
>> +				num-paths) The number of these entries *must*
>> +				be same as num-cases.
> 
> If it has to be in DT, could we use this [1] instead of the above? The 
> patches
> are not merged yet, but this might be the direction we want to go.
> 
> Thanks,
> Georgi
> 
> [1] 
> http://lore.kernel.org/r/20190807223111.230846-1-saravanak@google.com
> 

Sure, i will make the change based on [1] in my next patch set.

>> +
>>  Example:
>> 
>>  	sdhc_1: sdhci@f9824900 {
>> @@ -56,6 +75,19 @@ Example:
>> 
>>  		clocks = <&gcc GCC_SDCC1_APPS_CLK>, <&gcc GCC_SDCC1_AHB_CLK>;
>>  		clock-names = "core", "iface";
>> +		interconnects = <&qnoc 50 &qnoc 512>,
>> +				<&qnoc 1 &qnoc 544>;
>> +		interconnect-names = "sdhc-ddr","cpu-sdhc";
>> +		msm-bus,name = "sdhc1";
>> +		msm-bus,num-cases = <3>;
>> +		msm-bus,num-paths = <2>;
>> +		msm-bus,vectors-KBps =
>> +		/* No Vote */
>> +		<0 0>, <0 0>,
>> +		/* 50 MB/s */
>> +		<130718 200000>, <133320 133320>,
>> +		/* 200 MB/s */
>> +		<1338562 4096000>, <1338562 4096000>;
>>  	};
>> 
>>  	sdhc_2: sdhci@f98a4900 {
>>