On Mon, Feb 26, 2018 at 8:52 AM, Jyri Sarha <jsarha@xxxxxx> wrote: > On 25/02/18 11:22, Lukas Wunner wrote: >> On Thu, Feb 22, 2018 at 07:42:46PM +0200, Jyri Sarha wrote: >>> Put consumer device to deferred probe list if it is unbound due to a >>> dropped link to a supplier. >>> >>> When a device link supplier is unbound (either manually or because one >>> of its own suppliers was unbound), its consumers are unbound as >>> well. Currently if the supplier binds again after this the consumer >>> does not automatically probe again. With this patch it does. >> >> Yes I think this makes sense, based on the rationale that the consumer >> was automatically unbound, so by symmetry it should also be automatically >> rebound. >> >> The only thing I don't understand is you wrote in an earlier e-mail of a >> difference in behavior depending on whether driver_deferred_probe_add() >> is called before or after device_release_driver_internal(). >> That's really odd, it shouldn't make a difference. >> > > In that version there was a couple of other bugs elsewhere in the > system. I tried to reproduce that situation again multiple times, but I > could not (even write those bugs back in there, and move the link > creation back to panel bridge code). But even from reading the code that > difference did not make any sense. I suspect a heisen-bug, after I have > read the code and understood that the crash is not possible, it does not > happen anymore :). > > With the current version I could not find any difference in behaviour > depending on the order of device_release_driver_internal() and > driver_deferred_probe_add() calls. I just thought having them in this > order just lookes nicer. > > I stress tested the code by unloading and loading the panel driver in a > tight loop, for several minutes, but it simply wont crash (not in this > setup anyway). The probe of tilcdc eventually gracefully fails in a CMA > failure. Is this still moving or stuck? Pinging since Sean just brought up the drm_bridge lifetime issues, and I think it'd be nice if we could solve those by using device_link, like we've added already for drm_panel. >From my very layman understanding of the discussions, device_link not quite working for deferred probing is the blocker for this plan. Also adding Sean. -Daniel > > Best regards, > Jyri > >> Thanks, >> >> Lukas >> >>> >>> If this patch is not acceptable as such, how about adding this >>> behavior behind a new device link flag? >>> >>> The idea to this patch was gotten from this post by Lucas Wunner: >>> https://www.spinics.net/lists/dri-devel/msg166318.html >>> >>> Part of the code and the description is borrowed from him. >>> >>> cc: Lukas Wunner <lukas@xxxxxxxxx> >>> cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> >>> cc: Thierry Reding <thierry.reding@xxxxxxxxx> >>> Signed-off-by: Jyri Sarha <jsarha@xxxxxx> >>> --- >>> drivers/base/base.h | 1 + >>> drivers/base/core.c | 2 ++ >>> drivers/base/dd.c | 2 +- >>> 3 files changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/base/base.h b/drivers/base/base.h >>> index d800de6..39370eb 100644 >>> --- a/drivers/base/base.h >>> +++ b/drivers/base/base.h >>> @@ -114,6 +114,7 @@ extern void device_release_driver_internal(struct device *dev, >>> >>> extern void driver_detach(struct device_driver *drv); >>> extern int driver_probe_device(struct device_driver *drv, struct device *dev); >>> +extern void driver_deferred_probe_add(struct device *dev); >>> extern void driver_deferred_probe_del(struct device *dev); >>> static inline int driver_match_device(struct device_driver *drv, >>> struct device *dev) >>> diff --git a/drivers/base/core.c b/drivers/base/core.c >>> index b2261f9..0964ed5 100644 >>> --- a/drivers/base/core.c >>> +++ b/drivers/base/core.c >>> @@ -570,6 +570,8 @@ void device_links_unbind_consumers(struct device *dev) >>> >>> device_release_driver_internal(consumer, NULL, >>> consumer->parent); >>> + driver_deferred_probe_add(consumer); >>> + >>> put_device(consumer); >>> goto start; >>> } >>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c >>> index de6fd09..846ae78 100644 >>> --- a/drivers/base/dd.c >>> +++ b/drivers/base/dd.c >>> @@ -140,7 +140,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)) { > > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
