From patchwork Tue Mar 17 06:54:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Saravana Kannan X-Patchwork-Id: 11441871 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E79DB1668 for ; Tue, 17 Mar 2020 06:55:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C6EA720674 for ; Tue, 17 Mar 2020 06:55:03 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="tdBd3JXM" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726019AbgCQGzD (ORCPT ); Tue, 17 Mar 2020 02:55:03 -0400 Received: from mail-qt1-f202.google.com ([209.85.160.202]:44835 "EHLO mail-qt1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726005AbgCQGzC (ORCPT ); Tue, 17 Mar 2020 02:55:02 -0400 Received: by mail-qt1-f202.google.com with SMTP id l29so6804460qtu.11 for ; Mon, 16 Mar 2020 23:55:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=Vz8Rl+tO3s2Fvzy9mMmth3PcVEenCGW3CyqKGLCsJ9M=; b=tdBd3JXMEOE0YT+L7rwOPZHaK8kRVwY4smj7X3+QwEyDXEwn7tlhTF4Ytp0NC86X6q fT+BVMQpi67m9Q7lWDaj5yz1NykcSCPNpGQVIgCGqBeH1Z3YwPU9zQxS2D9ZnXph+o+9 hQBq1hYrgu7RWtKAszJvhlM964Knk9Pwoh/q+j+w4Kvxg0e3dKq1Mkb9+356AB/d/H7h iAD7OzZ0QrCyxEdaBvXRTCqKVt8QlI4DQXlT6N+Mz+wM845Q62Q3guDM61RAa9rg7bzG /LgCZAZbkmbfP7IiHAyTZtQ6S9+jLL1Ghvq0dxxWFL35GKxD7ldotwJLihuEfRF9jzU2 dnsg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=Vz8Rl+tO3s2Fvzy9mMmth3PcVEenCGW3CyqKGLCsJ9M=; b=fMRsTkSJb0RZdu2IC4wesdAp1itNxwdVSSMuSTd2JvMVeeUW0mC35dJnqEWW1SrCJX qvdvLDM0f2DAoeqVWgE1KMTQPrVAEPbWp9dPzM0bAaNK+o4emrg4RrSRbX1g6qqBNOFx iG/Qv91B8iHfs8FOhUz38FkxiASS8q4xN9TdrOHXZe2d5iJb8RpS+Pb3rFd7z2+6KBPF yz0D77YVdWYS5LCeVmBa9riSqlTrJChOfhusf2SlP9AJgXTRECz8ML3RSizMW7LK+kLT za5jbGw1FlfEr7yKZxV4N3aGBjlHi9syezHpZydAwJFTsMl/6OmGekf0w4f8LCk5FW96 jAlw== X-Gm-Message-State: ANhLgQ1t3aVYtrxirUSl8hcZ2MkouHWW0awtx9rA/FzpDBfW9Y3vkOSN UjKceXk4AQWp8oRRXxWInJJVJyOeJAXSyL8= X-Google-Smtp-Source: ADFU+vslXcTXZHxFbul+vKyVDDYbE109vB2qzmfCtTVliZ++Zd5h8tOc4jJH81ED+tXuc5j8cMzPwT/wJ2uvyfk= X-Received: by 2002:ac8:5193:: with SMTP id c19mr3907270qtn.204.1584428101406; Mon, 16 Mar 2020 23:55:01 -0700 (PDT) Date: Mon, 16 Mar 2020 23:54:47 -0700 In-Reply-To: <20200317065452.236670-1-saravanak@google.com> Message-Id: <20200317065452.236670-2-saravanak@google.com> Mime-Version: 1.0 References: <20200317065452.236670-1-saravanak@google.com> X-Mailer: git-send-email 2.25.1.481.gfbce0eb801-goog Subject: [PATCH v1 1/6] driver core: Remove the link if there is no driver with AUTO flag From: Saravana Kannan To: stable@vger.kernel.org, Jonathan Corbet , Greg Kroah-Hartman , "Rafael J. Wysocki" , Len Brown , Pavel Machek , Matthias Brugger Cc: Saravana Kannan , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, kernel-team@android.com, Yong Wu Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: Yong Wu DL_FLAG_AUTOREMOVE_CONSUMER/SUPPLIER means "Remove the link automatically on consumer/supplier driver unbind", that means we should remove whole the device_link when there is no this driver no matter what the ref_count of the link is. CC: Greg Kroah-Hartman Signed-off-by: Yong Wu Signed-off-by: Greg Kroah-Hartman (cherry picked from commit 0fe6f7874d467456da6f6a221dd92499a3ab1780) Signed-off-by: Saravana Kannan --- drivers/base/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 742bc60e9cca..b354fdd7ce75 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -546,7 +546,7 @@ static void __device_links_no_driver(struct device *dev) continue; if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) - kref_put(&link->kref, __device_link_del); + __device_link_del(&link->kref); else if (link->status != DL_STATE_SUPPLIER_UNBIND) WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } @@ -591,7 +591,7 @@ void device_links_driver_cleanup(struct device *dev) */ if (link->status == DL_STATE_SUPPLIER_UNBIND && link->flags & DL_FLAG_AUTOREMOVE_SUPPLIER) - kref_put(&link->kref, __device_link_del); + __device_link_del(&link->kref); WRITE_ONCE(link->status, DL_STATE_DORMANT); } From patchwork Tue Mar 17 06:54:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Saravana Kannan X-Patchwork-Id: 11441887 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 50ECE913 for ; Tue, 17 Mar 2020 06:55:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 27BD320754 for ; Tue, 17 Mar 2020 06:55:32 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="B4PdTjmn" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726605AbgCQGzb (ORCPT ); Tue, 17 Mar 2020 02:55:31 -0400 Received: from mail-pf1-f202.google.com ([209.85.210.202]:53299 "EHLO mail-pf1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726132AbgCQGzG (ORCPT ); Tue, 17 Mar 2020 02:55:06 -0400 Received: by mail-pf1-f202.google.com with SMTP id i26so6479923pfk.20 for ; Mon, 16 Mar 2020 23:55:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=E7+oI0v4YJiTcfxieAo+ZZx+f+6HuOczFo3lOATV4XM=; b=B4PdTjmn0c6lJeIMW8vcsIVdp4xg+iTkfxBW8URMxjsfTod/34RC00rlZ8dY9/NGHj rrKCTZusWClEMd6JNJPoxGp2JjQz3Vdcd8YYYLt20NfXX8ALy1ABiNkndW95RCqpE8hd prQy8VwzHquTUj0dlmzVMKgJZiHOcwoRwnil89BdWszSfGhsLF+e6ccAm7U7VcM+GpvA zmI9qXY0QpJJDmeCYFi7yAUGGnzoy7kTjPoTQHDso+wb2n8wl8eXnlryMojF8X+TorD1 AEuCw5Nq+ijxA2n3vndlx84VOq2VM3qpbtV13CO6bY/Gcj9J8dKvRO5GcexCO5mWqaVS ukZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=E7+oI0v4YJiTcfxieAo+ZZx+f+6HuOczFo3lOATV4XM=; b=Bl5ojoEKIceBYkVi0/u97NM+4veOFnEgFCjugUdwSFKk3cidsIr3HXY4nx1mMyh/VT Yea9AqHrJkO0660W3udURYiK8GCXsZsFrhjMb/5SMYGrcLU1AfDKkMZYHnG1nt35sD5l McU2gHTCWNn+6B7n3dmptEeck0gh//LZGwh8gJUQtRCll1upjbjcWAj9Z2k7UodJqPAF oWc1ZQ49L4RjVbvb6EUMZhrlm977iM3z+zck0lIzc0FGES+o8yA3au0j2WBkP3q9IMAh LAdNWpkgP0Ki/+ArE90ASR4F3qnxkIVorovao4gdbeqU6Hd2RctswTphzSBDegJhjIRQ qmYw== X-Gm-Message-State: ANhLgQ0DEcPmlA5WE0wFY2ShHdzMFGMwvKgbgtw0aFL5BpMsbHlbMz2R wPeNDOTepfh6Fh+JGku9kFJPtD7PsgcR3sg= X-Google-Smtp-Source: ADFU+vvIXvU94f5jKeoxuIFXK8fHLpbZJnHvhyMgI7zQmzpPqHJV9jhKFftzMx25WRb9Z932oqWKrkPXIha6jjs= X-Received: by 2002:a17:90a:d101:: with SMTP id l1mr3844319pju.130.1584428104661; Mon, 16 Mar 2020 23:55:04 -0700 (PDT) Date: Mon, 16 Mar 2020 23:54:48 -0700 In-Reply-To: <20200317065452.236670-1-saravanak@google.com> Message-Id: <20200317065452.236670-3-saravanak@google.com> Mime-Version: 1.0 References: <20200317065452.236670-1-saravanak@google.com> X-Mailer: git-send-email 2.25.1.481.gfbce0eb801-goog Subject: [PATCH v1 2/6] driver core: Fix adding device links to probing suppliers From: Saravana Kannan To: stable@vger.kernel.org, Jonathan Corbet , Greg Kroah-Hartman , "Rafael J. Wysocki" , Len Brown , Pavel Machek , Matthias Brugger Cc: Saravana Kannan , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, kernel-team@android.com, "Rafael J. Wysocki" Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: "Rafael J. Wysocki" Currently, it is not valid to add a device link from a consumer driver ->probe callback to a supplier that is still probing too, but generally this is a valid use case. For example, if the consumer has just acquired a resource that can only be available if the supplier is functional, adding a device link to that supplier right away should be safe (and even desirable arguably), but device_link_add() doesn't handle that case correctly and the initial state of the link created by it is wrong then. To address this problem, change the initial state of device links added between a probing supplier and a probing consumer to DL_STATE_CONSUMER_PROBE and update device_links_driver_bound() to skip such links on the supplier side. With this change, if the supplier probe completes first, device_links_driver_bound() called for it will skip the link state update and when it is called for the consumer, the link state will be updated to "active". In turn, if the consumer probe completes first, device_links_driver_bound() called for it will change the state of the link to "active" and when it is called for the supplier, the link status update will be skipped. However, in principle the supplier or consumer probe may still fail after the link has been added, so modify device_links_no_driver() to change device links in the "active" or "consumer probe" state to "dormant" on the supplier side and update __device_links_no_driver() to change the link state to "available" only if it is "consumer probe" or "active". Then, if the supplier probe fails first, the leftover link to the probing consumer will become "dormant" and device_links_no_driver() called for the consumer (when its probe fails) will clean it up. In turn, if the consumer probe fails first, it will either drop the link, or change its state to "available" and, in the latter case, when device_links_no_driver() is called for the supplier, it will update the link state to "dormant". [If the supplier probe fails, but the consumer probe succeeds, which should not happen as long as the consumer driver is correct, the link still will be around, but it will be "dormant" until the supplier is probed again.] Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman (cherry picked from commit 15cfb094160385cc0b303c4cda483caa102af654) Signed-off-by: Saravana Kannan --- Documentation/driver-api/device_link.rst | 10 +++- drivers/base/core.c | 74 +++++++++++++++++++++--- 2 files changed, 73 insertions(+), 11 deletions(-) diff --git a/Documentation/driver-api/device_link.rst b/Documentation/driver-api/device_link.rst index d6763272e747..5c7178189612 100644 --- a/Documentation/driver-api/device_link.rst +++ b/Documentation/driver-api/device_link.rst @@ -59,11 +59,15 @@ device ``->probe`` callback or a boot-time PCI quirk. Another example for an inconsistent state would be a device link that represents a driver presence dependency, yet is added from the consumer's -``->probe`` callback while the supplier hasn't probed yet: Had the driver -core known about the device link earlier, it wouldn't have probed the +``->probe`` callback while the supplier hasn't started to probe yet: Had the +driver core known about the device link earlier, it wouldn't have probed the consumer in the first place. The onus is thus on the consumer to check presence of the supplier after adding the link, and defer probing on -non-presence. +non-presence. [Note that it is valid to create a link from the consumer's +``->probe`` callback while the supplier is still probing, but the consumer must +know that the supplier is functional already at the link creation time (that is +the case, for instance, if the consumer has just acquired some resources that +would not have been available had the supplier not been functional then).] If a device link is added in the ``->probe`` callback of the supplier or consumer driver, it is typically deleted in its ``->remove`` callback for diff --git a/drivers/base/core.c b/drivers/base/core.c index b354fdd7ce75..ae2f85ca78fb 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -287,17 +287,26 @@ struct device_link *device_link_add(struct device *consumer, link->status = DL_STATE_NONE; } else { switch (supplier->links.status) { - case DL_DEV_DRIVER_BOUND: + case DL_DEV_PROBING: switch (consumer->links.status) { case DL_DEV_PROBING: /* - * Some callers expect the link creation during - * consumer driver probe to resume the supplier - * even without DL_FLAG_RPM_ACTIVE. + * A consumer driver can create a link to a + * supplier that has not completed its probing + * yet as long as it knows that the supplier is + * already functional (for example, it has just + * acquired some resources from the supplier). */ - if (flags & DL_FLAG_PM_RUNTIME) - pm_runtime_resume(supplier); - + link->status = DL_STATE_CONSUMER_PROBE; + break; + default: + link->status = DL_STATE_DORMANT; + break; + } + break; + case DL_DEV_DRIVER_BOUND: + switch (consumer->links.status) { + case DL_DEV_PROBING: link->status = DL_STATE_CONSUMER_PROBE; break; case DL_DEV_DRIVER_BOUND: @@ -317,6 +326,14 @@ struct device_link *device_link_add(struct device *consumer, } } + /* + * Some callers expect the link creation during consumer driver probe to + * resume the supplier even without DL_FLAG_RPM_ACTIVE. + */ + if (link->status == DL_STATE_CONSUMER_PROBE && + flags & DL_FLAG_PM_RUNTIME) + pm_runtime_resume(supplier); + /* * Move the consumer and all of the devices depending on it to the end * of dpm_list and the devices_kset list. @@ -508,6 +525,16 @@ void device_links_driver_bound(struct device *dev) if (link->flags & DL_FLAG_STATELESS) continue; + /* + * Links created during consumer probe may be in the "consumer + * probe" state to start with if the supplier is still probing + * when they are created and they may become "active" if the + * consumer probe returns first. Skip them here. + */ + if (link->status == DL_STATE_CONSUMER_PROBE || + link->status == DL_STATE_ACTIVE) + continue; + WARN_ON(link->status != DL_STATE_DORMANT); WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } @@ -547,17 +574,48 @@ static void __device_links_no_driver(struct device *dev) if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) __device_link_del(&link->kref); - else if (link->status != DL_STATE_SUPPLIER_UNBIND) + else if (link->status == DL_STATE_CONSUMER_PROBE || + link->status == DL_STATE_ACTIVE) WRITE_ONCE(link->status, DL_STATE_AVAILABLE); } dev->links.status = DL_DEV_NO_DRIVER; } +/** + * device_links_no_driver - Update links after failing driver probe. + * @dev: Device whose driver has just failed to probe. + * + * Clean up leftover links to consumers for @dev and invoke + * %__device_links_no_driver() to update links to suppliers for it as + * appropriate. + * + * Links with the DL_FLAG_STATELESS flag set are ignored. + */ void device_links_no_driver(struct device *dev) { + struct device_link *link; + device_links_write_lock(); + + list_for_each_entry(link, &dev->links.consumers, s_node) { + if (link->flags & DL_FLAG_STATELESS) + continue; + + /* + * The probe has failed, so if the status of the link is + * "consumer probe" or "active", it must have been added by + * a probing consumer while this device was still probing. + * Change its state to "dormant", as it represents a valid + * relationship, but it is not functionally meaningful. + */ + if (link->status == DL_STATE_CONSUMER_PROBE || + link->status == DL_STATE_ACTIVE) + WRITE_ONCE(link->status, DL_STATE_DORMANT); + } + __device_links_no_driver(dev); + device_links_write_unlock(); } From patchwork Tue Mar 17 06:54:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Saravana Kannan X-Patchwork-Id: 11441877 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 7F7C81668 for ; Tue, 17 Mar 2020 06:55:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4CD6D2073E for ; Tue, 17 Mar 2020 06:55:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="wPH+ZNue" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726530AbgCQGzN (ORCPT ); Tue, 17 Mar 2020 02:55:13 -0400 Received: from mail-pl1-f202.google.com ([209.85.214.202]:37932 "EHLO mail-pl1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726345AbgCQGzK (ORCPT ); Tue, 17 Mar 2020 02:55:10 -0400 Received: by mail-pl1-f202.google.com with SMTP id f20so11949732plj.5 for ; Mon, 16 Mar 2020 23:55:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=AhDNtOnE0i0fbvNrO5Hy/wYWuzw/VPXEbOKSweJpsf4=; b=wPH+ZNue3MAR6e1XSTYBhe/11JXInDuWWXzaIdDLj6ZrHDYT77PY79OhR+WRTt/2lX lhvOTWaOj+7VclxkB62+XMab5YrZ5ctJUl+/RkVJMkmB007BhW2oOqlG7SgJ6L1dB/oG HjPt3qSwFtfcNr7PhP0LPel+skTIylPhHOMgAWlo+8K74T8IpRw+NDpYJBjvLCXh3LBS eDCRwvQigaB1NLKJAiv//yde0zyS1WNHgD/k7MLRxNb6iRgNENMat7tHRv2wIByRQzfD 7aEFIw2zOu9CPPTe1KQmctOEHAXicUmiktlhfql+0oCGiwJtByZOf2b+c4Bzo5K0siiK uT4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=AhDNtOnE0i0fbvNrO5Hy/wYWuzw/VPXEbOKSweJpsf4=; b=hdvnj7pNBmxw+Y1zQdNuXEg2v6X1q4MdZjRINgUUMd66C7Yup89hWHpZAxZnI9nO9I B0aljrLBu4ce0RmV789JSwfJkuvmGDKUvF8b2mcSbYC86nvO/qyPuo1bHegEgbE8khMI 6KMYwzmbXT8Ofo7NOK0qU7r+Kp4ncMhITPVy/a3nrjYNIhu31xJK+b76RAZ7AGlRbFTu hXM6Fu5IRjvIKmkETSp+Nm3geHIhqz934FDA6+7ZFkPq8uaC4WsoZHWErVYGHuV/uO54 Mn1KWbqxVewaZe/iFMIRY9SBAMRiKJYuhTMYchrnDF1jU7WHzAmN1/oN/6zGPPM4BX2Z nKGw== X-Gm-Message-State: ANhLgQ14npW3UkCYz+8mUP2HH9+nu2Nkjpq1d1DzLumdbZIZK4sqtxmi GU8AFMBXOc/XbqtPMmE+8baxOeRJAVKOFYg= X-Google-Smtp-Source: ADFU+vvl2cl8n4lzMBjDD8zqvg5K6fNDYrXAwCc02UcwgL8/e9RticTUSfTj8N+XhNr2UOwP0gkSKrKkqpSimEA= X-Received: by 2002:a17:90a:2147:: with SMTP id a65mr3660422pje.176.1584428107828; Mon, 16 Mar 2020 23:55:07 -0700 (PDT) Date: Mon, 16 Mar 2020 23:54:49 -0700 In-Reply-To: <20200317065452.236670-1-saravanak@google.com> Message-Id: <20200317065452.236670-4-saravanak@google.com> Mime-Version: 1.0 References: <20200317065452.236670-1-saravanak@google.com> X-Mailer: git-send-email 2.25.1.481.gfbce0eb801-goog Subject: [PATCH v1 3/6] driver core: Make driver core own stateful device links From: Saravana Kannan To: stable@vger.kernel.org, Jonathan Corbet , Greg Kroah-Hartman , "Rafael J. Wysocki" , Len Brown , Pavel Machek , Matthias Brugger Cc: Saravana Kannan , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, kernel-team@android.com, "Rafael J. Wysocki" Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: "Rafael J. Wysocki" Even though stateful device links are managed by the driver core in principle, their creators are allowed and sometimes even expected to drop references to them via device_link_del() or device_link_remove(), but that doesn't really play well with the "persistent" link concept. If "persistent" managed device links are created from driver probe callbacks, device_link_add() called to do that will take a new reference on the link each time the callback runs and those references will never be dropped, which kind of isn't nice. This issues arises because of the link reference counting carried out by device_link_add() for existing links, but that is only done to avoid deleting device links that may still be necessary, which shouldn't be a concern for managed (stateful) links. These device links are managed by the driver core and whoever creates one of them will need it at least as long as until the consumer driver is detached from its device and deleting it may be left to the driver core just fine. For this reason, rework device_link_add() to apply the reference counting to stateless links only and make device_link_del() and device_link_remove() drop references to stateless links only too. After this change, if called to add a stateful device link for a consumer-supplier pair for which a stateful device link is present already, device_link_add() will return the existing link without incrementing its reference counter. Accordingly, device_link_del() and device_link_remove() will WARN() and do nothing when called to drop a reference to a stateful link. Thus, effectively, all stateful device links will be owned by the driver core. In addition, clean up the handling of the link management flags, DL_FLAG_AUTOREMOVE_CONSUMER and DL_FLAG_AUTOREMOVE_SUPPLIER, so that (a) they are never set at the same time and (b) if device_link_add() is called for a consumer-supplier pair with an existing stateful link between them, the flags of that link will be combined with the flags passed to device_link_add() to ensure that the life time of the link is sufficient for all of the callers of device_link_add() for the same consumer-supplier pair. Update the device_link_add() kerneldoc comment to reflect the above changes. Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman (cherry picked from commit 72175d4ea4c442d95cf690c3e968eeee90fd43ca) Signed-off-by: Saravana Kannan --- Documentation/driver-api/device_link.rst | 42 +++++++++------ drivers/base/core.c | 69 ++++++++++++++++++------ 2 files changed, 79 insertions(+), 32 deletions(-) diff --git a/Documentation/driver-api/device_link.rst b/Documentation/driver-api/device_link.rst index 5c7178189612..e249e074a8d2 100644 --- a/Documentation/driver-api/device_link.rst +++ b/Documentation/driver-api/device_link.rst @@ -25,8 +25,8 @@ suspend/resume and shutdown ordering. Device links allow representation of such dependencies in the driver core. -In its standard form, a device link combines *both* dependency types: -It guarantees correct suspend/resume and shutdown ordering between a +In its standard or *managed* form, a device link combines *both* dependency +types: It guarantees correct suspend/resume and shutdown ordering between a "supplier" device and its "consumer" devices, and it guarantees driver presence on the supplier. The consumer devices are not probed before the supplier is bound to a driver, and they're unbound before the supplier @@ -69,12 +69,14 @@ know that the supplier is functional already at the link creation time (that is the case, for instance, if the consumer has just acquired some resources that would not have been available had the supplier not been functional then).] -If a device link is added in the ``->probe`` callback of the supplier or -consumer driver, it is typically deleted in its ``->remove`` callback for -symmetry. That way, if the driver is compiled as a module, the device -link is added on module load and orderly deleted on unload. The same -restrictions that apply to device link addition (e.g. exclusion of a -parallel suspend/resume transition) apply equally to deletion. +If a device link with ``DL_FLAG_STATELESS`` set (i.e. a stateless device link) +is added in the ``->probe`` callback of the supplier or consumer driver, it is +typically deleted in its ``->remove`` callback for symmetry. That way, if the +driver is compiled as a module, the device link is added on module load and +orderly deleted on unload. The same restrictions that apply to device link +addition (e.g. exclusion of a parallel suspend/resume transition) apply equally +to deletion. Device links with ``DL_FLAG_STATELESS`` unset (i.e. managed +device links) are deleted automatically by the driver core. Several flags may be specified on device link addition, two of which have already been mentioned above: ``DL_FLAG_STATELESS`` to express that no @@ -87,8 +89,6 @@ link is added from the consumer's ``->probe`` callback: ``DL_FLAG_RPM_ACTIVE`` can be specified to runtime resume the supplier upon addition of the device link. ``DL_FLAG_AUTOREMOVE_CONSUMER`` causes the device link to be automatically purged when the consumer fails to probe or later unbinds. -This obviates the need to explicitly delete the link in the ``->remove`` -callback or in the error path of the ``->probe`` callback. Similarly, when the device link is added from supplier's ``->probe`` callback, ``DL_FLAG_AUTOREMOVE_SUPPLIER`` causes the device link to be automatically @@ -97,12 +97,20 @@ purged when the supplier fails to probe or later unbinds. Limitations =========== -Driver authors should be aware that a driver presence dependency (i.e. when -``DL_FLAG_STATELESS`` is not specified on link addition) may cause probing of -the consumer to be deferred indefinitely. This can become a problem if the -consumer is required to probe before a certain initcall level is reached. -Worse, if the supplier driver is blacklisted or missing, the consumer will -never be probed. +Driver authors should be aware that a driver presence dependency for managed +device links (i.e. when ``DL_FLAG_STATELESS`` is not specified on link addition) +may cause probing of the consumer to be deferred indefinitely. This can become +a problem if the consumer is required to probe before a certain initcall level +is reached. Worse, if the supplier driver is blacklisted or missing, the +consumer will never be probed. + +Moreover, managed device links cannot be deleted directly. They are deleted +by the driver core when they are not necessary any more in accordance with the +``DL_FLAG_AUTOREMOVE_CONSUMER`` and ``DL_FLAG_AUTOREMOVE_SUPPLIER`` flags. +However, stateless device links (i.e. device links with ``DL_FLAG_STATELESS`` +set) are expected to be removed by whoever called :c:func:`device_link_add()` +to add them with the help of either :c:func:`device_link_del()` or +:c:func:`device_link_remove()`. Sometimes drivers depend on optional resources. They are able to operate in a degraded mode (reduced feature set or performance) when those resources @@ -286,4 +294,4 @@ API === .. kernel-doc:: drivers/base/core.c - :functions: device_link_add device_link_del + :functions: device_link_add device_link_del device_link_remove diff --git a/drivers/base/core.c b/drivers/base/core.c index ae2f85ca78fb..d8273792950b 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -179,10 +179,21 @@ void device_pm_move_to_tail(struct device *dev) * of the link. If DL_FLAG_PM_RUNTIME is not set, DL_FLAG_RPM_ACTIVE will be * ignored. * - * If the DL_FLAG_AUTOREMOVE_CONSUMER flag is set, the link will be removed - * automatically when the consumer device driver unbinds from it. Analogously, - * if DL_FLAG_AUTOREMOVE_SUPPLIER is set in @flags, the link will be removed - * automatically when the supplier device driver unbinds from it. + * If DL_FLAG_STATELESS is set in @flags, the link is not going to be managed by + * the driver core and, in particular, the caller of this function is expected + * to drop the reference to the link acquired by it directly. + * + * If that flag is not set, however, the caller of this function is handing the + * management of the link over to the driver core entirely and its return value + * can only be used to check whether or not the link is present. In that case, + * the DL_FLAG_AUTOREMOVE_CONSUMER and DL_FLAG_AUTOREMOVE_SUPPLIER device link + * flags can be used to indicate to the driver core when the link can be safely + * deleted. Namely, setting one of them in @flags indicates to the driver core + * that the link is not going to be used (by the given caller of this function) + * after unbinding the consumer or supplier driver, respectively, from its + * device, so the link can be deleted at that point. If none of them is set, + * the link will be maintained until one of the devices pointed to by it (either + * the consumer or the supplier) is unregistered. * * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and @@ -228,6 +239,14 @@ struct device_link *device_link_add(struct device *consumer, goto out; } + /* + * DL_FLAG_AUTOREMOVE_SUPPLIER indicates that the link will be needed + * longer than for DL_FLAG_AUTOREMOVE_CONSUMER and setting them both + * together doesn't make sense, so prefer DL_FLAG_AUTOREMOVE_SUPPLIER. + */ + if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) + flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER; + list_for_each_entry(link, &supplier->links.consumers, s_node) { if (link->consumer != consumer) continue; @@ -241,12 +260,6 @@ struct device_link *device_link_add(struct device *consumer, goto out; } - if (flags & DL_FLAG_AUTOREMOVE_CONSUMER) - link->flags |= DL_FLAG_AUTOREMOVE_CONSUMER; - - if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) - link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER; - if (flags & DL_FLAG_PM_RUNTIME) { if (!(link->flags & DL_FLAG_PM_RUNTIME)) { pm_runtime_new_link(consumer); @@ -256,7 +269,25 @@ struct device_link *device_link_add(struct device *consumer, refcount_inc(&link->rpm_active); } - kref_get(&link->kref); + if (flags & DL_FLAG_STATELESS) { + kref_get(&link->kref); + goto out; + } + + /* + * If the life time of the link following from the new flags is + * longer than indicated by the flags of the existing link, + * update the existing link to stay around longer. + */ + if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) { + if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) { + link->flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER; + link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER; + } + } else if (!(flags & DL_FLAG_AUTOREMOVE_CONSUMER)) { + link->flags &= ~(DL_FLAG_AUTOREMOVE_CONSUMER | + DL_FLAG_AUTOREMOVE_SUPPLIER); + } goto out; } @@ -406,8 +437,16 @@ static void __device_link_del(struct kref *kref) } #endif /* !CONFIG_SRCU */ +static void device_link_put_kref(struct device_link *link) +{ + if (link->flags & DL_FLAG_STATELESS) + kref_put(&link->kref, __device_link_del); + else + WARN(1, "Unable to drop a managed device link reference\n"); +} + /** - * device_link_del - Delete a link between two devices. + * device_link_del - Delete a stateless link between two devices. * @link: Device link to delete. * * The caller must ensure proper synchronization of this function with runtime @@ -419,14 +458,14 @@ void device_link_del(struct device_link *link) { device_links_write_lock(); device_pm_lock(); - kref_put(&link->kref, __device_link_del); + device_link_put_kref(link); device_pm_unlock(); device_links_write_unlock(); } EXPORT_SYMBOL_GPL(device_link_del); /** - * device_link_remove - remove a link between two devices. + * device_link_remove - Delete a stateless link between two devices. * @consumer: Consumer end of the link. * @supplier: Supplier end of the link. * @@ -445,7 +484,7 @@ void device_link_remove(void *consumer, struct device *supplier) list_for_each_entry(link, &supplier->links.consumers, s_node) { if (link->consumer == consumer) { - kref_put(&link->kref, __device_link_del); + device_link_put_kref(link); break; } } From patchwork Tue Mar 17 06:54:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Saravana Kannan X-Patchwork-Id: 11441881 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 7FCA9913 for ; Tue, 17 Mar 2020 06:55:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5814E20757 for ; Tue, 17 Mar 2020 06:55:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="Nab2iMTk" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726192AbgCQGzQ (ORCPT ); Tue, 17 Mar 2020 02:55:16 -0400 Received: from mail-qv1-f73.google.com ([209.85.219.73]:47708 "EHLO mail-qv1-f73.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726506AbgCQGzO (ORCPT ); Tue, 17 Mar 2020 02:55:14 -0400 Received: by mail-qv1-f73.google.com with SMTP id o102so8556167qvo.14 for ; Mon, 16 Mar 2020 23:55:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=7ngORi0qjHGKSTPYP6jqwB2CTxTKHni0v/JyFS4TsJY=; b=Nab2iMTktoEDgr02ucqDK9isAmOo4ipKNIJmcdWX9C+9c6ibZ0ACWVVDLKrmvFK2Nv +n/CdhwFe7D0fcDDF+KueDiskyTSirfZzgH7ApN17BUlo33WGiDWYQxSdr+88om+U7ug yLoYp18sCc0bH6o010hAp7CyOux2D4YkIDi5T36j6CpG9ul39ZcoEkPKPhqNjdZI5Hbq 126yvaty609YLDursNVsFv1zTmvhwtCRCM5ONcqGkUz4vJt9r3XlTRWSa/0fb+rJO1Jk ZCqKS5PHRoyv6IVk7cQ1Tmnu4uXf0ZkfZkTpEZsAsodLYoQolYkSZVpKlgOncY4D208G e0aQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=7ngORi0qjHGKSTPYP6jqwB2CTxTKHni0v/JyFS4TsJY=; b=Q9jRncl0p0MOOHuUjNz62EVugdvgo/ism0g1iyIZCCFxLmVYP63z3S5VUpllPh3hw6 oySBhCRm3PulLTLN62u39Ry5XCCwE+A03n/h4xn4VRZNVm1ZGvQM/01ut65JfoHCbbbF 9TlqYVtKJSXuGXm8kUfdZFDNGJdwwoobePXn0HtC3DdN0ZyiQdjf0L14+BVdfv7AiQeJ l5SmRNdim9vqLm9t0TYL8YHcwYhodNoqg/1ezgYCHaE+WyUFHqOKW45PC+W3bOAaZuHC ot/9xf/jMx6dND66B9oUFcWoNF46otIUU7QtUYYKLJUID4kVuKz2dkQB/+qfO/z6me0j dhVw== X-Gm-Message-State: ANhLgQ1AN5EwTjk5tiLw+IcnWzDUD5C2WyaYaWVWSM4aqnlvxfd2awp+ HwUpbKeEXBMRBs04ZT4vz7q0TzTPt8IEXpU= X-Google-Smtp-Source: ADFU+vu6Q/y175df99s4giYR9vMy2KQkhva5yiVcjNt6xji0hnXCikbzl84Cs4t7LJdeoDn8q4XTKNSqQvCeU0w= X-Received: by 2002:ac8:3003:: with SMTP id f3mr3817009qte.293.1584428111301; Mon, 16 Mar 2020 23:55:11 -0700 (PDT) Date: Mon, 16 Mar 2020 23:54:50 -0700 In-Reply-To: <20200317065452.236670-1-saravanak@google.com> Message-Id: <20200317065452.236670-5-saravanak@google.com> Mime-Version: 1.0 References: <20200317065452.236670-1-saravanak@google.com> X-Mailer: git-send-email 2.25.1.481.gfbce0eb801-goog Subject: [PATCH v1 4/6] driver core: Add device link flag DL_FLAG_AUTOPROBE_CONSUMER From: Saravana Kannan To: stable@vger.kernel.org, Jonathan Corbet , Greg Kroah-Hartman , "Rafael J. Wysocki" , Len Brown , Pavel Machek , Matthias Brugger Cc: Saravana Kannan , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, kernel-team@android.com, "Rafael J. Wysocki" Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: "Rafael J. Wysocki" Add a new device link flag, DL_FLAG_AUTOPROBE_CONSUMER, to request the driver core to probe for a consumer driver automatically after binding a driver to the supplier device on a persistent managed device link. As unbinding the supplier driver on a managed device link causes the consumer driver to be detached from its device automatically, this flag provides a complementary mechanism which is needed to address some "composite device" use cases. Signed-off-by: Rafael J. Wysocki Signed-off-by: Greg Kroah-Hartman (cherry picked from commit e7dd40105aac9ba051e44ad711123bc53a5e4c71) Signed-off-by: Saravana Kannan --- Documentation/driver-api/device_link.rst | 9 +++++++++ drivers/base/core.c | 16 +++++++++++++++- drivers/base/dd.c | 2 +- include/linux/device.h | 3 +++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/driver-api/device_link.rst b/Documentation/driver-api/device_link.rst index e249e074a8d2..c764755121c7 100644 --- a/Documentation/driver-api/device_link.rst +++ b/Documentation/driver-api/device_link.rst @@ -94,6 +94,15 @@ Similarly, when the device link is added from supplier's ``->probe`` callback, ``DL_FLAG_AUTOREMOVE_SUPPLIER`` causes the device link to be automatically purged when the supplier fails to probe or later unbinds. +If neither ``DL_FLAG_AUTOREMOVE_CONSUMER`` nor ``DL_FLAG_AUTOREMOVE_SUPPLIER`` +is set, ``DL_FLAG_AUTOPROBE_CONSUMER`` can be used to request the driver core +to probe for a driver for the consumer driver on the link automatically after +a driver has been bound to the supplier device. + +Note, however, that any combinations of ``DL_FLAG_AUTOREMOVE_CONSUMER``, +``DL_FLAG_AUTOREMOVE_SUPPLIER`` or ``DL_FLAG_AUTOPROBE_CONSUMER`` with +``DL_FLAG_STATELESS`` are invalid and cannot be used. + Limitations =========== diff --git a/drivers/base/core.c b/drivers/base/core.c index d8273792950b..3495da13b18a 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -195,6 +195,12 @@ void device_pm_move_to_tail(struct device *dev) * the link will be maintained until one of the devices pointed to by it (either * the consumer or the supplier) is unregistered. * + * Also, if DL_FLAG_STATELESS, DL_FLAG_AUTOREMOVE_CONSUMER and + * DL_FLAG_AUTOREMOVE_SUPPLIER are not set in @flags (that is, a persistent + * managed device link is being added), the DL_FLAG_AUTOPROBE_CONSUMER flag can + * be used to request the driver core to automaticall probe for a consmer + * driver after successfully binding a driver to the supplier device. + * * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and * will cause NULL to be returned upfront. @@ -215,7 +221,12 @@ struct device_link *device_link_add(struct device *consumer, if (!consumer || !supplier || (flags & DL_FLAG_STATELESS && - flags & (DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER))) + flags & (DL_FLAG_AUTOREMOVE_CONSUMER | + DL_FLAG_AUTOREMOVE_SUPPLIER | + DL_FLAG_AUTOPROBE_CONSUMER)) || + (flags & DL_FLAG_AUTOPROBE_CONSUMER && + flags & (DL_FLAG_AUTOREMOVE_CONSUMER | + DL_FLAG_AUTOREMOVE_SUPPLIER))) return NULL; if (flags & DL_FLAG_PM_RUNTIME && flags & DL_FLAG_RPM_ACTIVE) { @@ -576,6 +587,9 @@ void device_links_driver_bound(struct device *dev) WARN_ON(link->status != DL_STATE_DORMANT); WRITE_ONCE(link->status, DL_STATE_AVAILABLE); + + if (link->flags & DL_FLAG_AUTOPROBE_CONSUMER) + driver_deferred_probe_add(link->consumer); } list_for_each_entry(link, &dev->links.suppliers, c_node) { diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 5f6416e6ba96..caaeb7910a04 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -116,7 +116,7 @@ static void deferred_probe_work_func(struct work_struct *work) } static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func); -static void driver_deferred_probe_add(struct device *dev) +void driver_deferred_probe_add(struct device *dev) { mutex_lock(&deferred_probe_mutex); if (list_empty(&dev->p->deferred_probe)) { diff --git a/include/linux/device.h b/include/linux/device.h index c74ce473589a..7569cff2f4dc 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -339,6 +339,7 @@ struct device *driver_find_device(struct device_driver *drv, struct device *start, void *data, int (*match)(struct device *dev, void *data)); +void driver_deferred_probe_add(struct device *dev); int driver_deferred_probe_check_state(struct device *dev); /** @@ -824,12 +825,14 @@ enum device_link_state { * PM_RUNTIME: If set, the runtime PM framework will use this link. * RPM_ACTIVE: Run pm_runtime_get_sync() on the supplier during link creation. * AUTOREMOVE_SUPPLIER: Remove the link automatically on supplier driver unbind. + * AUTOPROBE_CONSUMER: Probe consumer driver automatically after supplier binds. */ #define DL_FLAG_STATELESS BIT(0) #define DL_FLAG_AUTOREMOVE_CONSUMER BIT(1) #define DL_FLAG_PM_RUNTIME BIT(2) #define DL_FLAG_RPM_ACTIVE BIT(3) #define DL_FLAG_AUTOREMOVE_SUPPLIER BIT(4) +#define DL_FLAG_AUTOPROBE_CONSUMER BIT(5) /** * struct device_link - Device link representation. From patchwork Tue Mar 17 06:54:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Saravana Kannan X-Patchwork-Id: 11441883 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 17E101820 for ; Tue, 17 Mar 2020 06:55:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D21B520754 for ; Tue, 17 Mar 2020 06:55:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="koHbH84M" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726535AbgCQGz2 (ORCPT ); Tue, 17 Mar 2020 02:55:28 -0400 Received: from mail-pf1-f201.google.com ([209.85.210.201]:56373 "EHLO mail-pf1-f201.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725868AbgCQGzQ (ORCPT ); Tue, 17 Mar 2020 02:55:16 -0400 Received: by mail-pf1-f201.google.com with SMTP id s8so9136668pfd.23 for ; Mon, 16 Mar 2020 23:55:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=dw0Iez42+toJbxxFGiBDBV1QvMjdPRv2uGpEwHepw7s=; b=koHbH84McSuSIRzLGk3G78b/WTysI4PFNn2eqYP7QlIO0U3/yFA9FlQDsZNTyh6O4e +4C06oWQCqwVBAwn/+S3/s59kpUfqLaOGDtBqnMe59EFiHoqfndJ8lE8KIMPpSkUtkTQ vfxleCdqm8uTGi9XXxTOPbiycq6m8Zf8r2L0QbRvJGxrzdaltg2rpn9ttdZxWT2vLLYF 5JBvUNjZnid5R4DBWupbIHW07RzTAaJ+zyGPjTSCE0NMl7eRNMa4DQm8lK3juLbZphKp ctFOd4LEc/WHixe7BIDkfBuT3hymj85tg1qqjyNgpxci8+EiU7otorSPutTMhiCo6T38 cryA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=dw0Iez42+toJbxxFGiBDBV1QvMjdPRv2uGpEwHepw7s=; b=B0Qm9MykrxTeb/7gyvVrT0SbmGbLG3wmYqEXO9nmRrSb5tbL03+ov7cIaYvnaGYupT iOPDRi3Em2QV8nQ86m0HWUn92TvUy+6y6QfUBHmZ6VO+dfr5k6LwmRpseFhBsqjyQZXM NK4xAdwoINmuRxUpSU+l1uVj4f7E7D+184jDuDCRRXVmg6sgkHObk3tjs3HClB+rmJfS 6VX2PK9Htr/DvXQdCXdWbkqpA8OYhmyRZSJQ/oM//1IS2fUdaLKCSS3Yn3I72RnB4hra uo90Qtl+4nHwNs98pBxl/bR4VrMxDPKRI+8hXn19q3/ku56yc/F4gIGcDtmxJMSLwA1O DU/A== X-Gm-Message-State: ANhLgQ1D0+PCkJLeG4sMDC53bu2c4BL81RzUKpQuCg2ihkXPRr4V40PW EYoBycEiQclY/4rdLxGUHs9ciBCYT2A7w78= X-Google-Smtp-Source: ADFU+vtD8dv+wxxcFh5YwXw7fU5EdGA5e/bnNhJ6qN2zy+KreB7GHsNCzvqytJsJh+gZIiGTrL0nKre2eQCRBe4= X-Received: by 2002:a17:90b:3793:: with SMTP id mz19mr3854219pjb.194.1584428114447; Mon, 16 Mar 2020 23:55:14 -0700 (PDT) Date: Mon, 16 Mar 2020 23:54:51 -0700 In-Reply-To: <20200317065452.236670-1-saravanak@google.com> Message-Id: <20200317065452.236670-6-saravanak@google.com> Mime-Version: 1.0 References: <20200317065452.236670-1-saravanak@google.com> X-Mailer: git-send-email 2.25.1.481.gfbce0eb801-goog Subject: [PATCH v1 5/6] driver core: Remove device link creation limitation From: Saravana Kannan To: stable@vger.kernel.org, Jonathan Corbet , Greg Kroah-Hartman , "Rafael J. Wysocki" , Len Brown , Pavel Machek , Matthias Brugger Cc: Saravana Kannan , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, kernel-team@android.com, "Rafael J. Wysocki" , Marek Szyprowski Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: "Rafael J. Wysocki" If device_link_add() is called for a consumer/supplier pair with an existing device link between them and the existing link's type is not in agreement with the flags passed to that function by its caller, NULL will be returned. That is seriously inconvenient, because it forces the callers of device_link_add() to worry about what others may or may not do even if that is not relevant to them for any other reasons. It turns out, however, that this limitation can be made go away relatively easily. The underlying observation is that if DL_FLAG_STATELESS has been passed to device_link_add() in flags for the given consumer/supplier pair at least once, calling either device_link_del() or device_link_remove() to release the link returned by it should work, but there are no other requirements associated with that flag. In turn, if at least one of the callers of device_link_add() for the given consumer/supplier pair has not passed DL_FLAG_STATELESS to it in flags, the driver core should track the status of the link and act on it as appropriate (ie. the link should be treated as "managed"). This means that DL_FLAG_STATELESS needs to be set for managed device links and it should be valid to call device_link_del() or device_link_remove() to drop references to them in certain sutiations. To allow that to happen, introduce a new (internal) device link flag called DL_FLAG_MANAGED and make device_link_add() set it automatically whenever DL_FLAG_STATELESS is not passed to it. Also make it take additional references to existing device links that were previously stateless (that is, with DL_FLAG_STATELESS set and DL_FLAG_MANAGED unset) and will need to be managed going forward and initialize their status (which has been DL_STATE_NONE so far). Accordingly, when a managed device link is dropped automatically by the driver core, make it clear DL_FLAG_MANAGED, reset the link's status back to DL_STATE_NONE and drop the reference to it associated with DL_FLAG_MANAGED instead of just deleting it right away (to allow it to stay around in case it still needs to be released explicitly by someone). With that, since setting DL_FLAG_STATELESS doesn't mean that the device link in question is not managed any more, replace all of the status-tracking checks against DL_FLAG_STATELESS with analogous checks against DL_FLAG_MANAGED and update the documentation to reflect these changes. While at it, make device_link_add() reject flags that it does not recognize, including DL_FLAG_MANAGED. Signed-off-by: Rafael J. Wysocki Reviewed-by: Saravana Kannan Tested-by: Marek Szyprowski Review-by: Saravana Kannan Link: https://lore.kernel.org/r/2305283.AStDPdUUnE@kreacher Signed-off-by: Greg Kroah-Hartman (cherry picked from commit 515db266a9dace92b0cbaed9a6044dd5304b8ca9) Signed-off-by: Saravana Kannan --- Documentation/driver-api/device_link.rst | 4 +- drivers/base/core.c | 176 +++++++++++++---------- drivers/base/power/runtime.c | 4 +- include/linux/device.h | 4 +- 4 files changed, 106 insertions(+), 82 deletions(-) diff --git a/Documentation/driver-api/device_link.rst b/Documentation/driver-api/device_link.rst index c764755121c7..e8b0a8fd1ae0 100644 --- a/Documentation/driver-api/device_link.rst +++ b/Documentation/driver-api/device_link.rst @@ -75,8 +75,8 @@ typically deleted in its ``->remove`` callback for symmetry. That way, if the driver is compiled as a module, the device link is added on module load and orderly deleted on unload. The same restrictions that apply to device link addition (e.g. exclusion of a parallel suspend/resume transition) apply equally -to deletion. Device links with ``DL_FLAG_STATELESS`` unset (i.e. managed -device links) are deleted automatically by the driver core. +to deletion. Device links managed by the driver core are deleted automatically +by it. Several flags may be specified on device link addition, two of which have already been mentioned above: ``DL_FLAG_STATELESS`` to express that no diff --git a/drivers/base/core.c b/drivers/base/core.c index 3495da13b18a..31007af4cb79 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -124,6 +124,50 @@ static int device_is_dependent(struct device *dev, void *target) return ret; } +static void device_link_init_status(struct device_link *link, + struct device *consumer, + struct device *supplier) +{ + switch (supplier->links.status) { + case DL_DEV_PROBING: + switch (consumer->links.status) { + case DL_DEV_PROBING: + /* + * A consumer driver can create a link to a supplier + * that has not completed its probing yet as long as it + * knows that the supplier is already functional (for + * example, it has just acquired some resources from the + * supplier). + */ + link->status = DL_STATE_CONSUMER_PROBE; + break; + default: + link->status = DL_STATE_DORMANT; + break; + } + break; + case DL_DEV_DRIVER_BOUND: + switch (consumer->links.status) { + case DL_DEV_PROBING: + link->status = DL_STATE_CONSUMER_PROBE; + break; + case DL_DEV_DRIVER_BOUND: + link->status = DL_STATE_ACTIVE; + break; + default: + link->status = DL_STATE_AVAILABLE; + break; + } + break; + case DL_DEV_UNBINDING: + link->status = DL_STATE_SUPPLIER_UNBIND; + break; + default: + link->status = DL_STATE_DORMANT; + break; + } +} + static int device_reorder_to_tail(struct device *dev, void *not_used) { struct device_link *link; @@ -165,6 +209,10 @@ void device_pm_move_to_tail(struct device *dev) device_links_read_unlock(idx); } +#define DL_MANAGED_LINK_FLAGS (DL_FLAG_AUTOREMOVE_CONSUMER | \ + DL_FLAG_AUTOREMOVE_SUPPLIER | \ + DL_FLAG_AUTOPROBE_CONSUMER) + /** * device_link_add - Create a link between two devices. * @consumer: Consumer end of the link. @@ -179,9 +227,9 @@ void device_pm_move_to_tail(struct device *dev) * of the link. If DL_FLAG_PM_RUNTIME is not set, DL_FLAG_RPM_ACTIVE will be * ignored. * - * If DL_FLAG_STATELESS is set in @flags, the link is not going to be managed by - * the driver core and, in particular, the caller of this function is expected - * to drop the reference to the link acquired by it directly. + * If DL_FLAG_STATELESS is set in @flags, the caller of this function is + * expected to release the link returned by it directly with the help of either + * device_link_del() or device_link_remove(). * * If that flag is not set, however, the caller of this function is handing the * management of the link over to the driver core entirely and its return value @@ -201,9 +249,16 @@ void device_pm_move_to_tail(struct device *dev) * be used to request the driver core to automaticall probe for a consmer * driver after successfully binding a driver to the supplier device. * - * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER - * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and - * will cause NULL to be returned upfront. + * The combination of DL_FLAG_STATELESS and one of DL_FLAG_AUTOREMOVE_CONSUMER, + * DL_FLAG_AUTOREMOVE_SUPPLIER, or DL_FLAG_AUTOPROBE_CONSUMER set in @flags at + * the same time is invalid and will cause NULL to be returned upfront. + * However, if a device link between the given @consumer and @supplier pair + * exists already when this function is called for them, the existing link will + * be returned regardless of its current type and status (the link's flags may + * be modified then). The caller of this function is then expected to treat + * the link as though it has just been created, so (in particular) if + * DL_FLAG_STATELESS was passed in @flags, the link needs to be released + * explicitly when not needed any more (as stated above). * * A side effect of the link creation is re-ordering of dpm_list and the * devices_kset list by moving the consumer device and all devices depending @@ -220,10 +275,8 @@ struct device_link *device_link_add(struct device *consumer, struct device_link *link; if (!consumer || !supplier || - (flags & DL_FLAG_STATELESS && - flags & (DL_FLAG_AUTOREMOVE_CONSUMER | - DL_FLAG_AUTOREMOVE_SUPPLIER | - DL_FLAG_AUTOPROBE_CONSUMER)) || + (flags & ~(DL_FLAG_STATELESS | DL_MANAGED_LINK_FLAGS)) || + (flags & DL_FLAG_STATELESS && flags & DL_MANAGED_LINK_FLAGS) || (flags & DL_FLAG_AUTOPROBE_CONSUMER && flags & (DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER))) @@ -236,6 +289,9 @@ struct device_link *device_link_add(struct device *consumer, } } + if (!(flags & DL_FLAG_STATELESS)) + flags |= DL_FLAG_MANAGED; + device_links_write_lock(); device_pm_lock(); @@ -262,15 +318,6 @@ struct device_link *device_link_add(struct device *consumer, if (link->consumer != consumer) continue; - /* - * Don't return a stateless link if the caller wants a stateful - * one and vice versa. - */ - if (WARN_ON((flags & DL_FLAG_STATELESS) != (link->flags & DL_FLAG_STATELESS))) { - link = NULL; - goto out; - } - if (flags & DL_FLAG_PM_RUNTIME) { if (!(link->flags & DL_FLAG_PM_RUNTIME)) { pm_runtime_new_link(consumer); @@ -281,6 +328,7 @@ struct device_link *device_link_add(struct device *consumer, } if (flags & DL_FLAG_STATELESS) { + link->flags |= DL_FLAG_STATELESS; kref_get(&link->kref); goto out; } @@ -299,6 +347,11 @@ struct device_link *device_link_add(struct device *consumer, link->flags &= ~(DL_FLAG_AUTOREMOVE_CONSUMER | DL_FLAG_AUTOREMOVE_SUPPLIER); } + if (!(link->flags & DL_FLAG_MANAGED)) { + kref_get(&link->kref); + link->flags |= DL_FLAG_MANAGED; + device_link_init_status(link, consumer, supplier); + } goto out; } @@ -325,48 +378,10 @@ struct device_link *device_link_add(struct device *consumer, kref_init(&link->kref); /* Determine the initial link state. */ - if (flags & DL_FLAG_STATELESS) { + if (flags & DL_FLAG_STATELESS) link->status = DL_STATE_NONE; - } else { - switch (supplier->links.status) { - case DL_DEV_PROBING: - switch (consumer->links.status) { - case DL_DEV_PROBING: - /* - * A consumer driver can create a link to a - * supplier that has not completed its probing - * yet as long as it knows that the supplier is - * already functional (for example, it has just - * acquired some resources from the supplier). - */ - link->status = DL_STATE_CONSUMER_PROBE; - break; - default: - link->status = DL_STATE_DORMANT; - break; - } - break; - case DL_DEV_DRIVER_BOUND: - switch (consumer->links.status) { - case DL_DEV_PROBING: - link->status = DL_STATE_CONSUMER_PROBE; - break; - case DL_DEV_DRIVER_BOUND: - link->status = DL_STATE_ACTIVE; - break; - default: - link->status = DL_STATE_AVAILABLE; - break; - } - break; - case DL_DEV_UNBINDING: - link->status = DL_STATE_SUPPLIER_UNBIND; - break; - default: - link->status = DL_STATE_DORMANT; - break; - } - } + else + device_link_init_status(link, consumer, supplier); /* * Some callers expect the link creation during consumer driver probe to @@ -528,7 +543,7 @@ static void device_links_missing_supplier(struct device *dev) * mark the link as "consumer probe in progress" to make the supplier removal * wait for us to complete (or bad things may happen). * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ int device_links_check_suppliers(struct device *dev) { @@ -538,7 +553,7 @@ int device_links_check_suppliers(struct device *dev) device_links_write_lock(); list_for_each_entry(link, &dev->links.suppliers, c_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; if (link->status != DL_STATE_AVAILABLE) { @@ -563,7 +578,7 @@ int device_links_check_suppliers(struct device *dev) * * Also change the status of @dev's links to suppliers to "active". * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ void device_links_driver_bound(struct device *dev) { @@ -572,7 +587,7 @@ void device_links_driver_bound(struct device *dev) device_links_write_lock(); list_for_each_entry(link, &dev->links.consumers, s_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; /* @@ -593,7 +608,7 @@ void device_links_driver_bound(struct device *dev) } list_for_each_entry(link, &dev->links.suppliers, c_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; WARN_ON(link->status != DL_STATE_CONSUMER_PROBE); @@ -605,6 +620,13 @@ void device_links_driver_bound(struct device *dev) device_links_write_unlock(); } +static void device_link_drop_managed(struct device_link *link) +{ + link->flags &= ~DL_FLAG_MANAGED; + WRITE_ONCE(link->status, DL_STATE_NONE); + kref_put(&link->kref, __device_link_del); +} + /** * __device_links_no_driver - Update links of a device without a driver. * @dev: Device without a drvier. @@ -615,18 +637,18 @@ void device_links_driver_bound(struct device *dev) * unless they already are in the "supplier unbind in progress" state in which * case they need not be updated. * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ static void __device_links_no_driver(struct device *dev) { struct device_link *link, *ln; list_for_each_entry_safe_reverse(link, ln, &dev->links.suppliers, c_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) - __device_link_del(&link->kref); + device_link_drop_managed(link); else if (link->status == DL_STATE_CONSUMER_PROBE || link->status == DL_STATE_ACTIVE) WRITE_ONCE(link->status, DL_STATE_AVAILABLE); @@ -643,7 +665,7 @@ static void __device_links_no_driver(struct device *dev) * %__device_links_no_driver() to update links to suppliers for it as * appropriate. * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ void device_links_no_driver(struct device *dev) { @@ -652,7 +674,7 @@ void device_links_no_driver(struct device *dev) device_links_write_lock(); list_for_each_entry(link, &dev->links.consumers, s_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; /* @@ -680,7 +702,7 @@ void device_links_no_driver(struct device *dev) * invoke %__device_links_no_driver() to update links to suppliers for it as * appropriate. * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ void device_links_driver_cleanup(struct device *dev) { @@ -689,7 +711,7 @@ void device_links_driver_cleanup(struct device *dev) device_links_write_lock(); list_for_each_entry_safe(link, ln, &dev->links.consumers, s_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; WARN_ON(link->flags & DL_FLAG_AUTOREMOVE_CONSUMER); @@ -702,7 +724,7 @@ void device_links_driver_cleanup(struct device *dev) */ if (link->status == DL_STATE_SUPPLIER_UNBIND && link->flags & DL_FLAG_AUTOREMOVE_SUPPLIER) - __device_link_del(&link->kref); + device_link_drop_managed(link); WRITE_ONCE(link->status, DL_STATE_DORMANT); } @@ -724,7 +746,7 @@ void device_links_driver_cleanup(struct device *dev) * * Return 'false' if there are no probing or active consumers. * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ bool device_links_busy(struct device *dev) { @@ -734,7 +756,7 @@ bool device_links_busy(struct device *dev) device_links_write_lock(); list_for_each_entry(link, &dev->links.consumers, s_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; if (link->status == DL_STATE_CONSUMER_PROBE @@ -764,7 +786,7 @@ bool device_links_busy(struct device *dev) * driver to unbind and start over (the consumer will not re-probe as we have * changed the state of the link already). * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links without the DL_FLAG_MANAGED flag set are ignored. */ void device_links_unbind_consumers(struct device *dev) { @@ -776,7 +798,7 @@ void device_links_unbind_consumers(struct device *dev) list_for_each_entry(link, &dev->links.consumers, s_node) { enum device_link_state status; - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; status = link->status; diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 303ce7d54a30..2c99f93020bc 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1531,7 +1531,7 @@ void pm_runtime_remove(struct device *dev) * runtime PM references to the device, drop the usage counter of the device * (as many times as needed). * - * Links with the DL_FLAG_STATELESS flag set are ignored. + * Links with the DL_FLAG_MANAGED flag unset are ignored. * * Since the device is guaranteed to be runtime-active at the point this is * called, nothing else needs to be done here. @@ -1548,7 +1548,7 @@ void pm_runtime_clean_up_links(struct device *dev) idx = device_links_read_lock(); list_for_each_entry_rcu(link, &dev->links.consumers, s_node) { - if (link->flags & DL_FLAG_STATELESS) + if (!(link->flags & DL_FLAG_MANAGED)) continue; while (refcount_dec_not_one(&link->rpm_active)) diff --git a/include/linux/device.h b/include/linux/device.h index 7569cff2f4dc..b1c8150e9ea5 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -820,12 +820,13 @@ enum device_link_state { /* * Device link flags. * - * STATELESS: The core won't track the presence of supplier/consumer drivers. + * STATELESS: The core will not remove this link automatically. * AUTOREMOVE_CONSUMER: Remove the link automatically on consumer driver unbind. * PM_RUNTIME: If set, the runtime PM framework will use this link. * RPM_ACTIVE: Run pm_runtime_get_sync() on the supplier during link creation. * AUTOREMOVE_SUPPLIER: Remove the link automatically on supplier driver unbind. * AUTOPROBE_CONSUMER: Probe consumer driver automatically after supplier binds. + * MANAGED: The core tracks presence of supplier/consumer drivers (internal). */ #define DL_FLAG_STATELESS BIT(0) #define DL_FLAG_AUTOREMOVE_CONSUMER BIT(1) @@ -833,6 +834,7 @@ enum device_link_state { #define DL_FLAG_RPM_ACTIVE BIT(3) #define DL_FLAG_AUTOREMOVE_SUPPLIER BIT(4) #define DL_FLAG_AUTOPROBE_CONSUMER BIT(5) +#define DL_FLAG_MANAGED BIT(6) /** * struct device_link - Device link representation. From patchwork Tue Mar 17 06:54:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Saravana Kannan X-Patchwork-Id: 11441879 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 91C01913 for ; Tue, 17 Mar 2020 06:55:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 724462074D for ; Tue, 17 Mar 2020 06:55:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="UledzLdG" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726494AbgCQGzU (ORCPT ); Tue, 17 Mar 2020 02:55:20 -0400 Received: from mail-pg1-f202.google.com ([209.85.215.202]:48626 "EHLO mail-pg1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726575AbgCQGzT (ORCPT ); Tue, 17 Mar 2020 02:55:19 -0400 Received: by mail-pg1-f202.google.com with SMTP id f14so13836162pgj.15 for ; Mon, 16 Mar 2020 23:55:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=pBzGjleoRDABxMsluExcps57eD2wDVloI24KvIbaURE=; b=UledzLdGZZjZgCktwDrReM83P2/tmN5F4Qnd1xTlFNMGgY4exTx7PpUlYpVJcYkHn2 qOqSDPrlyo1Tg8E7TNmeCbcaRvzN8X/3p3klGc1pbS5gp4joG2QV+m7zPocAKbdG/djV mjbSeXmumJlZpNyD2H16Tr4UjjvSjJ9VFvp84bYCSC8nAKD0ptzQmWlkEpmb2H+YyfEx Q5CmFKr/pxFMWAzVJsJwrAUZbJgxAjQUKhVZinz2cSSZ3AoAfMo2MvYj7YHTC2PK7FYy y7OzDTmhrmK/3gX546uw6Y/xdIMPy5ZSM/4+vdG2TJbkV4Jqx4cGvW2bLV42+qET+yvr zeNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=pBzGjleoRDABxMsluExcps57eD2wDVloI24KvIbaURE=; b=F25hXIpTPAl1zXv5ZZIiiEVxFC7UyKaO/Po2upe32o12kS9KB3GjwixshRSuLxK9bj n83/Qawb6mxzW4jKg7ajjZZjD8oa2v+SOWvOoMDDVHt4/e02WEwNeK0r2iJa1PLqwP1p GfmPP8C0DmOsiHO8qoLLDzuL4xiLg6+ri+TBNBRjWYTuQOzIlKUU1sUvhJSWqNVINglf Oj6gD+WVaD3QlrA6khZNnjvg7KhzfSoNQYFHiDTsOqArve11uvbERgej6fDnbu2jjxHb BkigYOhOytpNHYTfwxej3DPnbdy32z0JkGJOzHdiOi+AYb6bLOWK8O+hcSl1FoZd819t V07A== X-Gm-Message-State: ANhLgQ3a8i6KscyBEuwEuD9HKioIYvMFE37ffef1nggs59foJBs/Jdh1 xm+Kg14qTq/nlsHFXqelUNb9/n8shZ2dfKk= X-Google-Smtp-Source: ADFU+vsI46mjOrzK35IlZFKtn+I16f/ZJ36YpcLgiFZqwRzWEH2bTScx3tkAxfqOUW2oCR5e/5W0bdip3gPKHnQ= X-Received: by 2002:a17:90a:178e:: with SMTP id q14mr3975908pja.132.1584428117265; Mon, 16 Mar 2020 23:55:17 -0700 (PDT) Date: Mon, 16 Mar 2020 23:54:52 -0700 In-Reply-To: <20200317065452.236670-1-saravanak@google.com> Message-Id: <20200317065452.236670-7-saravanak@google.com> Mime-Version: 1.0 References: <20200317065452.236670-1-saravanak@google.com> X-Mailer: git-send-email 2.25.1.481.gfbce0eb801-goog Subject: [PATCH v1 6/6] driver core: Fix creation of device links with PM-runtime flags From: Saravana Kannan To: stable@vger.kernel.org, Jonathan Corbet , Greg Kroah-Hartman , "Rafael J. Wysocki" , Len Brown , Pavel Machek , Matthias Brugger Cc: Saravana Kannan , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, kernel-team@android.com, "Rafael J. Wysocki" , Dmitry Osipenko , Jon Hunter , Marek Szyprowski Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: "Rafael J. Wysocki" After commit 515db266a9da ("driver core: Remove device link creation limitation"), if PM-runtime flags are passed to device_link_add(), it will fail (returning NULL) due to an overly restrictive flags check introduced by that commit. Fix this issue by extending the check in question to cover the PM-runtime flags too. Fixes: 515db266a9da ("driver core: Remove device link creation limitation") Reported-by: Dmitry Osipenko Tested-by: Jon Hunter Signed-off-by: Rafael J. Wysocki Tested-by: Dmitry Osipenko Tested-by: Marek Szyprowski Link: https://lore.kernel.org/r/7674989.cD04D8YV3U@kreacher Signed-off-by: Greg Kroah-Hartman (cherry picked from commit fb583c8eeeb1fd57e24ef41ed94c9112067aeac9) Signed-off-by: Saravana Kannan --- drivers/base/core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 31007af4cb79..928fc1532a70 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -213,6 +213,9 @@ void device_pm_move_to_tail(struct device *dev) DL_FLAG_AUTOREMOVE_SUPPLIER | \ DL_FLAG_AUTOPROBE_CONSUMER) +#define DL_ADD_VALID_FLAGS (DL_MANAGED_LINK_FLAGS | DL_FLAG_STATELESS | \ + DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE) + /** * device_link_add - Create a link between two devices. * @consumer: Consumer end of the link. @@ -274,8 +277,7 @@ struct device_link *device_link_add(struct device *consumer, { struct device_link *link; - if (!consumer || !supplier || - (flags & ~(DL_FLAG_STATELESS | DL_MANAGED_LINK_FLAGS)) || + if (!consumer || !supplier || flags & ~DL_ADD_VALID_FLAGS || (flags & DL_FLAG_STATELESS && flags & DL_MANAGED_LINK_FLAGS) || (flags & DL_FLAG_AUTOPROBE_CONSUMER && flags & (DL_FLAG_AUTOREMOVE_CONSUMER |