From: Vivek Gautam <vivek.gautam@codeaurora.org>
To: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: gregkh@linuxfoundation.org, lukas@wunner.de,
dmitry.torokhov@gmail.com, aspriel@gmail.com,
robin.murphy@arm.com, linux-kernel@vger.kernel.org,
linux-pm@vger.kernel.org
Subject: Re: [PATCH 1/1] device core: Add flag to autoremove device link on supplier unbind
Date: Wed, 30 May 2018 18:21:02 +0530 [thread overview]
Message-ID: <541c84e9-47fc-e7bc-8249-1477b9e63455@codeaurora.org> (raw)
In-Reply-To: <a85d44e7-3bb1-f282-088d-c4311a108bcd@intel.com>
Hi Rafael,
On 5/30/2018 4:29 PM, Rafael J. Wysocki wrote:
> On 5/30/2018 11:57 AM, Vivek Gautam wrote:
>> When using the device links without the consumers or
>> suppliers maintaining pointers to these links, a flag can
>> help in autoremoving the links on supplier driver unbind.
>> We remove these links only when the supplier's link to its
>> consumers has gone in DL_STATE_SUPPLIER_UNBIND state.
>>
>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> Suggested-by: Lukas Wunner <lukas@wunner.de>
>> ---
>>
>> Lukas, as suggested in the thread [1] this change adds additional flag
>> to autoremove device links on supplier unbind.
>> For arm-smmu, we want to _not_ keep references to the device links
>> added between arm-smmu, and consumer devices.
>> Robin also pointed to [2] the need to autoremove the device link on
>> supplier unbind rather than consumer unbind.
>
> Please CC device links patches to linux-pm.
Thanks for the quick review. Sure, will keep this noted from now on.
>
>> [1] https://lkml.org/lkml/2018/3/14/390
>> [2] https://lkml.org/lkml/2018/5/21/381
>>
>> drivers/base/core.c | 10 ++++++++++
>> include/linux/device.h | 2 ++
>> 2 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index b610816eb887..52c7222bb3c4 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -490,6 +490,16 @@ void device_links_driver_cleanup(struct device
>> *dev)
>> WARN_ON(link->flags & DL_FLAG_AUTOREMOVE);
>> WARN_ON(link->status != DL_STATE_SUPPLIER_UNBIND);
>> +
>> + /*
>> + * autoremove the links between this @dev and its consumer
>> + * devices that are not active, i.e. where the link state
>> + * has moved to DL_STATE_SUPPLIER_UNBIND.
>> + */
>> + if (link->status == DL_STATE_SUPPLIER_UNBIND &&
>> + link->flags & DL_FLAG_AUTOREMOVE_S)
>> + kref_put(&link->kref, __device_link_del);
>> +
>> WRITE_ONCE(link->status, DL_STATE_DORMANT);
>> }
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 477956990f5e..6033bf58453d 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -779,11 +779,13 @@ enum device_link_state {
>> * AUTOREMOVE: Remove this 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_S: Remove this link automatically on supplier driver
>> unbind.
>> */
>> #define DL_FLAG_STATELESS BIT(0)
>> #define DL_FLAG_AUTOREMOVE BIT(1)
>> #define DL_FLAG_PM_RUNTIME BIT(2)
>> #define DL_FLAG_RPM_ACTIVE BIT(3)
>> +#define DL_FLAG_AUTOREMOVE_S BIT(4)
>
> Couldn't you invent a better name for this one?
Frankly, I wanted to have something like DL_FLAG_AUTOREMOVE_SUPPLIER,
but that
felt a bit too long considering other flags.
Can you please consider suggesting a concise name?
Regards
Vivek
>
>> /**
>> * struct device_link - Device link representation.
>
>
next prev parent reply other threads:[~2018-05-30 12:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-30 9:57 [PATCH 1/1] device core: Add flag to autoremove device link on supplier unbind Vivek Gautam
2018-05-30 10:59 ` Rafael J. Wysocki
2018-05-30 12:51 ` Vivek Gautam [this message]
2018-06-26 10:03 ` Vivek Gautam
2018-06-26 10:19 ` Rafael J. Wysocki
2018-06-26 11:48 ` Vivek Gautam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=541c84e9-47fc-e7bc-8249-1477b9e63455@codeaurora.org \
--to=vivek.gautam@codeaurora.org \
--cc=aspriel@gmail.com \
--cc=dmitry.torokhov@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=rafael.j.wysocki@intel.com \
--cc=robin.murphy@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).