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=-2.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no 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 B1933C433DF for ; Tue, 7 Jul 2020 13:28:07 +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 6E80F206E2 for ; Tue, 7 Jul 2020 13:28:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="YF3I+geH" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6E80F206E2 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-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Y6T8FZLj+03P9/mcja3xRHmirTyIUEn6H7w9D1A632I=; b=YF3I+geHzzhYZaZzKGBoyp5RT ADEjW8StdQKtQeVgB6qIC9V7QIu9B7c4jol96uDqXcqx2l4wAkstqyDgQ/KfRMkdEDViDutaXWIt9 nfqBKJMUgjS09hl6X1LbzBxmM+ITr/KhQUFSHRYgDkYb7hk2yfKRK/6nHQS1SkCmK0qIP76PlUV7L 39vZ0vdWN2c3mxsSaBmtiF+bKg0xhSOa0EZ1WV1Vs0nQqXIiW6obu+elQkPWkiV6AMZKxrZE3odMh VHpfadOR+WebVkVJJzYr502dFk0S+tVMLxvoZTDIOi6RTApyiF2GKsv8T7Z4fLsPLlHr9xLm31s8C NJvcp+f0A==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jsncT-0002TH-5P; Tue, 07 Jul 2020 13:26:45 +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 1jsncP-0002Sq-Gj for linux-arm-kernel@lists.infradead.org; Tue, 07 Jul 2020 13:26:42 +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 3D699C0A; Tue, 7 Jul 2020 06:26:36 -0700 (PDT) Received: from [10.37.12.65] (unknown [10.37.12.65]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0B6033F71E; Tue, 7 Jul 2020 06:26:32 -0700 (PDT) Subject: Re: [PATCH 0/5] cpuidle: psci: Various improvements for PSCI PM domains To: Ulf Hansson References: <20200615152054.6819-1-ulf.hansson@linaro.org> From: Lukasz Luba Message-ID: <894922ca-6ae0-6397-117c-561919bc1ef3@arm.com> Date: Tue, 7 Jul 2020 14:26:30 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200707_092641_670488_7588F8EE X-CRM114-Status: GOOD ( 28.64 ) 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: Lorenzo Pieralisi , Vincent Guittot , Benjamin Gaignard , Saravana Kannan , Linux PM , Stephen Boyd , Daniel Lezcano , "Rafael J . Wysocki" , Lina Iyer , Bjorn Andersson , Sudeep Holla , Linux ARM Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 7/7/20 1:51 PM, Ulf Hansson wrote: > On Tue, 7 Jul 2020 at 14:37, Lukasz Luba wrote: >> >> >> >> On 7/7/20 12:53 PM, Ulf Hansson wrote: >>> Hi Lukaz, >>> >>> On Tue, 30 Jun 2020 at 12:23, Lukasz Luba wrote: >>>> >>>> Hi Ulf, >>>> >>>> On 6/15/20 4:20 PM, Ulf Hansson wrote: >>>>> The main change in this series is done in patch 5/5, which implements support >>>>> to prevent domain idlestates until all PSCI PM domain consumers are ready for >>>>> it. To reach that point the corresponding code for cpuidle-psci and >>>>> cpuidle-psci-domain, needed to be converted into platform drivers, which is >>>>> done by the earlier changes in the series. >>>>> >>>>> Additionally, some improvements have been made to the error path, which becomes >>>>> easier when the code gets converted to platform drivers. >>>>> >>>>> Deployment for a Qcom SoC, which actually takes full benefit of these changes >>>>> are also in the pipe, but deferring then a bit until $subject series have been >>>>> discussed. >>>>> >>>>> Kind regards >>>>> Ulf Hansson >>>>> >>>>> Ulf Hansson (5): >>>>> cpuidle: psci: Fail cpuidle registration if set OSI mode failed >>>>> cpuidle: psci: Fix error path via converting to a platform driver >>>>> cpuidle: psci: Split into two separate build objects >>>>> cpuidle: psci: Convert PM domain to platform driver >>>>> cpuidle: psci: Prevent domain idlestates until consumers are ready >>>>> >>>>> drivers/cpuidle/Kconfig.arm | 10 ++ >>>>> drivers/cpuidle/Makefile | 5 +- >>>>> drivers/cpuidle/cpuidle-psci-domain.c | 74 +++++++++----- >>>>> drivers/cpuidle/cpuidle-psci.c | 141 +++++++++++++++----------- >>>>> drivers/cpuidle/cpuidle-psci.h | 11 +- >>>>> 5 files changed, 150 insertions(+), 91 deletions(-) >>>>> >>>> >>>> Since I am interested in some CPU idle statistics (residency, etc), >>>> I would like to help you and Sudeep in reviewing the patch set. >>> >>> Thanks, much appreciated! >>> >>>> >>>> Could you clarify some bit below? >>>> 1. There is Qcom SoC which has dependencies between PSCI PM domain >>>> consumers and this CPU idle - thus we cannot register and use CPU >>>> idle driver till related drivers fully setup. >>> >>> I think you got it right, but let me clarify. >>> >>> On Qcom SoC, PSCI PM domain consumers aren't solely CPU devices, but >>> also the 'qcom,rpmh-rsc' device is a consumer, for example. >>> >>> That doesn't mean the CPU idle driver can't be probed/initialized, but >>> rather that the PSCI PM domain must not be powered off. The power off >>> needs to wait until all the consumers of the PSCI PM domain have been >>> registered/probed. >>> >>> See more details in the commit message of patch5. >>> >>>> 2. The proposed solution is to use platform driver and plat. device >>>> for this CPU idle driver, to have access to deferred probe mechanism and >>>> wait for the consumer drivers fully probed state. >>> >>> Correct, but let me fill in some more. >>> >>> I would like to use the ->sync_state() callback of the PSCI PM domain >>> driver, as a way to understand that all consumers have been probed. >>> >>>> 3. Do you have maybe some estimations how long it takes for these >>>> consumers to be fully probed? >>> >>> I am not sure I understand the reason for the question. >> >> I was wondering if this is because of HW issue of long setup, thus >> we need to wait a bit longer with drivers deferred probing. >> But this is not the case as I can see now. >> >> >>> >>> Anyway, at this point, I am looking at the qcom,rpmh-rsc device, which >>> is being probed by the drivers/soc/qcom/rpmh-rsc.c driver. Moving >>> forward, in principle it can be any device/driver that is a consumer >>> of the PSCI PM domain. I am not even excluding that drivers can be >>> modules. It should work for all cases. >> >> The late_initcall won't help, this is a really tough requirement: >> being a module... >> >> >>> >>>> 4. Changing just this CPU idle driver registration point (to >>>> late_initcall()) phase in time is not a solution. >>> >>> Correct, it doesn't work. >>> >>> Playing with initcalls doesn't guarantee anything when it comes to >>> making sure all consumers are ready. >> >> I agree, especially when modules are involved. >> >>> >>> Hope this clarifies things a bit for you, but just tell me if there is >>> anything more I can do to further explain things. >> >> Yes, thank you. I can see now why you need this explicit ->sync_state() >> callback. >> I don't see better solution to what you have proposed here. >> I will go through the patches once again to check and add some >> reviewed-by. >> > > Thanks a lot for your time! I am just about to post a v2, to re-order > the series so patch 3 comes first, according to suggestions from Lina. > > Please move over to review that version instead, in a few minutes. OK, got it in my mailbox, thanks! Regards, Lukasz _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel