Re: [PATCH v7 3/4] PCI: tegra: Add Tegra264 support

[Manikanta Maddireddy]

On 30/06/26 1:33 pm, Manivannan Sadhasivam wrote:
> > > > +static void tegra264_pcie_remove(struct platform_device *pdev)
> > > > +{
> > > > +	struct tegra264_pcie *pcie = platform_get_drvdata(pdev);
> > > > +
> > > > +	/*
> > > > +	 * If we undo tegra264_pcie_init() then link goes down and need
> > > > +	 * controller reset to bring up the link again. Remove intention is
> > > > +	 * to clean up the root bridge and re-enumerate during bind.
> > > But the controller will be consuming power even if PCIe is not used. Do you
> > > really want that? Can't tegra264_pcie_init() handle the initialization? I'm
> > > wondering how tegra264_pcie_deinit() in tegra264_pcie_suspend() works then.
> > I had to clarify this with the PCI team and they indicated that
> > tegra264_pcie_deinit() is actually useless and maybe even harmful. The
> > reason is that there's a processor on these boards (BPMP) that takes
> > care of power sequencing and it will automatically take the PCI links
> > to L2 on suspend and assert PERST#.
> Then why are you calling tegra264_pcie_deinit() in tegra264_pcie_suspend()? If
> tegra264_pcie_deinit() is harmful, then calling it during suspend should also
> be, right?
>
> Or tegra264_pcie_deinit() has to be paired with BPMP doing its own power
> sequencing?
>
> Not a big deal, but it just feels weird to see suspend() and remove() doing
> different things.

tegra264_pcie_deinit() should be removed in tegra264_pcie_suspend(), 
BPMP-FW takes care of L2+assert PERST# sequence during suspend.

- Manikanta
--
nvpublic