From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757448AbcIMJ7F (ORCPT ); Tue, 13 Sep 2016 05:59:05 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:37734 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755509AbcIMJ7D (ORCPT ); Tue, 13 Sep 2016 05:59:03 -0400 X-AuditID: cbfec7f5-f79ce6d000004c54-28-57d7cde26a8d Subject: Re: [RFC/RFT][PATCH v2 0/7] Functional dependencies between devices To: "Rafael J. Wysocki" , Linux PM list Cc: Greg Kroah-Hartman , Alan Stern , Linux Kernel Mailing List , Tomeu Vizoso , Mark Brown , Lukas Wunner , Kevin Hilman , Ulf Hansson , "Luis R. Rodriguez" From: Marek Szyprowski Message-id: <291f428a-da27-1c69-b0fb-c78f7a4665eb@samsung.com> Date: Tue, 13 Sep 2016 11:58:56 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-version: 1.0 In-reply-to: <27296716.H9VWo8ShOm@vostro.rjw.lan> Content-type: text/plain; charset=utf-8; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrNKsWRmVeSWpSXmKPExsWy7djPc7qPzl4PN9jar29xu30+k0Xz4vVs Fk83P2ayuLxrDpvF594jjBYvnktbPD+5l9nizOlLrBYTfl9gs+hbe4nN4vjacAdujx13lzB6 zJ+3jdFj06pONo871/aweeyfu4bdY8vVdhaP2Xd/MHqs33KVxePzJjmPGe3bWAO4orhsUlJz MstSi/TtErgy3i3azFKwy6zi4ONrTA2Mm7W7GDk5JARMJH52TWOHsMUkLtxbz9bFyMUhJLCU UWLtis8sEM5nRonFi1vZYTr+TbnFDpFYxiixZcp3JgjnOaNEw5ydLCBVwgI+Eo8+TmECsUUE wiWObVrFCmIzC/xiknizLwHEZhMwlOh628UGYvMK2En8eLIDzGYRUJXYd2AamC0qECNx98ZZ qBpBiR+T74HN5wTqfXHhMRPETCuJZ/9aoebLS2xe85YZ4tKX7BKz7th2MXIA2bISmw5AhV0k FvTvZ4WwhSVeHd8C9ZiMxOXJ3SwQdj+jRFMrNIhmMEqce8sLYVtLHD5+EWoVn8SkbdOZIcbz SnS0CUGUeEisaz3EBGE7SvTtnA0NnhZGiRMtz5gmMMrPQvLNLCQfzELywQJG5lWMIqmlxbnp qcWmesWJucWleel6yfm5mxiBSer0v+NfdzAuPWZ1iFGAg1GJh7dh9bVwIdbEsuLK3EOMEhzM SiK8nWeuhwvxpiRWVqUW5ccXleakFh9ilOZgURLn3bPgSriQQHpiSWp2ampBahFMlomDU6qB 0d90z8wfnt0/LxhqHix2ln7otvwn+5awBHfu5quCkfbLbe+8iLvUo9T5PVVo/QbpGWWaR107 w/c4Hg3vcmxsMArZ4+/0z9zvRWPiLZ3pu7JcejYaTyyKuZF4LqXG1mxB8K1NL+Qrdb9Irriz +9k0tdBkpZ6O+xfeJIrct65dqPGQcarbuV2zlFiKMxINtZiLihMBf/7A0E4DAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrNIsWRmVeSWpSXmKPExsVy+t/xy7obz14PN5j6UtPidvt8JovmxevZ LJ5ufsxkcXnXHDaLz71HGC1ePJe2eH5yL7PFmdOXWC0m/L7AZtG39hKbxfG14Q7cHjvuLmH0 mD9vG6PHplWdbB53ru1h89g/dw27x5ar7Swes+/+YPRYv+Uqi8fnTXIeM9q3sQZwRbnZZKQm pqQWKaTmJeenZOal2yqFhrjpWigp5CXmptoqRej6hgQpKZQl5pQCeUYGaMDBOcA9WEnfLsEt 492izSwFu8wqDj6+xtTAuFm7i5GTQ0LAROLflFvsELaYxIV769m6GLk4hASWMEo0b37ACuE8 Z5TY+HcHG0iVsICPxKOPU5hAbBGBcIlfJ36wdDFyABUZSFzs5gOpZxb4xSTxccJPZpAaNgFD ia63XWC9vAJ2Ej+eQMxhEVCV2HdgGhtIr6hAjMT6vgSIEkGJH5PvsYDYnECtLy48BlvFLGAm 8eXlYVYIW15i85q3zBMYBWYhaZmFpGwWkrIFjMyrGEVSS4tz03OLjfSKE3OLS/PS9ZLzczcx AiN227GfW3Ywdr0LPsQowMGoxMPbsPpauBBrYllxZe4hRgkOZiUR3s4z18OFeFMSK6tSi/Lj i0pzUosPMZoC/TCRWUo0OR+YTPJK4g1NDM0tDY2MLSzMjYyUxHmnfrgSLiSQnliSmp2aWpBa BNPHxMEp1cB4piWF66KLPY9hV26Cte33HxxrPH41+DsV1+ZW7bpluIXnUtiteSd+tMYtZGVm tJO8uIk7aF3zjqYTvwSnZrWIvld5ynKO0TzI9rv9/Qe2wmfn6V/Ri738XfVr7ruPX98v8ZpU VWX4gJfLaGXhezY/PoHrd86JBexYPpO5Nzv3de8dwWoJWQklluKMREMt5qLiRADjjdEm7gIA AA== X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20160913095858eucas1p267ec2397c9e4577f94557e4a38498164 X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 X-Local-Sender: =?UTF-8?B?TWFyZWsgU3p5cHJvd3NraRtTUlBPTC1LZXJuZWwgKFRQKRs=?= =?UTF-8?B?7IK87ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?TWFyZWsgU3p5cHJvd3NraRtTUlBPTC1LZXJuZWwgKFRQKRtT?= =?UTF-8?B?YW1zdW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20160913095858eucas1p267ec2397c9e4577f94557e4a38498164 X-RootMTR: 20160913095858eucas1p267ec2397c9e4577f94557e4a38498164 References: <27296716.H9VWo8ShOm@vostro.rjw.lan> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Rafael, On 2016-09-08 23:25, Rafael J. Wysocki wrote: > Hi Everyone, > > This is a refresh of the functional dependencies series that I posted last > year and which has picked up by Marek quite recently. For reference, appended > is my introductory message sent previously (which may be slightly outdated now). > > As last time, the first patch rearranges the code around __device_release_driver() > a bit to prepare it for the next one (it actually hasn't changed AFAICS). > > The second patch introduces the actual device links mechanics, but without > system suspend/resume and runtime PM support which are added by the subsequent > patches. > > Some bugs found by Marek during his work on these patches should be fixed > here. In particular, the endless recursion in device_reorder_to_tail() > which simply was broken before. > > There are two additional patches to address the issue with runtime PM support > that occured when runtime PM was disabled for some suppliers due to a PM > sleep transition in progress. Those patches simply make runtime PM helpers > return 0 in that case which may be controversial, so please let me know if > there are concerns about those. > > The way device_link_add() works is a bit different, as it takes an additional > status argument now. That makes it possible to create a link in any state, > with extra care of course, and should address the problem pointed to by Lukas > during the previous discussion. > > Also some comments from Tomeu have been addressed. > > This hasn't been really tested yet and I'm sort of relying on Marek to test > it, because he has a use case ready. Hence, the RFT tag on the series. I've checked it with my updated Exynos SYSMMU patches and it works really well. Both runtime pm and system sleep. It took some time to do the test, because this patchset changed somehow the probe order of the devices, what revealed some issues related to incorrect driver and device registration in Samsung Exynos FIMC-IS driver, but those were not related to Rafael's changes. I will post my patches in a few minutes. Tested-by: Marek Szyprowski Rafael, BTW, didn't you plan to change the name of the device_link_add() function to device_dependency_add() to avoid confusion with network device "link"? > Overall, please let me know what you think. > > Thanks, > Rafael > > > Introduction: > > As discussed in the recent "On-demand device probing" thread and in a Kernel > Summit session earlier today, there is a problem with handling cases where > functional dependencies between devices are involved. > > What I mean by a "functional dependency" is when the driver of device B needs > both device A and its driver to be present and functional to be able to work. > This implies that the driver of A needs to be working for B to be probed > successfully and it cannot be unbound from the device before the B's driver. > This also has certain consequences for power management of these devices > (suspend/resume and runtime PM ordering). > > So I want to be able to represent those functional dependencies between devices > and I'd like the driver core to track them and act on them in certain cases > where they matter. The argument for doing that in the driver core is that > there are quite a few distinct use cases related to that, they are relatively > hard to get right in a driver (if one wants to address all of them properly) > and it only gets worse if multiplied by the number of drivers potentially > needing to do it. Morever, at least one case (asynchronous system suspend/resume) > cannot be handled in a single driver at all, because it requires the driver of A > to wait for B to suspend (during system suspend) and the driver of B to wait for > A to resume (during system resume). > > My idea is to represent a supplier-consumer dependency between devices (or > more precisely between device+driver combos) as a "link" object containing > pointers to the devices in question, a list node for each of them and some > additional information related to the management of those objects, ie. > something like: > > struct device_link { > struct device *supplier; > struct list_head supplier_node; > struct device *consumer; > struct list_head consumer_node; > > }; > > In general, there will be two lists of those things per device, one list > of links to consumers and one list of links to suppliers. > > In that picture, links will be created by calling, say: > > int device_add_link(struct device *me, struct device *my_supplier, unsigned int flags); > > and they will be deleted by the driver core when not needed any more. The > creation of a link should also cause dpm_list and the list used during shutdown > to be reordered if needed. > > In principle, it seems usefult to consider two types of links, one created > at device registration time (when registering the second device from the linked > pair, whichever it is) and one created at probe time (of the consumer device). > I'll refer to them as "permanent" and "probe-time" links, respectively. > > The permanent links (created at device registration time) will stay around > until one of the linked devices is unregistered (at which time the driver > core will drop the link along with the device going away). The probe-time > ones will be dropped (automatically) at the consumer device driver unbind time. > > There's a question about what if the supplier device is being unbound before > the consumer one (for example, as a result of a hotplug event). My current > view on that is that the consumer needs to be force-unbound in that case too, > but I guess I may be persuaded otherwise given sufficiently convincing > arguments. Anyway, there are reasons to do that, like for example it may > help with the synchronization. Namely, if there's a rule that suppliers > cannot be unbound before any consumers linked to them, than the list of links > to suppliers for a consumer can only change at its registration/probe or > unbind/remove times (which simplifies things quite a bit). > > With that, the permanent links existing at the probe time for a consumer > device can be used to check whether or not to defer the probing of it > even before executing its probe callback. In turn, system suspend > synchronization should be a matter of calling device_pm_wait_for_dev() > for all consumers of a supplier device, in analogy with dpm_wait_for_children(), > and so on. > > Of course, the new lists have to be stable during those operations and ensuring > that is going to be somewhat tricky (AFAICS right now at least), but apart from > that the whole concept looks reasonably straightforward to me. > > > Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland