Messages in this thread | | | From | Saravana Kannan <> | Date | Thu, 28 Jul 2022 11:29:57 -0700 | Subject | Re: [PATCH v6] amba: Remove deferred device addition |
| |
On Thu, Jul 28, 2022 at 7:16 AM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Wed, Jul 27, 2022 at 05:10:57PM -0700, Saravana Kannan wrote: > > On Wed, Jul 27, 2022 at 3:16 PM Russell King (Oracle) > > <linux@armlinux.org.uk> wrote: > > > > > > On Wed, Jul 27, 2022 at 11:19:35AM -0700, Saravana Kannan wrote: > > > > The uevents generated for an amba device need PID and CID information > > > > that's available only when the amba device is powered on, clocked and > > > > out of reset. So, if those resources aren't available, the information > > > > can't be read to generate the uevents. To workaround this requirement, > > > > if the resources weren't available, the device addition was deferred and > > > > retried periodically. > > > > > > > > However, this deferred addition retry isn't based on resources becoming > > > > available. Instead, it's retried every 5 seconds and causes arbitrary > > > > probe delays for amba devices and their consumers. > > > > > > > > Also, maintaining a separate deferred-probe like mechanism is > > > > maintenance headache. > > > > > > > > With this commit, instead of deferring the device addition, we simply > > > > defer the generation of uevents for the device and probing of the device > > > > (because drivers needs PID and CID to match) until the PID and CID > > > > information can be read. This allows us to delete all the amba specific > > > > deferring code and also avoid the arbitrary probing delays. > > > > > > Oh, this is absolutely horrible. I can apply it cleanly to my "misc" > > > branch, but it then conflicts when I re-merge my tree for the for-next > > > thing (which is only supposed to be for sfr - the hint is in the name!) > > > for-next is basically my "fixes" plus "misc" branch and anything else I > > > want sfr to pick up for the -next tree. > > > > Btw, this is the repo I was using because I couldn't find any amba repo. > > git://git.armlinux.org.uk/~rmk/linux-arm.git > > I don't maintain a separate repo for amba stuff. > > > Is the misc branch visible somewhere because I don't see it in that > > repo? Or is that just a local branch? Also, what does sfr stand for > > (s* for next)? > > > > In case this helps, all the conflicts are due to this commit: > > 8030aa3ce12e ARM: 9207/1: amba: fix refcount underflow if > > amba_device_add() fails > > > > It's fixing bugs in code I'm deleting. So if you revert that, I can > > give you a patch that'll apply across 5.18 and 5.19. > > > > Let me know if you want me to do that. > > I dug into what had happened - the patch you mentioned patch 9207/1, > and this also applies to 9208/1 as well although that isn't relevant > for your patch. These two were merged as fixes in v5.19-rc7 via my > fixes branch. > > They also appeared for some reason in the misc branch as entirely > separate commits. Because 9207/1 appears in both, your patch applies > cleanly on misc, but when fixes is then merged, it touches the same > code and causes a conflict. > > Reverting the commit you mention won't actually fix anything - it'll > only make things much worse, with a conflict then appearing between > my tree and Linus'. > > The only sensible thing would be to delete those two duplicated > commits from the misc branch, rebase misc on v5.19-rc7 (thus picking > up those two commits that are already in mainline) and then putting > your patch on top of misc. > > This is doable, because there's five commits there, most of them are > trivially small changes, so its not a great loss in terms of the > testing that's already been done. > > That appears to work fine - I've just pushed that out with your commit > included, so should be in the final linux-next prior to the merge > window opening this Sunday.
Thanks a lot for taking care of this Russell!
-Saravana
| |