All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] cdx: Enable COMPILE_TEST
@ 2023-12-07 16:31 Rob Herring
  2023-12-07 16:31 ` [PATCH 2/2] cdx: Explicitly include correct DT includes, again Rob Herring
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Rob Herring @ 2023-12-07 16:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Nipun Gupta, Nikhil Agarwal; +Cc: linux-kernel

There is no reason CDX needs to depend on ARM64 other than limiting
visibility. So let's also enable building with COMPILE_TEST.

The CONFIG_OF dependency is redundant as ARM64 always enables it and all
the DT functions have empty stubs.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/cdx/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cdx/Kconfig b/drivers/cdx/Kconfig
index a08958485e31..7cdb7c414453 100644
--- a/drivers/cdx/Kconfig
+++ b/drivers/cdx/Kconfig
@@ -7,7 +7,7 @@
 
 config CDX_BUS
 	bool "CDX Bus driver"
-	depends on OF && ARM64
+	depends on ARM64 || COMPILE_TEST
 	help
 	  Driver to enable Composable DMA Transfer(CDX) Bus. CDX bus
 	  exposes Fabric devices which uses composable DMA IP to the
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] cdx: Explicitly include correct DT includes, again
  2023-12-07 16:31 [PATCH 1/2] cdx: Enable COMPILE_TEST Rob Herring
@ 2023-12-07 16:31 ` Rob Herring
  2023-12-12  4:40   ` Agarwal, Nikhil
  2023-12-09  2:39 ` [PATCH 1/2] cdx: Enable COMPILE_TEST kernel test robot
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2023-12-07 16:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Nipun Gupta, Nikhil Agarwal, Abhijit Gangurde
  Cc: linux-kernel

The DT of_device.h and of_platform.h date back to the separate
of_platform_bus_type before it was merged into the regular platform bus.
As part of that merge prepping Arm DT support 13 years ago, they
"temporarily" include each other. They also include platform_device.h
and of.h. As a result, there's a pretty much random mix of those include
files used throughout the tree. In order to detangle these headers and
replace the implicit includes with struct declarations, users need to
explicitly include the correct includes.

