* [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms @ 2025-01-28 4:42 Vidya Sagar 2025-01-28 12:04 ` Niklas Cassel 2025-04-10 19:45 ` [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency Vidya Sagar 0 siblings, 2 replies; 15+ messages in thread From: Vidya Sagar @ 2025-01-28 4:42 UTC (permalink / raw) To: lpieralisi, kw, manivannan.sadhasivam, robh, bhelgaas, quic_schintav, johan+linaro, cassel Cc: linux-pci, linux-kernel, treding, jonathanh, kthota, mmaddireddy, vidyas, sagar.tv Add PCIe RC & EP support for Tegra234 Platforms. Signed-off-by: Vidya Sagar <vidyas@nvidia.com> --- drivers/pci/controller/dwc/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig index b6d6778b0698..6dd232cf8064 100644 --- a/drivers/pci/controller/dwc/Kconfig +++ b/drivers/pci/controller/dwc/Kconfig @@ -205,7 +205,7 @@ config PCIE_TEGRA194 config PCIE_TEGRA194_HOST tristate "NVIDIA Tegra194 (and later) PCIe controller (host mode)" - depends on ARCH_TEGRA_194_SOC || COMPILE_TEST + depends on ARCH_TEGRA_194_SOC || ARCH_TEGRA_234_SOC || COMPILE_TEST depends on PCI_MSI select PCIE_DW_HOST select PHY_TEGRA194_P2U @@ -220,7 +220,7 @@ config PCIE_TEGRA194_HOST config PCIE_TEGRA194_EP tristate "NVIDIA Tegra194 (and later) PCIe controller (endpoint mode)" - depends on ARCH_TEGRA_194_SOC || COMPILE_TEST + depends on ARCH_TEGRA_194_SOC || ARCH_TEGRA_234_SOC || COMPILE_TEST depends on PCI_ENDPOINT select PCIE_DW_EP select PHY_TEGRA194_P2U -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms 2025-01-28 4:42 [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms Vidya Sagar @ 2025-01-28 12:04 ` Niklas Cassel 2025-02-03 16:59 ` Manivannan Sadhasivam ` (2 more replies) 2025-04-10 19:45 ` [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency Vidya Sagar 1 sibling, 3 replies; 15+ messages in thread From: Niklas Cassel @ 2025-01-28 12:04 UTC (permalink / raw) To: Vidya Sagar Cc: lpieralisi, kw, manivannan.sadhasivam, robh, bhelgaas, quic_schintav, johan+linaro, linux-pci, linux-kernel, treding, jonathanh, kthota, mmaddireddy, sagar.tv 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") Kind regards, Niklas ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms 2025-01-28 12:04 ` Niklas Cassel @ 2025-02-03 16:59 ` Manivannan Sadhasivam 2025-02-04 17:19 ` Thierry Reding 2025-02-04 17:12 ` Thierry Reding [not found] ` <BN9PR12MB5323E59E530FA1F87DE3C7FCB8F22@BN9PR12MB5323.namprd12.prod.outlook.com> 2 siblings, 1 reply; 15+ messages in thread From: Manivannan Sadhasivam @ 2025-02-03 16:59 UTC (permalink / raw) To: Niklas Cassel Cc: Vidya Sagar, lpieralisi, kw, robh, bhelgaas, quic_schintav, johan+linaro, linux-pci, linux-kernel, treding, jonathanh, kthota, mmaddireddy, sagar.tv 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 -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms 2025-02-03 16:59 ` Manivannan Sadhasivam @ 2025-02-04 17:19 ` Thierry Reding 2025-02-07 10:41 ` Niklas Cassel 2025-02-10 17:49 ` Manivannan Sadhasivam 0 siblings, 2 replies; 15+ messages in thread From: Thierry Reding @ 2025-02-04 17:19 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: Niklas Cassel, Vidya Sagar, lpieralisi, kw, robh, bhelgaas, quic_schintav, johan+linaro, linux-pci, linux-kernel, jonathanh, kthota, mmaddireddy, sagar.tv [-- Attachment #1: Type: text/plain, Size: 2695 bytes --] On Mon, Feb 03, 2025 at 10:29:32PM +0530, Manivannan Sadhasivam 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") > > > > 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. ARCH_TEGRA is a symbol that exists both on 32-bit and 64-bit ARM. This driver is completely useless on 32-bit ARM and only used on a very small subset of 64-bit ARM devices. It doesn't make sense to be able to enable this if you want to build a kernel for say Tegra210. The relevant defconfig in this case would be the arm64 defconfig, which isn't very authoritative. > 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. Well, it's certainly not used as a workaround for defconfig updates because the change itself doesn't enable this symbol. You'd still need a defconfig change to do that. Also, we do this primarily because we've always done things this way on Tegra. As I said, for a lot of drivers it doesn't make sense to include them in a 32-bit build or 64-bit build because the hardware simply doesn't exist. Having per-SoC generation Kconfig symbols allows this to be modelled more accurately and if desired to build compact images. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms 2025-02-04 17:19 ` Thierry Reding @ 2025-02-07 10:41 ` Niklas Cassel 2025-02-10 17:49 ` Manivannan Sadhasivam 1 sibling, 0 replies; 15+ messages in thread From: Niklas Cassel @ 2025-02-07 10:41 UTC (permalink / raw) To: Thierry Reding Cc: Manivannan Sadhasivam, Vidya Sagar, lpieralisi, kw, robh, bhelgaas, quic_schintav, johan+linaro, linux-pci, linux-kernel, jonathanh, kthota, mmaddireddy, sagar.tv On Tue, Feb 04, 2025 at 06:19:51PM +0100, Thierry Reding wrote: > On Mon, Feb 03, 2025 at 10:29:32PM +0530, Manivannan Sadhasivam 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") > > > > > > > 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. > > ARCH_TEGRA is a symbol that exists both on 32-bit and 64-bit ARM. This > driver is completely useless on 32-bit ARM and only used on a very small > subset of 64-bit ARM devices. It doesn't make sense to be able to enable > this if you want to build a kernel for say Tegra210. Well, if you look in drivers/pci/controller/dwc/Kconfig there are quite a few drivers that does: depends on ARM64 or ARM and then a ARCH_ something. I don't see why you can't do depends on (ARCH_TEGRA && ARM64) || COMPILE_TEST Kind regards, Niklas ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms 2025-02-04 17:19 ` Thierry Reding 2025-02-07 10:41 ` Niklas Cassel @ 2025-02-10 17:49 ` Manivannan Sadhasivam 1 sibling, 0 replies; 15+ messages in thread From: Manivannan Sadhasivam @ 2025-02-10 17:49 UTC (permalink / raw) To: Thierry Reding Cc: Niklas Cassel, Vidya Sagar, lpieralisi, kw, robh, bhelgaas, quic_schintav, johan+linaro, linux-pci, linux-kernel, jonathanh, kthota, mmaddireddy, sagar.tv On Tue, Feb 04, 2025 at 06:19:51PM +0100, Thierry Reding wrote: > On Mon, Feb 03, 2025 at 10:29:32PM +0530, Manivannan Sadhasivam 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") > > > > > > > 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. > > ARCH_TEGRA is a symbol that exists both on 32-bit and 64-bit ARM. This > driver is completely useless on 32-bit ARM and only used on a very small > subset of 64-bit ARM devices. It doesn't make sense to be able to enable > this if you want to build a kernel for say Tegra210. > As Niklas pointed out, why can't you have (ARCH_TEGRA && ARM) in Kconfig? > The relevant defconfig in this case would be the arm64 defconfig, which > isn't very authoritative. > > > 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. > > Well, it's certainly not used as a workaround for defconfig updates > because the change itself doesn't enable this symbol. You'd still need a > defconfig change to do that. > > Also, we do this primarily because we've always done things this way on > Tegra. As I said, for a lot of drivers it doesn't make sense to include > them in a 32-bit build or 64-bit build because the hardware simply > doesn't exist. Having per-SoC generation Kconfig symbols allows this to > be modelled more accurately and if desired to build compact images. > Well, certainly other SoC architectures are not following what Tegra does and I don't see why Tegra should be different than others. - Mani -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms 2025-01-28 12:04 ` Niklas Cassel 2025-02-03 16:59 ` Manivannan Sadhasivam @ 2025-02-04 17:12 ` Thierry Reding 2025-02-07 10:32 ` Niklas Cassel [not found] ` <BN9PR12MB5323E59E530FA1F87DE3C7FCB8F22@BN9PR12MB5323.namprd12.prod.outlook.com> 2 siblings, 1 reply; 15+ messages in thread From: Thierry Reding @ 2025-02-04 17:12 UTC (permalink / raw) To: Niklas Cassel Cc: Vidya Sagar, lpieralisi, kw, manivannan.sadhasivam, robh, bhelgaas, quic_schintav, johan+linaro, linux-pci, linux-kernel, jonathanh, kthota, mmaddireddy, sagar.tv [-- Attachment #1: Type: text/plain, Size: 1946 bytes --] 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). 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. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms 2025-02-04 17:12 ` Thierry Reding @ 2025-02-07 10:32 ` Niklas Cassel 0 siblings, 0 replies; 15+ messages in thread From: Niklas Cassel @ 2025-02-07 10:32 UTC (permalink / raw) To: Thierry Reding Cc: Vidya Sagar, lpieralisi, kw, manivannan.sadhasivam, robh, bhelgaas, quic_schintav, johan+linaro, linux-pci, linux-kernel, jonathanh, kthota, mmaddireddy, sagar.tv 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <BN9PR12MB5323E59E530FA1F87DE3C7FCB8F22@BN9PR12MB5323.namprd12.prod.outlook.com>]
* Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms [not found] ` <BN9PR12MB5323E59E530FA1F87DE3C7FCB8F22@BN9PR12MB5323.namprd12.prod.outlook.com> @ 2025-02-10 17:50 ` manivannan.sadhasivam 2025-02-10 17:56 ` Vidya Sagar 0 siblings, 1 reply; 15+ messages in thread From: manivannan.sadhasivam @ 2025-02-10 17:50 UTC (permalink / raw) To: Vidya Sagar Cc: Niklas Cassel, lpieralisi@kernel.org, kw@linux.com, robh@kernel.org, bhelgaas@google.com, quic_schintav@quicinc.com, johan+linaro@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Thierry Reding, Jon Hunter, Krishna Thota, Manikanta Maddireddy, sagar.tv@gmail.com On Mon, Feb 10, 2025 at 02:25:43PM +0000, Vidya Sagar wrote: > Thanks Niklas for the review. > I'll rewrite the commit message add the following line > Fixes: a54e19073718 ("PCI: tegra194: Add Tegra234 PCIe support") > No, please. Try to use the generic ARCH symbol dependency as suggested. - Mani > Thanks > Vidya Sagar > > ________________________________ > From: Niklas Cassel <cassel@kernel.org> > Sent: Tuesday, January 28, 2025 17:34 > To: Vidya Sagar <vidyas@nvidia.com> > Cc: lpieralisi@kernel.org <lpieralisi@kernel.org>; kw@linux.com <kw@linux.com>; manivannan.sadhasivam@linaro.org <manivannan.sadhasivam@linaro.org>; robh@kernel.org <robh@kernel.org>; bhelgaas@google.com <bhelgaas@google.com>; quic_schintav@quicinc.com <quic_schintav@quicinc.com>; johan+linaro@kernel.org <johan+linaro@kernel.org>; linux-pci@vger.kernel.org <linux-pci@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; Thierry Reding <treding@nvidia.com>; Jon Hunter <jonathanh@nvidia.com>; Krishna Thota <kthota@nvidia.com>; Manikanta Maddireddy <mmaddireddy@nvidia.com>; sagar.tv@gmail.com <sagar.tv@gmail.com> > Subject: Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms > > External email: Use caution opening links or attachments > > > 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") > > > Kind regards, > Niklas -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms 2025-02-10 17:50 ` manivannan.sadhasivam @ 2025-02-10 17:56 ` Vidya Sagar 0 siblings, 0 replies; 15+ messages in thread From: Vidya Sagar @ 2025-02-10 17:56 UTC (permalink / raw) To: manivannan.sadhasivam@linaro.org Cc: Niklas Cassel, lpieralisi@kernel.org, kw@linux.com, robh@kernel.org, bhelgaas@google.com, quic_schintav@quicinc.com, johan+linaro@kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Thierry Reding, Jon Hunter, Krishna Thota, Manikanta Maddireddy, sagar.tv@gmail.com > -----Original Message----- > From: manivannan.sadhasivam@linaro.org <manivannan.sadhasivam@linaro.org> > Sent: 10 February 2025 23:21 > To: Vidya Sagar <vidyas@nvidia.com> > Cc: Niklas Cassel <cassel@kernel.org>; lpieralisi@kernel.org; kw@linux.com; > robh@kernel.org; bhelgaas@google.com; quic_schintav@quicinc.com; > johan+linaro@kernel.org; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org; > Thierry Reding <treding@nvidia.com>; Jon Hunter <jonathanh@nvidia.com>; Krishna > Thota <kthota@nvidia.com>; Manikanta Maddireddy <mmaddireddy@nvidia.com>; > sagar.tv@gmail.com > Subject: Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 > Platforms > > External email: Use caution opening links or attachments > > > On Mon, Feb 10, 2025 at 02:25:43PM +0000, Vidya Sagar wrote: > > Thanks Niklas for the review. > > I'll rewrite the commit message add the following line > > Fixes: a54e19073718 ("PCI: tegra194: Add Tegra234 PCIe support") > > > > No, please. Try to use the generic ARCH symbol dependency as suggested. We wanted to have more granularity in terms of enabling the driver for only those SoCs where the respective hardware is available. But, if the community has strong preference towards enabling the drivers at a higher level, I'll modify and send next version of the patch accordingly. - Vidya Sagar > > - Mani > > > Thanks > > Vidya Sagar > > > > ________________________________ > > From: Niklas Cassel <cassel@kernel.org> > > Sent: Tuesday, January 28, 2025 17:34 > > To: Vidya Sagar <vidyas@nvidia.com> > > Cc: lpieralisi@kernel.org <lpieralisi@kernel.org>; kw@linux.com > > <kw@linux.com>; manivannan.sadhasivam@linaro.org > > <manivannan.sadhasivam@linaro.org>; robh@kernel.org <robh@kernel.org>; > > bhelgaas@google.com <bhelgaas@google.com>; quic_schintav@quicinc.com > > <quic_schintav@quicinc.com>; johan+linaro@kernel.org > > <johan+linaro@kernel.org>; linux-pci@vger.kernel.org > > <linux-pci@vger.kernel.org>; linux-kernel@vger.kernel.org > > <linux-kernel@vger.kernel.org>; Thierry Reding <treding@nvidia.com>; > > Jon Hunter <jonathanh@nvidia.com>; Krishna Thota <kthota@nvidia.com>; > > Manikanta Maddireddy <mmaddireddy@nvidia.com>; sagar.tv@gmail.com > > <sagar.tv@gmail.com> > > Subject: Re: [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in > > Tegra234 Platforms > > > > External email: Use caution opening links or attachments > > > > > > 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") > > > > > > Kind regards, > > Niklas > > -- > மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency 2025-01-28 4:42 [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms Vidya Sagar 2025-01-28 12:04 ` Niklas Cassel @ 2025-04-10 19:45 ` Vidya Sagar 2025-04-11 5:40 ` Niklas Cassel ` (2 more replies) 1 sibling, 3 replies; 15+ messages in thread From: Vidya Sagar @ 2025-04-10 19:45 UTC (permalink / raw) To: lpieralisi, kw, manivannan.sadhasivam, robh, bhelgaas, cassel Cc: linux-pci, linux-kernel, treding, jonathanh, kthota, mmaddireddy, vidyas, sagar.tv Replace ARCH_TEGRA_194_SOC dependency with a more generic ARCH_TEGRA check, allowing the PCIe controller to be built on Tegra platforms beyond Tegra194. Additionally, ensure compatibility by requiring ARM64 or COMPILE_TEST. Link: https://patchwork.kernel.org/project/linux-pci/patch/20250128044244.2766334-1-vidyas@nvidia.com/ Signed-off-by: Vidya Sagar <vidyas@nvidia.com> --- v2: * Addressed review comments from Niklas Cassel and Manivannan Sadhasivam drivers/pci/controller/dwc/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig index d9f0386396ed..815b6e0d6a0c 100644 --- a/drivers/pci/controller/dwc/Kconfig +++ b/drivers/pci/controller/dwc/Kconfig @@ -226,7 +226,7 @@ config PCIE_TEGRA194 config PCIE_TEGRA194_HOST tristate "NVIDIA Tegra194 (and later) PCIe controller (host mode)" - depends on ARCH_TEGRA_194_SOC || COMPILE_TEST + depends on ARCH_TEGRA && (ARM64 || COMPILE_TEST) depends on PCI_MSI select PCIE_DW_HOST select PHY_TEGRA194_P2U @@ -241,7 +241,7 @@ config PCIE_TEGRA194_HOST config PCIE_TEGRA194_EP tristate "NVIDIA Tegra194 (and later) PCIe controller (endpoint mode)" - depends on ARCH_TEGRA_194_SOC || COMPILE_TEST + depends on ARCH_TEGRA && (ARM64 || COMPILE_TEST) depends on PCI_ENDPOINT select PCIE_DW_EP select PHY_TEGRA194_P2U -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency 2025-04-10 19:45 ` [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency Vidya Sagar @ 2025-04-11 5:40 ` Niklas Cassel 2025-04-16 15:43 ` kernel test robot 2025-04-16 17:15 ` kernel test robot 2 siblings, 0 replies; 15+ messages in thread From: Niklas Cassel @ 2025-04-11 5:40 UTC (permalink / raw) To: Vidya Sagar Cc: lpieralisi, kw, manivannan.sadhasivam, robh, bhelgaas, linux-pci, linux-kernel, treding, jonathanh, kthota, mmaddireddy, sagar.tv On Fri, Apr 11, 2025 at 01:15:52AM +0530, Vidya Sagar wrote: > Replace ARCH_TEGRA_194_SOC dependency with a more generic ARCH_TEGRA > check, allowing the PCIe controller to be built on Tegra platforms > beyond Tegra194. Additionally, ensure compatibility by requiring > ARM64 or COMPILE_TEST. > > Link: https://patchwork.kernel.org/project/linux-pci/patch/20250128044244.2766334-1-vidyas@nvidia.com/ > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > v2: > * Addressed review comments from Niklas Cassel and Manivannan Sadhasivam > > drivers/pci/controller/dwc/Kconfig | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig > index d9f0386396ed..815b6e0d6a0c 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -226,7 +226,7 @@ config PCIE_TEGRA194 > > config PCIE_TEGRA194_HOST > tristate "NVIDIA Tegra194 (and later) PCIe controller (host mode)" > - depends on ARCH_TEGRA_194_SOC || COMPILE_TEST > + depends on ARCH_TEGRA && (ARM64 || COMPILE_TEST) > depends on PCI_MSI > select PCIE_DW_HOST > select PHY_TEGRA194_P2U > @@ -241,7 +241,7 @@ config PCIE_TEGRA194_HOST > > config PCIE_TEGRA194_EP > tristate "NVIDIA Tegra194 (and later) PCIe controller (endpoint mode)" > - depends on ARCH_TEGRA_194_SOC || COMPILE_TEST > + depends on ARCH_TEGRA && (ARM64 || COMPILE_TEST) > depends on PCI_ENDPOINT > select PCIE_DW_EP > select PHY_TEGRA194_P2U > -- > 2.25.1 > Reviewed-by: Niklas Cassel <cassel@kernel.org> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency 2025-04-10 19:45 ` [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency Vidya Sagar 2025-04-11 5:40 ` Niklas Cassel @ 2025-04-16 15:43 ` kernel test robot 2025-04-16 16:09 ` Niklas Cassel 2025-04-16 17:15 ` kernel test robot 2 siblings, 1 reply; 15+ messages in thread From: kernel test robot @ 2025-04-16 15:43 UTC (permalink / raw) To: Vidya Sagar, lpieralisi, kw, manivannan.sadhasivam, robh, bhelgaas, cassel Cc: Paul Gazzillo, Necip Fazil Yildiran, oe-kbuild-all, linux-pci, linux-kernel, treding, jonathanh, kthota, mmaddireddy, vidyas, sagar.tv Hi Vidya, kernel test robot noticed the following build warnings: [auto build test WARNING on pci/next] [also build test WARNING on pci/for-linus linus/master v6.15-rc2 next-20250416] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Vidya-Sagar/PCI-dwc-tegra194-Broaden-architecture-dependency/20250411-035134 base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next patch link: https://lore.kernel.org/r/20250410194552.944818-1-vidyas%40nvidia.com patch subject: [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency config: arm64-kismet-CONFIG_PHY_TEGRA194_P2U-CONFIG_PCIE_TEGRA194_EP-0-0 (https://download.01.org/0day-ci/archive/20250416/202504162332.fwaFxVrL-lkp@intel.com/config) reproduce: (https://download.01.org/0day-ci/archive/20250416/202504162332.fwaFxVrL-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202504162332.fwaFxVrL-lkp@intel.com/ kismet warnings: (new ones prefixed by >>) >> kismet: WARNING: unmet direct dependencies detected for PHY_TEGRA194_P2U when selected by PCIE_TEGRA194_EP WARNING: unmet direct dependencies detected for PHY_TEGRA194_P2U Depends on [n]: ARCH_TEGRA_194_SOC [=n] || ARCH_TEGRA_234_SOC [=n] || COMPILE_TEST [=n] Selected by [y]: - PCIE_TEGRA194_EP [=y] && PCI [=y] && ARCH_TEGRA [=y] && (ARM64 [=y] || COMPILE_TEST [=n]) && PCI_ENDPOINT [=y] -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency 2025-04-16 15:43 ` kernel test robot @ 2025-04-16 16:09 ` Niklas Cassel 0 siblings, 0 replies; 15+ messages in thread From: Niklas Cassel @ 2025-04-16 16:09 UTC (permalink / raw) To: kernel test robot Cc: Vidya Sagar, lpieralisi, kw, manivannan.sadhasivam, robh, bhelgaas, Paul Gazzillo, Necip Fazil Yildiran, oe-kbuild-all, linux-pci, linux-kernel, treding, jonathanh, kthota, mmaddireddy, sagar.tv On Wed, Apr 16, 2025 at 11:43:25PM +0800, kernel test robot wrote: > Hi Vidya, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on pci/next] > [also build test WARNING on pci/for-linus linus/master v6.15-rc2 next-20250416] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Vidya-Sagar/PCI-dwc-tegra194-Broaden-architecture-dependency/20250411-035134 > base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next > patch link: https://lore.kernel.org/r/20250410194552.944818-1-vidyas%40nvidia.com > patch subject: [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency > config: arm64-kismet-CONFIG_PHY_TEGRA194_P2U-CONFIG_PCIE_TEGRA194_EP-0-0 (https://download.01.org/0day-ci/archive/20250416/202504162332.fwaFxVrL-lkp@intel.com/config) > reproduce: (https://download.01.org/0day-ci/archive/20250416/202504162332.fwaFxVrL-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202504162332.fwaFxVrL-lkp@intel.com/ > > kismet warnings: (new ones prefixed by >>) > >> kismet: WARNING: unmet direct dependencies detected for PHY_TEGRA194_P2U when selected by PCIE_TEGRA194_EP > WARNING: unmet direct dependencies detected for PHY_TEGRA194_P2U > Depends on [n]: ARCH_TEGRA_194_SOC [=n] || ARCH_TEGRA_234_SOC [=n] || COMPILE_TEST [=n] > Selected by [y]: > - PCIE_TEGRA194_EP [=y] && PCI [=y] && ARCH_TEGRA [=y] && (ARM64 [=y] || COMPILE_TEST [=n]) && PCI_ENDPOINT [=y] > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests/wiki My guess is that the easiest fix is to change drivers/phy/tegra/Kconfig config PHY_TEGRA194_P2U to depends on ARCH_TEGRA || COMPILE_TEST Kind regards, Niklas ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency 2025-04-10 19:45 ` [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency Vidya Sagar 2025-04-11 5:40 ` Niklas Cassel 2025-04-16 15:43 ` kernel test robot @ 2025-04-16 17:15 ` kernel test robot 2 siblings, 0 replies; 15+ messages in thread From: kernel test robot @ 2025-04-16 17:15 UTC (permalink / raw) To: Vidya Sagar, lpieralisi, kw, manivannan.sadhasivam, robh, bhelgaas, cassel Cc: Paul Gazzillo, Necip Fazil Yildiran, oe-kbuild-all, linux-pci, linux-kernel, treding, jonathanh, kthota, mmaddireddy, vidyas, sagar.tv Hi Vidya, kernel test robot noticed the following build warnings: [auto build test WARNING on pci/next] [also build test WARNING on pci/for-linus linus/master v6.15-rc2 next-20250416] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Vidya-Sagar/PCI-dwc-tegra194-Broaden-architecture-dependency/20250411-035134 base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next patch link: https://lore.kernel.org/r/20250410194552.944818-1-vidyas%40nvidia.com patch subject: [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency config: arm64-kismet-CONFIG_PHY_TEGRA194_P2U-CONFIG_PCIE_TEGRA194_HOST-0-0 (https://download.01.org/0day-ci/archive/20250417/202504170034.ZUFz0z7Z-lkp@intel.com/config) reproduce: (https://download.01.org/0day-ci/archive/20250417/202504170034.ZUFz0z7Z-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202504170034.ZUFz0z7Z-lkp@intel.com/ kismet warnings: (new ones prefixed by >>) >> kismet: WARNING: unmet direct dependencies detected for PHY_TEGRA194_P2U when selected by PCIE_TEGRA194_HOST WARNING: unmet direct dependencies detected for PHY_TEGRA194_P2U Depends on [n]: ARCH_TEGRA_194_SOC [=n] || ARCH_TEGRA_234_SOC [=n] || COMPILE_TEST [=n] Selected by [y]: - PCIE_TEGRA194_HOST [=y] && PCI [=y] && ARCH_TEGRA [=y] && (ARM64 [=y] || COMPILE_TEST [=n]) && PCI_MSI [=y] -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-04-16 17:16 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-28 4:42 [PATCH V1] PCI: tegra194: Add support for PCIe RC & EP in Tegra234 Platforms Vidya Sagar
2025-01-28 12:04 ` Niklas Cassel
2025-02-03 16:59 ` Manivannan Sadhasivam
2025-02-04 17:19 ` Thierry Reding
2025-02-07 10:41 ` Niklas Cassel
2025-02-10 17:49 ` Manivannan Sadhasivam
2025-02-04 17:12 ` Thierry Reding
2025-02-07 10:32 ` Niklas Cassel
[not found] ` <BN9PR12MB5323E59E530FA1F87DE3C7FCB8F22@BN9PR12MB5323.namprd12.prod.outlook.com>
2025-02-10 17:50 ` manivannan.sadhasivam
2025-02-10 17:56 ` Vidya Sagar
2025-04-10 19:45 ` [PATCH V2] PCI: dwc: tegra194: Broaden architecture dependency Vidya Sagar
2025-04-11 5:40 ` Niklas Cassel
2025-04-16 15:43 ` kernel test robot
2025-04-16 16:09 ` Niklas Cassel
2025-04-16 17:15 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox