[U-Boot,v1,3/4] i.MX6: add enable_spi_clk()

add enable_spi_clk(), so board code can enable spi clocks.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Eric Nelson <eric.nelson@boundarydevices.com>
Cc: Stefano Babic <sbabic@denx.de>
---
 arch/arm/cpu/armv7/mx6/clock.c        | 17 +++++++++++++++++
 arch/arm/include/asm/arch-mx6/clock.h |  1 +
 2 files changed, 18 insertions(+)

Hi Heiko,


On 28/05/2014 12:16, Heiko Schocher wrote:
> add enable_spi_clk(), so board code can enable spi clocks.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Eric Nelson <eric.nelson@boundarydevices.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>  arch/arm/cpu/armv7/mx6/clock.c        | 17 +++++++++++++++++
>  arch/arm/include/asm/arch-mx6/clock.h |  1 +
>  2 files changed, 18 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c
> index bd65a08..4735368 100644
> --- a/arch/arm/cpu/armv7/mx6/clock.c
> +++ b/arch/arm/cpu/armv7/mx6/clock.c
> @@ -71,6 +71,23 @@ int enable_i2c_clk(unsigned char enable, unsigned i2c_num)
>  }
>  #endif
>  
> +int enable_spi_clk(unsigned char enable, unsigned spi_num)

Add a comment here to understand that spi_num starts from 0.

> +{
> +	u32 reg;
> +	u32 mask;
> +
> +	if (spi_num > 3)
> +		return -EINVAL;

The maximum number should be a #define in imx-regs.h, in case we will
reuse the code with a new variation of the SOC.

> +
> +	mask = MXC_CCM_CCGR_CG_MASK << (spi_num << 1);



> +	reg = __raw_readl(&imx_ccm->CCGR1);
> +	if (enable)
> +		reg |= mask;
> +	else
> +		reg &= ~mask;
> +	__raw_writel(reg, &imx_ccm->CCGR1);
> +	return 0;
> +}
>  static u32 decode_pll(enum pll_clocks pll, u32 infreq)
>  {
>  	u32 div;
> diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h
> index 1b4ded7..339c789 100644
> --- a/arch/arm/include/asm/arch-mx6/clock.h
> +++ b/arch/arm/include/asm/arch-mx6/clock.h
> @@ -57,6 +57,7 @@ void enable_usboh3_clk(unsigned char enable);
>  int enable_sata_clock(void);
>  int enable_pcie_clock(void);
>  int enable_i2c_clk(unsigned char enable, unsigned i2c_num);
> +int enable_spi_clk(unsigned char enable, unsigned spi_num);
>  void enable_ipu_clock(void);
>  int enable_fec_anatop_clock(enum enet_freq freq);
>  #endif /* __ASM_ARCH_CLOCK_H */
> 

Best regards,
Stefano Babic

Hi Heiko,

On Wed, May 28, 2014 at 7:16 AM, Heiko Schocher <hs@denx.de> wrote:

> --- a/arch/arm/include/asm/arch-mx6/clock.h
> +++ b/arch/arm/include/asm/arch-mx6/clock.h
> @@ -57,6 +57,7 @@ void enable_usboh3_clk(unsigned char enable);
>  int enable_sata_clock(void);
>  int enable_pcie_clock(void);
>  int enable_i2c_clk(unsigned char enable, unsigned i2c_num);
> +int enable_spi_clk(unsigned char enable, unsigned spi_num);
>  void enable_ipu_clock(void);
>  int enable_fec_anatop_clock(enum enet_freq freq);

Apart from comments that Stefano pointed out, the patch looks good.

However, this approach doesn't scale very well. In the future, we
should be looking into adding Common Clock Framework into U-boot, so
that we can better control the clocks like we do in the kernel.

Not sure if there is anyone interested or willing to work on this topic though.

Regards,

Fabio Estevam

Hi Fabio,

On 11/07/2014 15:22, Fabio Estevam wrote:
> Hi Heiko,
> 
> On Wed, May 28, 2014 at 7:16 AM, Heiko Schocher <hs@denx.de> wrote:
> 
>> --- a/arch/arm/include/asm/arch-mx6/clock.h
>> +++ b/arch/arm/include/asm/arch-mx6/clock.h
>> @@ -57,6 +57,7 @@ void enable_usboh3_clk(unsigned char enable);
>>  int enable_sata_clock(void);
>>  int enable_pcie_clock(void);
>>  int enable_i2c_clk(unsigned char enable, unsigned i2c_num);
>> +int enable_spi_clk(unsigned char enable, unsigned spi_num);
>>  void enable_ipu_clock(void);
>>  int enable_fec_anatop_clock(enum enet_freq freq);
> 
> Apart from comments that Stefano pointed out, the patch looks good.
> 
> However, this approach doesn't scale very well. In the future, we
> should be looking into adding Common Clock Framework into U-boot, so
> that we can better control the clocks like we do in the kernel.
> 

I fully agree.

> Not sure if there is anyone interested or willing to work on this topic though.

Anyway, it is good to see this issue because it shows the right way to
follow, even if there is not (yet) anybody ready to implement it.

Regards,
Stefano