[PATCH] watchdog: dw_wdt: remove build dependencies

[PATCH] watchdog: dw_wdt: remove build dependencies

[Baruch Siach]

There is nothing ARM specific in this driver. Also, clk.h now includes stubs
for !CONFIG_HAVE_CLK, so the driver should build anyway.

Cc: Jamie Iles <jamie@jamieiles.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/watchdog/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 5be6e91..5ffac39 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -214,7 +214,6 @@ config SA1100_WATCHDOG
 
 config DW_WATCHDOG
 	tristate "Synopsys DesignWare watchdog"
-	depends on ARM && HAVE_CLK
 	help
 	  Say Y here if to include support for the Synopsys DesignWare
 	  watchdog timer found in many ARM chips.
-- 
1.8.5.2

Re: [PATCH] watchdog: dw_wdt: remove build dependencies

[Wim Van Sebroeck]

Hi Baruch,

If this is ARM hardware, then this should have the dependency "depends on ARM".
Only if other architectures start using this watchdog device, then the "depends on ARM" can go.

Kind regards,
Wim.

> There is nothing ARM specific in this driver. Also, clk.h now includes stubs
> for !CONFIG_HAVE_CLK, so the driver should build anyway.
> 
> Cc: Jamie Iles <jamie@jamieiles.com>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  drivers/watchdog/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 5be6e91..5ffac39 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -214,7 +214,6 @@ config SA1100_WATCHDOG
>  
>  config DW_WATCHDOG
>  	tristate "Synopsys DesignWare watchdog"
> -	depends on ARM && HAVE_CLK
>  	help
>  	  Say Y here if to include support for the Synopsys DesignWare
>  	  watchdog timer found in many ARM chips.
> -- 
> 1.8.5.2
> 

Re: [PATCH] watchdog: dw_wdt: remove build dependencies

[Baruch Siach]

Hi Wim,

On Mon, Dec 30, 2013 at 09:37:55AM +0100, Wim Van Sebroeck wrote:
> If this is ARM hardware, then this should have the dependency "depends on 
> ARM".
> Only if other architectures start using this watchdog device, then the 
> "depends on ARM" can go.

Well, we intend to use this driver on Xtensa platform, does this count?

As a general rule wouldn't it be better to have the widest possible build 
coverage, and encourage arch neutral driver coding?

baruch

> > There is nothing ARM specific in this driver. Also, clk.h now includes 
> > stubs
> > for !CONFIG_HAVE_CLK, so the driver should build anyway.
> > 
> > Cc: Jamie Iles <jamie@jamieiles.com>
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> >  drivers/watchdog/Kconfig | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 5be6e91..5ffac39 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -214,7 +214,6 @@ config SA1100_WATCHDOG
> >  
> >  config DW_WATCHDOG
> >  	tristate "Synopsys DesignWare watchdog"
> > -	depends on ARM && HAVE_CLK
> >  	help
> >  	  Say Y here if to include support for the Synopsys DesignWare
> >  	  watchdog timer found in many ARM chips.

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

Re: [PATCH] watchdog: dw_wdt: remove build dependencies

[Wim Van Sebroeck]

Hi Baruch,

> > If this is ARM hardware, then this should have the dependency "depends on 
> > ARM".
> > Only if other architectures start using this watchdog device, then the 
> > "depends on ARM" can go.
> 
> Well, we intend to use this driver on Xtensa platform, does this count?

Xtensa is indeed another Platform, but then I expect the patch to:
1) make this clear in the Kconfig file
2) have a commit-message that also explains this.

> As a general rule wouldn't it be better to have the widest possible build 
> coverage, and encourage arch neutral driver coding?

We try to have as much arch neutral driver encoding (see the Kconfig & Makefile in the drivers/watchdog directory and you will see that we have drivers that are not depending on a specific hardware architecture), but we should not forget that certain hardware is only available in a certain architecture and that it thus does not make any sense to compile kernels with drivers that never will be used. So the general rule still is: use common sense and look at the broader picture.

Kind regards,
Wim.

Re: [PATCH] watchdog: dw_wdt: remove build dependencies

[Baruch Siach]

Hi Wim,

On Mon, Dec 30, 2013 at 12:03:36PM +0100, Wim Van Sebroeck wrote:
> > > If this is ARM hardware, then this should have the dependency "depends 
> > > on ARM".
> > > Only if other architectures start using this watchdog device, then the 
> > > "depends on ARM" can go.
> > 
> > Well, we intend to use this driver on Xtensa platform, does this count?
> 
> Xtensa is indeed another Platform, but then I expect the patch to:
> 1) make this clear in the Kconfig file

OK. So should I add a dependency on CONFIG_XTENSA, or just add a comment 
saying that this driver is cross platform.

