[PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Javier Martinez Canillas]

These drivers only have runtime but no build time dependencies so can be
built for testing purposes if the Kconfig COMPILE_TEST option is enabled.

This is useful to have more build coverage and make sure that drivers are
not affected by changes that could cause build regressions.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---

 drivers/clk/Kconfig           | 8 ++++----
 drivers/clk/bcm/Kconfig       | 4 ++--
 drivers/clk/versatile/Kconfig | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 573517151976..57316528e924 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -124,7 +124,7 @@ config CLK_TWL6040
 
 config COMMON_CLK_AXI_CLKGEN
 	tristate "AXI clkgen driver"
-	depends on ARCH_ZYNQ || MICROBLAZE
+	depends on ARCH_ZYNQ || MICROBLAZE || COMPILE_TEST
 	help
 	---help---
 	  Support for the Analog Devices axi-clkgen pcore clock generator for Xilinx
@@ -132,7 +132,7 @@ config COMMON_CLK_AXI_CLKGEN
 
 config CLK_QORIQ
 	bool "Clock driver for Freescale QorIQ platforms"
-	depends on (PPC_E500MC || ARM) && OF
+	depends on (PPC_E500MC || ARM || COMPILE_TEST) && OF
 	---help---
 	  This adds the clock driver support for Freescale QorIQ platforms
 	  using common clock framework.
@@ -140,13 +140,13 @@ config CLK_QORIQ
 config COMMON_CLK_XGENE
 	bool "Clock driver for APM XGene SoC"
 	default y
-	depends on ARM64
+	depends on ARM64 || COMPILE_TEST
 	---help---
 	  Sypport for the APM X-Gene SoC reference, PLL, and device clocks.
 
 config COMMON_CLK_KEYSTONE
 	tristate "Clock drivers for Keystone based SOCs"
-	depends on ARCH_KEYSTONE && OF
+	depends on (ARCH_KEYSTONE || COMPILE_TEST) && OF
 	---help---
           Supports clock drivers for Keystone based SOCs. These SOCs have local
 	  a power sleep control module that gate the clock to the IPs and PLLs.
diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
index 88febf53b276..561e9dc6d40d 100644
--- a/drivers/clk/bcm/Kconfig
+++ b/drivers/clk/bcm/Kconfig
@@ -1,6 +1,6 @@
 config CLK_BCM_KONA
 	bool "Broadcom Kona CCU clock support"
-	depends on ARCH_BCM_MOBILE
+	depends on ARCH_BCM_MOBILE || COMPILE_TEST
 	depends on COMMON_CLK
 	default y
 	help
@@ -10,7 +10,7 @@ config CLK_BCM_KONA
 
 config COMMON_CLK_IPROC
 	bool "Broadcom iProc clock support"
-	depends on ARCH_BCM_IPROC
+	depends on ARCH_BCM_IPROC || COMPILE_TEST
 	depends on COMMON_CLK
 	default ARCH_BCM_IPROC
 	help
diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
index 1530c9352a76..fc50b6264bed 100644
--- a/drivers/clk/versatile/Kconfig
+++ b/drivers/clk/versatile/Kconfig
@@ -1,6 +1,6 @@
 config COMMON_CLK_VERSATILE
 	bool "Clock driver for ARM Reference designs"
-	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
+	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64 || COMPILE_TEST
 	---help---
           Supports clocking on ARM Reference designs:
 	  - Integrator/AP and Integrator/CP
-- 
2.4.3

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[kbuild test robot]

Hi Javier,