CDX was fixed once, but commit ("cdx: Remove cdx controller list from cdx
bus system") added another occurrence.

Fixes: 54b406e10f03 ("cdx: Remove cdx controller list from cdx bus system")
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/cdx/cdx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
index 4461c6c9313f..d84d153078d7 100644
--- a/drivers/cdx/cdx.c
+++ b/drivers/cdx/cdx.c
@@ -57,7 +57,10 @@
 
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/mm.h>
 #include <linux/idr.h>
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] cdx: Enable COMPILE_TEST
  2023-12-07 16:31 [PATCH 1/2] cdx: Enable COMPILE_TEST Rob Herring
  2023-12-07 16:31 ` [PATCH 2/2] cdx: Explicitly include correct DT includes, again Rob Herring
@ 2023-12-09  2:39 ` kernel test robot
  2023-12-12  4:40 ` Agarwal, Nikhil
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-12-09  2:39 UTC (permalink / raw)
  To: Rob Herring; +Cc: Paul Gazzillo, Necip Fazil Yildiran, oe-kbuild-all

Hi Rob,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.7-rc4 next-20231208]
[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/Rob-Herring/cdx-Explicitly-include-correct-DT-includes-again/20231208-003335
base:   linus/master
patch link:    https://lore.kernel.org/r/20231207163128.2707993-1-robh%40kernel.org
patch subject: [PATCH 1/2] cdx: Enable COMPILE_TEST
config: m68k-kismet-CONFIG_REMOTEPROC-CONFIG_CDX_CONTROLLER-0-0 (https://download.01.org/0day-ci/archive/20231209/202312091016.deeonCay-lkp@intel.com/config)
reproduce: (https://download.01.org/0day-ci/archive/20231209/202312091016.deeonCay-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/202312091016.deeonCay-lkp@intel.com/

kismet warnings: (new ones prefixed by >>)
>> kismet: WARNING: unmet direct dependencies detected for REMOTEPROC when selected by CDX_CONTROLLER
   
   WARNING: unmet direct dependencies detected for REMOTEPROC
     Depends on [n]: HAS_DMA [=n]
     Selected by [y]:
     - CDX_CONTROLLER [=y] && CDX_BUS [=y]

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 1/2] cdx: Enable COMPILE_TEST
  2023-12-07 16:31 [PATCH 1/2] cdx: Enable COMPILE_TEST Rob Herring
  2023-12-07 16:31 ` [PATCH 2/2] cdx: Explicitly include correct DT includes, again Rob Herring
  2023-12-09  2:39 ` [PATCH 1/2] cdx: Enable COMPILE_TEST kernel test robot
@ 2023-12-12  4:40 ` Agarwal, Nikhil
  2023-12-12 14:26   ` Rob Herring
  2023-12-15 16:09 ` Greg Kroah-Hartman
  2023-12-18  4:40 ` Agarwal, Nikhil
  4 siblings, 1 reply; 14+ messages in thread
From: Agarwal, Nikhil @ 2023-12-12  4:40 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman, Gupta, Nipun
  Cc: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Thursday, December 7, 2023 10:01 PM
> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Gupta, Nipun
> <Nipun.Gupta@amd.com>; Agarwal, Nikhil <nikhil.agarwal@amd.com>
> Cc: linux-kernel@vger.kernel.org
> Subject: [PATCH 1/2] cdx: Enable COMPILE_TEST
> 
> There is no reason CDX needs to depend on ARM64 other than limiting
> visibility. So let's also enable building with COMPILE_TEST.
> 
> The CONFIG_OF dependency is redundant as ARM64 always enables it and all
> the DT functions have empty stubs.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/cdx/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cdx/Kconfig b/drivers/cdx/Kconfig index
> a08958485e31..7cdb7c414453 100644
> --- a/drivers/cdx/Kconfig
> +++ b/drivers/cdx/Kconfig
> @@ -7,7 +7,7 @@
> 
>  config CDX_BUS
>  	bool "CDX Bus driver"
> -	depends on OF && ARM64
> +	depends on ARM64 || COMPILE_TEST
Hi Rob,

There is a CDX MSI support patch
https://lore.kernel.org/lkml/20231116125609.245206-1-nipun.gupta@amd.com/ which is in
review and is dependent on ARM64( msi_alloc_info_t definition differs on x86). So, the
COMPILE_TEST would break once the MSI changes are added.

Regards
Nikhil


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 2/2] cdx: Explicitly include correct DT includes, again
  2023-12-07 16:31 ` [PATCH 2/2] cdx: Explicitly include correct DT includes, again Rob Herring
@ 2023-12-12  4:40   ` Agarwal, Nikhil
  0 siblings, 0 replies; 14+ messages in thread
From: Agarwal, Nikhil @ 2023-12-12  4:40 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman, Gupta, Nipun, Gangurde, Abhijit
  Cc: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Thursday, December 7, 2023 10:01 PM
> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Gupta, Nipun
> <Nipun.Gupta@amd.com>; Agarwal, Nikhil <nikhil.agarwal@amd.com>;
> Gangurde, Abhijit <abhijit.gangurde@amd.com>
> Cc: linux-kernel@vger.kernel.org
> Subject: [PATCH 2/2] cdx: Explicitly include correct DT includes, again
> 
> The DT of_device.h and of_platform.h date back to the separate
> of_platform_bus_type before it was merged into the regular platform bus.
> As part of that merge prepping Arm DT support 13 years ago, they
> "temporarily" include each other. They also include platform_device.h and
> of.h. As a result, there's a pretty much random mix of those include files used
> throughout the tree. In order to detangle these headers and replace the
> implicit includes with struct declarations, users need to explicitly include the
> correct includes.
> 
> CDX was fixed once, but commit ("cdx: Remove cdx controller list from cdx bus
> system") added another occurrence.
> 
> Fixes: 54b406e10f03 ("cdx: Remove cdx controller list from cdx bus system")
> Signed-off-by: Rob Herring <robh@kernel.org>

Acked-by: Nikhil Agarwal <Nikhil.agarwal@amd.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] cdx: Enable COMPILE_TEST
  2023-12-12  4:40 ` Agarwal, Nikhil
