On 14/08/18 17:17, Daniel Vetter wrote: > 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. > I would say stuck. As there is has not been any traffic regarding this patch after Lukas' reply. And I've been busy with other things since then. Perhaps I did not know to cc the right people. A reviewed-by or tested-by from someone would probably help to get this spinning again. I am happy to do changes or improvements etc., but for that I would need suggestions. As far as I can tell the patch is good to apply as it is, but then again I am no device core expert. Best regards, Jyri > 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 > > > -- 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