> 2) have a commit-message that also explains this.

Will do.

> 
> > As a general rule wouldn't it be better to have the widest possible build 
> > coverage, and encourage arch neutral driver coding?
> 
> We try to have as much arch neutral driver encoding (see the Kconfig & 
> Makefile in the drivers/watchdog directory and you will see that we have 
> drivers that are not depending on a specific hardware architecture), but we 
> should not forget that certain hardware is only available in a certain 
> architecture and that it thus does not make any sense to compile kernels 
> with drivers that never will be used. So the general rule still is: use 
> common sense and look at the broader picture.

This watchdog is part of the DesignWare family of IP blocks. These IP block 
are being reused on many platforms. The kernel already has drivers for 
DesignWare SPI, I2C, USB, and hopefully also GPIO soon. None of these drivers 
depend on any specific architecture. IMO it makes sense to drop arch 
dependency entirely in this case.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

Re: [PATCH] watchdog: dw_wdt: remove build dependencies

[Wim Van Sebroeck]

Hi Baruch,

> > > > If this is ARM hardware, then this should have the dependency "depends 
> > > > on ARM".
> > > > Only if other architectures start using this watchdog device, then the 
> > > > "depends on ARM" can go.
> > > 
> > > Well, we intend to use this driver on Xtensa platform, does this count?
> > 
> > Xtensa is indeed another Platform, but then I expect the patch to:
> > 1) make this clear in the Kconfig file
> 
> OK. So should I add a dependency on CONFIG_XTENSA, or just add a comment 
> saying that this driver is cross platform.

The help text references only designware on ARM platforms, So the text needs
to be changed.

> > 2) have a commit-message that also explains this.
> 
> Will do.
> 
> > 
> > > As a general rule wouldn't it be better to have the widest possible build 
> > > coverage, and encourage arch neutral driver coding?
> > 
> > We try to have as much arch neutral driver encoding (see the Kconfig & 
> > Makefile in the drivers/watchdog directory and you will see that we have 
> > drivers that are not depending on a specific hardware architecture), but we 
> > should not forget that certain hardware is only available in a certain 
> > architecture and that it thus does not make any sense to compile kernels 
> > with drivers that never will be used. So the general rule still is: use 
> > common sense and look at the broader picture.
> 
> This watchdog is part of the DesignWare family of IP blocks. These IP block 
> are being reused on many platforms. The kernel already has drivers for 
> DesignWare SPI, I2C, USB, and hopefully also GPIO soon. None of these drivers 
> depend on any specific architecture. IMO it makes sense to drop arch 
> dependency entirely in this case.

Correct.

Kind regards,
Wim.

Re: [PATCH] watchdog: dw_wdt: remove build dependencies

[Baruch Siach]

Hi Wim,

On Mon, Dec 30, 2013 at 01:08:20PM +0100, Wim Van Sebroeck wrote:
> > > > > If this is ARM hardware, then this should have the dependency 
> > > > > "depends on ARM".
> > > > > Only if other architectures start using this watchdog device, then the 
> > > > > "depends on ARM" can go.
> > > > 
> > > > Well, we intend to use this driver on Xtensa platform, does this count?
> > > 
> > > Xtensa is indeed another Platform, but then I expect the patch to:
> > > 1) make this clear in the Kconfig file
> > 
> > OK. So should I add a dependency on CONFIG_XTENSA, or just add a comment 
> > saying that this driver is cross platform.
> 
> The help text references only designware on ARM platforms, So the text needs
> to be changed.

Ah, I see that now. I'll also update the comment at the beginning of dw_wdt.c 
file likewise.

> > > 2) have a commit-message that also explains this.
> > 
> > Will do.
> > 
> > > 
> > > > As a general rule wouldn't it be better to have the widest possible build 
> > > > coverage, and encourage arch neutral driver coding?
> > > 
> > > We try to have as much arch neutral driver encoding (see the Kconfig & 
> > > Makefile in the drivers/watchdog directory and you will see that we have 
> > > drivers that are not depending on a specific hardware architecture), but we 
> > > should not forget that certain hardware is only available in a certain 
> > > architecture and that it thus does not make any sense to compile kernels 
> > > with drivers that never will be used. So the general rule still is: use 
> > > common sense and look at the broader picture.
> > 
> > This watchdog is part of the DesignWare family of IP blocks. These IP block 
> > are being reused on many platforms. The kernel already has drivers for 
> > DesignWare SPI, I2C, USB, and hopefully also GPIO soon. None of these drivers 
> > depend on any specific architecture. IMO it makes sense to drop arch 
> > dependency entirely in this case.
> 
> Correct.

Thanks for reviewing.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -