Received: by 2002:ad5:474a:0:0:0:0:0 with SMTP id i10csp2952457imu; Sun, 11 Nov 2018 04:43:16 -0800 (PST) X-Google-Smtp-Source: AJdET5cR5Bo+a98eIcd9SDOdMC1fyy244sILQo+vcBE8yOHMe3Ipa9wrezczq8GRLDeZQdnvCWt5 X-Received: by 2002:a17:902:5590:: with SMTP id g16-v6mr16018496pli.56.1541940195944; Sun, 11 Nov 2018 04:43:15 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1541940195; cv=none; d=google.com; s=arc-20160816; b=VS2cuvI2SoelC49X6uLiFvZi+mYFSkgVuHp4qmb5SXaZQi+h1Nlyl4AfRNSU8ntfWJ D2CGE8u6Nqq1Klzkr1wwsUFibZVyOPzi6UvQXMLutR82aq8+eWLkDKJ9LBz+Axop4OdU toHzNjCp+IN8ioYzVfpGuLfjDIWH7SvaxRdP++LObFDM2WoY7tA/77nHrgb0PLrq2at0 PtqRdRpwLyBKKTjRrFv7nRvRMGvnoYn6XlO1zMTLvtHdtrOveehWA+EHAq1PRHp0mnKH jm2U3Mj2I1ET9FvORCK7aoRLugDLrGXInANfD0dZKY3dorvYNenKMKvtTolrzNeTxg2p MEig== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:dmarc-filter :dkim-signature:dkim-signature; bh=ikY0fv9ECMOHrA0YUm5rr2R4c99RE1eCvrCrFKqMjkM=; b=AxVvZjA7qRrVmtldvvj3TnRcPuT777IM7sWs7hrGeyJftRbrq3GWcUhrCPAv3DDG70 x2SvAElO9+z8XrpbpnjNChu9OciCaHcyKa3blyWMNx9Dbq565LAXIxJYx/OT/uI28BGo b3wFbwfdwjf/gTNi+zeVPwoJ7YVbmXuX+Y0jaxVW3oGzBeWHWAnF5YoG8b3afPY3V7C8 jTyY5u8N3sQ+uyA1z7/o9Vj+OpSMhhuzZWpsCkO7YJVKKejRDBk119S7G/i5XUwRRcpo aXOk7UnDeHoPnLDpgpAgP5f7Ereinj/E9qkmzFKfAE88nalcsKTPzzFzScEcVR6yw5rS QR+w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@codeaurora.org header.s=default header.b=m28mhrX9; dkim=pass header.i=@codeaurora.org header.s=default header.b="dR1hRky/"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id i187-v6si7003432pfb.91.2018.11.11.04.43.00; Sun, 11 Nov 2018 04:43:15 -0800 (PST) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@codeaurora.org header.s=default header.b=m28mhrX9; dkim=pass header.i=@codeaurora.org header.s=default header.b="dR1hRky/"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728043AbeKKWbJ (ORCPT + 99 others); Sun, 11 Nov 2018 17:31:09 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:33378 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727594AbeKKWbJ (ORCPT ); Sun, 11 Nov 2018 17:31:09 -0500 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id B5C4560A33; Sun, 11 Nov 2018 12:42:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1541940159; bh=hF/omAr0+4O+t6KrEhSBC45nS/s2dupbv5nPlOR0+0A=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=m28mhrX9XOpS6LIDqkOYa1Crw0+FGDXOzSlr9UyhO5n+vZRuIrUqQjarDAONR7eT3 E8Le5704bR0Oyvy6hcO4zG2bXUVSP1K5rSma6v5WEtpYpbN49iFPDdmwh3+yTeUubD 6VafLwUlUi32WeItV7qX98vLVUeVMcVdpCTgnM30= X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on pdx-caf-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.7 required=2.0 tests=ALL_TRUSTED,BAYES_00, DKIM_INVALID,DKIM_SIGNED autolearn=no autolearn_force=no version=3.4.0 Received: from [10.79.169.103] (blr-bdr-fw-01_globalnat_allzones-outside.qualcomm.com [103.229.18.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: tdas@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 777F060249; Sun, 11 Nov 2018 12:42:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1541940158; bh=hF/omAr0+4O+t6KrEhSBC45nS/s2dupbv5nPlOR0+0A=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=dR1hRky/UCSbmQJhX7BWdl3/3n71uVs3rtshOG8ggZxLFcrTLxIY4Z9JZwkUYfIRd W8XAycjiCTqRnryFMA33tvMkwU59xBvPJt1PHNhB1dW1uyo0dHaw5VnptxgUq3RyTa S5nKEBnDed7VMQQoCfd5aonczDQpHPZVjEapPICY= DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 777F060249 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=tdas@codeaurora.org Subject: Re: [PATCH 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver To: Stephen Boyd , "Rafael J. Wysocki" , Viresh Kumar , linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org Cc: Rajendra Nayak , devicetree@vger.kernel.org, robh@kernel.org, skannan@codeaurora.org, linux-arm-msm@vger.kernel.org, amit.kucheria@linaro.org, evgreen@google.com References: <1539257761-23023-1-git-send-email-tdas@codeaurora.org> <1539257761-23023-3-git-send-email-tdas@codeaurora.org> <153981915373.5275.15971019914218464179@swboyd.mtv.corp.google.com> <0c51a12e-38d3-2df5-4f5f-6a687727e9bf@codeaurora.org> <154130523254.88331.12609105382114756048@swboyd.mtv.corp.google.com> From: Taniya Das Message-ID: <3aa7b871-9cf9-9626-11fe-b9aa6009b477@codeaurora.org> Date: Sun, 11 Nov 2018 18:12:29 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <154130523254.88331.12609105382114756048@swboyd.mtv.corp.google.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Stephen, Thanks for your comments. On 11/4/2018 9:50 AM, Stephen Boyd wrote: > Quoting Taniya Das (2018-11-02 20:06:00) >> Hello Stephen, >> >> On 10/18/2018 5:02 AM, Stephen Boyd wrote: >>> Quoting Taniya Das (2018-10-11 04:36:01) >>>> --- a/drivers/cpufreq/Kconfig.arm >>>> +++ b/drivers/cpufreq/Kconfig.arm >>>> @@ -121,6 +121,17 @@ config ARM_QCOM_CPUFREQ_KRYO >>>> >>>> If in doubt, say N. >>>> >>>> +config ARM_QCOM_CPUFREQ_HW >>>> + bool "QCOM CPUFreq HW driver" >>> >>> Is there any reason this can't be a module? >>> >> >> We do not have any use cases where we need to support it as module. > > Ok, so it could easily be tristate then? Why not allow it? > I have checked other vendors CPUfreq drivers and those too support only "bool". >> >>>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c >>>> new file mode 100644 >>>> index 0000000..fe1c264 >>>> --- /dev/null >>>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c >>>> @@ -0,0 +1,354 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/* >>>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. >>>> + */ > [...] >>>> + >>>> +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = { >>> >>> Is this going to change in the future? >>> >> >> Yes, they could change and that was the reason to introduce the offsets. >> This was discussed earlier too with Sudeep and was to add them. >> >>>> + [REG_ENABLE] = 0x0, > > This is only used once? Maybe it could be removed. > >>>> + [REG_LUT_TABLE] = 0x110, > > And this is only used during probe to figure out the supported > frequencies. So we definitely don't need to store around the registers > after probe in an array of iomem pointers. The only one that we need > after probe is the one below. > >>>> + [REG_PERF_STATE] = 0x920, >>>> +}; >>>> + As these address offsets could change, so I am of the opinion to leave them as it is. >>>> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS]; >>>> + >>>> +static int >>>> +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, >>>> + unsigned int index) >>>> +{ >>>> + struct cpufreq_qcom *c = policy->driver_data; >>>> + >>>> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]); >>> >>> Why can't we avoid the indirection here and store the perf_state pointer >>> in probe? Then we don't have to indirect through a table to perform the >>> register write. >>> >> >> As the offsets could change and that was the reason to add this. > > With fast switching we can avoid incurring any extra instructions, so > please make another iomem pointer in the cpufreq_qcom struct just for > writing the index or if possible, just pass the iomem pointer that > points to the REG_PERF_STATE as the policy->driver_data variable here. > Then we have the address in hand without any extra load. If my > understanding is correct, we don't need to keep around anything besides > this register address anyway so we should be able to just load it and > write it immediately. > The c->reg_bases[] is just an index to the updated bases addresses. I am not clear as to why it would incur an extra instruction. The below code would already take care of it. + for (i = REG_ENABLE; i < REG_ARRAY_SIZE; i++) + c->reg_bases[i] = base + offsets[i]; + -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --