On Tue, Jan 4, 2022 at 9:18 AM Lorenzo Pieralisi
<lorenzo.pieralisi@xxxxxxx> wrote:
>
> On Fri, Dec 10, 2021 at 12:31:10PM -0800, Florian Fainelli wrote:
> > On 12/10/21 10:44 AM, Rob Herring wrote:
> > > On Thu, Dec 09, 2021 at 04:13:58PM -0500, Jim Quinlan wrote:
> > >> v10 -- Bindings commit example: in comment, refer to bridge under
> > >> controller node as a root port. (Pali)
> > >> -- Bindings commit example: remove three properties that are not
> > >> appropriate for a PCIe endpoint node. (Rob)
> > >>
> > >> v9 -- Simplify where this mechanism works: instead of looking for
> > >> regulators below every bridge, just look for them at the
> > >> bridge under the root bus (root port). Now there is no
> > >> modification of portdrv_{pci,core}.c in this submission.
> > >> -- Although Pali is working on support for probing native
> > >> PCIe controller drivers, this work may take some time to
> > >> implement and it still might not be able to accomodate
> > >> our driver's requirements (e.g. vreg suspend/resume control).
> > >> -- Move regulator suspend/resume control to Brcm RC driver. It
> > >> must reside there because (a) in order to know when to
> > >> initiate linkup during resume and (b) to turn on the
> > >> regulators before any config-space accesses occur.
> > >
> > > You now have a mixture of 'generic' add/remove_bus hooks and the host
> > > controller suspend/resume managing the regulators. I think long term,
> > > the portdrv is going to be the right place for all of this with some
> > > interface defined for link control. So I think this solution moves
> > > sideways rather than towards anything common.
> > >
> > > Unfortunately, the only leverage maintainers have to get folks to care
> > > about any refactoring is to reject features. We're lucky to find anyone
> > > to test refactoring when posted if done independently. There's a long
> > > list of commits of PCI hosts that I've broken to prove that. So it's
> > > up to Lorenzo and Bjorn on what they want to do here.
> >
> > After version 10, it would seem pretty clear that we are still very much
> > committed to and interested in getting that set merged and do it the
> > most acceptable way possible. Common code with a single user is always a
> > little bit of a grey area to me as it tends to be developed to cater for
> > the specific needs of that single user, so the entire common aspect is
> > debatable. I suppose as long as we have the binding right, the code can
> > change at will.
> >
> > Not trying to coerce Bjorn and Lorenzo into accepting these patches if
> > they don't feel comfortable, but what about getting it included so we
> > can sort of move on from that topic for a little bit (as we have other
> > PCIe changes coming in, supporting additional chips etc.) and we work
> > with Pali on a common solution and ensure it works on our pcie-brcmstb.c
> > based devices? We are not going to vanish and not come back looking at this.
>
> Sorry for being late on reviewing this set. I agree with both of you.
>
> I don't think Bjorn had a chance to have a look at patch (4) now I am
> delegating it to him; I am not very keen on adding functionality to PCI
> core where it is still a question whether it can be reused by other
> drivers (forgive me if I missed some details on previous review
> versions).
>
> Is it possible to keep patch (4) brcmstb specific (ie keep the code
> out of PCI core for now), we then merge this series and help Pali
> implement a generic version based on Rob's suggestion ?
>
> Just let me know please, thanks.
Hi Lorenzo,
That sounds good to me. Pullreq coming tomorrow.
Thanks,
Jim Quinlan
Broadcom STB
>
> Lorenzo
