Re: [PATCH v6] amba: Remove deferred device addition

[Saravana Kannan]

On Thu, Jul 28, 2022 at 7:16 AM Russell King (Oracle)
<linux@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> 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