@ 2023-12-12 14:26   ` Rob Herring
  2023-12-18  4:39     ` Agarwal, Nikhil
  0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2023-12-12 14:26 UTC (permalink / raw)
  To: Agarwal, Nikhil
  Cc: Greg Kroah-Hartman, Gupta, Nipun, linux-kernel@vger.kernel.org

On Mon, Dec 11, 2023 at 10:40 PM Agarwal, Nikhil <nikhil.agarwal@amd.com> wrote:
>
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: Thursday, December 7, 2023 10:01 PM
> > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Gupta, Nipun
> > <Nipun.Gupta@amd.com>; Agarwal, Nikhil <nikhil.agarwal@amd.com>
> > Cc: linux-kernel@vger.kernel.org
> > Subject: [PATCH 1/2] cdx: Enable COMPILE_TEST
> >
> > There is no reason CDX needs to depend on ARM64 other than limiting
> > visibility. So let's also enable building with COMPILE_TEST.
> >
> > The CONFIG_OF dependency is redundant as ARM64 always enables it and all
> > the DT functions have empty stubs.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >  drivers/cdx/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cdx/Kconfig b/drivers/cdx/Kconfig index
> > a08958485e31..7cdb7c414453 100644
> > --- a/drivers/cdx/Kconfig
> > +++ b/drivers/cdx/Kconfig
> > @@ -7,7 +7,7 @@
> >
> >  config CDX_BUS
> >       bool "CDX Bus driver"
> > -     depends on OF && ARM64
> > +     depends on ARM64 || COMPILE_TEST
> Hi Rob,
>
> There is a CDX MSI support patch
> https://lore.kernel.org/lkml/20231116125609.245206-1-nipun.gupta@amd.com/ which is in
> review and is dependent on ARM64( msi_alloc_info_t definition differs on x86). So, the
> COMPILE_TEST would break once the MSI changes are added.

An ifdef around 'scratchpad' should fix that. It is worthwhile to get
all this to build on x86 allyesconfig builds at least because that is
frequently built by the various CI systems. arm64 is getting there,
but x86 is first for many.

Rob

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] cdx: Enable COMPILE_TEST
  2023-12-07 16:31 [PATCH 1/2] cdx: Enable COMPILE_TEST Rob Herring
                   ` (2 preceding siblings ...)
  2023-12-12  4:40 ` Agarwal, Nikhil
@ 2023-12-15 16:09 ` Greg Kroah-Hartman
  2023-12-15 16:10   ` Greg Kroah-Hartman
  2023-12-18  4:40 ` Agarwal, Nikhil
  4 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2023-12-15 16:09 UTC (permalink / raw)
  To: Rob Herring; +Cc: Nipun Gupta, Nikhil Agarwal, linux-kernel

On Thu, Dec 07, 2023 at 10:31:26AM -0600, Rob Herring wrote:
> There is no reason CDX needs to depend on ARM64 other than limiting
> visibility. So let's also enable building with COMPILE_TEST.
> 
> The CONFIG_OF dependency is redundant as ARM64 always enables it and all
> the DT functions have empty stubs.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/cdx/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cdx/Kconfig b/drivers/cdx/Kconfig
> index a08958485e31..7cdb7c414453 100644
> --- a/drivers/cdx/Kconfig
> +++ b/drivers/cdx/Kconfig
> @@ -7,7 +7,7 @@
>  
>  config CDX_BUS
>  	bool "CDX Bus driver"
> -	depends on OF && ARM64
> +	depends on ARM64 || COMPILE_TEST

Ok, good start, now we need to turn this into a module, what's keeping
it from being able to be built as a tristate?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] cdx: Enable COMPILE_TEST
  2023-12-15 16:09 ` Greg Kroah-Hartman
@ 2023-12-15 16:10   ` Greg Kroah-Hartman
  2023-12-21  6:42     ` Gangurde, Abhijit
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2023-12-15 16:10 UTC (permalink / raw)
  To: Rob Herring; +Cc: Nipun Gupta, Nikhil Agarwal, linux-kernel

