* [PATCH 1/1] device core: Add flag to autoremove device link on supplier unbind @ 2018-05-30 9:57 Vivek Gautam 2018-05-30 10:59 ` Rafael J. Wysocki 0 siblings, 1 reply; 6+ messages in thread From: Vivek Gautam @ 2018-05-30 9:57 UTC (permalink / raw) To: gregkh, rafael.j.wysocki, lukas Cc: dmitry.torokhov, aspriel, robin.murphy, linux-kernel, linux-arm-msm, Vivek Gautam 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. [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) /** * struct device_link - Device link representation. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] device core: Add flag to autoremove device link on supplier unbind 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 0 siblings, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2018-05-30 10:59 UTC (permalink / raw) To: Vivek Gautam Cc: gregkh, lukas, dmitry.torokhov, aspriel, robin.murphy, linux-kernel, linux-pm 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. > [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? > > /** > * struct device_link - Device link representation. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] device core: Add flag to autoremove device link on supplier unbind 2018-05-30 10:59 ` Rafael J. Wysocki @ 2018-05-30 12:51 ` Vivek Gautam 2018-06-26 10:03 ` Vivek Gautam 0 siblings, 1 reply; 6+ messages in thread From: Vivek Gautam @ 2018-05-30 12:51 UTC (permalink / raw) To: Rafael J. Wysocki Cc: gregkh, lukas, dmitry.torokhov, aspriel, robin.murphy, linux-kernel, linux-pm 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. > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] device core: Add flag to autoremove device link on supplier unbind 2018-05-30 12:51 ` Vivek Gautam @ 2018-06-26 10:03 ` Vivek Gautam 2018-06-26 10:19 ` Rafael J. Wysocki 0 siblings, 1 reply; 6+ messages in thread From: Vivek Gautam @ 2018-06-26 10:03 UTC (permalink / raw) To: Rafael J. Wysocki, Ulf Hansson, Marek Szyprowski Cc: Greg KH, lukas, dmitry.torokhov, aspriel, Robin Murphy, open list, Linux PM On Wed, May 30, 2018 at 6:21 PM, Vivek Gautam <vivek.gautam@codeaurora.org> wrote: Adding Ulf and Marek for their side of comments. In summary, we do not want to not maintain a device link pointer when adding a device link in supplier's driver, to delete the link later. An autoremove flag from the suppliers side (similar to what we already have for consumer side) can help autoremove the device link when supplier driver goes away. Hi Rafael, Lukas, Gentle ping. Can you please consider reviewing this patch. Best regards Vivek > 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. >> >> >> > -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] device core: Add flag to autoremove device link on supplier unbind 2018-06-26 10:03 ` Vivek Gautam @ 2018-06-26 10:19 ` Rafael J. Wysocki 2018-06-26 11:48 ` Vivek Gautam 0 siblings, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2018-06-26 10:19 UTC (permalink / raw) To: Vivek Gautam Cc: Rafael J. Wysocki, Ulf Hansson, Marek Szyprowski, Greg KH, lukas, dmitry.torokhov, aspriel, Robin Murphy, open list, Linux PM On Tuesday, June 26, 2018 12:03:44 PM CEST Vivek Gautam wrote: > On Wed, May 30, 2018 at 6:21 PM, Vivek Gautam > <vivek.gautam@codeaurora.org> wrote: > > Adding Ulf and Marek for their side of comments. > In summary, we do not want to not maintain a device link pointer > when adding a device link in supplier's driver, to delete the link later. > An autoremove flag from the suppliers side (similar to what we already > have for consumer side) can help autoremove the device link > when supplier driver goes away. > > Hi Rafael, Lukas, > Gentle ping. Can you please consider reviewing this patch. ISTR telling you that I didn't like the DL_FLAG_AUTOREMOVE_S name as it is too similar to another existing flag. That hasn't changed. BTW, please CC the patch to linux-pm when you resend it. Thanks, Rafael ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] device core: Add flag to autoremove device link on supplier unbind 2018-06-26 10:19 ` Rafael J. Wysocki @ 2018-06-26 11:48 ` Vivek Gautam 0 siblings, 0 replies; 6+ messages in thread From: Vivek Gautam @ 2018-06-26 11:48 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Rafael J. Wysocki, Ulf Hansson, Marek Szyprowski, Greg KH, lukas, dmitry.torokhov, aspriel, Robin Murphy, open list, Linux PM Hi Rafael, On Tue, Jun 26, 2018 at 3:49 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Tuesday, June 26, 2018 12:03:44 PM CEST Vivek Gautam wrote: >> On Wed, May 30, 2018 at 6:21 PM, Vivek Gautam >> <vivek.gautam@codeaurora.org> wrote: >> >> Adding Ulf and Marek for their side of comments. >> In summary, we do not want to not maintain a device link pointer >> when adding a device link in supplier's driver, to delete the link later. >> An autoremove flag from the suppliers side (similar to what we already >> have for consumer side) can help autoremove the device link >> when supplier driver goes away. >> >> Hi Rafael, Lukas, >> Gentle ping. Can you please consider reviewing this patch. > > ISTR telling you that I didn't like the DL_FLAG_AUTOREMOVE_S name > as it is too similar to another existing flag. That hasn't changed. Right. I thought to gather more comments about the patch overall before attempting to respin with the changed name. I will send the next version. > > BTW, please CC the patch to linux-pm when you resend it. Sure. I will do that. Thanks. Best regards Vivek > > Thanks, > Rafael > -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-26 11:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 2018-06-26 10:03 ` Vivek Gautam 2018-06-26 10:19 ` Rafael J. Wysocki 2018-06-26 11:48 ` Vivek Gautam
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).