Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
On Thu, Sep 22, 2011 at 09:48:27AM -0400, Felipe Contreras wrote: > On Tue, Sep 20, 2011 at 1:54 PM, Laurent Pinchart > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
On Tue, Sep 20, 2011 at 1:54 PM, Laurent Pinchart 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
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 > Cc: io...@lists.linux-foundation.org > Cc: linux-ker...@vger.kernel.org > Signed-off-by: Joerg Roedel > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
On Thu, Sep 15, 2011 at 1:45 PM, Roedel, Joerg 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 (cc'ing Laurent for the VIDEO_OMAP3 part) -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
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 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iommu/omap: Fix build error with !IOMMU_SUPPORT
On Wed, Sep 14, 2011 at 5:07 PM, 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. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
