[U-Boot] [PATCH 09/10] AVR32: CPU support for AT32UC3A0xxx CPUs

[Wolfgang Denk]

Dear Olav Morken,

In message <fa1e825be37320d862f752c6962c81f9b19ed3db.1223643536.git.olavmrk at gmail.com> you wrote:
> This patch adds support for the AT32UC3A0xxx chips.
> 
> Signed-off-by: Gunnar Rangoy <gunnar at rangoy.com>
> Signed-off-by: Paul Driveklepp <pauldriveklepp at gmail.com>
> Signed-off-by: Olav Morken <olavmrk at gmail.com>

Coding style violations: C++ comments, indentation not by TAB, too
long lines, incorrect multiline comment style.

...
> +static inline unsigned long get_hsb_clk_rate(void)
> +{
> +	//TODO HSB is always the same as cpu-rate
-------^^
> +	return MAIN_CLK_RATE >> CFG_CLKDIV_CPU;
> +}
> +static inline unsigned long get_pba_clk_rate(void)
> +{
> +	return MAIN_CLK_RATE >> CFG_CLKDIV_PBA;
> +}
> +static inline unsigned long get_pbb_clk_rate(void)
> +{
> +	return MAIN_CLK_RATE >> CFG_CLKDIV_PBB;
> +}
> +
> +/* Accessors for specific devices. More will be added as needed. */
> +static inline unsigned long get_sdram_clk_rate(void)
> +{
> +	return get_hsb_clk_rate();
> +}
> +#ifdef AT32UC3A0xxx_CHIP_HAS_USART
> +static inline unsigned long get_usart_clk_rate(unsigned int dev_id)
> +{
> +	return get_pba_clk_rate();
> +}
> +#endif
> +#ifdef AT32UC3A0xxx_CHIP_HAS_MACB
> +static inline unsigned long get_macb_pclk_rate(unsigned int dev_id)
> +{
> +	return get_pbb_clk_rate();
> +}
> +static inline unsigned long get_macb_hclk_rate(unsigned int dev_id)
> +{
> +	return get_hsb_clk_rate();
> +}
> +#endif
> +#ifdef AT32UC3A0xxx_CHIP_HAS_SPI
> +static inline unsigned long get_spi_clk_rate(unsigned int dev_id)
> +{
> +	return get_pba_clk_rate();
> +}
> +#endif

Would it make  sense  to  provide  weak  functions  the  get  defined
accordingly?  A  function  which  only  calls  another function looks
stupid to me.

> +#ifdef AT32UC3A0xxx_CHIP_HAS_USART
> +static inline void portmux_enable_usart0(unsigned long drive_strength)
> +{
> +	portmux_select_peripheral(PORTMUX_PORT_A, (1 << 0) | (1 << 1),
> +			PORTMUX_FUNC_A, 0);
> +}
> +
> +static inline void portmux_enable_usart1(unsigned long drive_strength)
> +{
> +	portmux_select_peripheral(PORTMUX_PORT_A, (1 << 5) | (1 << 6),
> +			PORTMUX_FUNC_A, 0);
> +}
> +
> +static inline void portmux_enable_usart2(unsigned long drive_strength)
> +{
> +	portmux_select_peripheral(PORTMUX_PORT_B, (1 << 29) | (1 << 30),
> +			PORTMUX_FUNC_A, 0);
> +}
> +
> +static inline void portmux_enable_usart3(unsigned long drive_strength)
> +{
> +	portmux_select_peripheral(PORTMUX_PORT_B, (1 << 10) | (1 << 11),
> +			PORTMUX_FUNC_B, 0);
> +}
> +#endif

I don't like this either.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The universe is all a spin-off of the Big Bang.