* [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
@ 2011-09-14 14:07 ` Joerg Roedel
0 siblings, 0 replies; 10+ messages in thread
From: Joerg Roedel @ 2011-09-14 14:07 UTC (permalink / raw)
To: linux-omap; +Cc: Joerg Roedel, Ohad Ben-Cohen, iommu, linux-kernel
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
@ 2011-09-14 14:07 ` Joerg Roedel
0 siblings, 0 replies; 10+ messages in thread
From: Joerg Roedel @ 2011-09-14 14:07 UTC (permalink / raw)
To: linux-omap; +Cc: Joerg Roedel, Ohad Ben-Cohen, iommu, linux-kernel
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
2011-09-14 14:07 ` Joerg Roedel
(?)
@ 2011-09-14 18:57 ` Ohad Ben-Cohen
2011-09-15 10:45 ` Roedel, Joerg
-1 siblings, 1 reply; 10+ messages in thread
From: Ohad Ben-Cohen @ 2011-09-14 18:57 UTC (permalink / raw)
To: Joerg Roedel; +Cc: linux-omap, iommu, linux-kernel
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
2011-09-14 18:57 ` Ohad Ben-Cohen
@ 2011-09-15 10:45 ` Roedel, Joerg
2011-09-16 9:57 ` Ohad Ben-Cohen
0 siblings, 1 reply; 10+ messages in thread
From: Roedel, Joerg @ 2011-09-15 10:45 UTC (permalink / raw)
To: Ohad Ben-Cohen; +Cc: linux-omap, iommu, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
2011-09-15 10:45 ` Roedel, Joerg
@ 2011-09-16 9:57 ` Ohad Ben-Cohen
0 siblings, 0 replies; 10+ messages in thread
From: Ohad Ben-Cohen @ 2011-09-16 9:57 UTC (permalink / raw)
To: Roedel, Joerg, Laurent Pinchart; +Cc: linux-omap, iommu, linux-kernel
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)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
2011-09-14 14:07 ` Joerg Roedel
(?)
(?)
@ 2011-09-18 0:02 ` Laurent Pinchart
2011-09-20 10:01 ` Roedel, Joerg
-1 siblings, 1 reply; 10+ messages in thread
From: Laurent Pinchart @ 2011-09-18 0:02 UTC (permalink / raw)
To: Joerg Roedel; +Cc: linux-omap, Ohad Ben-Cohen, iommu, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
2011-09-18 0:02 ` Laurent Pinchart
@ 2011-09-20 10:01 ` Roedel, Joerg
2011-09-20 10:54 ` Laurent Pinchart
0 siblings, 1 reply; 10+ messages in thread
From: Roedel, Joerg @ 2011-09-20 10:01 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-omap, Ohad Ben-Cohen, iommu, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
2011-09-20 10:01 ` Roedel, Joerg
@ 2011-09-20 10:54 ` Laurent Pinchart
2011-09-22 13:48 ` Felipe Contreras
0 siblings, 1 reply; 10+ messages in thread
From: Laurent Pinchart @ 2011-09-20 10:54 UTC (permalink / raw)
To: Roedel, Joerg; +Cc: linux-omap, Ohad Ben-Cohen, iommu, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
2011-09-20 10:54 ` Laurent Pinchart
@ 2011-09-22 13:48 ` Felipe Contreras
2011-09-23 9:05 ` Roedel, Joerg
0 siblings, 1 reply; 10+ messages in thread
From: Felipe Contreras @ 2011-09-22 13:48 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Roedel, Joerg, linux-omap, Ohad Ben-Cohen, iommu, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
2011-09-22 13:48 ` Felipe Contreras
@ 2011-09-23 9:05 ` Roedel, Joerg
0 siblings, 0 replies; 10+ messages in thread
From: Roedel, Joerg @ 2011-09-23 9:05 UTC (permalink / raw)
To: Felipe Contreras
Cc: Laurent Pinchart, linux-omap, Ohad Ben-Cohen, iommu, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-09-23 9:06 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-14 14:07 [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT Joerg Roedel
2011-09-14 14:07 ` Joerg Roedel
2011-09-14 18:57 ` Ohad Ben-Cohen
2011-09-15 10:45 ` Roedel, Joerg
2011-09-16 9:57 ` Ohad Ben-Cohen
2011-09-18 0:02 ` Laurent Pinchart
2011-09-20 10:01 ` Roedel, Joerg
2011-09-20 10:54 ` Laurent Pinchart
2011-09-22 13:48 ` Felipe Contreras
2011-09-23 9:05 ` Roedel, Joerg
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.