Re: [PATCH 0/4] PCI: brcmstb: Revert subdevice regulator stuff

[Jim Quinlan <jim2101024@gmail.com>]

On Mon, Jun 13, 2022 at 8:00 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Jun 13, 2022 at 10:06:12AM -0700, Florian Fainelli wrote:
> > On 5/11/22 13:39, Bjorn Helgaas wrote:
> > > On Wed, May 11, 2022 at 01:24:55PM -0700, Florian Fainelli wrote:
> > > > On 5/11/22 13:18, Bjorn Helgaas wrote:
> > > > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > > >
> > > > > Cyril reported that 830aa6f29f07 ("PCI: brcmstb: Split brcm_pcie_setup()
> > > > > into two funcs"), which appeared in v5.17-rc1, broke booting on the
> > > > > Raspberry Pi Compute Module 4.  Revert 830aa6f29f07 and subsequent patches
> > > > > for now.
> > > >
> > > > How about we get a chance to fix this? Where, when and how was this even
> > > > reported?
> > >
> > > Sorry, I forgot to cc you, that's my fault:
> > >    https://lore.kernel.org/r/CABhMZUWjZCwK1_qT2ghTSu2dguJBzBTpiTqKohyA72OSGMsaeg@mail.gmail.com
> > >
> > > If you come up with a fix, I'll drop the reverts, of course.
>
> > What is even better is that meanwhile there was already a candidate fix
> > proposed on May 18th, and a v2 on May 28th, so still an alternative to the
> > reverts making it to Linus' tree, or so I thought.
>
> I hoped for a fix, but neither of those seemed to be clearly better.
>
> > - the history for pcie-brcmstb.c is now looking super ugly because we have 4
> > commits getting reverted and if we were to add back the original feature
> > being added now what? Do we come up with reverts of reverts, or the modified
> > (with the fix) original commits applied on top, are not we going to sign
> > ourselves for another 13 or so round of patches before we all agree on the
> > solution?
>
> I agree on the ugliness and I try hard to avoid that.  In this case I
> waited too long after the regression was discovered, hoping for a fix
> that was better than the revert.  And I should have asked for
> trade-offs between the revert and the the CM4 regression.
>
> > - we could have just fixed this with proper communication from the get go
> > about the regression in the first place, which remains the failure in
> > communicating appropriately with driver authors/maintainers
>
> I apologized earlier for omitting you when the regression was
> discovered, and I'm still sorry.
>
> > I appreciate that as a maintainer you are very sensitive to regressions and
> > want to be responsive and responsible but this is not leaving just a
> > slightest chance to right a wrong. Can we not do that again please?
>
> Cyril opened the bugzilla April 30 and I forwarded it to linux-pci and
> to Jim (but not you; again, I'm sorry for that omission) on May 2.
> From my perspective we had almost a month to push this forward, but we
> didn't quite make it.
Hello Bjorn,

Can you elaborate this? On May 18 I submitted v1, a viable fix.
At no point did you say "you need to get v2 in ASAP because I am planning on
reverting the entire original patchset in N days".  If I had known
this was the situation,
I could have had you a v2 on May 19th, but as it was I let the v1
email review thread
die out before submitting v2.

The original patchset was and is controversial, as it is basically a square peg
that does not fit nicely into a round Linux hole. It took 11 versions
of following reviewers'
suggestions until it was accepted.  And now it has been reverted, I am
wondering if it will ever be let in again or whether I should even try.

Regards,
Jim Quinlan
Broadcom STB


>
> I posted the reverts May 11, but I did not realize the regression to
> you and other users they would cause.  I apologize for that.
>
> Bjorn