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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 28B03C4332F for ; Wed, 20 Apr 2022 10:04:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1377432AbiDTKHL (ORCPT ); Wed, 20 Apr 2022 06:07:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39698 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1377464AbiDTKHK (ORCPT ); Wed, 20 Apr 2022 06:07:10 -0400 Received: from mail-ed1-x530.google.com (mail-ed1-x530.google.com [IPv6:2a00:1450:4864:20::530]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 746D03ED27 for ; Wed, 20 Apr 2022 03:04:23 -0700 (PDT) Received: by mail-ed1-x530.google.com with SMTP id b24so1570970edu.10 for ; Wed, 20 Apr 2022 03:04:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=O/+Bq4GD0i1aeDJZB6KF270FrYsDWC9ZLK3WKl7pvtw=; b=iQg0xYNVkHIZo0BFivFUh7zcofwerY9qIBuS1eg1pq1Wa+PK0moNksnXr2ibxPi8Nt U4lJJn9AKf4k/TObba/Cc6oO4TsZpgIvDsGSOI4d+MrBlA6kaCK7lviv+aa2F++37KgR N/r+QzoV+OnRGchrRwCncCZWkuUNshBr9/El/cp2eZVL39X0sLxwhSLBbrR2SosL706Q QGwTbqgSZE0x2tcPBwFlyXbQQGpGtE5ZbEhr9iglUhXQg2XhGbPhvxh1iQ7sSxD8Ekyp pSwmybpiz//I9XHylVbYKw8D03YZv3IELSfETGs8FfNIZ0DB7WpSpuQJKb7nPEVuVJ9w vvUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=O/+Bq4GD0i1aeDJZB6KF270FrYsDWC9ZLK3WKl7pvtw=; b=sQobqbKtBMNMc5QGaR/YreYizGYCNwRm5qpha4bKamGRTgMsmZSfrYkTh8atij2Csb nD98F8z2vJ8InHfNOtudhqD3tEse6wz77J0nnQOUuvxXO4ejuuOoj/tSi+f3dtA8gU/r 5gZXNR3tFol2PR7QtuFWJKK91kk7v6/PH5d6N0KuWZl0But6aOsczErwZ555Qs+JhW5i OSDOV3AV6Mi6NWESOkxUCLyN/y45CVLikYbt7+nkWuUgmYHwvX0L6NaEqPYlKcZUjsVi 8REkEM6CvDV8IX2H2LMLfLZoQbY+PZRuXxjUxvqXl+B23OQdzigVkNz48ePU242f/YtJ Ookg== X-Gm-Message-State: AOAM532Tf5HneeIoHD4cTi/QMx4RVJ8xH9I0Q71kHVuNTiy3rY3CKVRo w83HPmrLtbhNZi62emUn24wzKw== X-Google-Smtp-Source: ABdhPJzL1lJX7sEUE2x5amjeU9v9hVjpMQnJ3qC7rbHT6pjZahpCdoZq2EmtCY7VhfWQbkBr/EYySQ== X-Received: by 2002:a05:6402:31ee:b0:41d:7038:c04a with SMTP id dy14-20020a05640231ee00b0041d7038c04amr22411161edb.142.1650449061949; Wed, 20 Apr 2022 03:04:21 -0700 (PDT) Received: from [192.168.0.224] (xdsl-188-155-176-92.adslplus.ch. [188.155.176.92]) by smtp.gmail.com with ESMTPSA id kw5-20020a170907770500b006db075e5358sm6599740ejc.66.2022.04.20.03.04.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 20 Apr 2022 03:04:21 -0700 (PDT) Message-ID: <6dc5e28b-e84d-95c3-3967-476b2126314e@linaro.org> Date: Wed, 20 Apr 2022 12:04:20 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [RFC PATCH v2 5/6] ufs: use PM OPP when scaling gears Content-Language: en-US To: Manivannan Sadhasivam Cc: Andy Gross , Bjorn Andersson , Michael Turquette , Stephen Boyd , Rob Herring , Krzysztof Kozlowski , Viresh Kumar , Nishanth Menon , Alim Akhtar , Avri Altman , "James E.J. Bottomley" , "Martin K. Petersen" , "Rafael J. Wysocki" , Taniya Das , linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-scsi@vger.kernel.org References: <20220411154347.491396-1-krzysztof.kozlowski@linaro.org> <20220411154347.491396-6-krzysztof.kozlowski@linaro.org> <20220419170149.GB8699@thinkpad> From: Krzysztof Kozlowski In-Reply-To: <20220419170149.GB8699@thinkpad> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-clk@vger.kernel.org On 19/04/2022 19:01, Manivannan Sadhasivam wrote: > On Mon, Apr 11, 2022 at 05:43:46PM +0200, Krzysztof Kozlowski wrote: >> Scaling gears requires not only scaling clocks, but also voltage levels, >> e.g. via performance states. >> >> Use the provided OPP table, to set proper OPP frequency which through >> required-opps will trigger performance state change. This deprecates >> the old freq-table-hz Devicetree property and old clock scaling method >> in favor of PM core code. >> >> Signed-off-by: Krzysztof Kozlowski >> --- >> drivers/scsi/ufs/ufshcd-pltfrm.c | 69 +++++++++++++++++++ >> drivers/scsi/ufs/ufshcd.c | 115 +++++++++++++++++++++++-------- >> drivers/scsi/ufs/ufshcd.h | 4 ++ >> 3 files changed, 158 insertions(+), 30 deletions(-) >> >> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c >> index c1d8b6f46868..edba585db0c1 100644 >> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c >> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c >> @@ -107,6 +107,69 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba) >> return ret; >> } >> >> +static int ufshcd_parse_operating_points(struct ufs_hba *hba) >> +{ >> + struct device *dev = hba->dev; >> + struct device_node *np = dev->of_node; >> + struct ufs_clk_info *clki; >> + const char *names[16]; >> + bool clocks_done; > > Maybe freq_table? ok > >> + int cnt, i, ret; >> + >> + if (!of_find_property(dev->of_node, "operating-points-v2", NULL)) >> + return 0; >> + >> + cnt = of_property_count_strings(np, "clock-names"); >> + if (cnt <= 0) { >> + dev_warn(dev, "%s: Missing clock-names\n", >> + __func__); >> + return -EINVAL; >> + } >> + >> + if (cnt > ARRAY_SIZE(names)) { >> + dev_info(dev, "%s: Too many clock-names\n", __func__); >> + return -EINVAL; >> + } > > How did you come up with 16 as the max clock count? Is this check necessary? 16 was an arbitrary choice, also mentioned in the bindings: https://lore.kernel.org/all/20220411154347.491396-3-krzysztof.kozlowski@linaro.org/ The check is necessary from current code point of view - array is locally allocated with fixed size. Since bindings do not allow more than 16, I am not sure if there is a point to make the code flexible now... but if this is something you wish, I can change. > >> + >> + /* clocks parsed by ufshcd_parse_clock_info() */ >> + clocks_done = !!of_find_property(np, "freq-table-hz", NULL); > > freq-table-hz and opp-table are mutually exclusive, isn't it? You're right, this should be an exit. (...) >> ufshcd_init_lanes_per_dir(hba); >> >> err = ufshcd_init(hba, mmio_base, irq); >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index 5bfa62fa288a..aec7da18a550 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -1022,6 +1022,9 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) >> int ret = 0; >> ktime_t start = ktime_get(); >> >> + if (hba->use_pm_opp) >> + return 0; >> + > > So you don't need pre and post clock changes below? That's tricky. The UFS HCD core does not need it, but of course the question is about the drivers actually using ufshcd_vops_clk_scale_notify(). Only QCOM UFS driver implements it and actually we might need it. Qcom driver changes DME_VS_CORE_CLK_CTRL, so maybe this should be done here as well. I don't know yet how to incorporate it into PM-opp framework, because now changing frequencies and voltage is atomic from the UFS driver perspective. Before it was not - for example first clock (with these pre/post changes) and then voltage. I will need to solve it somehow... Best regards, Krzysztof