From patchwork Thu Aug 22 22:45:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulf Hansson X-Patchwork-Id: 13774318 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 9E63AC3DA4A for ; Thu, 22 Aug 2024 22:48:38 +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=lvx3dtDyuaLtWvQBN1ccLzAdA+uGExJd6lwl1p/+zuE=; b=V5z+7LVxnyAxtHyfcHCRI5z4vr LGScajL64bZtRhL4FeyzvdLHUTrnftMrwvk0c8Ro8vg35w4uAgJe37k7Rt2SIARjlxXdv8anRbAci edyXUY6sDKTUzKJ2UguWdVB4dsvROMr/OM93F8g+KfYi+xh8CVAxLEQW+Qza2O8wp7PE+lHfnV2QK c4lQZT/EpwrrHNP0J9QY4TRTNfpDQvnr6/YucsQx0e6/dsKmDGg5W5HA2lR7m916ZiL5h7oz1J3Sh Mv5tqPhJbqUOqh4Tw94bVTUR784WYvAWry1rADCVV0n22NmwaRsNkz1DyP/gcezdZ4oTcTturNn9c yWXGDchA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1shGbS-0000000EWDt-2lLq; Thu, 22 Aug 2024 22:48:26 +0000 Received: from mail-lf1-x12d.google.com ([2a00:1450:4864:20::12d]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1shGZF-0000000EVdA-29G7 for linux-arm-kernel@lists.infradead.org; Thu, 22 Aug 2024 22:46:12 +0000 Received: by mail-lf1-x12d.google.com with SMTP id 2adb3069b0e04-5334c018913so1279963e87.0 for ; Thu, 22 Aug 2024 15:46:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1724366767; x=1724971567; 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=lvx3dtDyuaLtWvQBN1ccLzAdA+uGExJd6lwl1p/+zuE=; b=A8NUF5BNr0a4aF/9raNypw6pPaZ1qjLgTq2VpErcNyOYMl5SBebU7JFPcgZJ9/AQF0 m+mK+gfKAu/e6C6MhKL8EZ8O5mmWJoOeL4WFvKwBVyfgFu582R1qJnRTit5oyqX8goGD DRpsryXjIN+XKthzHGJ3NcAbO7Qgbo1y2FEsiGoSosiKFCUiGjlDzEzh8Hga28fAb0ZG 2qPM8ETY+giigzk1xdvhtxPuVLDxjeIUkqfJf84wjCkT3n4fUVq5uqlfb0F7KskDNomz SCgVJd4Xhnb6QO5uCI3be6T9TA+k7xCKtzLjjUwW9WvBXiqtVAbrvsOM8Q9KWDhmXvOq +P6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724366767; x=1724971567; 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=lvx3dtDyuaLtWvQBN1ccLzAdA+uGExJd6lwl1p/+zuE=; b=ttYllWU1qyvTXecj4gljy7e7xSiA8GDZ4ZYH6VKD3iZsMeyhnJVRAzfvujEsJDXG8i MgEA9STXn+ljb/bfXfSvembmHBJb7Aj3qFWGxfLuRpooBe5xtdQGWY4WX5CHcHtXY/IS nI7t+6bMMK/dAvdNkfN8F/5q4nyHkouIZBupbf/KRp2aDYUTEKR3+3pIwJUZ+DQYOO1i w0cF6lRSN0mnFL0r6HcRkYBprbL40q4YuudCrVvooyJulIO/8ACmCQPGVVnvnMjKKMp/ /4/2aIosgmmYq/Ua3+GVE6TREFuNmh8gb4/VtnQ7yQP1mFM0BXB6wtT0uRrnyRCKnb68 RwrA== X-Forwarded-Encrypted: i=1; AJvYcCVwge6r4bV7ucUEuZHoZtuBTWLVRHPZzH5x5Y2JOuF9/32rSkGH1k4r6FhyQ02hU2oFuHP8NSVrNHHScEIUOx2d@lists.infradead.org X-Gm-Message-State: AOJu0YwlakL5Kfc7Q9e+jDTABZi4iVaGnPuwt/0LbuGjki2d9TOCKLZK UNG3Dv9/JSqZN8cGe8Czku+CM2VgIvGZSgLm6TXXn+kDNLt9IUloXNul0ZjK7no= X-Google-Smtp-Source: AGHT+IGxvA4Uq9Y8lvyO5rIqpEo5VlzIQ99oKaiGIrLuFmsy7/2BQLTwlcgbwEWVVwymuZXSfzR59g== X-Received: by 2002:ac2:4e09:0:b0:52e:941d:7039 with SMTP id 2adb3069b0e04-5343886c9bdmr183956e87.59.1724366767061; Thu, 22 Aug 2024 15:46:07 -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-5334ea362a4sm379443e87.66.2024.08.22.15.46.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Aug 2024 15:46:06 -0700 (PDT) From: Ulf Hansson To: Viresh Kumar , Nishanth Menon , Stephen Boyd 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 v3 02/10] OPP: Rework _set_required_devs() to manage a single device per call Date: Fri, 23 Aug 2024 00:45:39 +0200 Message-Id: <20240822224547.385095-3-ulf.hansson@linaro.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240822224547.385095-1-ulf.hansson@linaro.org> References: <20240822224547.385095-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-20240822_154609_577735_2B8CA2A7 X-CRM114-Status: GOOD ( 32.11 ) 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 At this point there are no consumer drivers that makes use of _set_required_devs(), hence it should be straightforward to rework the code to enable it to better integrate with the genpd attach procedure. During attach, one device at the time is being hooked up to its corresponding PM domain. Therefore, let's update the _set_required_devs() to align to this behaviour, allowing callers to fill out one required_dev per call. As shown from a subsequent change, genpd becomes the first user of the reworked _set_required_dev(). Signed-off-by: Ulf Hansson --- Changes in v3: - Clarified the commitmsg. Changes in v2: - Addressed some comments from Viresh. - Drop calls to _add_opp_dev() for required_devs. --- drivers/opp/core.c | 91 +++++++++++++++++++++++++++++------------- drivers/opp/opp.h | 4 +- include/linux/pm_opp.h | 10 +++-- 3 files changed, 73 insertions(+), 32 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 494f8860220d..b6a699286aaa 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -2473,9 +2473,10 @@ static int _opp_attach_genpd(struct opp_table *opp_table, struct device *dev, } -static int _opp_set_required_devs(struct opp_table *opp_table, - struct device *dev, - struct device **required_devs) +static int _opp_set_required_dev(struct opp_table *opp_table, + struct device *dev, + struct device *required_dev, + struct opp_table *required_opp_table) { int i; @@ -2484,36 +2485,70 @@ static int _opp_set_required_devs(struct opp_table *opp_table, return -EINVAL; } - /* Another device that shares the OPP table has set the required devs ? */ - if (opp_table->required_devs[0]) - return 0; + /* Genpd core takes care of propagation to parent genpd */ + if (opp_table->is_genpd) { + dev_err(dev, "%s: Operation not supported for genpds\n", __func__); + return -EOPNOTSUPP; + } for (i = 0; i < opp_table->required_opp_count; i++) { - /* Genpd core takes care of propagation to parent genpd */ - if (required_devs[i] && opp_table->is_genpd && - opp_table->required_opp_tables[i]->is_genpd) { - dev_err(dev, "%s: Operation not supported for genpds\n", __func__); - return -EOPNOTSUPP; - } + struct opp_table *table = opp_table->required_opp_tables[i]; + + /* + * The OPP table should be available at this point. If not, it's + * not the one we are looking for. + */ + if (IS_ERR(table)) + continue; - opp_table->required_devs[i] = required_devs[i]; + /* Move to the next available index. */ + if (opp_table->required_devs[i]) + continue; + + /* + * 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) { + /* + * The required_opp_tables parsing is not perfect, as + * the OPP core does the parsing solely based on the DT + * node pointers. The core sets the required_opp_tables + * entry to the first OPP table in the "opp_tables" + * list, that matches with the node pointer. + * + * If the target DT OPP table is used by multiple + * devices and they all create separate instances of + * 'struct opp_table' from it, then it is possible that + * the required_opp_tables entry may be set to the + * incorrect sibling device. Cross check it again and + * fix if required. + */ + if (required_opp_table != table) { + dev_pm_opp_put_opp_table(table); + _get_opp_table_kref(required_opp_table); + opp_table->required_opp_tables[i] = required_opp_table; + } + + opp_table->required_devs[i] = required_dev; + return i; + } } - return 0; + dev_err(dev, "Missing OPP table, unable to set the required dev\n"); + return -ENODEV; } -static void _opp_put_required_devs(struct opp_table *opp_table) +static void _opp_put_required_dev(struct opp_table *opp_table, + unsigned int index) { - int i; - - for (i = 0; i < opp_table->required_opp_count; i++) - opp_table->required_devs[i] = NULL; + opp_table->required_devs[index] = NULL; } static void _opp_clear_config(struct opp_config_data *data) { - if (data->flags & OPP_CONFIG_REQUIRED_DEVS) - _opp_put_required_devs(data->opp_table); + if (data->flags & OPP_CONFIG_REQUIRED_DEV) + _opp_put_required_dev(data->opp_table, data->index); else if (data->flags & OPP_CONFIG_GENPD) _opp_detach_genpd(data->opp_table); @@ -2630,7 +2665,7 @@ int dev_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config) /* Attach genpds */ if (config->genpd_names) { - if (config->required_devs) + if (config->required_dev) goto err; ret = _opp_attach_genpd(opp_table, dev, config->genpd_names, @@ -2639,13 +2674,15 @@ int dev_pm_opp_set_config(struct device *dev, struct dev_pm_opp_config *config) goto err; data->flags |= OPP_CONFIG_GENPD; - } else if (config->required_devs) { - ret = _opp_set_required_devs(opp_table, dev, - config->required_devs); - if (ret) + } else if (config->required_dev && config->required_opp_table) { + ret = _opp_set_required_dev(opp_table, dev, + config->required_dev, + config->required_opp_table); + if (ret < 0) goto err; - data->flags |= OPP_CONFIG_REQUIRED_DEVS; + data->index = ret; + data->flags |= OPP_CONFIG_REQUIRED_DEV; } ret = xa_alloc(&opp_configs, &id, data, XA_LIMIT(1, INT_MAX), diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index cff1fabd1ae3..5b5a4bd89c9e 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -35,12 +35,13 @@ extern struct list_head opp_tables; #define OPP_CONFIG_PROP_NAME BIT(3) #define OPP_CONFIG_SUPPORTED_HW BIT(4) #define OPP_CONFIG_GENPD BIT(5) -#define OPP_CONFIG_REQUIRED_DEVS BIT(6) +#define OPP_CONFIG_REQUIRED_DEV BIT(6) /** * struct opp_config_data - data for set config operations * @opp_table: OPP table * @flags: OPP config flags + * @index: The position in the array of required_devs * * This structure stores the OPP config information for each OPP table * configuration by the callers. @@ -48,6 +49,7 @@ extern struct list_head opp_tables; struct opp_config_data { struct opp_table *opp_table; unsigned int flags; + unsigned int index; }; /** diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 6424692c30b7..5fade5c4de40 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -63,10 +63,11 @@ 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. * @genpd_names: Null terminated array of pointers containing names of genpd to - * attach. Mutually exclusive with required_devs. + * attach. Mutually exclusive with required_dev. * @virt_devs: Pointer to return the array of genpd virtual devices. Mutually - * exclusive with required_devs. - * @required_devs: Required OPP devices. Mutually exclusive with genpd_names/virt_devs. + * exclusive with required_dev. + * @required_dev: Required OPP device. Mutually exclusive with genpd_names/virt_devs. + * @required_opp_table: The corresponding required OPP table for @required_dev. * * This structure contains platform specific OPP configurations for the device. */ @@ -81,7 +82,8 @@ struct dev_pm_opp_config { const char * const *regulator_names; const char * const *genpd_names; struct device ***virt_devs; - struct device **required_devs; + struct device *required_dev; + struct opp_table *required_opp_table; }; #define OPP_LEVEL_UNSET U32_MAX