[auto build test WARNING on clk/clk-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Javier-Martinez-Canillas/clk-Allow-drivers-to-build-if-COMPILE_TEST-is-enabled/20151013-222133
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/clk/clk-xgene.c:217:12: sparse: context imbalance in 'xgene_clk_enable' - different lock contexts for basic block
>> drivers/clk/clk-xgene.c:285:9: sparse: context imbalance in 'xgene_clk_disable' - different lock contexts for basic block
>> drivers/clk/clk-xgene.c:368:9: sparse: context imbalance in 'xgene_clk_set_rate' - different lock contexts for basic block
--
>> drivers/clk/keystone/gate.c:117:12: sparse: context imbalance in 'keystone_clk_enable' - different lock contexts for basic block
>> drivers/clk/keystone/gate.c:147:9: sparse: context imbalance in 'keystone_clk_disable' - different lock contexts for basic block

vim +/xgene_clk_enable +217 drivers/clk/clk-xgene.c

308964ca Loc Ho           2013-06-26  211  	spinlock_t	*lock;
308964ca Loc Ho           2013-06-26  212  	struct xgene_dev_parameters	param;
308964ca Loc Ho           2013-06-26  213  };
308964ca Loc Ho           2013-06-26  214  
308964ca Loc Ho           2013-06-26  215  #define to_xgene_clk(_hw) container_of(_hw, struct xgene_clk, hw)
308964ca Loc Ho           2013-06-26  216  
308964ca Loc Ho           2013-06-26 @217  static int xgene_clk_enable(struct clk_hw *hw)
308964ca Loc Ho           2013-06-26  218  {
308964ca Loc Ho           2013-06-26  219  	struct xgene_clk *pclk = to_xgene_clk(hw);
308964ca Loc Ho           2013-06-26  220  	unsigned long flags = 0;
308964ca Loc Ho           2013-06-26  221  	u32 data;
6ae5fd38 Stephen Boyd     2015-05-01  222  	phys_addr_t reg;
308964ca Loc Ho           2013-06-26  223  
308964ca Loc Ho           2013-06-26  224  	if (pclk->lock)
308964ca Loc Ho           2013-06-26  225  		spin_lock_irqsave(pclk->lock, flags);
308964ca Loc Ho           2013-06-26  226  
308964ca Loc Ho           2013-06-26  227  	if (pclk->param.csr_reg != NULL) {
836ee0f7 Stephen Boyd     2015-08-12  228  		pr_debug("%s clock enabled\n", clk_hw_get_name(hw));
6ae5fd38 Stephen Boyd     2015-05-01  229  		reg = __pa(pclk->param.csr_reg);
308964ca Loc Ho           2013-06-26  230  		/* First enable the clock */
308964ca Loc Ho           2013-06-26  231  		data = xgene_clk_read(pclk->param.csr_reg +
308964ca Loc Ho           2013-06-26  232  					pclk->param.reg_clk_offset);
308964ca Loc Ho           2013-06-26  233  		data |= pclk->param.reg_clk_mask;
308964ca Loc Ho           2013-06-26  234  		xgene_clk_write(data, pclk->param.csr_reg +
308964ca Loc Ho           2013-06-26  235  					pclk->param.reg_clk_offset);
6ae5fd38 Stephen Boyd     2015-05-01  236  		pr_debug("%s clock PADDR base %pa clk offset 0x%08X mask 0x%08X value 0x%08X\n",
836ee0f7 Stephen Boyd     2015-08-12  237  			clk_hw_get_name(hw), &reg,
308964ca Loc Ho           2013-06-26  238  			pclk->param.reg_clk_offset, pclk->param.reg_clk_mask,
308964ca Loc Ho           2013-06-26  239  			data);
308964ca Loc Ho           2013-06-26  240  
308964ca Loc Ho           2013-06-26  241  		/* Second enable the CSR */
308964ca Loc Ho           2013-06-26  242  		data = xgene_clk_read(pclk->param.csr_reg +
308964ca Loc Ho           2013-06-26  243  					pclk->param.reg_csr_offset);
308964ca Loc Ho           2013-06-26  244  		data &= ~pclk->param.reg_csr_mask;
308964ca Loc Ho           2013-06-26  245  		xgene_clk_write(data, pclk->param.csr_reg +
308964ca Loc Ho           2013-06-26  246  					pclk->param.reg_csr_offset);
6ae5fd38 Stephen Boyd     2015-05-01  247  		pr_debug("%s CSR RESET PADDR base %pa csr offset 0x%08X mask 0x%08X value 0x%08X\n",
836ee0f7 Stephen Boyd     2015-08-12  248  			clk_hw_get_name(hw), &reg,
308964ca Loc Ho           2013-06-26  249  			pclk->param.reg_csr_offset, pclk->param.reg_csr_mask,
308964ca Loc Ho           2013-06-26  250  			data);
308964ca Loc Ho           2013-06-26  251  	}
308964ca Loc Ho           2013-06-26  252  
308964ca Loc Ho           2013-06-26  253  	if (pclk->lock)
308964ca Loc Ho           2013-06-26  254  		spin_unlock_irqrestore(pclk->lock, flags);
308964ca Loc Ho           2013-06-26  255  
308964ca Loc Ho           2013-06-26  256  	return 0;
308964ca Loc Ho           2013-06-26  257  }
308964ca Loc Ho           2013-06-26  258  
308964ca Loc Ho           2013-06-26  259  static void xgene_clk_disable(struct clk_hw *hw)
308964ca Loc Ho           2013-06-26  260  {
308964ca Loc Ho           2013-06-26  261  	struct xgene_clk *pclk = to_xgene_clk(hw);
308964ca Loc Ho           2013-06-26  262  	unsigned long flags = 0;
308964ca Loc Ho           2013-06-26  263  	u32 data;
308964ca Loc Ho           2013-06-26  264  
308964ca Loc Ho           2013-06-26  265  	if (pclk->lock)
308964ca Loc Ho           2013-06-26  266  		spin_lock_irqsave(pclk->lock, flags);
308964ca Loc Ho           2013-06-26  267  
308964ca Loc Ho           2013-06-26  268  	if (pclk->param.csr_reg != NULL) {
836ee0f7 Stephen Boyd     2015-08-12  269  		pr_debug("%s clock disabled\n", clk_hw_get_name(hw));
308964ca Loc Ho           2013-06-26  270  		/* First put the CSR in reset */
308964ca Loc Ho           2013-06-26  271  		data = xgene_clk_read(pclk->param.csr_reg +
308964ca Loc Ho           2013-06-26  272  					pclk->param.reg_csr_offset);
308964ca Loc Ho           2013-06-26  273  		data |= pclk->param.reg_csr_mask;
308964ca Loc Ho           2013-06-26  274  		xgene_clk_write(data, pclk->param.csr_reg +
308964ca Loc Ho           2013-06-26  275  					pclk->param.reg_csr_offset);
308964ca Loc Ho           2013-06-26  276  
308964ca Loc Ho           2013-06-26  277  		/* Second disable the clock */
308964ca Loc Ho           2013-06-26  278  		data = xgene_clk_read(pclk->param.csr_reg +
308964ca Loc Ho           2013-06-26  279  					pclk->param.reg_clk_offset);
308964ca Loc Ho           2013-06-26  280  		data &= ~pclk->param.reg_clk_mask;
308964ca Loc Ho           2013-06-26  281  		xgene_clk_write(data, pclk->param.csr_reg +
308964ca Loc Ho           2013-06-26  282  					pclk->param.reg_clk_offset);
308964ca Loc Ho           2013-06-26  283  	}
308964ca Loc Ho           2013-06-26  284  
308964ca Loc Ho           2013-06-26 @285  	if (pclk->lock)
308964ca Loc Ho           2013-06-26  286  		spin_unlock_irqrestore(pclk->lock, flags);
308964ca Loc Ho           2013-06-26  287  }
308964ca Loc Ho           2013-06-26  288  
308964ca Loc Ho           2013-06-26  289  static int xgene_clk_is_enabled(struct clk_hw *hw)
308964ca Loc Ho           2013-06-26  290  {
308964ca Loc Ho           2013-06-26  291  	struct xgene_clk *pclk = to_xgene_clk(hw);
308964ca Loc Ho           2013-06-26  292  	u32 data = 0;
308964ca Loc Ho           2013-06-26  293  
308964ca Loc Ho           2013-06-26  294  	if (pclk->param.csr_reg != NULL) {
836ee0f7 Stephen Boyd     2015-08-12  295  		pr_debug("%s clock checking\n", clk_hw_get_name(hw));
308964ca Loc Ho           2013-06-26  296  		data = xgene_clk_read(pclk->param.csr_reg +
308964ca Loc Ho           2013-06-26  297  					pclk->param.reg_clk_offset);
836ee0f7 Stephen Boyd     2015-08-12  298  		pr_debug("%s clock is %s\n", clk_hw_get_name(hw),
308964ca Loc Ho           2013-06-26  299  			data & pclk->param.reg_clk_mask ? "enabled" :
308964ca Loc Ho           2013-06-26  300  							"disabled");
308964ca Loc Ho           2013-06-26  301  	}
308964ca Loc Ho           2013-06-26  302  
308964ca Loc Ho           2013-06-26  303  	if (pclk->param.csr_reg == NULL)
308964ca Loc Ho           2013-06-26  304  		return 1;
308964ca Loc Ho           2013-06-26  305  	return data & pclk->param.reg_clk_mask ? 1 : 0;
308964ca Loc Ho           2013-06-26  306  }
308964ca Loc Ho           2013-06-26  307  
308964ca Loc Ho           2013-06-26  308  static unsigned long xgene_clk_recalc_rate(struct clk_hw *hw,
308964ca Loc Ho           2013-06-26  309  				unsigned long parent_rate)
308964ca Loc Ho           2013-06-26  310  {
308964ca Loc Ho           2013-06-26  311  	struct xgene_clk *pclk = to_xgene_clk(hw);
308964ca Loc Ho           2013-06-26  312  	u32 data;
308964ca Loc Ho           2013-06-26  313  
308964ca Loc Ho           2013-06-26  314  	if (pclk->param.divider_reg) {
308964ca Loc Ho           2013-06-26  315  		data = xgene_clk_read(pclk->param.divider_reg +
308964ca Loc Ho           2013-06-26  316  					pclk->param.reg_divider_offset);
308964ca Loc Ho           2013-06-26  317  		data >>= pclk->param.reg_divider_shift;
308964ca Loc Ho           2013-06-26  318  		data &= (1 << pclk->param.reg_divider_width) - 1;
308964ca Loc Ho           2013-06-26  319  
308964ca Loc Ho           2013-06-26  320  		pr_debug("%s clock recalc rate %ld parent %ld\n",
836ee0f7 Stephen Boyd     2015-08-12  321  			clk_hw_get_name(hw),
78e50c6d Matthias Brugger 2015-06-17  322  			parent_rate / data, parent_rate);
78e50c6d Matthias Brugger 2015-06-17  323  
308964ca Loc Ho           2013-06-26  324  		return parent_rate / data;
308964ca Loc Ho           2013-06-26  325  	} else {
308964ca Loc Ho           2013-06-26  326  		pr_debug("%s clock recalc rate %ld parent %ld\n",
836ee0f7 Stephen Boyd     2015-08-12  327  			clk_hw_get_name(hw), parent_rate, parent_rate);
308964ca Loc Ho           2013-06-26  328  		return parent_rate;
308964ca Loc Ho           2013-06-26  329  	}
308964ca Loc Ho           2013-06-26  330  }
308964ca Loc Ho           2013-06-26  331  
308964ca Loc Ho           2013-06-26  332  static int xgene_clk_set_rate(struct clk_hw *hw, unsigned long rate,
308964ca Loc Ho           2013-06-26  333  				unsigned long parent_rate)
308964ca Loc Ho           2013-06-26  334  {
308964ca Loc Ho           2013-06-26  335  	struct xgene_clk *pclk = to_xgene_clk(hw);
308964ca Loc Ho           2013-06-26  336  	unsigned long flags = 0;
308964ca Loc Ho           2013-06-26  337  	u32 data;
308964ca Loc Ho           2013-06-26  338  	u32 divider;
308964ca Loc Ho           2013-06-26  339  	u32 divider_save;
308964ca Loc Ho           2013-06-26  340  
308964ca Loc Ho           2013-06-26  341  	if (pclk->lock)
308964ca Loc Ho           2013-06-26  342  		spin_lock_irqsave(pclk->lock, flags);
308964ca Loc Ho           2013-06-26  343  
308964ca Loc Ho           2013-06-26  344  	if (pclk->param.divider_reg) {
308964ca Loc Ho           2013-06-26  345  		/* Let's compute the divider */
308964ca Loc Ho           2013-06-26  346  		if (rate > parent_rate)
308964ca Loc Ho           2013-06-26  347  			rate = parent_rate;
308964ca Loc Ho           2013-06-26  348  		divider_save = divider = parent_rate / rate; /* Rounded down */
308964ca Loc Ho           2013-06-26  349  		divider &= (1 << pclk->param.reg_divider_width) - 1;
308964ca Loc Ho           2013-06-26  350  		divider <<= pclk->param.reg_divider_shift;
308964ca Loc Ho           2013-06-26  351  
308964ca Loc Ho           2013-06-26  352  		/* Set new divider */
308964ca Loc Ho           2013-06-26  353  		data = xgene_clk_read(pclk->param.divider_reg +
308964ca Loc Ho           2013-06-26  354  				pclk->param.reg_divider_offset);
308964ca Loc Ho           2013-06-26  355  		data &= ~((1 << pclk->param.reg_divider_width) - 1);
308964ca Loc Ho           2013-06-26  356  		data |= divider;
308964ca Loc Ho           2013-06-26  357  		xgene_clk_write(data, pclk->param.divider_reg +
308964ca Loc Ho           2013-06-26  358  					pclk->param.reg_divider_offset);
836ee0f7 Stephen Boyd     2015-08-12  359  		pr_debug("%s clock set rate %ld\n", clk_hw_get_name(hw),
308964ca Loc Ho           2013-06-26  360  			parent_rate / divider_save);
308964ca Loc Ho           2013-06-26  361  	} else {
308964ca Loc Ho           2013-06-26  362  		divider_save = 1;
308964ca Loc Ho           2013-06-26  363  	}
308964ca Loc Ho           2013-06-26  364  
308964ca Loc Ho           2013-06-26  365  	if (pclk->lock)
308964ca Loc Ho           2013-06-26  366  		spin_unlock_irqrestore(pclk->lock, flags);
308964ca Loc Ho           2013-06-26  367  
308964ca Loc Ho           2013-06-26 @368  	return parent_rate / divider_save;
308964ca Loc Ho           2013-06-26  369  }
308964ca Loc Ho           2013-06-26  370  
308964ca Loc Ho           2013-06-26  371  static long xgene_clk_round_rate(struct clk_hw *hw, unsigned long rate,

:::::: The code at line 217 was first introduced by commit
:::::: 308964caeebc45eb7723c87818076f61fa1a2e1b clk: Add APM X-Gene SoC clock driver

:::::: TO: Loc Ho <lho@apm.com>
:::::: CC: Mike Turquette <mturquette@linaro.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Scott Branden]