On Fri, Dec 15, 2023 at 05:09:15PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Dec 07, 2023 at 10:31:26AM -0600, Rob Herring wrote:
> > There is no reason CDX needs to depend on ARM64 other than limiting
> > visibility. So let's also enable building with COMPILE_TEST.
> > 
> > The CONFIG_OF dependency is redundant as ARM64 always enables it and all
> > the DT functions have empty stubs.
> > 
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >  drivers/cdx/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cdx/Kconfig b/drivers/cdx/Kconfig
> > index a08958485e31..7cdb7c414453 100644
> > --- a/drivers/cdx/Kconfig
> > +++ b/drivers/cdx/Kconfig
> > @@ -7,7 +7,7 @@
> >  
> >  config CDX_BUS
> >  	bool "CDX Bus driver"
> > -	depends on OF && ARM64
> > +	depends on ARM64 || COMPILE_TEST
> 
> Ok, good start, now we need to turn this into a module, what's keeping
> it from being able to be built as a tristate?

To answer my own question, the following errors:

ERROR: modpost: missing MODULE_LICENSE() in drivers/cdx/cdx.o
ERROR: modpost: "iommu_device_unuse_default_domain" [drivers/cdx/cdx.ko] undefined!
ERROR: modpost: "iommu_device_use_default_domain" [drivers/cdx/cdx.ko] undefined!

Would be great for someone to fix this up...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 1/2] cdx: Enable COMPILE_TEST
  2023-12-12 14:26   ` Rob Herring
@ 2023-12-18  4:39     ` Agarwal, Nikhil
  0 siblings, 0 replies; 14+ messages in thread
From: Agarwal, Nikhil @ 2023-12-18  4:39 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Gupta, Nipun, linux-kernel@vger.kernel.org,
	Gangurde, Abhijit



> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, December 12, 2023 7:56 PM
> To: Agarwal, Nikhil <nikhil.agarwal@amd.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Gupta, Nipun
> <Nipun.Gupta@amd.com>; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] cdx: Enable COMPILE_TEST
> 
> On Mon, Dec 11, 2023 at 10:40 PM Agarwal, Nikhil
> <nikhil.agarwal@amd.com> wrote:
> >
> > > -----Original Message-----
> > > From: Rob Herring <robh@kernel.org>
> > > Sent: Thursday, December 7, 2023 10:01 PM
> > > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Gupta, Nipun
> > > <Nipun.Gupta@amd.com>; Agarwal, Nikhil <nikhil.agarwal@amd.com>
> > > Cc: linux-kernel@vger.kernel.org
> > > Subject: [PATCH 1/2] cdx: Enable COMPILE_TEST
> > >
> > > There is no reason CDX needs to depend on ARM64 other than limiting
> > > visibility. So let's also enable building with COMPILE_TEST.
> > >
> > > The CONFIG_OF dependency is redundant as ARM64 always enables it and
> > > all the DT functions have empty stubs.
> > >
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > ---
> > >  drivers/cdx/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/cdx/Kconfig b/drivers/cdx/Kconfig index
> > > a08958485e31..7cdb7c414453 100644
> > > --- a/drivers/cdx/Kconfig
> > > +++ b/drivers/cdx/Kconfig
> > > @@ -7,7 +7,7 @@
> > >
> > >  config CDX_BUS
> > >       bool "CDX Bus driver"
> > > -     depends on OF && ARM64
> > > +     depends on ARM64 || COMPILE_TEST
> > Hi Rob,
> >
> > There is a CDX MSI support patch
> > https://lore.kernel.org/lkml/20231116125609.245206-1-
> nipun.gupta@amd.c
> > om/ which is in review and is dependent on ARM64( msi_alloc_info_t
> > definition differs on x86). So, the COMPILE_TEST would break once the MSI
> changes are added.
> 
> An ifdef around 'scratchpad' should fix that. It is worthwhile to get all this to
> build on x86 allyesconfig builds at least because that is frequently built by the
> various CI systems. arm64 is getting there, but x86 is first for many.
> 
> Rob

