Received: by 2002:a25:d7c1:0:0:0:0:0 with SMTP id o184csp1196017ybg; Fri, 18 Oct 2019 13:36:34 -0700 (PDT) X-Google-Smtp-Source: APXvYqyE6eu7gXGJ2SGagmZLBCOBQqMO74HUrRoioxzQF0oByrjqztsg3onBVa8Zebnq3KEHnSaI X-Received: by 2002:a17:906:448e:: with SMTP id y14mr10879160ejo.136.1571430994203; Fri, 18 Oct 2019 13:36:34 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1571430994; cv=none; d=google.com; s=arc-20160816; b=hUtnKpegW8XPE8Mr0/cZg2fTbtHX/SeDQL/pAKw/OR8WVH53EK2kKnZDn8rwQAuzJl jjeSsFTnREeWowi4TIRsnN4dzPy0/3GAgWcG4/SNa9i9mocE6kQgc25kkRm1sQAmx0hU vMnHQgQsVM+C0UjVt3v5d0FM4tL4xjmuAjGkwk7rbbLHiJnKJDigL+t4bbExRGxCOFmZ km+ERQHm24BqbXAPXZ6qZ2SoxpksmvEiqaxeXYXvEEEUO8UH8ltxfWpdgFFsz8vmcjhl qtxupPk/TDZGeEwL+B/ZMpSrWl7hudcAn1t0/dD/tvlArMouy0g/zubJ9AATliE3FhNi DeBg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:in-reply-to :mime-version:user-agent:date:message-id:from:cc:references:to :subject:dkim-signature; bh=EInTdxDjDLXDstDiLqEeeyfhKiB3PsTEBGK0hKU+4QE=; b=X9WXCH+aBWx8JRJWKdwQklMEMQIWLVGty0eadceXeEnVHserA5TM2+UoO0OyXJSAuT TucD5Iq2Jk/TYWPkYd0pM5k9B7jWuMwgfDrtneyRWH8mhbS4DwjXrsGKPiDVs0yosDVy ff/tcvNMXa/7LkgzAfIhn/DItUJ/i5tJQsI0XAOvS2RO7dKxynlgRqZJs29mOp/HaKyY 7z4XZMYpK9+eMWQWC/ZSruMs1aQlGepfB+5cD9KX7aJqhPKZE1DXgV4S57evUjtZTwi8 ecb6rlexJyarJTPvXLJYC0G9NlqTMUcZMiqZKs/SMcUZGbAWh4Z09/qfFR0StmWv9NYg nxJA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=wcVh7+Fm; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id n8si4222651ejh.169.2019.10.18.13.36.11; Fri, 18 Oct 2019 13:36:34 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=wcVh7+Fm; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390093AbfJQPqE (ORCPT + 99 others); Thu, 17 Oct 2019 11:46:04 -0400 Received: from mail-qk1-f195.google.com ([209.85.222.195]:35819 "EHLO mail-qk1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730879AbfJQPqE (ORCPT ); Thu, 17 Oct 2019 11:46:04 -0400 Received: by mail-qk1-f195.google.com with SMTP id w2so2321856qkf.2 for ; Thu, 17 Oct 2019 08:46:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=EInTdxDjDLXDstDiLqEeeyfhKiB3PsTEBGK0hKU+4QE=; b=wcVh7+FmV6UtER8BX9mpGqMwW+bqm65EPR2eJoBAs1jnfHWhQLDWZ4G1vTJIZzH2fl 2hlOplz2HcgHYUJ2f3gQ1zfzcCpHk1V5W7mtExYsVRW0bP9l0a0SPET/eGjkp4bpOuhs q9sYFFXTLNJHQbO4lhkdccjbVG88CGWl2Ag2wtKlm9//XT4savE30zoQuS2XAjbS71fC BE9VosMPbMWj6Vt0IPfTHjuSUdeLHrg+ss2T8PWZCOAZLGJZSDfwU3hUoz3X0p6CVH49 XStCqwW5j6yPp8l+IvM/gyblpTMvdVsHVS061meUaBMxIlhipFooO32mB55u7/bH65sj n4eQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=EInTdxDjDLXDstDiLqEeeyfhKiB3PsTEBGK0hKU+4QE=; b=gsyMWNaQ9p23QV4LrtKKeIC7Y3CEgZLatTGTTZsqZ/T7rvpKRZoFgyNS0hxlAz50mz SdCZx+k6NPj3KetLOG6LtuoVp/c0LXP5/9b+6yF5OiPdx9HEptPT6mEmGl/JsRhBsMKF cUkITAUlwopiyBu/QI4tJB/KvsbI/sXhjZesOhA7tA2BO6WmKQwinUpZZacr+JHZORbx e2ANySHijggO84+P0tns4uhzlze0vZX6TOVVKv6yDVzbCmjW9k6CtLQWlQ7LFtX88mcL F6tURhCuPYH6Bxml/krPAmPRWDPooEXvKvhVqDFs6NEKOVJwXVQ3vVHVXL+t5Ui/NGwn bSug== X-Gm-Message-State: APjAAAXWk8lD6mstzratrHEW8Ugp4gZBRi2YrYn/gd7LNhioKrAL21/4 WhqmLk6nHTadjbYTpSkqSxHVCbKS5csmeg== X-Received: by 2002:ae9:e513:: with SMTP id w19mr3920393qkf.308.1571327162320; Thu, 17 Oct 2019 08:46:02 -0700 (PDT) Received: from [192.168.1.169] (pool-71-255-246-27.washdc.fios.verizon.net. [71.255.246.27]) by smtp.gmail.com with ESMTPSA id c126sm1360612qkd.13.2019.10.17.08.46.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 17 Oct 2019 08:46:01 -0700 (PDT) Subject: Re: [PATCH v3 4/7] thermal: Add generic power domain warming device driver. To: Ulf Hansson References: <1571254641-13626-1-git-send-email-thara.gopinath@linaro.org> <1571254641-13626-5-git-send-email-thara.gopinath@linaro.org> Cc: Eduardo Valentin , Zhang Rui , Daniel Lezcano , Bjorn Andersson , Andy Gross , amit.kucheria@verdurent.com, Mark Rutland , "Rafael J. Wysocki" , Linux PM , DTML , linux-arm-msm , Linux Kernel Mailing List From: Thara Gopinath Message-ID: <5DA88CB8.6050206@linaro.org> Date: Thu, 17 Oct 2019 11:46:00 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/17/2019 04:47 AM, Ulf Hansson wrote: > On Wed, 16 Oct 2019 at 21:37, Thara Gopinath wrote: >> >> Resources modeled as power domains in linux kenrel >> can be used to warm the SoC(eg. mx power domain on sdm845). >> To support this feature, introduce a generic power domain >> warming device driver that can be plugged into the thermal framework >> (The thermal framework itself requires further modifiction to >> support a warming device in place of a cooling device. >> Those extensions are not introduced in this patch series). >> >> Signed-off-by: Thara Gopinath >> --- >> drivers/thermal/Kconfig | 10 +++ >> drivers/thermal/Makefile | 2 + >> drivers/thermal/pwr_domain_warming.c | 136 +++++++++++++++++++++++++++++++++++ >> include/linux/pwr_domain_warming.h | 31 ++++++++ >> 4 files changed, 179 insertions(+) >> create mode 100644 drivers/thermal/pwr_domain_warming.c >> create mode 100644 include/linux/pwr_domain_warming.h >> >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig >> index 001a21a..0c5c93e 100644 >> --- a/drivers/thermal/Kconfig >> +++ b/drivers/thermal/Kconfig >> @@ -187,6 +187,16 @@ config DEVFREQ_THERMAL >> >> If you want this support, you should say Y here. >> >> +config PWR_DOMAIN_WARMING_THERMAL >> + bool "Power Domain based warming device" >> + depends on PM_GENERIC_DOMAINS_OF >> + help >> + This implements the generic power domain based warming >> + mechanism through increasing the performance state of >> + a power domain. >> + >> + If you want this support, you should say Y here. >> + >> config THERMAL_EMULATION >> bool "Thermal emulation mode support" >> help >> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile >> index 74a37c7..382c64a 100644 >> --- a/drivers/thermal/Makefile >> +++ b/drivers/thermal/Makefile >> @@ -27,6 +27,8 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o >> # devfreq cooling >> thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o >> >> +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL) += pwr_domain_warming.o >> + >> # platform thermal drivers >> obj-y += broadcom/ >> obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o >> diff --git a/drivers/thermal/pwr_domain_warming.c b/drivers/thermal/pwr_domain_warming.c >> new file mode 100644 >> index 0000000..60fae3e >> --- /dev/null >> +++ b/drivers/thermal/pwr_domain_warming.c >> @@ -0,0 +1,136 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2019, Linaro Ltd >> + */ >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +struct pd_warming_device { >> + struct thermal_cooling_device *cdev; >> + struct generic_pm_domain *gpd; > > No, this isn't a genpd provider and thus we should not need to carry > the above pointer in the struct pd_warming_device. I store this to attach the device in late_init. More about this approach below. > >> + struct device *dev; >> + int max_state; >> + int cur_state; >> + bool runtime_resumed; >> +}; >> + >> +static int pd_wdev_get_max_state(struct thermal_cooling_device *cdev, >> + unsigned long *state) >> +{ >> + struct pd_warming_device *pd_wdev = cdev->devdata; >> + >> + *state = pd_wdev->max_state; >> + return 0; >> +} >> + >> +static int pd_wdev_get_cur_state(struct thermal_cooling_device *cdev, >> + unsigned long *state) >> +{ >> + struct pd_warming_device *pd_wdev = cdev->devdata; >> + >> + *state = dev_pm_genpd_get_performance_state(pd_wdev->dev); >> + >> + return 0; >> +} >> + >> +static int pd_wdev_set_cur_state(struct thermal_cooling_device *cdev, >> + unsigned long state) >> +{ >> + struct pd_warming_device *pd_wdev = cdev->devdata; >> + struct device *dev = pd_wdev->dev; >> + int ret; >> + >> + ret = dev_pm_genpd_set_performance_state(dev, state); >> + >> + if (ret) >> + return ret; >> + >> + if (state && !pd_wdev->runtime_resumed) { >> + ret = pm_runtime_get_sync(dev); >> + pd_wdev->runtime_resumed = true; >> + } else if (!state && pd_wdev->runtime_resumed) { >> + ret = pm_runtime_put(dev); >> + pd_wdev->runtime_resumed = false; >> + } >> + >> + return ret; >> +} >> + >> +static int pd_wdev_late_init(struct thermal_cooling_device *cdev) >> +{ >> + struct pd_warming_device *pd_wdev = cdev->devdata; >> + struct device *dev = &cdev->device; >> + int state_count, ret; >> + >> + ret = pm_genpd_add_device(pd_wdev->gpd, dev); > > The pm_genpd_add_device() is a legacy interface and we are striving to > remove it. I think there are two better options for you to use to deal > with the attach thingy. I was not aware of this. Apologies. > > 1. The easiest one is probably just to convert into using > of_genpd_add_device() instead. I think you already have the > information that you need in the ->cdev pointer to do that. However, > that also means you need to add the ->late_init() callback to the > struct thermal_cooling_device_ops, like what you do here. > > 2. Maybe the most correct solution is, rather than attaching > &cdev->device to the PM domain, to register a separate new device > (device_register()) and assign it the corresponding OF node as the > genpd provider's subnode and then attach this one instead. If > "power-domains" can be specified in the subnode, you can even use > dev_pm_domain_attach() to attach the device to the PM domain, else > of_genpd_add_device() should work. In the second step, when > registering the cooling device, the new device above should be > assigned as parent to the device that is about to be registered via > thermal_of_cooling_device_register(). In other words, the > thermal_of_cooling_device_register() needs to be extended to cope with > receiving a parent device as an in-parameter, but that should be > doable I think. In this way, you don't need to add the ->late_init() > callback at all, but you can instead just use the parent device when > operating on the PM domain. I did toy with registering a separate device vs reusing cdev device. My rational was, the power domain is actually controlled/needed by the cdev and hence should be attached to it. For me either solution is acceptable . It is a trade off between creating a new device and registering it as a parent of cooling device vs introducing a late init. With the second approach I should be able to do away with the generic_pm_domain pointer in pd_warming_device. To register a parent for a cooling device, I will have to introduce a new API in the thermal framework. Like thermal_of_cooling_device_parent_register. I am ok with this as well. I would like to hear on what some of the thermal maintainers/reviewers have to say about both the approaches and which is better. I will wait a few days for others to review and if there are no major comments, I will send across the series after updating it to the second approach. -- Warm Regards Thara