For BCM portion:

Acked-by: Scott Branden <sbranden@broadcom.com>

On 15-10-13 07:18 AM, Javier Martinez Canillas wrote:
> These drivers only have runtime but no build time dependencies so can be
> built for testing purposes if the Kconfig COMPILE_TEST option is enabled.
>
> This is useful to have more build coverage and make sure that drivers are
> not affected by changes that could cause build regressions.
>
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>
> ---
>
>   drivers/clk/Kconfig           | 8 ++++----
>   drivers/clk/bcm/Kconfig       | 4 ++--
>   drivers/clk/versatile/Kconfig | 2 +-
>   3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 573517151976..57316528e924 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -124,7 +124,7 @@ config CLK_TWL6040
>
>   config COMMON_CLK_AXI_CLKGEN
>   	tristate "AXI clkgen driver"
> -	depends on ARCH_ZYNQ || MICROBLAZE
> +	depends on ARCH_ZYNQ || MICROBLAZE || COMPILE_TEST
>   	help
>   	---help---
>   	  Support for the Analog Devices axi-clkgen pcore clock generator for Xilinx
> @@ -132,7 +132,7 @@ config COMMON_CLK_AXI_CLKGEN
>
>   config CLK_QORIQ
>   	bool "Clock driver for Freescale QorIQ platforms"
> -	depends on (PPC_E500MC || ARM) && OF
> +	depends on (PPC_E500MC || ARM || COMPILE_TEST) && OF
>   	---help---
>   	  This adds the clock driver support for Freescale QorIQ platforms
>   	  using common clock framework.
> @@ -140,13 +140,13 @@ config CLK_QORIQ
>   config COMMON_CLK_XGENE
>   	bool "Clock driver for APM XGene SoC"
>   	default y
> -	depends on ARM64
> +	depends on ARM64 || COMPILE_TEST
>   	---help---
>   	  Sypport for the APM X-Gene SoC reference, PLL, and device clocks.
>
>   config COMMON_CLK_KEYSTONE
>   	tristate "Clock drivers for Keystone based SOCs"
> -	depends on ARCH_KEYSTONE && OF
> +	depends on (ARCH_KEYSTONE || COMPILE_TEST) && OF
>   	---help---
>             Supports clock drivers for Keystone based SOCs. These SOCs have local
>   	  a power sleep control module that gate the clock to the IPs and PLLs.
> diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
> index 88febf53b276..561e9dc6d40d 100644
> --- a/drivers/clk/bcm/Kconfig
> +++ b/drivers/clk/bcm/Kconfig
> @@ -1,6 +1,6 @@
>   config CLK_BCM_KONA
>   	bool "Broadcom Kona CCU clock support"
> -	depends on ARCH_BCM_MOBILE
> +	depends on ARCH_BCM_MOBILE || COMPILE_TEST
>   	depends on COMMON_CLK
>   	default y
>   	help
> @@ -10,7 +10,7 @@ config CLK_BCM_KONA
>
>   config COMMON_CLK_IPROC
>   	bool "Broadcom iProc clock support"
> -	depends on ARCH_BCM_IPROC
> +	depends on ARCH_BCM_IPROC || COMPILE_TEST
>   	depends on COMMON_CLK
>   	default ARCH_BCM_IPROC
>   	help
> diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
> index 1530c9352a76..fc50b6264bed 100644
> --- a/drivers/clk/versatile/Kconfig
> +++ b/drivers/clk/versatile/Kconfig
> @@ -1,6 +1,6 @@
>   config COMMON_CLK_VERSATILE
>   	bool "Clock driver for ARM Reference designs"
> -	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
> +	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64 || COMPILE_TEST
>   	---help---
>             Supports clocking on ARM Reference designs:
>   	  - Integrator/AP and Integrator/CP
>

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Stephen Boyd]

On 10/13, Javier Martinez Canillas wrote:
> diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
> index 1530c9352a76..fc50b6264bed 100644
> --- a/drivers/clk/versatile/Kconfig
> +++ b/drivers/clk/versatile/Kconfig
> @@ -1,6 +1,6 @@
>  config COMMON_CLK_VERSATILE
>  	bool "Clock driver for ARM Reference designs"
> -	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
> +	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64 || COMPILE_TEST

Have you compiled these drivers on an architecture that doesn't
have IOMEM? Perhaps tile or um? I'm all for more build coverage,
but it's not always as simple as just sprinkling some
COMPILE_TEST around the Kconfigs.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Javier Martinez Canillas]

Hello Stephen,

On 10/14/2015 08:38 PM, Stephen Boyd wrote:
> On 10/13, Javier Martinez Canillas wrote:
>> diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
>> index 1530c9352a76..fc50b6264bed 100644
>> --- a/drivers/clk/versatile/Kconfig
>> +++ b/drivers/clk/versatile/Kconfig
>> @@ -1,6 +1,6 @@
>>  config COMMON_CLK_VERSATILE
>>  	bool "Clock driver for ARM Reference designs"
>> -	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
>> +	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64 || COMPILE_TEST
> 
> Have you compiled these drivers on an architecture that doesn't
> have IOMEM? Perhaps tile or um? I'm all for more build coverage,
> but it's not always as simple as just sprinkling some
> COMPILE_TEST around the Kconfigs.
>