Thanks for the suggestion, Rob. We will change the MSI patch accordingly.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 1/2] cdx: Enable COMPILE_TEST
  2023-12-07 16:31 [PATCH 1/2] cdx: Enable COMPILE_TEST Rob Herring
                   ` (3 preceding siblings ...)
  2023-12-15 16:09 ` Greg Kroah-Hartman
@ 2023-12-18  4:40 ` Agarwal, Nikhil
  4 siblings, 0 replies; 14+ messages in thread
From: Agarwal, Nikhil @ 2023-12-18  4:40 UTC (permalink / raw)
  To: Rob Herring, Greg Kroah-Hartman, Gupta, Nipun
  Cc: linux-kernel@vger.kernel.org, Gangurde, Abhijit

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Thursday, December 7, 2023 10:01 PM
> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Gupta, Nipun
> <Nipun.Gupta@amd.com>; Agarwal, Nikhil <nikhil.agarwal@amd.com>
> Cc: linux-kernel@vger.kernel.org
> Subject: [PATCH 1/2] cdx: Enable COMPILE_TEST
> 
> There is no reason CDX needs to depend on ARM64 other than limiting
> visibility. So let's also enable building with COMPILE_TEST.
> 
> The CONFIG_OF dependency is redundant as ARM64 always enables it and all
> the DT functions have empty stubs.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>

Acked-by: Nikhil Agarwal <Nikhil.agarwal@amd.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 1/2] cdx: Enable COMPILE_TEST
  2023-12-15 16:10   ` Greg Kroah-Hartman
@ 2023-12-21  6:42     ` Gangurde, Abhijit
  2023-12-21  8:07       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: Gangurde, Abhijit @ 2023-12-21  6:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Gupta, Nipun, Agarwal, Nikhil, linux-kernel@vger.kernel.org,
	Rob Herring

> > On Thu, Dec 07, 2023 at 10:31:26AM -0600, Rob Herring wrote:
> > > There is no reason CDX needs to depend on ARM64 other than limiting
> > > visibility. So let's also enable building with COMPILE_TEST.
> > >
> > > The CONFIG_OF dependency is redundant as ARM64 always enables it and
> all
> > > the DT functions have empty stubs.
> > >
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > ---
> > >  drivers/cdx/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/cdx/Kconfig b/drivers/cdx/Kconfig
> > > index a08958485e31..7cdb7c414453 100644
> > > --- a/drivers/cdx/Kconfig
> > > +++ b/drivers/cdx/Kconfig
> > > @@ -7,7 +7,7 @@
> > >
> > >  config CDX_BUS
> > >  	bool "CDX Bus driver"
> > > -	depends on OF && ARM64
> > > +	depends on ARM64 || COMPILE_TEST
> >
> > Ok, good start, now we need to turn this into a module, what's keeping
> > it from being able to be built as a tristate?
> 
> To answer my own question, the following errors:
> 
> ERROR: modpost: missing MODULE_LICENSE() in drivers/cdx/cdx.o
> ERROR: modpost: "iommu_device_unuse_default_domain"
> [drivers/cdx/cdx.ko] undefined!
> ERROR: modpost: "iommu_device_use_default_domain" [drivers/cdx/cdx.ko]
> undefined!
> 
> Would be great for someone to fix this up...

I did look at this code. There are 2 issues here for it to be a module.
1. There are many symbols in iommu, msi and other module which are not exported.
Most of other busses like amba, fslmc, pci are bool only.
2. As of now, iommu has static list on bus types (static struct bus_type * const iommu_buses[])
 which is initializes the notifier at init time. So, if we change cdx bus to be a module
cdx devices would miss these mappings.

Thanks,
Abhijit

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] cdx: Enable COMPILE_TEST
  2023-12-21  6:42     ` Gangurde, Abhijit
@ 2023-12-21  8:07       ` Greg Kroah-Hartman
  2023-12-21  8:40         ` Gangurde, Abhijit
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2023-12-21  8:07 UTC (permalink / raw)
  To: Gangurde, Abhijit
  Cc: Gupta, Nipun, Agarwal, Nikhil, linux-kernel@vger.kernel.org,
	Rob Herring

