[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(-)
  

> -----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
  

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
  

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
  

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
  

> -----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.
  

> -----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>
  

> > 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
  

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
  

> > > > 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
  

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 :)