No, I only build tested on arm32 and x86. The 0-day bot haven't reported a
build error yet and I didn't see any platform dependent code in the drivers.

But you are right that COMPILE_TEST is a little tricky, so maybe is safest
to drop this patch.

Is just that it would be nice to build as much as possible to make sure that
subystemms or kernel wide changes don't introduce regressions since that is
also tricky even when using tools like coccinelle.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Javier Martinez Canillas]

On 10/14/2015 09:08 PM, Javier Martinez Canillas wrote:
> Hello Stephen,
> 
> On 10/14/2015 08:38 PM, Stephen Boyd wrote:
>> On 10/13, Javier Martinez Canillas wrote:
>>> diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
>>> index 1530c9352a76..fc50b6264bed 100644
>>> --- a/drivers/clk/versatile/Kconfig
>>> +++ b/drivers/clk/versatile/Kconfig
>>> @@ -1,6 +1,6 @@
>>>  config COMMON_CLK_VERSATILE
>>>  	bool "Clock driver for ARM Reference designs"
>>> -	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
>>> +	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64 || COMPILE_TEST
>>
>> Have you compiled these drivers on an architecture that doesn't
>> have IOMEM? Perhaps tile or um? I'm all for more build coverage,
>> but it's not always as simple as just sprinkling some
>> COMPILE_TEST around the Kconfigs.
>>
> 
> No, I only build tested on arm32 and x86. The 0-day bot haven't reported a
> build error yet and I didn't see any platform dependent code in the drivers.
> 

BTW, all clk drivers depends on COMMON_CLK so these won't even be built in
tile or um since that symbol isn't selected there. Or did I misunderstand?

Having said that, I see that enabling COMPILE_TEST will attempt to build
in many archs [0] that I don't have a toolchain to test. So I'm OK if you
drop this patch and sorry for the inconvenience.

[0]:
$ git grep "select COMMON_CLK" arch/ | cut -d '/' -f2 | uniq 
arc
arm
arm64
h8300
microblaze
mips
powerpc
x86
xtensa

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Stephen Boyd]

On 10/14, Javier Martinez Canillas wrote:
> On 10/14/2015 09:08 PM, Javier Martinez Canillas wrote:
> > Hello Stephen,
> > 
> > On 10/14/2015 08:38 PM, Stephen Boyd wrote:
> >> On 10/13, Javier Martinez Canillas wrote:
> >>> diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
> >>> index 1530c9352a76..fc50b6264bed 100644
> >>> --- a/drivers/clk/versatile/Kconfig
> >>> +++ b/drivers/clk/versatile/Kconfig
> >>> @@ -1,6 +1,6 @@
> >>>  config COMMON_CLK_VERSATILE
> >>>  	bool "Clock driver for ARM Reference designs"
> >>> -	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
> >>> +	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64 || COMPILE_TEST
> >>
> >> Have you compiled these drivers on an architecture that doesn't
> >> have IOMEM? Perhaps tile or um? I'm all for more build coverage,
> >> but it's not always as simple as just sprinkling some
> >> COMPILE_TEST around the Kconfigs.
> >>
> > 
> > No, I only build tested on arm32 and x86. The 0-day bot haven't reported a
> > build error yet and I didn't see any platform dependent code in the drivers.
> > 
> 
> BTW, all clk drivers depends on COMMON_CLK so these won't even be built in
> tile or um since that symbol isn't selected there. Or did I misunderstand?
> 
> Having said that, I see that enabling COMPILE_TEST will attempt to build
> in many archs [0] that I don't have a toolchain to test. So I'm OK if you
> drop this patch and sorry for the inconvenience.
> 
> [0]:
> $ git grep "select COMMON_CLK" arch/ | cut -d '/' -f2 | uniq 
> arc
> arm
> arm64
> h8300
> microblaze
> mips
> powerpc
> x86
> xtensa
> 

Ah that's good. So assuming someone has built these patches with
these architectures then we've got it all covered. Do you know if
kbuild does that? There are a bunch of cross compilers on
kernel.org that may help[1]. I'll wait to apply the patch once we
get that confirmation.

[1] https://www.kernel.org/pub/tools/crosstool/

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Javier Martinez Canillas]

Hello Stephen,

On 10/14/2015 11:13 PM, Stephen Boyd wrote:
> On 10/14, Javier Martinez Canillas wrote:
>> On 10/14/2015 09:08 PM, Javier Martinez Canillas wrote:
>>> Hello Stephen,
>>>
>>> On 10/14/2015 08:38 PM, Stephen Boyd wrote:
>>>> On 10/13, Javier Martinez Canillas wrote:
>>>>> diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
>>>>> index 1530c9352a76..fc50b6264bed 100644
>>>>> --- a/drivers/clk/versatile/Kconfig
>>>>> +++ b/drivers/clk/versatile/Kconfig
>>>>> @@ -1,6 +1,6 @@
>>>>>  config COMMON_CLK_VERSATILE
>>>>>  	bool "Clock driver for ARM Reference designs"
>>>>> -	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
>>>>> +	depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64 || COMPILE_TEST
>>>>
>>>> Have you compiled these drivers on an architecture that doesn't
>>>> have IOMEM? Perhaps tile or um? I'm all for more build coverage,
>>>> but it's not always as simple as just sprinkling some
>>>> COMPILE_TEST around the Kconfigs.
>>>>
>>>
>>> No, I only build tested on arm32 and x86. The 0-day bot haven't reported a
>>> build error yet and I didn't see any platform dependent code in the drivers.
>>>
>>
>> BTW, all clk drivers depends on COMMON_CLK so these won't even be built in
>> tile or um since that symbol isn't selected there. Or did I misunderstand?
>>
>> Having said that, I see that enabling COMPILE_TEST will attempt to build
>> in many archs [0] that I don't have a toolchain to test. So I'm OK if you
>> drop this patch and sorry for the inconvenience.
>>
>> [0]:
>> $ git grep "select COMMON_CLK" arch/ | cut -d '/' -f2 | uniq 
>> arc
>> arm
>> arm64
>> h8300
>> microblaze
>> mips
>> powerpc
>> x86
>> xtensa
>>
> 
> Ah that's good. So assuming someone has built these patches with
> these architectures then we've got it all covered. Do you know if
> kbuild does that? There are a bunch of cross compilers on
> kernel.org that may help[1]. I'll wait to apply the patch once we
> get that confirmation.
> 
> [1] https://www.kernel.org/pub/tools/crosstool/
> 

I built on all those archs and neither allyesconfig nor allmodconfig
gives me build error or warnings for the partial build M=drivers/clk

I only had an issue with h8300 since the arch has hardcoded stuff
in its Makefile like the toolchain file name, I posted a patch [0]
to fix that but then got a linker issue. This not related to $SUBJECT
though but to the h8300 arch Makefile.

[0]: https://lkml.org/lkml/2015/10/16/342

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Stephen Boyd]

On 10/16, Javier Martinez Canillas wrote:
> I built on all those archs and neither allyesconfig nor allmodconfig
> gives me build error or warnings for the partial build M=drivers/clk
> 
> I only had an issue with h8300 since the arch has hardcoded stuff
> in its Makefile like the toolchain file name, I posted a patch [0]
> to fix that but then got a linker issue. This not related to $SUBJECT
> though but to the h8300 arch Makefile.

Ok, sounds good. Applied to clk-next.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Krzysztof Kozlowski]

2015-10-15 4:08 GMT+09:00 Javier Martinez Canillas <javier@osg.samsung.com>:
> Hello Stephen,
>
> On 10/14/2015 08:38 PM, Stephen Boyd wrote:
>> On 10/13, Javier Martinez Canillas wrote:
>>> diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
>>> index 1530c9352a76..fc50b6264bed 100644
>>> --- a/drivers/clk/versatile/Kconfig
>>> +++ b/drivers/clk/versatile/Kconfig
>>> @@ -1,6 +1,6 @@
>>>  config COMMON_CLK_VERSATILE
>>>      bool "Clock driver for ARM Reference designs"
>>> -    depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
>>> +    depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64 || COMPILE_TEST
>>
>> Have you compiled these drivers on an architecture that doesn't
>> have IOMEM? Perhaps tile or um? I'm all for more build coverage,
>> but it's not always as simple as just sprinkling some
>> COMPILE_TEST around the Kconfigs.
>>
>
> No, I only build tested on arm32 and x86. The 0-day bot haven't reported a
> build error yet and I didn't see any platform dependent code in the drivers.

I see you guys with Luis are adding a lot of COMPILE_TEST. But
building only on these two architectures *is not enough*. Run at least
armv8, PPC and the x86_64. MIPS would be nice as well (I use the
CodeSourcery's MIPS). All of these (ARM64, X86_64, PPC, MIPS) can be
easily installed on typical debian-like Linux distro. Really easily.

By adding this non-tested build coverage you can actually fail some
other architecture's allyesconfig/allmodconfig builds.

Best regards,
Krzysztof

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Javier Martinez Canillas]

Hello Krzysztof,

On 10/15/2015 04:04 AM, Krzysztof Kozlowski wrote:
> 2015-10-15 4:08 GMT+09:00 Javier Martinez Canillas <javier@osg.samsung.com>:
>> Hello Stephen,
>>
>> On 10/14/2015 08:38 PM, Stephen Boyd wrote:
>>> On 10/13, Javier Martinez Canillas wrote:
>>>> diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
>>>> index 1530c9352a76..fc50b6264bed 100644
>>>> --- a/drivers/clk/versatile/Kconfig
>>>> +++ b/drivers/clk/versatile/Kconfig
>>>> @@ -1,6 +1,6 @@
>>>>  config COMMON_CLK_VERSATILE
>>>>      bool "Clock driver for ARM Reference designs"
>>>> -    depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
>>>> +    depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64 || COMPILE_TEST
>>>
>>> Have you compiled these drivers on an architecture that doesn't
>>> have IOMEM? Perhaps tile or um? I'm all for more build coverage,
>>> but it's not always as simple as just sprinkling some
>>> COMPILE_TEST around the Kconfigs.
>>>
>>
>> No, I only build tested on arm32 and x86. The 0-day bot haven't reported a
>> build error yet and I didn't see any platform dependent code in the drivers.
> 
> I see you guys with Luis are adding a lot of COMPILE_TEST. But

Yes, the motivation for this was that I've been helping Mauro with a big
rework in the media subsystem [0] and was annoying to audit that all the
drivers were converted to the new APIs and no compile regressions were
introduced in drivers that could not be built with COMPILE_TEST enabled.

Most media drivers are able to be build though so I thought it would be
a good idea to extend the build coverage in all the other subsystems.

> building only on these two architectures *is not enough*. Run at least
> armv8, PPC and the x86_64. MIPS would be nice as well (I use the
> CodeSourcery's MIPS). All of these (ARM64, X86_64, PPC, MIPS) can be
> easily installed on typical debian-like Linux distro. Really easily.
>

Thanks, Stephen also pointed out to the toolchains in kernel.org [1].
 
> By adding this non-tested build coverage you can actually fail some
> other architecture's allyesconfig/allmodconfig builds.
>

Agreed, unfortunately having more build coverage is not as trivial as I
originally thought. Not only because it can break the build in obscure
archs that I don't have a toolchain to test but also exposes more build
warnings (as reported by the 0-day bot) that I've the bandwidth to fix.

So personally I'll stop trying to enabled COMPILE_TEST just to be safe.

> Best regards,
> Krzysztof
> 

[0]: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-August/367109.html
[1]: https://www.kernel.org/pub/tools/crosstool/

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Krzysztof Kozlowski]

