Re: [PATCH V5 4/7] spi: spi-geni-qcom: Add interconnect support

From: Akash Asthana <akashast@codeaurora.org>
To: Matthias Kaehlcke <mka@chromium.org>
Cc: gregkh@linuxfoundation.org, agross@kernel.org,
	bjorn.andersson@linaro.org, wsa@the-dreams.de,
	broonie@kernel.org, mark.rutland@arm.com, robh+dt@kernel.org,
	linux-i2c@vger.kernel.org, linux-spi@vger.kernel.org,
	devicetree@vger.kernel.org, swboyd@chromium.org,
	mgautam@codeaurora.org, linux-arm-msm@vger.kernel.org,
	linux-serial@vger.kernel.org, dianders@chromium.org,
	evgreen@chromium.org, georgi.djakov@linaro.org
Subject: Re: [PATCH V5 4/7] spi: spi-geni-qcom: Add interconnect support
Date: Thu, 14 May 2020 13:07:28 +0530	[thread overview]
Message-ID: <6a7b8129-6542-042f-e418-c4b49303d944@codeaurora.org> (raw)
In-Reply-To: <20200508182532.GD4525@google.com>

Hi Matthias,

....

;
>>   
>> +	ret = geni_icc_get(&mas->se, NULL);
>> +	if (ret)
>> +		goto spi_geni_probe_runtime_disable;
>> +	/* Set the bus quota to a reasonable value for register access */
>> +	geni_icc_bw_init(&mas->se.icc_paths[GENI_TO_CORE],
>> +			Bps_to_icc(CORE_2X_50_MHZ), 0);
>> +	geni_icc_bw_init(&mas->se.icc_paths[CPU_TO_GENI], GENI_DEFAULT_BW, 0);
>> +
>> +	/* Set BW for register access */
> This comment doesn't add any value. Register access is mentioned a few lines
> above and from the function name it's evident that it sets the ICC bandwidth.
ok
>
>> +	ret = geni_icc_set_bw(&mas->se);
>>
>> +		return ret;
>>   
>>   	return geni_se_resources_on(&mas->se);
>>   }
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project