Re: [PATCH v2 2/6] PCI: brcmstb: Split brcm_pcie_setup() into two funcs

[Jim Quinlan]

On Wed, Jul 20, 2022 at 4:37 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>
> On Mon, Jul 18, 2022 at 05:40:33PM -0500, Bjorn Helgaas wrote:
> > On Mon, Jul 18, 2022 at 01:14:25PM -0500, Bjorn Helgaas wrote:
> > > ...
> >
> > > So I think brcm_pcie_setup() does initialization that doesn't depend
> > > on the link or any downstream devices, and brcm_pcie_start_link() does
> > > things that depend on the link being up.  Right?
> > >
> > > If so, "start_link" might be a slight misnomer since AFAICT
> > > brcm_pcie_start_link() doesn't do anything to initiate link-up except
> > > maybe deasserting fundamental reset.  Some drivers start the LTSSM or
> > > explicitly enable link training, but brcm_pcie_start_link() doesn't
> > > seem to do anything like that.
> > >
> > > brcm_pcie_start_link() still does brcm_pcie_set_outbound_win().  Does
> > > that really depend on the link being up?  If that only affects the
> > > Root Port, maybe it could be done before link-up?
> >
> > What about the /* PCIe->SCB endian mode for BAR */ thing?  Does that
> > depend on the link being up?
> >
> > And the "Refclk from RC should be gated with CLKREQ#" part?  Does that
> > depend on the link being up?
> >
> > It seems obvious that brcm_pcie_set_ssc() and reading the negotiated
> > link speed and width depend on the link being up.
>
> Can we close on this?  Splitting into
Absolutely.

>
>   (a) stuff that can be initialized before the link is available and
>   (b) stuff that depends on the link
>
> makes good sense, but then (b) should only contain stuff that actually
> depends on the link.
>
> The "PCIe->SCB endian mode for BAR" *sounds* like something related to
> the primary side of the RP, not the link.
>
> Not sure about "Refclk from RC".  RC would certainly be primary side,
> but ASPM has to do with secondary (link) side.

I get the feedback, submission coming soon -- I was waiting for the
email thread to conclude.

Regards,
Jim Quinlan
Broadcom STB


>
> Bjorn

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature