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 X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7ACD6C433E3 for ; Thu, 9 Jul 2020 08:41:01 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4AA3520708 for ; Thu, 9 Jul 2020 08:41:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="0t9aLtJG" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4AA3520708 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject: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=7iVwhd6PQugwOxW+1AKnbEwkLmMEQ3S0diFUsS0+22Q=; b=0t9aLtJGN4e96x+hOxS1p1K05 LiodMIpCuyhtFOCDvcBLcX85m03e+TxLLL5Q/ww3kXxuGnffgDjYY3j50oeI/zqns+/ntdzuSAtcs b/or290At+UqPpecHfpfpsLTIWg6Lvuh4AIXhRcp62jy7/OH+no5Ca6xcNpATn4vhxG7+YRv1kMxu W3bzw5Nb5ClsdsBgi/vaXeP/d9HwcUFf9O/Wel5/SMR+RdMXQaFVfLHKA5T63IjpUjGXt9a9FKKNe TnbpSNSjhhGm4cRsP0OeuwUeCAXu722BK3v53uEhCeG/3ohcMI5uAT0NyuOQn0o5aad0+xIS41J/J hNDOHJuRg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jtS4P-0003AC-Rv; Thu, 09 Jul 2020 08:38:17 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jtS4N-00039C-2p for linux-arm-kernel@lists.infradead.org; Thu, 09 Jul 2020 08:38: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 96DF131B; Thu, 9 Jul 2020 01:38:13 -0700 (PDT) Received: from bogus (unknown [10.37.8.63]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D15183F887; Thu, 9 Jul 2020 01:38:11 -0700 (PDT) Date: Thu, 9 Jul 2020 09:38:09 +0100 From: Sudeep Holla To: Dien Pham Subject: Re: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted Message-ID: <20200709083808.GB31544@bogus> References: <20200708110725.18017-1-sudeep.holla@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200709_043815_242388_67C8B513 X-CRM114-Status: GOOD ( 31.88 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Stephen Boyd , Michael Turquette , "linux-clk@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" 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 Hi Dien-san, On Thu, Jul 09, 2020 at 08:20:51AM +0000, Dien Pham wrote: > Hi Sudeep, > > I share my build warning and some in-line comment below: > > CC drivers/firmware/arm_scmi/clock.o > drivers/firmware/arm_scmi/clock.c: In function 'rate_cmp_func': > drivers/firmware/arm_scmi/clock.c:127:12: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] > u64 *r1 = _r1, *r2 = _r2; > ^~~ > drivers/firmware/arm_scmi/clock.c:127:23: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] > u64 *r1 = _r1, *r2 = _r2; > ^~~ > CC arch/arm64/kernel/vdso.o > drivers/firmware/arm_scmi/clock.c: In function 'scmi_clock_protocol_init': > drivers/firmware/arm_scmi/clock.c:197:3: warning: 'rate' may be used uninitialized in this function [-Wmaybe-uninitialized] > sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > Sorry for this. I noticed yesterday when I built but strangely I had created patches before I fixed these and sent them instead of fixed version. My mistake. > >-----Original Message----- > >From: Sudeep Holla > >Sent: Wednesday, July 8, 2020 6:07 PM > >To: linux-arm-kernel@lists.infradead.org; linux-clk@vger.kernel.org; Stephen Boyd > >Cc: Sudeep Holla ; linux-kernel@vger.kernel.org; Michael Turquette ; Dien Pham > >Subject: [PATCH 1/2] firmware: arm_scmi: Keep the discrete clock rates sorted > > > >Instead of relying on the firmware to keep the clock rates sorted, let us sort the list. This is not essential for clock layer but it helps to find the min and max rates easily from the list. > > > >Fixes: 5f6c6430e904 ("firmware: arm_scmi: add initial support for clock protocol") > >Reported-by: Dien Pham > >Signed-off-by: Sudeep Holla > >--- > > drivers/firmware/arm_scmi/clock.c | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > >Hi Dien-san, > > > >If you could review/test these patches, I can queue them ASAP. > >I am planning to send the PR for ARM SoC later this week, so I need your tested-by. > > I applied the patch, > Although there are some build warnings, but the patch effect is ok. > Thanks for testing. > > > >Regards, > >Sudeep > > > >diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c > >index 4c2227662b26..2dd119cdebf6 100644 > >--- a/drivers/firmware/arm_scmi/clock.c > >+++ b/drivers/firmware/arm_scmi/clock.c > >@@ -5,6 +5,8 @@ > > * Copyright (C) 2018 ARM Ltd. > > */ > > > >+#include > >+ > > #include "common.h" > > > > enum scmi_clock_protocol_cmd { > >@@ -121,6 +123,13 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle, > > return ret; > > } > > > >+static int rate_cmp_func(const void *_r1, const void *_r2) { > >+ u64 *r1 = _r1, *r2 = _r2; > > It is better to add 'const' as below to avoid warning. > const u64 *r1 = _r1, *r2 = _r2; > Yes, I have this in the correct version which I sent as v2 this morning. > >+ > >+ return r1 - r2; > > r1 and r2 are u64, but returned value is 'int' type. > Do you think we should improve this ? e.g. return (int)r1 - r2; > Not changing to const above must suffice. > >+} > >+ > > static int > > scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > struct scmi_clock_info *clk) > >@@ -184,8 +193,10 @@ scmi_clock_describe_rates_get(const struct scmi_handle *handle, u32 clk_id, > > */ > > } while (num_returned && num_remaining); > > > >- if (rate_discrete) > >+ if (rate_discrete) { > > clk->list.num_rates = tot_rate_cnt; > >+ sort(rate, tot_rate_cnt, sizeof(*rate), rate_cmp_func, NULL); > > About warning of above line, I think it relates to below snip of code: > if (tot_rate_cnt + num_returned > SCMI_MAX_NUM_RATES) { > dev_err(handle->dev, "No. of rates > MAX_NUM_RATES"); > break; > } > I don't understand your comment and relation to above warning. > I see that in this case is true, it is not proceeded as error case, > If so I think you can update 'rate' for value from 'tot_rate_cnt' to SCMI_MAX_NUM_RATES at here. > How do you think ? > -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel