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

[Manikanta Maddireddy <mmaddireddy@nvidia.com>]

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