Message ID | 1473960849-23024-27-git-send-email-vinod.koul@intel.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
On 09/15/2016 07:34 PM, Vinod Koul wrote: > To get more coverage, enable COMPILE_TEST for this driver. > > Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com> > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > --- > drivers/dma/Kconfig | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig > index 82d6e374d82f..60341185a6b9 100644 > --- a/drivers/dma/Kconfig > +++ b/drivers/dma/Kconfig > @@ -451,8 +451,8 @@ config STM32_DMA > here. > > config S3C24XX_DMAC > - bool "Samsung S3C24XX DMA support" > - depends on ARCH_S3C24XX > + bool "Samsung S3C24XX DMA support" if COMPILE_TEST && !ARCH_S3C24XX > + default ARCH_S3C24XX This is not equivalent to previous code. I think it should be: bool "Samsung S3C24XX DMA support" if COMPILE_TEST depends on ARCH_S3C24XX || COMPILE_TEST 1. This way we do not enable it by default on the architecture (it wasn't enabled so far), 2. The option is visible on compile test, 3. The runtime dependency is preserved. Best regards, Krzysztof > select DMA_ENGINE > select DMA_VIRTUAL_CHANNELS > help > -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday, September 16, 2016 2:56:32 PM CEST Krzysztof Kozlowski wrote: > On 09/15/2016 07:34 PM, Vinod Koul wrote: > > To get more coverage, enable COMPILE_TEST for this driver. > > > > Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com> > > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > > --- > > drivers/dma/Kconfig | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig > > index 82d6e374d82f..60341185a6b9 100644 > > --- a/drivers/dma/Kconfig > > +++ b/drivers/dma/Kconfig > > @@ -451,8 +451,8 @@ config STM32_DMA > > here. > > > > config S3C24XX_DMAC > > - bool "Samsung S3C24XX DMA support" > > - depends on ARCH_S3C24XX > > + bool "Samsung S3C24XX DMA support" if COMPILE_TEST && !ARCH_S3C24XX > > + default ARCH_S3C24XX > > This is not equivalent to previous code. > > I think it should be: > bool "Samsung S3C24XX DMA support" if COMPILE_TEST > depends on ARCH_S3C24XX || COMPILE_TEST > > 1. This way we do not enable it by default on the architecture (it > wasn't enabled so far), > 2. The option is visible on compile test, > 3. The runtime dependency is preserved. Unless you add the "default ARCH_S3C24XX", it will be disabled for normal configurations and impossible to get turned on, which is also wrong. I think in this case we want: config S3C24XX_DMAC bool "Samsung S3C24XX DMA support" - depends on ARCH_S3C24XX + depends on ARCH_S3C24XX || COMPILE_TEST Arnd -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 16, 2016 at 02:56:32PM +0200, Krzysztof Kozlowski wrote: > On 09/15/2016 07:34 PM, Vinod Koul wrote: > > To get more coverage, enable COMPILE_TEST for this driver. > > > > Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com> > > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > > --- > > drivers/dma/Kconfig | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig > > index 82d6e374d82f..60341185a6b9 100644 > > --- a/drivers/dma/Kconfig > > +++ b/drivers/dma/Kconfig > > @@ -451,8 +451,8 @@ config STM32_DMA > > here. > > > > config S3C24XX_DMAC > > - bool "Samsung S3C24XX DMA support" > > - depends on ARCH_S3C24XX > > + bool "Samsung S3C24XX DMA support" if COMPILE_TEST && !ARCH_S3C24XX > > + default ARCH_S3C24XX > > This is not equivalent to previous code. Yeah it is not and enables the driver by default. My bad, I should have explicitly mentioned this. My though was to indeed enable them by default, do you see any issues with that? > > I think it should be: > bool "Samsung S3C24XX DMA support" if COMPILE_TEST > depends on ARCH_S3C24XX || COMPILE_TEST > > 1. This way we do not enable it by default on the architecture (it > wasn't enabled so far), > 2. The option is visible on compile test, > 3. The runtime dependency is preserved. > > Best regards, > Krzysztof > > > select DMA_ENGINE > > select DMA_VIRTUAL_CHANNELS > > help > > >
On Fri, Sep 16, 2016 at 04:03:57PM +0200, Arnd Bergmann wrote: > On Friday, September 16, 2016 2:56:32 PM CEST Krzysztof Kozlowski wrote: > > On 09/15/2016 07:34 PM, Vinod Koul wrote: > > > To get more coverage, enable COMPILE_TEST for this driver. > > > > > > Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com> > > > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > > > --- > > > drivers/dma/Kconfig | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig > > > index 82d6e374d82f..60341185a6b9 100644 > > > --- a/drivers/dma/Kconfig > > > +++ b/drivers/dma/Kconfig > > > @@ -451,8 +451,8 @@ config STM32_DMA > > > here. > > > > > > config S3C24XX_DMAC > > > - bool "Samsung S3C24XX DMA support" > > > - depends on ARCH_S3C24XX > > > + bool "Samsung S3C24XX DMA support" if COMPILE_TEST && !ARCH_S3C24XX > > > + default ARCH_S3C24XX > > > > This is not equivalent to previous code. > > > > I think it should be: > > bool "Samsung S3C24XX DMA support" if COMPILE_TEST > > depends on ARCH_S3C24XX || COMPILE_TEST > > > > 1. This way we do not enable it by default on the architecture (it > > wasn't enabled so far), > > 2. The option is visible on compile test, > > 3. The runtime dependency is preserved. > > Unless you add the "default ARCH_S3C24XX", it will be disabled for > normal configurations and impossible to get turned on, which is > also wrong. I think in this case we want: > > config S3C24XX_DMAC > bool "Samsung S3C24XX DMA support" > - depends on ARCH_S3C24XX > + depends on ARCH_S3C24XX || COMPILE_TEST Right, the bool should remain untouched. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 16, 2016 at 08:21:13PM +0530, Vinod Koul wrote: > On Fri, Sep 16, 2016 at 02:56:32PM +0200, Krzysztof Kozlowski wrote: > > On 09/15/2016 07:34 PM, Vinod Koul wrote: > > > To get more coverage, enable COMPILE_TEST for this driver. > > > > > > Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com> > > > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > > > --- > > > drivers/dma/Kconfig | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig > > > index 82d6e374d82f..60341185a6b9 100644 > > > --- a/drivers/dma/Kconfig > > > +++ b/drivers/dma/Kconfig > > > @@ -451,8 +451,8 @@ config STM32_DMA > > > here. > > > > > > config S3C24XX_DMAC > > > - bool "Samsung S3C24XX DMA support" > > > - depends on ARCH_S3C24XX > > > + bool "Samsung S3C24XX DMA support" if COMPILE_TEST && !ARCH_S3C24XX > > > + default ARCH_S3C24XX > > > > This is not equivalent to previous code. > > Yeah it is not and enables the driver by default. My bad, I should have > explicitly mentioned this. > > My though was to indeed enable them by default, do you see any issues with > that? I don't see any particular problems with that. Default value is just a hint and can be overridden. I think however that such change should be in separate patch. One thing is adding compile-test and second is turning this on for our defconfigs. They might have different outcomes (failing of various archs/configs for the first, failing of our defconfigs for the latter). Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index 82d6e374d82f..60341185a6b9 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -451,8 +451,8 @@ config STM32_DMA here. config S3C24XX_DMAC - bool "Samsung S3C24XX DMA support" - depends on ARCH_S3C24XX + bool "Samsung S3C24XX DMA support" if COMPILE_TEST && !ARCH_S3C24XX + default ARCH_S3C24XX select DMA_ENGINE select DMA_VIRTUAL_CHANNELS help
To get more coverage, enable COMPILE_TEST for this driver. Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com> Signed-off-by: Vinod Koul <vinod.koul@intel.com> --- drivers/dma/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)