Received: by 2002:a05:6a10:f3d0:0:0:0:0 with SMTP id a16csp5286573pxv; Tue, 6 Jul 2021 23:33:29 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxDKV27JxZk0/5itmiPNFH6rHIw/8tzSXkfDw+73mbyc6iktJ0v/kl3tRXb4rA6kmAWzFKh X-Received: by 2002:a17:907:970e:: with SMTP id jg14mr17728699ejc.105.1625639609498; Tue, 06 Jul 2021 23:33:29 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1625639609; cv=none; d=google.com; s=arc-20160816; b=nVNSYP+6BRNjTqdz94GFpDkmyJiEEIV28fyTkqv55tPeJXFYBwTLir3sleubd/+d7X w72fCoj91+eaGdA08vLt0wbuu22EevcebUPSnmW18iNHnbhLPcTNycgAYZnowefssu1K UMnpFAWfy2BBaH+m+fmVPOeReCOF2evlX16NkguGZJz/QK9FovDbyuMfKJ/gN0+VxOLa EPJqTCTEDeom2Kt4bvatjq+kwUlJZqIWxmutMXCSWcqBlhkz2gjYk+oBgK22GMryfnBu PwurFEkLMer1lcbnUhdeJrYLvYLdC6WciqSK0RrkWrXYU4GPHna7AwML06g/By9tmhn6 3Ffw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject:dmarc-filter:sender:dkim-signature; bh=ORSL9Q/hfYBNgLWGbd3sA9EpU/yVvo+awtIkSaX2qWw=; b=UbKM5d3zfhHwa07LfTAdWKtqoiJeusWSxukNjhgzazUV327BnNJ3tsIqZAekA2OuqT Tda6vh1C1rdkvEMdQVPiHThsM3s4BvXjEOvEfkxzJpYojfDYRxm+Uz7TA+cd0vywM+nz 7dvxtsE9ziriztJNSqBGZ19d0ikT49Ahk+jFPNH7pLcTzh6EVWFDorUZHkUXCAcZD0AQ H19gWFxpfqJ4Fsf803yGm+GXdYixdTlmKiiwDb4X76TN0JfgKCItnWL2EAlvKeo9amzk ZAR7x4xjT3+QBLKi/XEb1q1ru4NdblmBVyh+bh9GghXZKdfHj2/Jytnpj41wXD5cB7Qa gmwA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@mg.codeaurora.org header.s=smtp header.b=rdAJHMrF; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id o20si7332850ejy.635.2021.07.06.23.33.05; Tue, 06 Jul 2021 23:33:29 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@mg.codeaurora.org header.s=smtp header.b=rdAJHMrF; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230292AbhGGGea (ORCPT + 99 others); Wed, 7 Jul 2021 02:34:30 -0400 Received: from so254-9.mailgun.net ([198.61.254.9]:30712 "EHLO so254-9.mailgun.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230263AbhGGGe3 (ORCPT ); Wed, 7 Jul 2021 02:34:29 -0400 DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=mg.codeaurora.org; q=dns/txt; s=smtp; t=1625639510; h=Content-Transfer-Encoding: Content-Type: In-Reply-To: MIME-Version: Date: Message-ID: From: References: Cc: To: Subject: Sender; bh=ORSL9Q/hfYBNgLWGbd3sA9EpU/yVvo+awtIkSaX2qWw=; b=rdAJHMrFmOEMEOh7qICBw0gPZizPsXXR8ceMXZT1P0LjluD8F/9jH2eb//OLjL2DjqFnRMS4 nO/WA8fcLeuTyaJucafkn/9EbFmCMpYfzqon7f4zaDNQ4zMdo8QnLFHV3nxelPakxoLi7Zwr VQVGefAXr8mwLEmu7N1GYDnVHFo= X-Mailgun-Sending-Ip: 198.61.254.9 X-Mailgun-Sid: WyI0MWYwYSIsICJsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnIiwgImJlOWU0YSJd Received: from smtp.codeaurora.org (ec2-35-166-182-171.us-west-2.compute.amazonaws.com [35.166.182.171]) by smtp-out-n04.prod.us-west-2.postgun.com with SMTP id 60e54a50ad0600eedefdff57 (version=TLS1.2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256); Wed, 07 Jul 2021 06:31:44 GMT Sender: rnayak=codeaurora.org@mg.codeaurora.org Received: by smtp.codeaurora.org (Postfix, from userid 1001) id 9262FC433F1; Wed, 7 Jul 2021 06:31:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-caf-mail-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=ALL_TRUSTED,BAYES_00, NICE_REPLY_A,SPF_FAIL autolearn=no autolearn_force=no version=3.4.0 Received: from [10.50.35.89] (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: rnayak) by smtp.codeaurora.org (Postfix) with ESMTPSA id 9B36BC433F1; Wed, 7 Jul 2021 06:31:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 9B36BC433F1 Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; spf=fail smtp.mailfrom=rnayak@codeaurora.org Subject: Re: [PATCH 1/2] soc: qcom: rpmhpd: Use corner in power_off To: Bjorn Andersson Cc: Andy Gross , Ulf Hansson , Stephen Boyd , Dmitry Baryshkov , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org References: <20210703005416.2668319-1-bjorn.andersson@linaro.org> <20210703005416.2668319-2-bjorn.andersson@linaro.org> From: Rajendra Nayak Message-ID: <534fca70-d277-4154-b932-a4d6ab3b0b66@codeaurora.org> Date: Wed, 7 Jul 2021 12:01:39 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 7/7/2021 10:19 AM, Bjorn Andersson wrote: > On Mon 05 Jul 00:40 CDT 2021, Rajendra Nayak wrote: >> On 7/5/2021 10:36 AM, Bjorn Andersson wrote: >>> On Sun, Jul 4, 2021 at 11:27 PM Rajendra Nayak wrote: >>>> >>>> >>>> >>>> On 7/3/2021 6:24 AM, Bjorn Andersson wrote: >>>>> rpmhpd_aggregate_corner() takes a corner as parameter, but in >>>>> rpmhpd_power_off() the code requests the level of the first corner >>>>> instead. >>>>> >>>>> In all (known) current cases the first corner has level 0, so this >>>>> change should be a nop, but in case that there's a power domain with a >>>>> non-zero lowest level this makes sure that rpmhpd_power_off() actually >>>>> requests the lowest level - which is the closest to "power off" we can >>>>> get. >>>>> >>>>> While touching the code, also skip the unnecessary zero-initialization >>>>> of "ret". >>>>> >>>>> Fixes: 279b7e8a62cc ("soc: qcom: rpmhpd: Add RPMh power domain driver") >>>>> Signed-off-by: Bjorn Andersson >>>>> --- >>>>> drivers/soc/qcom/rpmhpd.c | 5 ++--- >>>>> 1 file changed, 2 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c >>>>> index 2daa17ba54a3..fa209b479ab3 100644 >>>>> --- a/drivers/soc/qcom/rpmhpd.c >>>>> +++ b/drivers/soc/qcom/rpmhpd.c >>>>> @@ -403,12 +403,11 @@ static int rpmhpd_power_on(struct generic_pm_domain *domain) >>>>> static int rpmhpd_power_off(struct generic_pm_domain *domain) >>>>> { >>>>> struct rpmhpd *pd = domain_to_rpmhpd(domain); >>>>> - int ret = 0; >>>>> + int ret; >>>>> >>>>> mutex_lock(&rpmhpd_lock); >>>>> >>>>> - ret = rpmhpd_aggregate_corner(pd, pd->level[0]); >>>>> - >>>>> + ret = rpmhpd_aggregate_corner(pd, 0); >>>> >>>> This won't work for cases where pd->level[0] != 0, rpmh would just ignore this and keep the >>>> resource at whatever corner it was previously at. >>>> (unless command DB tells you a 0 is 'valid' for a resource, sending a 0 is a nop) >>>> The right thing to do is to send in whatever command DB tells you is the lowest level that's valid, >>>> which is pd->level[0]. >>>> >>> >>> I'm afraid this doesn't make sense to me. >>> >>> In rpmh_power_on() if cmd-db tells us that we have [0, 64, ...] and we >>> request 64 we rpmhpd_aggregate_corner(pd, 1); but in power off, if >>> cmd-db would provide [64, ...] we would end up sending >>> rpmhpd_aggregate_corner(pd, 64); >>> So in power_on we request the corner (i.e. index in the array provided >>> in cmd-db) and in power-off the same function takes the level? >> >> ah that's right, I did not read the commit log properly and got confused. > > Thanks for confirming my understanding. > >> Looks like this bug existed from the day this driver for merged :/, thanks >> for catching it. >> Does it make sense to also mark this fix for stable? >> > > I can certainly add a Cc: stable@ as I'm applying this. sure, sounds good > May I have your R-b? Reviewed-by: Rajendra Nayak > > PS. Do you have any input on patch 2/2? That actually solves a practical > problem we're seeing. Would it perhaps aid in your need for the new > "assigned-opp-level" property? We would perhaps still need the 'assigned-opp-level' or equivalent since the default requirement of devices is not always the least level supported, in some cases it might be slightly higher corner which would then need to be set explicitly. I was hoping on getting some more testing done with that patch especially for any regression on the sc7180 and sc7280 devices, which I haven't got to yet. Are you getting these patches ready for merge for the -rc cycle or for the next merge window? -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation