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

["Russell King (Oracle)" <linux@armlinux.org.uk>]

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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!