[U-Boot] vf610twr: Enable all masks of CCGRx registers.

The CCGRx registers, or clock gating registers, can be set to disable or enable
clocks for devices on the vf610twr platform. Enabling masks for all CCGRx
registers allows Linux drivers, like snvs_rtc, and also MQX applications, to
work out-of-the-box. There are no real downsides to enabling all clocks, and
this has been done in previous Freescale releases of U-Boot for the Vybrid
Tower.

Signed-off-by: Anthony Felice <tony.felice@timesys.com>
---
 arch/arm/include/asm/arch-vf610/crm_regs.h |  1 +
 board/freescale/vf610twr/vf610twr.c        | 22 +++++++++-------------
 2 files changed, 10 insertions(+), 13 deletions(-)

On 2015-10-08 12:32, Anthony Felice wrote:
> The CCGRx registers, or clock gating registers, can be set to disable or enable
> clocks for devices on the vf610twr platform. Enabling masks for all CCGRx
> registers allows Linux drivers, like snvs_rtc, and also MQX applications, to
> work out-of-the-box. There are no real downsides to enabling all clocks, and
> this has been done in previous Freescale releases of U-Boot for the Vybrid
> Tower.

I don't really agree with this patch. Linux should handle clocks for
snvs_rtc properly, if the necessary clocks are assigned. At least it
seems to work on a recent mainline kernel.

As far as I remember there is a clock tree driver for Vybrid in MQX too,
so it should be able to handle clocks.

And, there is a downside with this approach: It will consume more power.
Although, Linux will automatically disable unused clocks, at least the
gates which it knows of. Hence, this will lead to a higher power
consumption not only during boot time, but also during run time.

--
Stefan


> 
> Signed-off-by: Anthony Felice <tony.felice@timesys.com>
> ---
>  arch/arm/include/asm/arch-vf610/crm_regs.h |  1 +
>  board/freescale/vf610twr/vf610twr.c        | 22 +++++++++-------------
>  2 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-vf610/crm_regs.h
> b/arch/arm/include/asm/arch-vf610/crm_regs.h
> index a46e396..11b87ec 100644
> --- a/arch/arm/include/asm/arch-vf610/crm_regs.h
> +++ b/arch/arm/include/asm/arch-vf610/crm_regs.h
> @@ -187,6 +187,7 @@ struct anadig_reg {
>  #define CCM_CSCMR2_RMII_CLK_SEL(v)		(((v) & 0x3) << 4)
>  
>  #define CCM_REG_CTRL_MASK			0xffffffff
> +#define CCM_CCGRX_ENABLE_ALL_CTRL_MASK		(0xffffffff)
>  #define CCM_CCGR0_UART0_CTRL_MASK               (0x3 << 14)
>  #define CCM_CCGR0_UART1_CTRL_MASK		(0x3 << 16)
>  #define CCM_CCGR0_DSPI0_CTRL_MASK		(0x3 << 24)
> diff --git a/board/freescale/vf610twr/vf610twr.c
> b/board/freescale/vf610twr/vf610twr.c
> index 7834931..a78e9e6 100644
> --- a/board/freescale/vf610twr/vf610twr.c
> +++ b/board/freescale/vf610twr/vf610twr.c
> @@ -272,27 +272,23 @@ static void clock_init(void)
>  	struct anadig_reg *anadig = (struct anadig_reg *)ANADIG_BASE_ADDR;
>  
>  	clrsetbits_le32(&ccm->ccgr0, CCM_REG_CTRL_MASK,
> -		CCM_CCGR0_UART1_CTRL_MASK);
> +		CCM_CCGRX_ENABLE_ALL_CTRL_MASK);
>  	clrsetbits_le32(&ccm->ccgr1, CCM_REG_CTRL_MASK,
> -		CCM_CCGR1_PIT_CTRL_MASK | CCM_CCGR1_WDOGA5_CTRL_MASK);
> +		CCM_CCGRX_ENABLE_ALL_CTRL_MASK);
>  	clrsetbits_le32(&ccm->ccgr2, CCM_REG_CTRL_MASK,
> -		CCM_CCGR2_IOMUXC_CTRL_MASK | CCM_CCGR2_PORTA_CTRL_MASK |
> -		CCM_CCGR2_PORTB_CTRL_MASK | CCM_CCGR2_PORTC_CTRL_MASK |
> -		CCM_CCGR2_PORTD_CTRL_MASK | CCM_CCGR2_PORTE_CTRL_MASK |
> -		CCM_CCGR2_QSPI0_CTRL_MASK);
> +		CCM_CCGRX_ENABLE_ALL_CTRL_MASK);
>  	clrsetbits_le32(&ccm->ccgr3, CCM_REG_CTRL_MASK,
> -		CCM_CCGR3_ANADIG_CTRL_MASK | CCM_CCGR3_SCSC_CTRL_MASK);
> +		CCM_CCGRX_ENABLE_ALL_CTRL_MASK);
>  	clrsetbits_le32(&ccm->ccgr4, CCM_REG_CTRL_MASK,
> -		CCM_CCGR4_WKUP_CTRL_MASK | CCM_CCGR4_CCM_CTRL_MASK |
> -		CCM_CCGR4_GPC_CTRL_MASK | CCM_CCGR4_I2C0_CTRL_MASK);
> +		CCM_CCGRX_ENABLE_ALL_CTRL_MASK);
>  	clrsetbits_le32(&ccm->ccgr6, CCM_REG_CTRL_MASK,
> -		CCM_CCGR6_OCOTP_CTRL_MASK | CCM_CCGR6_DDRMC_CTRL_MASK);
> +		CCM_CCGRX_ENABLE_ALL_CTRL_MASK);
>  	clrsetbits_le32(&ccm->ccgr7, CCM_REG_CTRL_MASK,
> -		CCM_CCGR7_SDHC1_CTRL_MASK);
> +		CCM_CCGRX_ENABLE_ALL_CTRL_MASK);
>  	clrsetbits_le32(&ccm->ccgr9, CCM_REG_CTRL_MASK,
> -		CCM_CCGR9_FEC0_CTRL_MASK | CCM_CCGR9_FEC1_CTRL_MASK);
> +		CCM_CCGRX_ENABLE_ALL_CTRL_MASK);
>  	clrsetbits_le32(&ccm->ccgr10, CCM_REG_CTRL_MASK,
> -		CCM_CCGR10_NFC_CTRL_MASK);
> +		CCM_CCGRX_ENABLE_ALL_CTRL_MASK);
>  
>  	clrsetbits_le32(&anadig->pll2_ctrl, ANADIG_PLL2_CTRL_POWERDOWN,
>  		ANADIG_PLL2_CTRL_ENABLE | ANADIG_PLL2_CTRL_DIV_SELECT);

On Mon, Oct 12, 2015 at 5:36 PM, Stefan Agner <stefan@agner.ch> wrote:
> On 2015-10-08 12:32, Anthony Felice wrote:
>> The CCGRx registers, or clock gating registers, can be set to disable or enable
>> clocks for devices on the vf610twr platform. Enabling masks for all CCGRx
>> registers allows Linux drivers, like snvs_rtc, and also MQX applications, to
>> work out-of-the-box. There are no real downsides to enabling all clocks, and
>> this has been done in previous Freescale releases of U-Boot for the Vybrid
>> Tower.
>
> I don't really agree with this patch. Linux should handle clocks for
> snvs_rtc properly, if the necessary clocks are assigned. At least it
> seems to work on a recent mainline kernel.
>
> As far as I remember there is a clock tree driver for Vybrid in MQX too,
> so it should be able to handle clocks.
>
> And, there is a downside with this approach: It will consume more power.
> Although, Linux will automatically disable unused clocks, at least the
> gates which it knows of. Hence, this will lead to a higher power
> consumption not only during boot time, but also during run time.

Agreed; the clocks enabled in the U-Boot should be the bare minimal to
get the CPU in an usable form, and all the rest should be handled by
the loaded operating system. There is no reason to increase power
consumption for every user when the kernel can handle it.