On 15.10.2015 16:11, Javier Martinez Canillas wrote:
> Hello Krzysztof,
>>>>
>>>
>>> No, I only build tested on arm32 and x86. The 0-day bot haven't reported a
>>> build error yet and I didn't see any platform dependent code in the drivers.
>>
>> I see you guys with Luis are adding a lot of COMPILE_TEST. But
> 
> Yes, the motivation for this was that I've been helping Mauro with a big
> rework in the media subsystem [0] and was annoying to audit that all the
> drivers were converted to the new APIs and no compile regressions were
> introduced in drivers that could not be built with COMPILE_TEST enabled.
> 
> Most media drivers are able to be build though so I thought it would be
> a good idea to extend the build coverage in all the other subsystems.
> 
>> building only on these two architectures *is not enough*. Run at least
>> armv8, PPC and the x86_64. MIPS would be nice as well (I use the
>> CodeSourcery's MIPS). All of these (ARM64, X86_64, PPC, MIPS) can be
>> easily installed on typical debian-like Linux distro. Really easily.
>>
> 
> Thanks, Stephen also pointed out to the toolchains in kernel.org [1].
>  
>> By adding this non-tested build coverage you can actually fail some
>> other architecture's allyesconfig/allmodconfig builds.
>>
> 
> Agreed, unfortunately having more build coverage is not as trivial as I
> originally thought. Not only because it can break the build in obscure
> archs that I don't have a toolchain to test but also exposes more build
> warnings (as reported by the 0-day bot) that I've the bandwidth to fix.
> 
> So personally I'll stop trying to enabled COMPILE_TEST just to be safe.

I mean that in general I agree with the idea of COMPILE_TEST. I had
similar problems when I was changing the power supply API. Build testing
of each modified file required a lot of effort... but it was doable. I
just set up a configuration for build server doing all necessary archs
and configs. With COMPILE_TEST it would be much, much easier!

You don't have to give up entirely. Just use more compilers and in the
same time fix the warnings and errors. Send a patch fixing driver and
another one adding COMPILE_TEST.

Best regards,
Krzysztof

Re: [PATCH] clk: Allow drivers to build if COMPILE_TEST is enabled

[Javier Martinez Canillas]

Hello Krzysztof,

On 10/15/2015 09:22 AM, Krzysztof Kozlowski wrote:
> On 15.10.2015 16:11, Javier Martinez Canillas wrote:
>> Hello Krzysztof,
>>>>>
>>>>
>>>> No, I only build tested on arm32 and x86. The 0-day bot haven't reported a
>>>> build error yet and I didn't see any platform dependent code in the drivers.
>>>
>>> I see you guys with Luis are adding a lot of COMPILE_TEST. But
>>
>> Yes, the motivation for this was that I've been helping Mauro with a big
>> rework in the media subsystem [0] and was annoying to audit that all the
>> drivers were converted to the new APIs and no compile regressions were
>> introduced in drivers that could not be built with COMPILE_TEST enabled.
>>
>> Most media drivers are able to be build though so I thought it would be
>> a good idea to extend the build coverage in all the other subsystems.
>>
>>> building only on these two architectures *is not enough*. Run at least
>>> armv8, PPC and the x86_64. MIPS would be nice as well (I use the
>>> CodeSourcery's MIPS). All of these (ARM64, X86_64, PPC, MIPS) can be
>>> easily installed on typical debian-like Linux distro. Really easily.
>>>
>>
>> Thanks, Stephen also pointed out to the toolchains in kernel.org [1].
>>  
>>> By adding this non-tested build coverage you can actually fail some
>>> other architecture's allyesconfig/allmodconfig builds.
>>>
>>
>> Agreed, unfortunately having more build coverage is not as trivial as I
>> originally thought. Not only because it can break the build in obscure
>> archs that I don't have a toolchain to test but also exposes more build
>> warnings (as reported by the 0-day bot) that I've the bandwidth to fix.
>>
>> So personally I'll stop trying to enabled COMPILE_TEST just to be safe.
> 
> I mean that in general I agree with the idea of COMPILE_TEST. I had
> similar problems when I was changing the power supply API. Build testing
> of each modified file required a lot of effort... but it was doable. I
> just set up a configuration for build server doing all necessary archs
> and configs. With COMPILE_TEST it would be much, much easier!
>

Indeed.
 
> You don't have to give up entirely. Just use more compilers and in the
> same time fix the warnings and errors. Send a patch fixing driver and
> another one adding COMPILE_TEST.
>

Yes, although as I said, you may miss some possible build setup and I
certainly don't want to be blamed for introducing a build regression.

Additionally, maybe I can ask Fengguang if I can get a branch pulled
by 0-day build bot so I can have early feedback of the patches before
are sent to the mailing lists and first fix all the build errors and
warnings before posting, to be sure that it won't cause any issues.
 
> Best regards,
> Krzysztof
> 
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America