On Thu, Dec 21, 2023 at 06:42:29AM +0000, Gangurde, Abhijit wrote:
> > > On Thu, Dec 07, 2023 at 10:31:26AM -0600, Rob Herring wrote:
> > > > There is no reason CDX needs to depend on ARM64 other than limiting
> > > > visibility. So let's also enable building with COMPILE_TEST.
> > > >
> > > > The CONFIG_OF dependency is redundant as ARM64 always enables it and
> > all
> > > > the DT functions have empty stubs.
> > > >
> > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > > ---
> > > >  drivers/cdx/Kconfig | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/cdx/Kconfig b/drivers/cdx/Kconfig
> > > > index a08958485e31..7cdb7c414453 100644
> > > > --- a/drivers/cdx/Kconfig
> > > > +++ b/drivers/cdx/Kconfig
> > > > @@ -7,7 +7,7 @@
> > > >
> > > >  config CDX_BUS
> > > >  	bool "CDX Bus driver"
> > > > -	depends on OF && ARM64
> > > > +	depends on ARM64 || COMPILE_TEST
> > >
> > > Ok, good start, now we need to turn this into a module, what's keeping
> > > it from being able to be built as a tristate?
> > 
> > To answer my own question, the following errors:
> > 
> > ERROR: modpost: missing MODULE_LICENSE() in drivers/cdx/cdx.o
> > ERROR: modpost: "iommu_device_unuse_default_domain"
> > [drivers/cdx/cdx.ko] undefined!
> > ERROR: modpost: "iommu_device_use_default_domain" [drivers/cdx/cdx.ko]
> > undefined!
> > 
> > Would be great for someone to fix this up...
> 
> I did look at this code. There are 2 issues here for it to be a module.
> 1. There are many symbols in iommu, msi and other module which are not exported.
> Most of other busses like amba, fslmc, pci are bool only.

I only see 2 symbols here, what other ones do you see?

> 2. As of now, iommu has static list on bus types (static struct bus_type * const iommu_buses[])
>  which is initializes the notifier at init time. So, if we change cdx bus to be a module
> cdx devices would miss these mappings.

That static bus list needs to be fixed up eventually anyway as we are
moving all bus structures to read-only-memory.  So it's on my todo
list...

Anyway, for now this is ok, but ideally if possible, and if you all want
more build testing, it should be made a module.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 1/2] cdx: Enable COMPILE_TEST
  2023-12-21  8:07       ` Greg Kroah-Hartman
@ 2023-12-21  8:40         ` Gangurde, Abhijit
  2023-12-21  8:46           ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: Gangurde, Abhijit @ 2023-12-21  8:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Gupta, Nipun, Agarwal, Nikhil, linux-kernel@vger.kernel.org,
	Rob Herring

> > > > On Thu, Dec 07, 2023 at 10:31:26AM -0600, Rob Herring wrote:
> > > > > There is no reason CDX needs to depend on ARM64 other than limiting
> > > > > visibility. So let's also enable building with COMPILE_TEST.
> > > > >
> > > > > The CONFIG_OF dependency is redundant as ARM64 always enables it
> and
> > > all
> > > > > the DT functions have empty stubs.
> > > > >
> > > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > > > ---
> > > > >  drivers/cdx/Kconfig | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/cdx/Kconfig b/drivers/cdx/Kconfig
> > > > > index a08958485e31..7cdb7c414453 100644
> > > > > --- a/drivers/cdx/Kconfig
> > > > > +++ b/drivers/cdx/Kconfig
> > > > > @@ -7,7 +7,7 @@
> > > > >
> > > > >  config CDX_BUS
> > > > >  	bool "CDX Bus driver"
> > > > > -	depends on OF && ARM64
> > > > > +	depends on ARM64 || COMPILE_TEST
> > > >
> > > > Ok, good start, now we need to turn this into a module, what's keeping
> > > > it from being able to be built as a tristate?
> > >
> > > To answer my own question, the following errors:
> > >
> > > ERROR: modpost: missing MODULE_LICENSE() in drivers/cdx/cdx.o
> > > ERROR: modpost: "iommu_device_unuse_default_domain"
> > > [drivers/cdx/cdx.ko] undefined!
> > > ERROR: modpost: "iommu_device_use_default_domain"
> [drivers/cdx/cdx.ko]
> > > undefined!
> > >
> > > Would be great for someone to fix this up...
> >
> > I did look at this code. There are 2 issues here for it to be a module.
> > 1. There are many symbols in iommu, msi and other module which are not
> exported.
> > Most of other busses like amba, fslmc, pci are bool only.
> 
> I only see 2 symbols here, what other ones do you see?

