From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CBB09C4167B for ; Thu, 30 Nov 2023 12:05:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=1Uh5Y3gkfWUwWLRr+D7+Jweq7w//gY/ES5MnvSnFja8=; b=eu17BBCIyzCbMb /dBlpwoNIGcOVG0zLSw2PNs2u1FFWtQzVALJaOJUdvi+zYEIiFK5AbIWynzhXivCbmrtfo/9ccOy0 80Ge+0SMhokmZbFkZKSDFahV/uqElW6apKjNnENwRrw12jjhEFSFW7Lgi6zYn2dbOKPq2pn4r3EYa gH5Tbef4VH0ZNedfjKfyVSeHcOv2zxzcE3nV+TBXUliSiyunLX//DTlgxIl5ro/6X6grO8MqDSU2s +wNu3OWC6fHmTaaVdrwiSs4vYC2XlGPKMlW38HT2sYIUEidGSLuasFdbBRb/yaWkhj9p1RSDXqyC0 KMPrn1hjIHTEwlI4sBfw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r8fnJ-00AnNZ-1T; Thu, 30 Nov 2023 12:05:25 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r8fn8-00AnIU-1h for linux-arm-kernel@lists.infradead.org; Thu, 30 Nov 2023 12:05:16 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 526E11042; Thu, 30 Nov 2023 04:05:56 -0800 (PST) Received: from bogus (e103737-lin.cambridge.arm.com [10.1.197.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 791C53F5A1; Thu, 30 Nov 2023 04:05:08 -0800 (PST) Date: Thu, 30 Nov 2023 12:05:06 +0000 From: Sudeep Holla To: Sibi Sankar Cc: , , Sudeep Holla , , , , , Subject: Re: [PATCH 2/3] firmware: arm_scmi: Fix freq/power truncation in the perf protocol Message-ID: References: <20231129065748.19871-1-quic_sibis@quicinc.com> <20231129065748.19871-3-quic_sibis@quicinc.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20231129065748.19871-3-quic_sibis@quicinc.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231130_040514_685396_8C99D405 X-CRM114-Status: GOOD ( 14.78 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Nov 29, 2023 at 12:27:47PM +0530, Sibi Sankar wrote: > Fix frequency and power truncation seen in the performance protocol by > casting it with the correct type. > While I always remembered to handle this when reviewing the spec, seem to have forgotten when it came to handling in the implementation :(. Thanks for spotting this. However I don't like the ugly type casting. I think we can do better. Also looking at the code around the recently added level index mode, I think we can simplify things like below patch. Cristian, What do you think ? Regards, Sudeep -->8 drivers/firmware/arm_scmi/perf.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index a648521e04a3..2e828b29efab 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -268,13 +268,14 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph, dom_info->sustained_perf_level = le32_to_cpu(attr->sustained_perf_level); if (!dom_info->sustained_freq_khz || - !dom_info->sustained_perf_level) + !dom_info->sustained_perf_level || + dom_info->level_indexing_mode) /* CPUFreq converts to kHz, hence default 1000 */ dom_info->mult_factor = 1000; else dom_info->mult_factor = - (dom_info->sustained_freq_khz * 1000) / - dom_info->sustained_perf_level; + (dom_info->sustained_freq_khz * 1000UL) + / dom_info->sustained_perf_level; strscpy(dom_info->info.name, attr->name, SCMI_SHORT_NAME_MAX_SIZE); } @@ -804,9 +805,10 @@ static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph, for (idx = 0; idx < dom->opp_count; idx++) { if (!dom->level_indexing_mode) - freq = dom->opp[idx].perf * dom->mult_factor; + freq = dom->opp[idx].perf; else - freq = dom->opp[idx].indicative_freq * 1000; + freq = dom->opp[idx].indicative_freq; + freq *= dom->mult_factor; data.level = dom->opp[idx].perf; data.freq = freq; @@ -879,7 +881,7 @@ static int scmi_dvfs_freq_get(const struct scmi_protocol_handle *ph, u32 domain, return ret; if (!dom->level_indexing_mode) { - *freq = level * dom->mult_factor; + *freq = level; } else { struct scmi_opp *opp; @@ -887,8 +889,9 @@ static int scmi_dvfs_freq_get(const struct scmi_protocol_handle *ph, u32 domain, if (!opp) return -EIO; - *freq = opp->indicative_freq * 1000; + *freq = opp->indicative_freq; } + freq *= dom->mult_factor; return ret; } @@ -908,9 +911,10 @@ static int scmi_dvfs_est_power_get(const struct scmi_protocol_handle *ph, for (opp = dom->opp, idx = 0; idx < dom->opp_count; idx++, opp++) { if (!dom->level_indexing_mode) - opp_freq = opp->perf * dom->mult_factor; + opp_freq = opp->perf; else - opp_freq = opp->indicative_freq * 1000; + opp_freq = opp->indicative_freq; + opp_freq *= dom->mult_factor; if (opp_freq < *freq) continue; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel