| Message ID | a49cc8aa-84fe-5752-efdc-113a116e588d@samsung.com |
|---|---|
| State | New |
| Headers | show |
Hi, On Thu, Oct 20, 2016 at 12:21 PM, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > Hi Rafael, > > > On 2016-10-19 13:57, Rafael J. Wysocki wrote: >> >> On Tue, Oct 18, 2016 at 12:46 PM, Marek Szyprowski >> <m.szyprowski@samsung.com> wrote: >>> >>> On 2016-10-10 14:36, Rafael J. Wysocki wrote: >>>> >>>> [...] >>>> >>>> One more update after some conversations during LinuxCon Europe. >>>> >>>> The main point was to make it possible for device_link_add() to figure >>>> out >>>> the >>>> initial state of the link instead of expecting the caller to provide it >>>> which >>>> might not be reliable enough in general. >>>> >>>> In this version device_link_add() takes three arguments, the supplier >>>> and >>>> consumer pointers and flags and it sets the correct initial state of the >>>> link >>>> automatically (unless invoked with the "stateless" flag, of course). >>>> The >>>> cost >>>> is one additional field in struct device (I moved all of the >>>> links-related >>>> fields in struct device to a separate sub-structure while at it) to >>>> track >>>> the "driver presence status" of the device (to be used by >>>> device_link_add()). >>>> >>>> In addition to that, the links list walks in the core.c and dd.c code >>>> are >>>> under the device links mutex now, so the iternal link spinlock is not >>>> needed >>>> any more and I have renamed symbols to distinguish between flags, link >>>> states >>>> and device "driver presence statuses". >>>> >>>> More information is there in the changelogs. >>> >>> Thanks for the update. This version is indeed easier to use and still >>> works >>> fine >>> with my Exynos IOMMU runtime pm rework. You can keep my: >>> >>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> >>> >>> I will send updated version of my patchset soon. >> >> Thanks for the testing, much appreciated! >> >> The series is in a new branch called "device-links-test" in my tree now: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git >> device-links-test > > > While working on integrating IOMMU deferred probing patches I found a bug, > which has been introduced in v4 of device dependency patchset (v3 worked > fine in this area, v5 also contains this bug). The following fixup is > needed to properly create links with DL_FLAG_PM_RUNTIME flag set during > consumer device probing: > > From: Marek Szyprowski <m.szyprowski@samsung.com> > Date: Thu, 20 Oct 2016 12:12:14 +0200 > Subject: [PATCH] driver core: fix runtime pm state for > DEVICE_LINK_CONSUMER_PROBE links > > If link is added during consumer probe with DL_FLAG_PM_RUNTIME flag set, > the code will do additional pm_runtime_put() on the supplier after > finishing consumer probing. This will break runtime pm operation for > the supplier device. To solve this issue, enforce additional call to > pm_runtime_get_sync() on the supplier device while creating such link. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/base/core.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 48bc5a362f7d..d4cc285a1df7 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -217,6 +217,9 @@ struct device_link *device_link_add(struct device > *consumer, > } > } > > + if (flags & DL_FLAG_PM_RUNTIME && status == DL_STATE_CONSUMER_PROBE) > + pm_runtime_get_sync(supplier); > + Good catch! I'd prefer to do this slightly differently, though, so I'll send an update of the runtime PM patch with this folded in shortly. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/base/core.c b/drivers/base/core.c index 48bc5a362f7d..d4cc285a1df7 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -217,6 +217,9 @@ struct device_link *device_link_add(struct device *consumer, } } + if (flags & DL_FLAG_PM_RUNTIME && status == DL_STATE_CONSUMER_PROBE) + pm_runtime_get_sync(supplier); + /* * Move the consumer and all of the devices depending on it to the end