drivers/pci/controller/cadence/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
The drivers associated with the PCIE_CADENCE, PCIE_CADENCE_HOST AND
PCIE_CADENCE_EP configs are used by multiple vendor drivers and serve as a
library of helpers. Since the vendor drivers could individually be built
as built-in or as loadable modules, it is possible to select a build
configuration wherein a vendor driver is built-in while the library is
built as a loadable module. This will result in a build error as reported
in the 'Closes' link below.
Address the build error by changing the library configs to be 'bool'
instead of 'tristate'.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202511111705.MZ7ls8Hm-lkp@intel.com/
Fixes: 1c72774df028 ("PCI: sg2042: Add Sophgo SG2042 PCIe driver")
Cc: <stable@vger.kernel.org>
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
drivers/pci/controller/cadence/Kconfig | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/cadence/Kconfig b/drivers/pci/controller/cadence/Kconfig
index 02a639e55fd8..980da64ce730 100644
--- a/drivers/pci/controller/cadence/Kconfig
+++ b/drivers/pci/controller/cadence/Kconfig
@@ -4,16 +4,16 @@ menu "Cadence-based PCIe controllers"
depends on PCI
config PCIE_CADENCE
- tristate
+ bool
config PCIE_CADENCE_HOST
- tristate
+ bool
depends on OF
select IRQ_DOMAIN
select PCIE_CADENCE
config PCIE_CADENCE_EP
- tristate
+ bool
depends on OF
depends on PCI_ENDPOINT
select PCIE_CADENCE
--
2.51.1
Hi Siddharth,
Your patch repeats this part.
https://patchwork.kernel.org/project/linux-pci/patch/20251108140305.1120117-2-hans.zhang@cixtech.com/
Best regards,
Hans
On 11/13/2025 5:27 PM, Siddharth Vadapalli wrote:
> EXTERNAL EMAIL
>
> The drivers associated with the PCIE_CADENCE, PCIE_CADENCE_HOST AND
> PCIE_CADENCE_EP configs are used by multiple vendor drivers and serve as a
> library of helpers. Since the vendor drivers could individually be built
> as built-in or as loadable modules, it is possible to select a build
> configuration wherein a vendor driver is built-in while the library is
> built as a loadable module. This will result in a build error as reported
> in the 'Closes' link below.
>
> Address the build error by changing the library configs to be 'bool'
> instead of 'tristate'.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202511111705.MZ7ls8Hm-lkp@intel.com/
> Fixes: 1c72774df028 ("PCI: sg2042: Add Sophgo SG2042 PCIe driver")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
> drivers/pci/controller/cadence/Kconfig | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/cadence/Kconfig b/drivers/pci/controller/cadence/Kconfig
> index 02a639e55fd8..980da64ce730 100644
> --- a/drivers/pci/controller/cadence/Kconfig
> +++ b/drivers/pci/controller/cadence/Kconfig
> @@ -4,16 +4,16 @@ menu "Cadence-based PCIe controllers"
> depends on PCI
>
> config PCIE_CADENCE
> - tristate
> + bool
>
> config PCIE_CADENCE_HOST
> - tristate
> + bool
> depends on OF
> select IRQ_DOMAIN
> select PCIE_CADENCE
>
> config PCIE_CADENCE_EP
> - tristate
> + bool
> depends on OF
> depends on PCI_ENDPOINT
> select PCIE_CADENCE
> --
> 2.51.1
>
>
On Thu, 2025-11-13 at 18:38 +0800, Hans Zhang wrote:
> Hi Siddharth,
>
>
> Your patch repeats this part.
I am not sure I understand the "repetition" that you are referring to. The
patch below is updating:
PCIE_CADENCE_PLAT, PCIE_CADENCE_PLAT_HOST and PCIE_CADENCE_PLAT_EP
from 'bool' to 'tristate'.
The current patch is updating:
PCIE_CADENCE, PCIE_CADENCE_HOST and PCIE_CADENCE_EP
[No 'PLAT' in the configs]
from 'tristate' to 'bool'.
>
>
> https://patchwork.kernel.org/project/linux-pci/patch/20251108140305.1120117-2-hans.zhang@cixtech.com/
>
> Best regards,
> Hans
>
> On 11/13/2025 5:27 PM, Siddharth Vadapalli wrote:
> > EXTERNAL EMAIL
> >
> > The drivers associated with the PCIE_CADENCE, PCIE_CADENCE_HOST AND
> > PCIE_CADENCE_EP configs are used by multiple vendor drivers and serve as a
> > library of helpers. Since the vendor drivers could individually be built
> > as built-in or as loadable modules, it is possible to select a build
> > configuration wherein a vendor driver is built-in while the library is
> > built as a loadable module. This will result in a build error as reported
> > in the 'Closes' link below.
> >
> > Address the build error by changing the library configs to be 'bool'
> > instead of 'tristate'.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202511111705.MZ7ls8Hm-lkp@intel.com/
> > Fixes: 1c72774df028 ("PCI: sg2042: Add Sophgo SG2042 PCIe driver")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> > ---
> > drivers/pci/controller/cadence/Kconfig | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/controller/cadence/Kconfig b/drivers/pci/controller/cadence/Kconfig
> > index 02a639e55fd8..980da64ce730 100644
> > --- a/drivers/pci/controller/cadence/Kconfig
> > +++ b/drivers/pci/controller/cadence/Kconfig
> > @@ -4,16 +4,16 @@ menu "Cadence-based PCIe controllers"
> > depends on PCI
> >
> > config PCIE_CADENCE
> > - tristate
> > + bool
> >
> > config PCIE_CADENCE_HOST
> > - tristate
> > + bool
> > depends on OF
> > select IRQ_DOMAIN
> > select PCIE_CADENCE
> >
> > config PCIE_CADENCE_EP
> > - tristate
> > + bool
> > depends on OF
> > depends on PCI_ENDPOINT
> > select PCIE_CADENCE
> > --
> > 2.51.1
> >
> >
Regards,
Siddharth.
On 11/13/2025 6:43 PM, Siddharth Vadapalli wrote:
> EXTERNAL EMAIL
>
> On Thu, 2025-11-13 at 18:38 +0800, Hans Zhang wrote:
>> Hi Siddharth,
>>
>>
>> Your patch repeats this part.
>
> I am not sure I understand the "repetition" that you are referring to. The
> patch below is updating:
> PCIE_CADENCE_PLAT, PCIE_CADENCE_PLAT_HOST and PCIE_CADENCE_PLAT_EP
> from 'bool' to 'tristate'.
>
> The current patch is updating:
> PCIE_CADENCE, PCIE_CADENCE_HOST and PCIE_CADENCE_EP
> [No 'PLAT' in the configs]
> from 'tristate' to 'bool'.
>
Hi Siddharth,
Sorry, I was mistaken.
Best regards,
Hans
>>
>>
>> https://patchwork.kernel.org/project/linux-pci/patch/20251108140305.1120117-2-hans.zhang@cixtech.com/
>>
>> Best regards,
>> Hans
>>
>> On 11/13/2025 5:27 PM, Siddharth Vadapalli wrote:
>>> EXTERNAL EMAIL
>>>
>>> The drivers associated with the PCIE_CADENCE, PCIE_CADENCE_HOST AND
>>> PCIE_CADENCE_EP configs are used by multiple vendor drivers and serve as a
>>> library of helpers. Since the vendor drivers could individually be built
>>> as built-in or as loadable modules, it is possible to select a build
>>> configuration wherein a vendor driver is built-in while the library is
>>> built as a loadable module. This will result in a build error as reported
>>> in the 'Closes' link below.
>>>
>>> Address the build error by changing the library configs to be 'bool'
>>> instead of 'tristate'.
>>>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Closes: https://lore.kernel.org/oe-kbuild-all/202511111705.MZ7ls8Hm-lkp@intel.com/
>>> Fixes: 1c72774df028 ("PCI: sg2042: Add Sophgo SG2042 PCIe driver")
>>> Cc: <stable@vger.kernel.org>
>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>> ---
>>> drivers/pci/controller/cadence/Kconfig | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/cadence/Kconfig b/drivers/pci/controller/cadence/Kconfig
>>> index 02a639e55fd8..980da64ce730 100644
>>> --- a/drivers/pci/controller/cadence/Kconfig
>>> +++ b/drivers/pci/controller/cadence/Kconfig
>>> @@ -4,16 +4,16 @@ menu "Cadence-based PCIe controllers"
>>> depends on PCI
>>>
>>> config PCIE_CADENCE
>>> - tristate
>>> + bool
>>>
>>> config PCIE_CADENCE_HOST
>>> - tristate
>>> + bool
>>> depends on OF
>>> select IRQ_DOMAIN
>>> select PCIE_CADENCE
>>>
>>> config PCIE_CADENCE_EP
>>> - tristate
>>> + bool
>>> depends on OF
>>> depends on PCI_ENDPOINT
>>> select PCIE_CADENCE
>>> --
>>> 2.51.1
>>>
>>>
>
> Regards,
> Siddharth.
On Thu, Nov 13, 2025, at 10:27, Siddharth Vadapalli wrote:
> The drivers associated with the PCIE_CADENCE, PCIE_CADENCE_HOST AND
> PCIE_CADENCE_EP configs are used by multiple vendor drivers and serve as a
> library of helpers. Since the vendor drivers could individually be built
> as built-in or as loadable modules, it is possible to select a build
> configuration wherein a vendor driver is built-in while the library is
> built as a loadable module. This will result in a build error as reported
> in the 'Closes' link below.
>
> Address the build error by changing the library configs to be 'bool'
> instead of 'tristate'.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202511111705.MZ7ls8Hm-lkp@intel.com/
> Fixes: 1c72774df028 ("PCI: sg2042: Add Sophgo SG2042 PCIe driver")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
I really think there has to be a better solution here, this is not
an unusual problem.
> @@ -4,16 +4,16 @@ menu "Cadence-based PCIe controllers"
> depends on PCI
>
> config PCIE_CADENCE
> - tristate
> + bool
>
> config PCIE_CADENCE_HOST
> - tristate
> + bool
> depends on OF
> select IRQ_DOMAIN
> select PCIE_CADENCE
>
> config PCIE_CADENCE_EP
> - tristate
> + bool
> depends on OF
> depends on PCI_ENDPOINT
> select PCIE_CADENCE
I think the easiest way would be to leave PCIE_CADENCE as
a 'tristate' symbol but make the other two 'bool', and then
adjust the Makefile logic to use CONFIG_PCIE_CADENCE as
the thing that controls how the individual drivers are built.
That way, if any platform specific driver is built-in, both
the EP and HOST support are built-in or disabled but never
loadable modules. As long as all platform drivers are
loadable modules, so would be the base support.
Arnd
On Thu, 2025-11-13 at 11:13 +0100, Arnd Bergmann wrote:
Hello Arnd,
> On Thu, Nov 13, 2025, at 10:27, Siddharth Vadapalli wrote:
> > The drivers associated with the PCIE_CADENCE, PCIE_CADENCE_HOST AND
> > PCIE_CADENCE_EP configs are used by multiple vendor drivers and serve as a
> > library of helpers. Since the vendor drivers could individually be built
> > as built-in or as loadable modules, it is possible to select a build
> > configuration wherein a vendor driver is built-in while the library is
> > built as a loadable module. This will result in a build error as reported
> > in the 'Closes' link below.
> >
> > Address the build error by changing the library configs to be 'bool'
> > instead of 'tristate'.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes:
> > https://lore.kernel.org/oe-kbuild-all/202511111705.MZ7ls8Hm-lkp@intel.com/
> > Fixes: 1c72774df028 ("PCI: sg2042: Add Sophgo SG2042 PCIe driver")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>
> I really think there has to be a better solution here, this is not
> an unusual problem.
>
> > @@ -4,16 +4,16 @@ menu "Cadence-based PCIe controllers"
> > depends on PCI
> >
> > config PCIE_CADENCE
> > - tristate
> > + bool
> >
> > config PCIE_CADENCE_HOST
> > - tristate
> > + bool
> > depends on OF
> > select IRQ_DOMAIN
> > select PCIE_CADENCE
> >
> > config PCIE_CADENCE_EP
> > - tristate
> > + bool
> > depends on OF
> > depends on PCI_ENDPOINT
> > select PCIE_CADENCE
>
> I think the easiest way would be to leave PCIE_CADENCE as
> a 'tristate' symbol but make the other two 'bool', and then
> adjust the Makefile logic to use CONFIG_PCIE_CADENCE as
> the thing that controls how the individual drivers are built.
>
> That way, if any platform specific driver is built-in, both
> the EP and HOST support are built-in or disabled but never
> loadable modules. As long as all platform drivers are
> loadable modules, so would be the base support.
Thank you for the suggestion. I think that the following Makefile changes
will be sufficient and Kconfig doesn't need to be modified:
diff --git a/drivers/pci/controller/cadence/Makefile
b/drivers/pci/controller/cadence/Makefile
index 5e23f8539ecc..1a97c9b249b8 100644
--- a/drivers/pci/controller/cadence/Makefile
+++ b/drivers/pci/controller/cadence/Makefile
@@ -4,4 +4,6 @@ obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o
obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o
obj-$(CONFIG_PCIE_CADENCE_PLAT) += pcie-cadence-plat.o
obj-$(CONFIG_PCI_J721E) += pci-j721e.o
+pci_j721e-y := pci-j721e.o pcie-cadence.o
obj-$(CONFIG_PCIE_SG2042_HOST) += pcie-sg2042.o
+pci_sg2042_host-y := pci-sg2042.o pcie-cadence.o
If either of PCI_J721E or SG2042_HOST is selected as a built-in module,
then pcie-cadence-host.c, pcie-cadence-ep.c and pcie-cadence.c drivers will
be built-in. If both PCI_J721E and SG2042_HOST are selected as loadable
modules, only then the library drivers will be enabled as loadable modules.
Please let me know what you think.
Regards,
Siddharth.
On Fri, Nov 14, 2025, at 06:47, Siddharth Vadapalli wrote:
> On Thu, 2025-11-13 at 11:13 +0100, Arnd Bergmann wrote:
>> On Thu, Nov 13, 2025, at 10:27, Siddharth Vadapalli wrote:
> Thank you for the suggestion. I think that the following Makefile changes
> will be sufficient and Kconfig doesn't need to be modified:
>
> diff --git a/drivers/pci/controller/cadence/Makefile
> b/drivers/pci/controller/cadence/Makefile
> index 5e23f8539ecc..1a97c9b249b8 100644
> --- a/drivers/pci/controller/cadence/Makefile
> +++ b/drivers/pci/controller/cadence/Makefile
> @@ -4,4 +4,6 @@ obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o
> obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o
> obj-$(CONFIG_PCIE_CADENCE_PLAT) += pcie-cadence-plat.o
> obj-$(CONFIG_PCI_J721E) += pci-j721e.o
> +pci_j721e-y := pci-j721e.o pcie-cadence.o
> obj-$(CONFIG_PCIE_SG2042_HOST) += pcie-sg2042.o
> +pci_sg2042_host-y := pci-sg2042.o pcie-cadence.o
>
> If either of PCI_J721E or SG2042_HOST is selected as a built-in module,
> then pcie-cadence-host.c, pcie-cadence-ep.c and pcie-cadence.c drivers will
> be built-in. If both PCI_J721E and SG2042_HOST are selected as loadable
> modules, only then the library drivers will be enabled as loadable modules.
>
> Please let me know what you think.
I don't think that the version above does what you want,
this would build the pcie-cadence.o file into three separate
modules and break in additional ways if a subset of them are
built-in.
I would still suggest combining pcie-cadence{,-ep,-host}.o into
one module that is used by the other drivers, as that would address
the build failure you are observing.
An alternative would be to change the pcie-j721e.c file to only
reference the host portion if host support is enabled for this
driver.
Arnd
On Fri, 2025-11-14 at 08:03 +0100, Arnd Bergmann wrote:
> On Fri, Nov 14, 2025, at 06:47, Siddharth Vadapalli wrote:
> > On Thu, 2025-11-13 at 11:13 +0100, Arnd Bergmann wrote:
> > > On Thu, Nov 13, 2025, at 10:27, Siddharth Vadapalli wrote:
>
> > Thank you for the suggestion. I think that the following Makefile changes
> > will be sufficient and Kconfig doesn't need to be modified:
> >
> > diff --git a/drivers/pci/controller/cadence/Makefile
> > b/drivers/pci/controller/cadence/Makefile
> > index 5e23f8539ecc..1a97c9b249b8 100644
> > --- a/drivers/pci/controller/cadence/Makefile
> > +++ b/drivers/pci/controller/cadence/Makefile
> > @@ -4,4 +4,6 @@ obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o
> > obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep.o
> > obj-$(CONFIG_PCIE_CADENCE_PLAT) += pcie-cadence-plat.o
> > obj-$(CONFIG_PCI_J721E) += pci-j721e.o
> > +pci_j721e-y := pci-j721e.o pcie-cadence.o
> > obj-$(CONFIG_PCIE_SG2042_HOST) += pcie-sg2042.o
> > +pci_sg2042_host-y := pci-sg2042.o pcie-cadence.o
> >
> > If either of PCI_J721E or SG2042_HOST is selected as a built-in module,
> > then pcie-cadence-host.c, pcie-cadence-ep.c and pcie-cadence.c drivers will
> > be built-in. If both PCI_J721E and SG2042_HOST are selected as loadable
> > modules, only then the library drivers will be enabled as loadable modules.
> >
> > Please let me know what you think.
>
> I don't think that the version above does what you want,
> this would build the pcie-cadence.o file into three separate
> modules and break in additional ways if a subset of them are
> built-in.
>
> I would still suggest combining pcie-cadence{,-ep,-host}.o into
> one module that is used by the other drivers, as that would address
> the build failure you are observing.
>
> An alternative would be to change the pcie-j721e.c file to only
> reference the host portion if host support is enabled for this
> driver.
While 'pcie-cadence.h' handles the case where 'PCIE_CADENCE_HOST' is not
defined:
#if IS_ENABLED(CONFIG_PCIE_CADENCE_HOST)
...
void cdns_pcie_host_disable(struct cdns_pcie_rc *rc);
...
#else
...
static inline void cdns_pcie_host_disable(struct cdns_pcie_rc *rc)
{
}
...
#endif
the issue occurs because PCIE_SG2042_HOST enables CONFIG_PCIE_CADENCE_HOST
but it is enabled as 'm'. As a result, the definition exists in pcie-
cadence-host.c that is built as a loadable module which is not accessible
by pci-j721e.c that is built-in.
I understand that the solution should be fixing the pci-j721e.c driver
rather than updating Kconfig or Makefile. Thank you for the feedback. I
will update the pci-j721e.c driver to handle the case that is triggering
the build error.
Regards,
Siddharth.
On Mon, Nov 17, 2025, at 07:05, Siddharth Vadapalli wrote:
> On Fri, 2025-11-14 at 08:03 +0100, Arnd Bergmann wrote:
>> On Fri, Nov 14, 2025, at 06:47, Siddharth Vadapalli wrote:
> I understand that the solution should be fixing the pci-j721e.c driver
> rather than updating Kconfig or Makefile. Thank you for the feedback. I
> will update the pci-j721e.c driver to handle the case that is triggering
> the build error.
Ok, thanks!
I think a single if(IS_ENABLED(CONFIG_PCI_J721E_HOST)) check
is probably enough to avoid the link failure
--- a/drivers/pci/controller/cadence/pci-j721e.c
+++ b/drivers/pci/controller/cadence/pci-j721e.c
@@ -669,7 +669,8 @@ static void j721e_pcie_remove(struct platform_device *pdev)
struct cdns_pcie_ep *ep;
struct cdns_pcie_rc *rc;
- if (pcie->mode == PCI_MODE_RC) {
+ if (IS_ENABLED(CONFIG_PCI_J721E_HOST) &&
+ pcie->mode == PCI_MODE_RC) {
rc = container_of(cdns_pcie, struct cdns_pcie_rc, pcie);
cdns_pcie_host_disable(rc);
but you may want to split it up further to get better dead
code elimination and prevent similar bugs from reappearing when
another call gets added without this type of check.
If you split j721e_pcie_driver into a host and an ep driver
structure with their own probe/remove callbacks, you can
move the IS_ENABLED() check all the way into module_init()
function.
Arnd
On Mon, 2025-11-17 at 10:06 +0100, Arnd Bergmann wrote:
> On Mon, Nov 17, 2025, at 07:05, Siddharth Vadapalli wrote:
> > On Fri, 2025-11-14 at 08:03 +0100, Arnd Bergmann wrote:
> > > On Fri, Nov 14, 2025, at 06:47, Siddharth Vadapalli wrote:
>
> > I understand that the solution should be fixing the pci-j721e.c driver
> > rather than updating Kconfig or Makefile. Thank you for the feedback. I
> > will update the pci-j721e.c driver to handle the case that is triggering
> > the build error.
>
> Ok, thanks!
>
> I think a single if(IS_ENABLED(CONFIG_PCI_J721E_HOST)) check
> is probably enough to avoid the link failure
>
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -669,7 +669,8 @@ static void j721e_pcie_remove(struct platform_device *pdev)
> struct cdns_pcie_ep *ep;
> struct cdns_pcie_rc *rc;
>
> - if (pcie->mode == PCI_MODE_RC) {
> + if (IS_ENABLED(CONFIG_PCI_J721E_HOST) &&
> + pcie->mode == PCI_MODE_RC) {
> rc = container_of(cdns_pcie, struct cdns_pcie_rc, pcie);
> cdns_pcie_host_disable(rc);
>
>
> but you may want to split it up further to get better dead
> code elimination and prevent similar bugs from reappearing when
> another call gets added without this type of check.
>
> If you split j721e_pcie_driver into a host and an ep driver
> structure with their own probe/remove callbacks, you can
> move the IS_ENABLED() check all the way into module_init()
> function.
Thank you for the suggestion :)
Would it work if I send a quick fix for `cdns_pcie_host_disable` using
IS_ENABLED in the existing driver implementation and then send the
refactoring series later? This is to resolve the build error quickly until
the refactoring series is ready.
On the other hand, if it should be fixed the right way by refactoring, I
will not post the temporary fix. Please let me know.
Regards,
Siddharth.
On Mon, Nov 17, 2025, at 10:23, Siddharth Vadapalli wrote:
> On Mon, 2025-11-17 at 10:06 +0100, Arnd Bergmann wrote:
>> On Mon, Nov 17, 2025, at 07:05, Siddharth Vadapalli wrote:
>>
>> but you may want to split it up further to get better dead
>> code elimination and prevent similar bugs from reappearing when
>> another call gets added without this type of check.
>>
>> If you split j721e_pcie_driver into a host and an ep driver
>> structure with their own probe/remove callbacks, you can
>> move the IS_ENABLED() check all the way into module_init()
>> function.
>
> Thank you for the suggestion :)
>
> Would it work if I send a quick fix for `cdns_pcie_host_disable` using
> IS_ENABLED in the existing driver implementation and then send the
> refactoring series later? This is to resolve the build error quickly until
> the refactoring series is ready.
>
> On the other hand, if it should be fixed the right way by refactoring, I
> will not post the temporary fix. Please let me know.
Please see if my suggestion works in practice using all the
combinations of build options, I may have missed something.
If it fixes all the build failures, merging and backporting
this first makes sense.
Arnd
On Mon, Nov 17, 2025 at 02:53:00PM +0530, Siddharth Vadapalli wrote:
> On Mon, 2025-11-17 at 10:06 +0100, Arnd Bergmann wrote:
> > On Mon, Nov 17, 2025, at 07:05, Siddharth Vadapalli wrote:
> > > On Fri, 2025-11-14 at 08:03 +0100, Arnd Bergmann wrote:
> > > > On Fri, Nov 14, 2025, at 06:47, Siddharth Vadapalli wrote:
> >
> > > I understand that the solution should be fixing the pci-j721e.c driver
> > > rather than updating Kconfig or Makefile. Thank you for the feedback. I
> > > will update the pci-j721e.c driver to handle the case that is triggering
> > > the build error.
> >
> > Ok, thanks!
> >
> > I think a single if(IS_ENABLED(CONFIG_PCI_J721E_HOST)) check
> > is probably enough to avoid the link failure
> >
> > --- a/drivers/pci/controller/cadence/pci-j721e.c
> > +++ b/drivers/pci/controller/cadence/pci-j721e.c
> > @@ -669,7 +669,8 @@ static void j721e_pcie_remove(struct platform_device *pdev)
> > struct cdns_pcie_ep *ep;
> > struct cdns_pcie_rc *rc;
> >
> > - if (pcie->mode == PCI_MODE_RC) {
> > + if (IS_ENABLED(CONFIG_PCI_J721E_HOST) &&
> > + pcie->mode == PCI_MODE_RC) {
> > rc = container_of(cdns_pcie, struct cdns_pcie_rc, pcie);
> > cdns_pcie_host_disable(rc);
> >
> >
> > but you may want to split it up further to get better dead
> > code elimination and prevent similar bugs from reappearing when
> > another call gets added without this type of check.
> >
> > If you split j721e_pcie_driver into a host and an ep driver
> > structure with their own probe/remove callbacks, you can
> > move the IS_ENABLED() check all the way into module_init()
> > function.
>
> Thank you for the suggestion :)
>
> Would it work if I send a quick fix for `cdns_pcie_host_disable` using
> IS_ENABLED in the existing driver implementation and then send the
> refactoring series later? This is to resolve the build error quickly until
> the refactoring series is ready.
>
This will work for me so that this fix can be backported.
- Mani
--
மணிவண்ணன் சதாசிவம்
© 2016 - 2026 Red Hat, Inc.