Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms

[Manivannan Sadhasivam]

On Tue, Jan 28, 2025 at 01:04:32PM +0100, Niklas Cassel wrote:
> Hello Vidya,
> 
> On Tue, Jan 28, 2025 at 10:12:44AM +0530, Vidya Sagar wrote:
> > Add PCIe RC & EP support for Tegra234 Platforms.
> 
> The commit log does leave quite a few questions unanswered.
> 
> Since you are just updating the Kconfig and nothing else:
> Does the DT binding already have support for the Tegra234 SoC?
> Does the driver already have support for the Tegra234 SoC?
> 
> Looking at the DT binding and driver, the answer to both questions
> is yes. (This should have been in the commit message IMO.)
> 
> 
> But that leads me to the question, since there is support for Tegra234
> SoC in the driver, does this means that this fixes a regression, e.g.
> the Kconfig ARCH_TEGRA_234_SOC was added after the driver support in
> this driver was added. In this case, you should have a Fixes: tag that
> points to the commit that added ARCH_TEGRA_234_SOC.
> 
> Or has the the driver support for Tegra234 been "dead-code" since it
> was originally added? (Because without this patch, no one can have
> tested it, at least not without COMPILE_TEST.)
> In this case, you should add:
> Fixes: a54e19073718 ("PCI: tegra194: Add Tegra234 PCIe support")
> 

TBH, I don't like muddling with Kconfig like this. Ideally, the driver should
just depend on ARCH_TEGRA || COMPILE_TEST and the driver should be selected by
the relevant defconfig.

And this is what all other rest of the platforms are doing. Why should Nvidia be
different? It makes me feel that this Kconfig dependency is used as a workaround
for defconfig updates.

- Mani

-- 
மணிவண்ணன் சதாசிவம்