* [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-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-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: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
* 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
[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