Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220727181936.3250466-1-saravanak@google.com>
 <YuG5KCksLWzThSmF@shell.armlinux.org.uk> <CAGETcx9HFQa=58XUQ07FVQ_Qqem2La4YcR8_qDbg4uqCT8w3CQ@mail.gmail.com>
 <YuKaI5jJOZ8WQQPU@shell.armlinux.org.uk>
In-Reply-To: <YuKaI5jJOZ8WQQPU@shell.armlinux.org.uk>
From:   Saravana Kannan <saravanak@google.com>
Date:   Thu, 28 Jul 2022 11:29:57 -0700
Message-ID: <CAGETcx8g1Sgs8Ev4Ab1E-fEGzo=Rx3Kx9WiT-3ekpQv2BmR_Qg@mail.gmail.com>
Subject: Re: [PATCH v6] amba: Remove deferred device addition
To:     "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc:     Philipp Zabel <p.zabel@pengutronix.de>,
        Rob Herring <robh@kernel.org>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Nicolas Saenz Julienne <nsaenz@kernel.org>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        Kefeng Wang <wangkefeng.wang@huawei.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        patches@armlinux.org.uk, kernel-team@android.com,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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