Received-SPF: pass (google.com: domain of linux-kernel+bounces-129357-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <91f59477-1799-4db6-bcc2-3f0c5225d1c8@quicinc.com>
Date: Wed, 3 Apr 2024 14:15:20 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 1/4] interconnect: qcom: icc-rpmh: Add QoS
 configuration support
To: Konrad Dybcio <konrad.dybcio@linaro.org>,
        Bjorn Andersson
	<andersson@kernel.org>,
        Georgi Djakov <djakov@kernel.org>, Rob Herring
	<robh@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>
CC: Kees Cook <keescook@chromium.org>, <cros-qcom-dts-watchers@chromium.org>,
        "Gustavo A . R . Silva" <gustavoars@kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-pm@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-hardening@vger.kernel.org>, <quic_rlaggysh@quicinc.com>,
        <quic_mdtipton@quicinc.com>
References: <20240325181628.9407-1-quic_okukatla@quicinc.com>
 <20240325181628.9407-2-quic_okukatla@quicinc.com>
 <d59896bb-a559-4013-a615-37bb43278b2e@linaro.org>
Content-Language: en-US
From: Odelu Kukatla <quic_okukatla@quicinc.com>
In-Reply-To: <d59896bb-a559-4013-a615-37bb43278b2e@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit


On 3/27/2024 2:26 AM, Konrad Dybcio wrote:
> On 25.03.2024 7:16 PM, Odelu Kukatla wrote:
>> It adds QoS support for QNOC device and includes support for
>> configuring priority, priority forward disable, urgency forwarding.
>> This helps in priortizing the traffic originating from different
>> interconnect masters at NoC(Network On Chip).
>>
>> Signed-off-by: Odelu Kukatla <quic_okukatla@quicinc.com>
>> ---
> 
> [...]
> 
>>  
>> +	if (desc->config) {
>> +		struct resource *res;
>> +		void __iomem *base;
>> +
>> +		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +		if (!res)
>> +			goto skip_qos_config;
>> +
>> +		base = devm_ioremap_resource(dev, res);
> 
> You were asked to substitute this call like 3 times already..
> 
> devm_platform_get_and_ioremap_resource
> 
> or even better, devm_platform_ioremap_resource
> 
> [...]
> 
>> @@ -70,6 +102,7 @@ struct qcom_icc_node {
>>  	u64 max_peak[QCOM_ICC_NUM_BUCKETS];
>>  	struct qcom_icc_bcm *bcms[MAX_BCM_PER_NODE];
>>  	size_t num_bcms;
>> +	const struct qcom_icc_qosbox *qosbox;
> 
> I believe I came up with a better approach for storing this.. see [1]
> 
> Konrad
> 
> [1] https://lore.kernel.org/linux-arm-msm/20240326-topic-rpm_icc_qos_cleanup-v1-4-357e736792be@linaro.org/
> 

I see in this series, QoS parameters are moved into struct qcom_icc_desc. 
Even though we program QoS at Provider/Bus level, it is property of the node/master connected to a Bus/NoC.
It will be easier later to know which master's QoS we are programming if we add in node data.
Readability point of view,  it might be good to keep QoS parameters in node data.  

Thanks,
Odelu