There are ~5 symbols from cdx msi patch

Thanks,
Abhijit

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] cdx: Enable COMPILE_TEST
  2023-12-21  8:40         ` Gangurde, Abhijit
@ 2023-12-21  8:46           ` Greg Kroah-Hartman
  0 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2023-12-21  8:46 UTC (permalink / raw)
  To: Gangurde, Abhijit
  Cc: Gupta, Nipun, Agarwal, Nikhil, linux-kernel@vger.kernel.org,
	Rob Herring

On Thu, Dec 21, 2023 at 08:40:05AM +0000, Gangurde, Abhijit wrote:
> > > > > On Thu, Dec 07, 2023 at 10:31:26AM -0600, Rob Herring wrote:
> > > > > > There is no reason CDX needs to depend on ARM64 other than limiting
> > > > > > visibility. So let's also enable building with COMPILE_TEST.
> > > > > >
> > > > > > The CONFIG_OF dependency is redundant as ARM64 always enables it
> > and
> > > > all
> > > > > > the DT functions have empty stubs.
> > > > > >
> > > > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > > > > ---
> > > > > >  drivers/cdx/Kconfig | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/cdx/Kconfig b/drivers/cdx/Kconfig
> > > > > > index a08958485e31..7cdb7c414453 100644
> > > > > > --- a/drivers/cdx/Kconfig
> > > > > > +++ b/drivers/cdx/Kconfig
> > > > > > @@ -7,7 +7,7 @@
> > > > > >
> > > > > >  config CDX_BUS
> > > > > >  	bool "CDX Bus driver"
> > > > > > -	depends on OF && ARM64
> > > > > > +	depends on ARM64 || COMPILE_TEST
> > > > >
> > > > > Ok, good start, now we need to turn this into a module, what's keeping
> > > > > it from being able to be built as a tristate?
> > > >
> > > > To answer my own question, the following errors:
> > > >
> > > > ERROR: modpost: missing MODULE_LICENSE() in drivers/cdx/cdx.o
> > > > ERROR: modpost: "iommu_device_unuse_default_domain"
> > > > [drivers/cdx/cdx.ko] undefined!
> > > > ERROR: modpost: "iommu_device_use_default_domain"
> > [drivers/cdx/cdx.ko]
> > > > undefined!
> > > >
> > > > Would be great for someone to fix this up...
> > >
> > > I did look at this code. There are 2 issues here for it to be a module.
> > > 1. There are many symbols in iommu, msi and other module which are not
> > exported.
> > > Most of other busses like amba, fslmc, pci are bool only.
> > 
> > I only see 2 symbols here, what other ones do you see?
> 
> There are ~5 symbols from cdx msi patch

That hasn't been merged yet :)

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-12-21  8:46 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-07 16:31 [PATCH 1/2] cdx: Enable COMPILE_TEST Rob Herring
2023-12-07 16:31 ` [PATCH 2/2] cdx: Explicitly include correct DT includes, again Rob Herring
2023-12-12  4:40   ` Agarwal, Nikhil
2023-12-09  2:39 ` [PATCH 1/2] cdx: Enable COMPILE_TEST kernel test robot
2023-12-12  4:40 ` Agarwal, Nikhil
2023-12-12 14:26   ` Rob Herring
2023-12-18  4:39     ` Agarwal, Nikhil
2023-12-15 16:09 ` Greg Kroah-Hartman
2023-12-15 16:10   ` Greg Kroah-Hartman
2023-12-21  6:42     ` Gangurde, Abhijit
2023-12-21  8:07       ` Greg Kroah-Hartman
2023-12-21  8:40         ` Gangurde, Abhijit
2023-12-21  8:46           ` Greg Kroah-Hartman
2023-12-18  4:40 ` Agarwal, Nikhil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.