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=-4.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham 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 0F41FC48BD6 for ; Wed, 26 Jun 2019 11:28:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DB13D2080C for ; Wed, 26 Jun 2019 11:28:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1561548511; bh=Rx1WkGoiSEWh2wSDBDE2RSmbZjM1VyJ2Sb77lEjvA2w=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=0GPn9aOB/bPTJKrXmGjBwkaqYWcFSM0cI+/KQV1kYZopNkDwyT3qVUMx68kpM1vqW IN6SEHBJUyQ4nLQrX8lEAgeuVXOcn0e5IBYYw0WtQQM4PDmfSp1oIR1WZrzzXu4lVd UeH4UXMjV+VWQ31qWgQp8I4KNFiiE+L3MwENNrio= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726347AbfFZL2b convert rfc822-to-8bit (ORCPT ); Wed, 26 Jun 2019 07:28:31 -0400 Received: from mail-ot1-f66.google.com ([209.85.210.66]:42749 "EHLO mail-ot1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726339AbfFZL2b (ORCPT ); Wed, 26 Jun 2019 07:28:31 -0400 Received: by mail-ot1-f66.google.com with SMTP id l15so2119547otn.9; Wed, 26 Jun 2019 04:28:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=zYG7+FYrQci0B0NIIaV4SdyVIkXx7HP7jsA5/CTPT+A=; b=I/rVUbKcJWx0fApH7NgpK6+ISwu3DE1elJmg+JSChHT5MddeovLDMcyCNgMsvF8L3V x0hldkNCghWWU4eFQfXRRC0jwZtLFcnIUNZPXrFckHL4NrQdVkkCyls+82x3i+rUtp5V 4j4WmRF8xVeF0AjlP5CPuMIKy2WUzTjIizmvGsJQdzUGo/B5MVQty3855y9tLGsf8IyA MHho5/17RwhTMyt3vCXJYwYJknCcPizUmh+G88iI3FaewxgAEYNnrBdRQLb8/4mtBvWa EJk1iTE5LTMfujhzApk9a3qnzsdLf9Wy5dlTWLaR5Xh7FRTTzqW8ONWwyjD8sA/OYgxs LT3A== X-Gm-Message-State: APjAAAWPMA82e1z6z6aBm9mXpacR1GSoy1biySEA77/lbDUww3tDNv89 cNMLcx3taCTUXLxUNJ2knGhcYm3DKe9lGxPEvOzgsbEg X-Google-Smtp-Source: APXvYqxQcfGkDZskFmpDKI+mK6sXWnA1QxcoJlpLtksnfosbfzd0ZwAqux1kiyfOCsazXPoyKgzI+/UQgzyr5VyCpbU= X-Received: by 2002:a9d:6959:: with SMTP id p25mr2713084oto.118.1561548510328; Wed, 26 Jun 2019 04:28:30 -0700 (PDT) MIME-Version: 1.0 References: <20190625113244.18146-1-daniel.lezcano@linaro.org> <20190625113244.18146-2-daniel.lezcano@linaro.org> <20190626025831.jmyzyypxr6ezpbtu@vireshk-i7> <20190626063716.cechnzsb75q5lclr@vireshk-i7> <8a9b7bd0-9b21-1ce1-6176-cffff4b8d739@linaro.org> In-Reply-To: <8a9b7bd0-9b21-1ce1-6176-cffff4b8d739@linaro.org> From: "Rafael J. Wysocki" Date: Wed, 26 Jun 2019 13:28:19 +0200 Message-ID: Subject: Re: [PATCH V3 2/3] thermal/drivers/cpu_cooling: Unregister with the policy To: Daniel Lezcano Cc: "Rafael J. Wysocki" , Viresh Kumar , "Rafael J. Wysocki" , Eduardo Valentin , Linux Kernel Mailing List , Sudeep Holla , Amit Daniel Kachhap , Javi Merino , Zhang Rui , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , Keerthy , "open list:CPU FREQUENCY DRIVERS - ARM BIG LITTLE" , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" , "open list:TI BANDGAP AND THERMAL DRIVER" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On Wed, Jun 26, 2019 at 12:19 PM Daniel Lezcano wrote: > > On 26/06/2019 11:06, Rafael J. Wysocki wrote: > > On Wed, Jun 26, 2019 at 8:37 AM Viresh Kumar wrote: > >> > >> On 26-06-19, 08:02, Daniel Lezcano wrote: > >>> On 26/06/2019 04:58, Viresh Kumar wrote: > >>>> On 25-06-19, 13:32, Daniel Lezcano wrote: > >>>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > >>>>> index aee024e42618..f07454249fbc 100644 > >>>>> --- a/drivers/cpufreq/cpufreq.c > >>>>> +++ b/drivers/cpufreq/cpufreq.c > >>>>> @@ -1379,8 +1379,8 @@ static int cpufreq_online(unsigned int cpu) > >>>>> cpufreq_driver->ready(policy); > >>>>> > >>>>> if (cpufreq_thermal_control_enabled(cpufreq_driver)) > >>>>> - policy->cdev = of_cpufreq_cooling_register(policy); > >>>>> - > >>>>> + of_cpufreq_cooling_register(policy); > >>>>> + > >>>> > >>>> We don't need any error checking here anymore ? > >>> > >>> There was no error checking initially. This comment and the others below > >>> are for an additional patch IMO, not a change in this one. > >> > >> right, but ... > >> > >>>>> -void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) > >>>>> +void cpufreq_cooling_unregister(struct cpufreq_policy *policy) > >>>>> { > >>>>> struct cpufreq_cooling_device *cpufreq_cdev; > >>>>> bool last; > >>>>> > >>>>> - if (!cdev) > >>>>> - return; > >> > >> we used to return without any errors from here. Now we will have > >> problems if regsitering fails for some reason. > > > > Specifically, the last cpufreq_cdev in the list will be unregistered > > AFAICS, and without removing it from the list for that matter, which > > isn't what the caller wants. > > Indeed, > > What about the resulting code above: > > void __cpufreq_cooling_unregister(struct cpufreq_cooling_device > *cpufreq_cdev, int last) > { > /* Unregister the notifier for the last cpufreq cooling device */ > if (last) > cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block, > CPUFREQ_POLICY_NOTIFIER); > Doesn't the notifier need to be unregistered under cooling_list_lock ? > thermal_cooling_device_unregister(cpufreq_cdev->cdev); > ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id); > kfree(cpufreq_cdev->idle_time); > kfree(cpufreq_cdev); > } > > /** > > * cpufreq_cooling_unregister - function to remove cpufreq cooling > device. > * @cdev: thermal cooling device pointer. > > * > > * This interface function unregisters the "thermal-cpufreq-%x" cooling > device. > */ > void cpufreq_cooling_unregister(struct cpufreq_policy *policy) > { > struct cpufreq_cooling_device *cpufreq_cdev; > bool last; > > mutex_lock(&cooling_list_lock); > list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) { > if (cpufreq_cdev->policy == policy) { > list_del(&cpufreq_cdev->node); > last = list_empty(&cpufreq_cdev_list); > break; > } > } > mutex_unlock(&cooling_list_lock); > > if (cpufreq_cdev->policy == policy) > __cpufreq_cooling_unregister(cpufreq_cdev, last); > } > EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister); > > > > > -- > Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: Facebook | > Twitter | > Blog >