From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Szyprowski Subject: Re: [RFC/RFT][PATCH v3 0/5] Functional dependencies between devices Date: Fri, 16 Sep 2016 09:57:10 +0200 Message-ID: <51bbee36-380a-6a42-db6b-959d5fec40f8@samsung.com> References: <27296716.H9VWo8ShOm@vostro.rjw.lan> <5257325.y9rG1UM74b@vostro.rjw.lan> <37bd85ce-95c9-4614-e1fa-2b189ff90d43@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mailout2.w1.samsung.com ([210.118.77.12]:38561 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752183AbcIPH5Q (ORCPT ); Fri, 16 Sep 2016 03:57:16 -0400 In-reply-to: <37bd85ce-95c9-4614-e1fa-2b189ff90d43@samsung.com> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org 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" Hi Everyone, On 2016-09-16 09:25, Marek Szyprowski wrote: > Hi Rafael, > > On 2016-09-16 00:03, Rafael J. Wysocki wrote: >> Hi Everyone, >> >> On Thursday, September 08, 2016 11:25:44 PM 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. >> An update here. >> >> The first patch hasn't changed, so I'm resending it. >> >> The majority of changes in the other patches are in order to address >> Lukas' >> comments. >> >> First off, I added a DEVICE_LINK_STATELESS flag that will prevent the >> driver >> core from trying to maintain device links having it set. >> >> Also, the DEVICE_LINK_PERSISTENT flag was dropped (as link >> "persistence" is the >> default behavior now) and there's a new one, DEVICE_LINK_AUTOREMOVE, >> that will >> cause the driver core to remove the link on the consumer driver unbind. >> >> Moreover, the code checks attempts to create a link between a parent >> and a child >> device now and actively prevents that from happening. >> >> The changelog of the second patch has been updated as requested by Ulf. >> >> The third patch was updated to fix a bug related to the (previously >> missing) >> clearing of power.direct_complete for supplier devices having >> consumers that >> don't use direct_complete. >> >> The next two (runtime PM) patches turned out to be unnecessary, so >> I've dropped >> them. >> >> The runtime PM patch [4/5] was reorganized somewhat to reduce the >> indentation >> level in there, but the code flow introduced by it is essentially the >> same >> and the last patch was simply rebased on top of the new series. >> >> If this version still works for Marek, I'll probably drop the RFC tag >> from it >> in the next iteration. > > Sadly, this version doesn't work. I get following kernel bug: > > [ 2.357622] BUG: spinlock bad magic on CPU#0, swapper/0/1 > [ 2.362361] lock: 0xeea2e294, .magic: ffffffff, .owner: /0, > .owner_cpu: -1 > [ 2.369389] CPU: 0 PID: 1 Comm: swapper/0 Not tainted > 4.8.0-rc6-00019-gd66d0028dd3c-dirty #651 > [ 2.377954] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > [ 2.384053] [] (unwind_backtrace) from [] > (show_stack+0x10/0x14) > [ 2.391766] [] (show_stack) from [] > (dump_stack+0x74/0x94) > [ 2.398970] [] (dump_stack) from [] > (do_raw_spin_lock+0x160/0x1a8) > [ 2.406870] [] (do_raw_spin_lock) from [] > (device_links_no_driver+0x64/0x98) > [ 2.415634] [] (device_links_no_driver) from [] > (driver_probe_device+0xa0/0x2bc) > [ 2.424744] [] (driver_probe_device) from [] > (__driver_attach+0xac/0xb0) > [ 2.433165] [] (__driver_attach) from [] > (bus_for_each_dev+0x54/0x88) > [ 2.441323] [] (bus_for_each_dev) from [] > (bus_add_driver+0xe8/0x1f4) > [ 2.449481] [] (bus_add_driver) from [] > (driver_register+0x78/0xf4) > [ 2.457469] [] (driver_register) from [] > (do_one_initcall+0x3c/0x16c) > [ 2.465632] [] (do_one_initcall) from [] > (kernel_init_freeable+0x120/0x1ec) > [ 2.474313] [] (kernel_init_freeable) from [] > (kernel_init+0x8/0x118) > [ 2.482470] [] (kernel_init) from [] > (ret_from_fork+0x14/0x3c) > > I'm checking what's wrong there. > The issue was caused by missing braces in device_links_no_driver() function. After fixing it the patches works fine, so you can add: Tested-by: Marek Szyprowski Rafael, how and when do you plan to merge them? I would like to know how to process further with my IOMMU patch, which is depends on your patches. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland