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=-17.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,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 7674EC433E6 for ; Tue, 19 Jan 2021 01:57:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 409CA2223E for ; Tue, 19 Jan 2021 01:57:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2393468AbhASB5b (ORCPT ); Mon, 18 Jan 2021 20:57:31 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60564 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387895AbhASB5Z (ORCPT ); Mon, 18 Jan 2021 20:57:25 -0500 Received: from mail-pl1-x62c.google.com (mail-pl1-x62c.google.com [IPv6:2607:f8b0:4864:20::62c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3036CC061574 for ; Mon, 18 Jan 2021 17:56:45 -0800 (PST) Received: by mail-pl1-x62c.google.com with SMTP id q4so9610669plr.7 for ; Mon, 18 Jan 2021 17:56:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=o1BhqLN64XSevTkLdXufW4kqzYTM3iysk6wH7Gij994=; b=EId4DXRCDyZtmTjha+Gyr7ue3DpJhyB/tvJCnOb3pb+TGA5uZGRNwNj4w4CfPRgzic U6QXj2/s1LV8ZaJGqJUJDvQUUlUAmlk/3Czpea+yUGDPr1plFzz/KeqHYTlPZzZ57eO1 V/7iuS3ApTxcue5pT5kxW3sDiVLIDkyKTA/MiMBEDFpLnDm9HRWXmPG3K1MWx9/osbdJ 5KRTpcdOw9ESs/qTbBBa0LPNBuwAizMKouyFfyhuUZKwWyHxzikuxgRRn8ZZreNSdm8k jeTHW7uTQ9eP3kL/ROVAKUZZqqzOt+1tsJqXFWWh8HA0LUW7aznVYZVBM9Ey82gNkjNU +W7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=o1BhqLN64XSevTkLdXufW4kqzYTM3iysk6wH7Gij994=; b=FQKuroV+c+0wv7yrASM3OuDJUABAj1zvC74M+9lHUGRiIPGXXQKkfDeMSdt2k0pJeq ajXu4R6pPUEFfFvOsqg2X54dN3/NVtuGsO4noN2fqxLDUzX8mmW2pfUtCC+62bDUWZ6e XiYkonoPHqdtX3nOa+eGx9bwwJeCvRZqfucJRyNrdLBhHiubGm4R0wPLGW0IpS7aAJdT EFhLXsnIZbs8hG+hdNVpEJ7FreNDSFFjpeva3LvH2yZq2jwEU0mnWMQp09B0GXFBhJ4A Oau6n6xCklBmu0xFUQLYbzo2+yyDHhbDLtX+ehkgpqI5nsNL9XuDxrNnGlA/SzqL7Qs3 WVGw== X-Gm-Message-State: AOAM5315RkzuJXV1wJYRxoFanzD5CoWa3g55HrlvyyB4HxiDdipzutdC 2JkY5nOfRKeX+qnQOUaafwaUAg== X-Google-Smtp-Source: ABdhPJzxuRg/AlkwIx2egrd1Q3zznzL19m3+GHhL2ShFF5AsybQmyH8ZOOx3W2zF8i3iBvecxPuPSA== X-Received: by 2002:a17:902:9a02:b029:dc:3481:3ff1 with SMTP id v2-20020a1709029a02b02900dc34813ff1mr2264038plp.28.1611021404442; Mon, 18 Jan 2021 17:56:44 -0800 (PST) Received: from dragon (80.251.214.228.16clouds.com. [80.251.214.228]) by smtp.gmail.com with ESMTPSA id w20sm14044834pga.90.2021.01.18.17.56.42 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 18 Jan 2021 17:56:43 -0800 (PST) Date: Tue, 19 Jan 2021 09:56:38 +0800 From: Shawn Guo To: Ionela Voinescu Cc: "Rafael J . Wysocki" , Viresh Kumar , Bjorn Andersson , Andy Gross , linux-pm@vger.kernel.org, linux-arm-msm@vger.kernel.org Subject: Re: [PATCH] cpufreq: qcom-hw: drop devm_xxx() calls from init/exit hooks Message-ID: <20210119015637.GA17701@dragon> References: <20210118130603.16176-1-shawn.guo@linaro.org> <20210118153823.GA3310@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210118153823.GA3310@arm.com> User-Agent: Mutt/1.9.4 (2018-02-28) Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On Mon, Jan 18, 2021 at 03:38:23PM +0000, Ionela Voinescu wrote: > Hi, > > On Monday 18 Jan 2021 at 21:06:03 (+0800), Shawn Guo wrote: > > Commit f17b3e44320b ("cpufreq: qcom-hw: Use > > devm_platform_ioremap_resource() to simplify code") introduces > > a regression on platforms using the driver, by failing to initialise > > a policy, when one is created post hotplug. > > > > When all the CPUs of a policy are hoptplugged out, the call to .exit() > > and later to devm_iounmap() does not release the memory region that was > > requested during devm_platform_ioremap_resource(). Therefore, > > a subsequent call to .init() will result in the following error, which > > will prevent a new policy to be initialised: > > > > [ 3395.915416] CPU4: shutdown > > [ 3395.938185] psci: CPU4 killed (polled 0 ms) > > [ 3399.071424] CPU5: shutdown > > [ 3399.094316] psci: CPU5 killed (polled 0 ms) > > [ 3402.139358] CPU6: shutdown > > [ 3402.161705] psci: CPU6 killed (polled 0 ms) > > [ 3404.742939] CPU7: shutdown > > [ 3404.765592] psci: CPU7 killed (polled 0 ms) > > [ 3411.492274] Detected VIPT I-cache on CPU4 > > [ 3411.492337] GICv3: CPU4: found redistributor 400 region 0:0x0000000017ae0000 > > [ 3411.492448] CPU4: Booted secondary processor 0x0000000400 [0x516f802d] > > [ 3411.503654] qcom-cpufreq-hw 17d43000.cpufreq: can't request region for resource [mem 0x17d45800-0x17d46bff] > > > > With that being said, the original code was tricky and skipping memory > > region request intentionally to hide this issue. The true cause is that > > those devm_xxx() device managed functions shouldn't be used for cpufreq > > init/exit hooks, because &pdev->dev is alive across the hooks and will > > not trigger auto resource free-up. Let's drop the use of device managed > > functions and manually allocate/free resources, so that the issue can be > > fixed properly. > > > > Fixes: f17b3e44320b ("cpufreq: qcom-hw: Use devm_platform_ioremap_resource() to simplify code") > > Suggested-by: Bjorn Andersson > > Signed-off-by: Shawn Guo > > --- > > > > I took some of the commit log from Ionela. > > > > drivers/cpufreq/qcom-cpufreq-hw.c | 43 ++++++++++++++++++++++++------- > > 1 file changed, 33 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > > index 9ed5341dc515..b529b49649e0 100644 > > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > > @@ -32,6 +32,7 @@ struct qcom_cpufreq_soc_data { > > > > struct qcom_cpufreq_data { > > void __iomem *base; > > + struct resource *res; > > const struct qcom_cpufreq_soc_data *soc_data; > > }; > > > > @@ -280,6 +281,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > > struct of_phandle_args args; > > struct device_node *cpu_np; > > struct device *cpu_dev; > > + struct resource *res; > > void __iomem *base; > > struct qcom_cpufreq_data *data; > > int ret, index; > > @@ -303,18 +305,33 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > > > > index = args.args[0]; > > > > - base = devm_platform_ioremap_resource(pdev, index); > > - if (IS_ERR(base)) > > - return PTR_ERR(base); > > + data = kzalloc(sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > > > Nit: you could move this allocation after all resource reservation and > mapping below, possibly to avoid doing it unless the base address and > the memory resource is actually valid. Or you can keep it here and > remove the use of the local variables, especially the "base" variable. It's a reasonable suggestion. I will send a new version to kill `base` variable, but still want to keep `res` as it saves some two level indirection. Shawn