Message-ID: <CAKMK7uGiYbRCS=eGLPn3XW29VvX3yo5TZVObc5LJJvZf2eswAQ@mail.gmail.com>
Date:   Tue, 14 Aug 2018 16:17:29 +0200
From:   Daniel Vetter <daniel@...ll.ch>
To:     Jyri Sarha <jsarha@...com>, Sean Paul <seanpaul@...omium.org>
Cc:     Lukas Wunner <lukas@...ner.de>,
        Greg KH <gregkh@...uxfoundation.org>,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Tomi Valkeinen <tomi.valkeinen@...com>,
        Thierry Reding <thierry.reding@...il.com>
Subject: Re: [PATCH RFC] driver core: Reprobe consumer if it was unbound by
 dropped device_link

On Mon, Feb 26, 2018 at 8:52 AM, Jyri Sarha <jsarha@...com> 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@...ner.de>
>>> cc: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>>> cc: Thierry Reding <thierry.reding@...il.com>
>>> Signed-off-by: Jyri Sarha <jsarha@...com>
>>> ---
>>>  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@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Powered by blists - more mailing lists