[PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT

[PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT

[Joerg Roedel]

Without this patch it is possible to select the VIDEO_OMAP3
driver which then selects OMAP_IOVMM. But the omap iommu
driver is not compiled without IOMMU_SUPPORT enabled. Fix
that by forcing OMAP_IOMMU and OMAP_IOVMM are enabled before
VIDEO_OMAP3 can be selected.

Cc: Ohad Ben-Cohen <ohad@wizery.com>
Cc: iommu@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/iommu/Kconfig       |    4 ++--
 drivers/media/video/Kconfig |    3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index d901930..ae46776 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -114,8 +114,8 @@ config OMAP_IOMMU
 	select IOMMU_API
 
 config OMAP_IOVMM
-	tristate
-	select OMAP_IOMMU
+	tristate "OMAP IO Virtual Memory Manager Support"
+	depends on OMAP_IOMMU
 
 config OMAP_IOMMU_DEBUG
        tristate "Export OMAP IOMMU/IOVMM internals in DebugFS"
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 6a25fad..6201069 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -763,8 +763,7 @@ source "drivers/media/video/m5mols/Kconfig"
 
 config VIDEO_OMAP3
 	tristate "OMAP 3 Camera support (EXPERIMENTAL)"
-	select OMAP_IOVMM
-	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 && EXPERIMENTAL
+	depends on OMAP_IOVMM && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 && EXPERIMENTAL
 	---help---
 	  Driver for an OMAP 3 camera controller.
 
-- 
1.7.4.1

[PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT

[Joerg Roedel]

Without this patch it is possible to select the VIDEO_OMAP3
driver which then selects OMAP_IOVMM. But the omap iommu
driver is not compiled without IOMMU_SUPPORT enabled. Fix
that by forcing OMAP_IOMMU and OMAP_IOVMM are enabled before
VIDEO_OMAP3 can be selected.

Cc: Ohad Ben-Cohen <ohad@wizery.com>
Cc: iommu@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/iommu/Kconfig       |    4 ++--
 drivers/media/video/Kconfig |    3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index d901930..ae46776 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -114,8 +114,8 @@ config OMAP_IOMMU
 	select IOMMU_API
 
 config OMAP_IOVMM
-	tristate
-	select OMAP_IOMMU
+	tristate "OMAP IO Virtual Memory Manager Support"
+	depends on OMAP_IOMMU
 
 config OMAP_IOMMU_DEBUG
        tristate "Export OMAP IOMMU/IOVMM internals in DebugFS"
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 6a25fad..6201069 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -763,8 +763,7 @@ source "drivers/media/video/m5mols/Kconfig"
 
 config VIDEO_OMAP3
 	tristate "OMAP 3 Camera support (EXPERIMENTAL)"
-	select OMAP_IOVMM
-	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 && EXPERIMENTAL
+	depends on OMAP_IOVMM && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 && EXPERIMENTAL
 	---help---
 	  Driver for an OMAP 3 camera controller.
 
-- 
1.7.4.1

Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT

[Ohad Ben-Cohen]

On Wed, Sep 14, 2011 at 5:07 PM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> Without this patch it is possible to select the VIDEO_OMAP3
> driver which then selects OMAP_IOVMM. But the omap iommu
> driver is not compiled without IOMMU_SUPPORT enabled. Fix
> that by forcing OMAP_IOMMU and OMAP_IOVMM are enabled before
> VIDEO_OMAP3 can be selected.

I'm ok with this but wouldn't it be simpler if we drop IOMMU_SUPPORT
altogether (and just use a plain menu instead) ?

Users will then be presented with only a single iommu-related
question: whether they want their iommu driver built or not.

Something like this:

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index d901930..7d6e5ad 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -2,16 +2,7 @@
 config IOMMU_API
        bool

-menuconfig IOMMU_SUPPORT
-       bool "IOMMU Hardware Support"
-       default y
-       ---help---
-         Say Y here if you want to compile device drivers for IO Memory
-         Management Units into the kernel. These devices usually allow to
-         remap DMA requests and/or remap interrupts from other devices on the
-         system.
-
-if IOMMU_SUPPORT
+menu "IOMMU Hardware Support"

 # MSM IOMMU support
 config MSM_IOMMU
@@ -126,4 +117,4 @@ config OMAP_IOMMU_DEBUG

          Say N unless you know you need this.

-endif # IOMMU_SUPPORT
+endmenu

Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT

[Roedel, Joerg]

On Wed, Sep 14, 2011 at 02:57:13PM -0400, Ohad Ben-Cohen wrote:
> On Wed, Sep 14, 2011 at 5:07 PM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> > Without this patch it is possible to select the VIDEO_OMAP3
> > driver which then selects OMAP_IOVMM. But the omap iommu
> > driver is not compiled without IOMMU_SUPPORT enabled. Fix
> > that by forcing OMAP_IOMMU and OMAP_IOVMM are enabled before
> > VIDEO_OMAP3 can be selected.
> 
> I'm ok with this but wouldn't it be simpler if we drop IOMMU_SUPPORT
> altogether (and just use a plain menu instead) ?
> 
> Users will then be presented with only a single iommu-related
> question: whether they want their iommu driver built or not.

This doesn't work on platforms that may have more than one possible
iommu driver, like x86 for example.

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT

[Ohad Ben-Cohen]

On Thu, Sep 15, 2011 at 1:45 PM, Roedel, Joerg <Joerg.Roedel@amd.com> wrote:
> This doesn't work on platforms that may have more than one possible
> iommu driver, like x86 for example.

Ok.

Acked-by: Ohad Ben-Cohen <ohad@wizery.com>

(cc'ing Laurent for the VIDEO_OMAP3 part)

Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT

[Laurent Pinchart]

Hi Joerg,

On Wednesday 14 September 2011 16:07:39 Joerg Roedel wrote:
> Without this patch it is possible to select the VIDEO_OMAP3
> driver which then selects OMAP_IOVMM. But the omap iommu
> driver is not compiled without IOMMU_SUPPORT enabled. Fix
> that by forcing OMAP_IOMMU and OMAP_IOVMM are enabled before
> VIDEO_OMAP3 can be selected.

What about making VIDEO_OMAP3 select IOMMU_SUPPORT instead then ? Your patch 
would make the OMAP3 ISP driver disappear from the menu until IOMMU_SUPPORT 
gets turned on, which can confuse users.

> Cc: Ohad Ben-Cohen <ohad@wizery.com>
> Cc: iommu@lists.linux-foundation.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  drivers/iommu/Kconfig       |    4 ++--
>  drivers/media/video/Kconfig |    3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index d901930..ae46776 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -114,8 +114,8 @@ config OMAP_IOMMU
>  	select IOMMU_API
> 
>  config OMAP_IOVMM
> -	tristate
> -	select OMAP_IOMMU
> +	tristate "OMAP IO Virtual Memory Manager Support"
> +	depends on OMAP_IOMMU
> 
>  config OMAP_IOMMU_DEBUG
>         tristate "Export OMAP IOMMU/IOVMM internals in DebugFS"
> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> index 6a25fad..6201069 100644
> --- a/drivers/media/video/Kconfig
> +++ b/drivers/media/video/Kconfig
> @@ -763,8 +763,7 @@ source "drivers/media/video/m5mols/Kconfig"
> 
>  config VIDEO_OMAP3
>  	tristate "OMAP 3 Camera support (EXPERIMENTAL)"
> -	select OMAP_IOVMM
> -	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 &&
> EXPERIMENTAL
> +	depends on OMAP_IOVMM && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API &&
> ARCH_OMAP3 && EXPERIMENTAL
> ---help---
>  	  Driver for an OMAP 3 camera controller.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT

[Roedel, Joerg]

Hi Laurent,

On Sat, Sep 17, 2011 at 08:02:22PM -0400, Laurent Pinchart wrote:
> On Wednesday 14 September 2011 16:07:39 Joerg Roedel wrote:
> > Without this patch it is possible to select the VIDEO_OMAP3
> > driver which then selects OMAP_IOVMM. But the omap iommu
> > driver is not compiled without IOMMU_SUPPORT enabled. Fix
> > that by forcing OMAP_IOMMU and OMAP_IOVMM are enabled before
> > VIDEO_OMAP3 can be selected.
> 
> What about making VIDEO_OMAP3 select IOMMU_SUPPORT instead then ? Your patch 
> would make the OMAP3 ISP driver disappear from the menu until IOMMU_SUPPORT 
> gets turned on, which can confuse users.

Using 'depends on' rather then 'selects' is common standard in Kconfig.
You can't select PCI drivers without selecting PCI first, for example.
Further selecting whole drivers implicitly isn't a good idea. This can
grow out of control very quickly.

Regards,

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT

[Laurent Pinchart]

Hi Joerg,

On Tuesday 20 September 2011 12:01:46 Roedel, Joerg wrote:
> On Sat, Sep 17, 2011 at 08:02:22PM -0400, Laurent Pinchart wrote:
> > On Wednesday 14 September 2011 16:07:39 Joerg Roedel wrote:
> > > Without this patch it is possible to select the VIDEO_OMAP3
> > > driver which then selects OMAP_IOVMM. But the omap iommu
> > > driver is not compiled without IOMMU_SUPPORT enabled. Fix
> > > that by forcing OMAP_IOMMU and OMAP_IOVMM are enabled before
> > > VIDEO_OMAP3 can be selected.
> > 
> > What about making VIDEO_OMAP3 select IOMMU_SUPPORT instead then ? Your
> > patch would make the OMAP3 ISP driver disappear from the menu until
> > IOMMU_SUPPORT gets turned on, which can confuse users.
> 
> Using 'depends on' rather then 'selects' is common standard in Kconfig.
> You can't select PCI drivers without selecting PCI first, for example.

You wouldn't expect a PCI driver to work without PCI support. My concern is 
that most OMAP3 ISP users won't know that IOMMU supports is required. Feel 
free to ignore it though :-)

> Further selecting whole drivers implicitly isn't a good idea. This can
> grow out of control very quickly.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT

[Felipe Contreras]

On Tue, Sep 20, 2011 at 1:54 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tuesday 20 September 2011 12:01:46 Roedel, Joerg wrote:
>> On Sat, Sep 17, 2011 at 08:02:22PM -0400, Laurent Pinchart wrote:
>> > On Wednesday 14 September 2011 16:07:39 Joerg Roedel wrote:
>> > > Without this patch it is possible to select the VIDEO_OMAP3
>> > > driver which then selects OMAP_IOVMM. But the omap iommu
>> > > driver is not compiled without IOMMU_SUPPORT enabled. Fix
>> > > that by forcing OMAP_IOMMU and OMAP_IOVMM are enabled before
>> > > VIDEO_OMAP3 can be selected.
>> >
>> > What about making VIDEO_OMAP3 select IOMMU_SUPPORT instead then ? Your
>> > patch would make the OMAP3 ISP driver disappear from the menu until
>> > IOMMU_SUPPORT gets turned on, which can confuse users.
>>
>> Using 'depends on' rather then 'selects' is common standard in Kconfig.
>> You can't select PCI drivers without selecting PCI first, for example.
>
> You wouldn't expect a PCI driver to work without PCI support. My concern is
> that most OMAP3 ISP users won't know that IOMMU supports is required. Feel
> free to ignore it though :-)

I agree with Laurent. As a user, why should I care about IOMMU? I just
want this thing.

That's why DRM_RADEON_KMS selects BACKLIGHT_CLASS_DEVICE; it's an
implementation detail that the user should not need to care about. Why
should I need to go see the dependencies of DRM_RADEON_KMS and
manually select all of them? I shouldn't, that's the reason 'select'
exists in the first place.

Cheers.

-- 
Felipe Contreras

Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT

[Roedel, Joerg]

On Thu, Sep 22, 2011 at 09:48:27AM -0400, Felipe Contreras wrote:
> On Tue, Sep 20, 2011 at 1:54 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Tuesday 20 September 2011 12:01:46 Roedel, Joerg wrote:
> >> On Sat, Sep 17, 2011 at 08:02:22PM -0400, Laurent Pinchart wrote:
> >> > On Wednesday 14 September 2011 16:07:39 Joerg Roedel wrote:
> >> > > Without this patch it is possible to select the VIDEO_OMAP3
> >> > > driver which then selects OMAP_IOVMM. But the omap iommu
> >> > > driver is not compiled without IOMMU_SUPPORT enabled. Fix
> >> > > that by forcing OMAP_IOMMU and OMAP_IOVMM are enabled before
> >> > > VIDEO_OMAP3 can be selected.
> >> >
> >> > What about making VIDEO_OMAP3 select IOMMU_SUPPORT instead then ? Your
> >> > patch would make the OMAP3 ISP driver disappear from the menu until
> >> > IOMMU_SUPPORT gets turned on, which can confuse users.
> >>
> >> Using 'depends on' rather then 'selects' is common standard in Kconfig.
> >> You can't select PCI drivers without selecting PCI first, for example.
> >
> > You wouldn't expect a PCI driver to work without PCI support. My concern is
> > that most OMAP3 ISP users won't know that IOMMU supports is required. Feel
> > free to ignore it though :-)
> 
> I agree with Laurent. As a user, why should I care about IOMMU? I just
> want this thing.

Because enabling IOMMU can have other side-effects on the system and as
a user you want to be aware that it is enabled. A user not wanting the
iommu driver can just disable it then without the need to de-select all
drivers depending on it first.

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632