Dear Olav Morken,
In message fa1e825be37320d862f752c6962c81f9b19ed3db.1223643536.git.olavmrk@gmail.com you wrote:
This patch adds support for the AT32UC3A0xxx chips.
Signed-off-by: Gunnar Rangoy gunnar@rangoy.com Signed-off-by: Paul Driveklepp pauldriveklepp@gmail.com Signed-off-by: Olav Morken olavmrk@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