From patchwork Mon Sep 2 22:48:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulf Hansson X-Patchwork-Id: 13787777 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B79ABCA0ED3 for ; Mon, 2 Sep 2024 22:51:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=ILko9GrGSTEsXrXVZflQuhfBdNy2+GsUHO1RFuWf+V0=; b=LgNiSXQA/YDre5wIILXhYkGlKx cuTtENyCjvqpkHVRhYhWCQd74qBiN+AKi1CfxOYqcf1ImbxvNMfs3vSkVxRc3sXGZxtzEpTVtF7F5 aM/EbLvR66tw3aRHZUKdV3I5EXxdgKj2uMZ2aSqgdTqXLN9rqkawZ/7Q6lqCshRVW8s/MCJIG7J4e Kvf6aN/UwWK+6DjpUYGLWmi7SgZQqry0SnvyK37ygQkWB1F9NeVCOfPzh3wdmZRj+Kuu4OppQndGZ 403Bqdd+ygm47vXMnu+GM8EUVYBHOxzBf1JPJakeKj720TLe7NuIJAdR7FvOz4RHF3yxoHdx5cYU9 DqqkNs0Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1slFt5-0000000FhxL-0U9A; Mon, 02 Sep 2024 22:51:07 +0000 Received: from mail-lf1-x12c.google.com ([2a00:1450:4864:20::12c]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1slFqV-0000000FhWW-1Vfo for linux-arm-kernel@lists.infradead.org; Mon, 02 Sep 2024 22:48:28 +0000 Received: by mail-lf1-x12c.google.com with SMTP id 2adb3069b0e04-5344ab30508so5279729e87.0 for ; Mon, 02 Sep 2024 15:48:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1725317305; x=1725922105; darn=lists.infradead.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ILko9GrGSTEsXrXVZflQuhfBdNy2+GsUHO1RFuWf+V0=; b=WiUnVXHxOsrkcCgMbF/yMV1bT6ltvom5/lxoJJrM1o55l6yBvNboeMLQV65mnVi3cJ r4KSl1iFxa7I3ynLlO7hh97urUOizE8xGNRzk8QAd3c1TiK9cS+zJyIxJ/kRfH8JQd+m tlpABxeZzqWTegw1/26iv3/88iMzlKi/vgSJM4GLWZFFMkHzUNv9kQUzWdJrnSxR/OtJ 9UsIxD1C/9BGFL1TZ2SprmomNBpC/DayX783BDaH3S8fi6yiVQU9G4oJJvNVQA/NvB3y HKpKi8NRblCpDxUOAfxEHZbjQyW9yfeJpsTthwPW01sJcig1oMyfLdyNA/hIw56FdTKh Bi1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725317305; x=1725922105; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ILko9GrGSTEsXrXVZflQuhfBdNy2+GsUHO1RFuWf+V0=; b=Iv+fjjYwJN9lFcRR+oNNA8FqRx3eNT+gnTDG+2c4T9kVnzOfqc5OXQYz5Dy1o/HnFu OHAyCLWQ6Rh+rrXsjVeCXOWPya+ehzGu41OAp1ObO7iH69c0Bg2XDfDTqiOLftRrvLbQ QZiNMmnbRkh5+E0YY3JrCdDCa47wFQkGpGRDYpzIgeQ0990raO+TXxbR+iTpJ+6WlpoJ 5rGQ7j4t6prlGTr2enuD8OWG3f21cKc0p7gOCzwIIzs+qsgyVxDf1EAZOSdbOmhppBI7 /FMeZhKcpyVtEyNpKavpCeC/BIdDsgKjd3UN3sRk+KLK4FCzHA88qMnvPuE70rBL50eZ 5Eeg== X-Forwarded-Encrypted: i=1; AJvYcCVD8kkifuRycvMKBF9zbxsueeOcRxszKktME0taxAEnPnofnXzp6YcxiId5BtnVznW4hqPvbVBSwzoGb7jfTNYU@lists.infradead.org X-Gm-Message-State: AOJu0Yz7r1NBK/1SXIt5ztJ7BDlh82YgkoRi0wzUJneOnYTw4alQ/DbB Jvy7v3iqRG1OuGZU+WvkAXZ1ISQreycINEFbgqECnosGQ+f98r70QH4X1xRfNdA= X-Google-Smtp-Source: AGHT+IFum6vAI4M7Ifv5kv5ZZ8Ww55Gm2DB955sOcPDG5+CQnwdg/OMQzsSkUUI+7Z9IA063dZuH2g== X-Received: by 2002:a05:6512:3088:b0:52c:dc06:d4ad with SMTP id 2adb3069b0e04-535462ee5demr3891202e87.6.1725317304291; Mon, 02 Sep 2024 15:48:24 -0700 (PDT) Received: from uffe-tuxpro14.. (h-178-174-189-39.A498.priv.bahnhof.se. [178.174.189.39]) by smtp.gmail.com with ESMTPSA id 2adb3069b0e04-5354079bbf2sm1790646e87.20.2024.09.02.15.48.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 02 Sep 2024 15:48:23 -0700 (PDT) From: Ulf Hansson To: Viresh Kumar , Nishanth Menon , Stephen Boyd , Dikshita Agarwal Cc: Bjorn Andersson , Konrad Dybcio , Nikunj Kela , Bryan O'Donoghue , Thierry Reding , Mikko Perttunen , Jonathan Hunter , Stephan Gerhold , Ilia Lin , Stanimir Varbanov , Vikash Garodia , Ulf Hansson , linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH 2/2] OPP/pmdomain: Fix the assignment of the required-devs Date: Tue, 3 Sep 2024 00:48:15 +0200 Message-Id: <20240902224815.78220-3-ulf.hansson@linaro.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240902224815.78220-1-ulf.hansson@linaro.org> References: <20240902224815.78220-1-ulf.hansson@linaro.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240902_154827_425000_9E4101D0 X-CRM114-Status: GOOD ( 28.10 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org The recent attempt to make genpd first lookup an OPP table for a device that has been attached to it and then let the OPP core validate whether the OPP table is correct, fails for some configurations. More precisely, if a device has multiple power-domains, which all has an OPP table associated, doesn't necessarily mean that the device's OPP table needs multiple phandles specified in the required-opps. Instead it's perfectly possible to use a single phandle in the required-opps, which is typically where the current code fails to assign the correct required-dev. To fix this problem, let's instead start by letting the OPP core find the device node for the required OPP table and then let genpd search for a corresponding OPP table, allowing us the find the correct required-dev to assign for it. Reported-by: Dikshita Agarwal Fixes: f0d2dcc9b087 ("OPP/pmdomain: Set the required_dev for a required OPP during genpd attach") Signed-off-by: Ulf Hansson --- drivers/opp/core.c | 15 +++++++----- drivers/pmdomain/core.c | 52 +++++++++++++++++++++++------------------ include/linux/pm_opp.h | 7 ++++-- 3 files changed, 43 insertions(+), 31 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 66cac7a1d9db..538612886446 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -2363,7 +2363,7 @@ static void _opp_put_config_regulators_helper(struct opp_table *opp_table) static int _opp_set_required_dev(struct opp_table *opp_table, struct device *dev, struct device *required_dev, - struct opp_table *required_opp_table) + config_required_dev_t config_required_dev) { int i; @@ -2380,6 +2380,7 @@ static int _opp_set_required_dev(struct opp_table *opp_table, for (i = 0; i < opp_table->required_opp_count; i++) { struct opp_table *table = opp_table->required_opp_tables[i]; + struct opp_table *required_opp_table; /* * The OPP table should be available at this point. If not, it's @@ -2396,7 +2397,9 @@ static int _opp_set_required_dev(struct opp_table *opp_table, * We need to compare the nodes for the OPP tables, rather than * the OPP tables themselves, as we may have separate instances. */ - if (required_opp_table->np == table->np) { + required_opp_table = config_required_dev(required_dev, + table->np); + if (required_opp_table) { /* * The required_opp_tables parsing is not perfect, as * the OPP core does the parsing solely based on the DT @@ -2422,8 +2425,8 @@ static int _opp_set_required_dev(struct opp_table *opp_table, } } - dev_err(dev, "Missing OPP table, unable to set the required dev\n"); - return -ENODEV; + /* No matching OPP table found for the required_dev. */ + return -ERANGE; } static void _opp_put_required_dev(struct opp_table *opp_table, @@ -2547,10 +2550,10 @@ int dev_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config) data->flags |= OPP_CONFIG_REGULATOR; } - if (config->required_dev && config->required_opp_table) { + if (config->required_dev && config->config_required_dev) { ret = _opp_set_required_dev(opp_table, dev, config->required_dev, - config->required_opp_table); + config->config_required_dev); if (ret < 0) goto err; diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c index edef1a520110..0ff0b513b2a1 100644 --- a/drivers/pmdomain/core.c +++ b/drivers/pmdomain/core.c @@ -2874,20 +2874,21 @@ static void genpd_dev_pm_sync(struct device *dev) genpd_queue_power_off_work(pd); } -static struct opp_table *genpd_find_opp_table(struct generic_pm_domain *genpd, - unsigned int depth) +static struct opp_table *_genpd_find_opp_table(struct generic_pm_domain *genpd, + struct device_node *np, + unsigned int depth) { - struct opp_table *opp_table; + struct opp_table *opp_table = genpd->opp_table; struct gpd_link *link; - if (genpd->opp_table) - return genpd->opp_table; + if (opp_table && (dev_pm_opp_table_to_of_node(opp_table) == np)) + return opp_table; list_for_each_entry(link, &genpd->child_links, child_node) { struct generic_pm_domain *parent = link->parent; genpd_lock_nested(parent, depth + 1); - opp_table = genpd_find_opp_table(parent, depth + 1); + opp_table = _genpd_find_opp_table(parent, np, depth + 1); genpd_unlock(parent); if (opp_table) @@ -2897,12 +2898,27 @@ static struct opp_table *genpd_find_opp_table(struct generic_pm_domain *genpd, return NULL; } -static int genpd_set_required_opp_dev(struct device *dev, - struct device *base_dev) +static struct opp_table *genpd_find_opp_table(struct device *dev, + struct device_node *np) { struct generic_pm_domain *genpd = dev_to_genpd(dev); struct opp_table *opp_table; - int ret = 0; + + genpd_lock(genpd); + opp_table = _genpd_find_opp_table(genpd, np, 0); + genpd_unlock(genpd); + + return opp_table; +} + +static int genpd_set_required_opp_dev(struct device *dev, + struct device *base_dev) +{ + struct dev_pm_opp_config config = { + .required_dev = dev, + .config_required_dev = genpd_find_opp_table, + }; + int ret; /* Limit support to non-providers for now. */ if (of_property_present(base_dev->of_node, "#power-domain-cells")) @@ -2911,20 +2927,10 @@ static int genpd_set_required_opp_dev(struct device *dev, if (!dev_pm_opp_of_has_required_opp(base_dev)) return 0; - genpd_lock(genpd); - opp_table = genpd_find_opp_table(genpd, 0); - genpd_unlock(genpd); - - if (opp_table) { - struct dev_pm_opp_config config = { - .required_dev = dev, - .required_opp_table = opp_table, - }; - - ret = devm_pm_opp_set_config(base_dev, &config); - if (ret < 0) - dev_err(dev, "failed to set opp config %d\n", ret); - } + ret = devm_pm_opp_set_config(base_dev, &config); + ret = ret == -ERANGE ? 0 : ret; + if (ret < 0) + dev_err(dev, "failed to set opp config %d\n", ret); return ret; } diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 7894e631cded..0ada4a5057c8 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -53,6 +53,9 @@ typedef int (*config_regulators_t)(struct device *dev, typedef int (*config_clks_t)(struct device *dev, struct opp_table *opp_table, struct dev_pm_opp *opp, void *data, bool scaling_down); +typedef struct opp_table *(*config_required_dev_t)(struct device *dev, + struct device_node *opp_table_np); + /** * struct dev_pm_opp_config - Device OPP configuration values * @clk_names: Clk names, NULL terminated array. @@ -63,7 +66,7 @@ typedef int (*config_clks_t)(struct device *dev, struct opp_table *opp_table, * @supported_hw_count: Number of elements in the array. * @regulator_names: Array of pointers to the names of the regulator, NULL terminated. * @required_dev: Required OPP device. - * @required_opp_table: The corresponding required OPP table for @required_dev. + * @config_required_dev: Custom helper to find the required OPP table for $required_dev. * * This structure contains platform specific OPP configurations for the device. */ @@ -77,7 +80,7 @@ struct dev_pm_opp_config { unsigned int supported_hw_count; const char * const *regulator_names; struct device *required_dev; - struct opp_table *required_opp_table; + config_required_dev_t config_required_dev; }; #define OPP_LEVEL_UNSET U32_MAX