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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z6XhVL_OckIOnqvV@ryzen>
Date: Fri, 7 Feb 2025 11:32:52 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Thierry Reding <treding@...dia.com>
Cc: Vidya Sagar <vidyas@...dia.com>, lpieralisi@...nel.org, kw@...ux.com,
	manivannan.sadhasivam@...aro.org, robh@...nel.org,
	bhelgaas@...gle.com, quic_schintav@...cinc.com,
	johan+linaro@...nel.org, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, jonathanh@...dia.com,
	kthota@...dia.com, mmaddireddy@...dia.com, sagar.tv@...il.com
Subject: Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in
 Tegra234 Platforms

On Tue, Feb 04, 2025 at 06:12:56PM +0100, Thierry Reding wrote:
> 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")
> 
> Typically we build the default configuration with some custom options
> (like everyone else, I assume) and usually in those configurations both
> Tegra194 and Tegra234 support will be enabled, so the code ends up
> enabled in most cases. I guess the commit message doesn't do a very good
> job of making this clear. Really what this commit does is enable the PCI
> controller driver for Tegra234-only configurations (i.e. no other Tegra
> generations are built-in).

Ok, fine by me.


> 
> Not sure about the Fixes: tag since this is fairly harmless. Worst case
> you'll need to enable Tegra194 support along with Tegra234 in order to
> be able to enable this driver, but that's almost always the case anyway.

I think it is quite a fundamental mistake that the commit that added
support for Tegra234, actually requires you to enabled support for a
completely different SoC to actually make use of that driver, so IMO
the Fixes tag is absolutely warrented.


Kind regards,
Niklas