[U-Boot] [PATCH v2 2/7] ARM: stm32: cleanup stm32f7 files

Vikas MANOCHA vikas.manocha at st.com
Thu Nov 24 02:48:03 CET 2016


Hi Michael,

> -----Original Message-----
> From: Michael Kurz [mailto:michi.kurz at gmail.com]
> Sent: Friday, November 04, 2016 12:21 PM
> To: u-boot at lists.denx.de
> Cc: Michael Kurz <michi.kurz at gmail.com>; Kamil Lulko <kamil.lulko at gmail.com>; Toshifumi NISHINAGA
> <tnishinaga.dev at gmail.com>; Vadzim Dambrouski <pftbest at gmail.com>; Albert Aribaud <albert.u.boot at aribaud.net>; Vikas
> MANOCHA <vikas.manocha at st.com>; Simon Glass <sjg at chromium.org>
> Subject: [PATCH v2 2/7] ARM: stm32: cleanup stm32f7 files
> 
> Cleanup stm32f7 files:
> - use BIT macro
> - use GENMASK macro
> - prefix all constants with STM32_

Adding no value to add this prefix.

> - remove double constants
> 
> Signed-off-by: Michael Kurz <michi.kurz at gmail.com>
> 
> ---
> 
> Changes in v2:
> - add cleanup patch
> 
>  arch/arm/include/asm/arch-stm32f4/stm32.h        |   2 +-
>  arch/arm/include/asm/arch-stm32f7/fmc.h          |   7 +-
>  arch/arm/include/asm/arch-stm32f7/gpt.h          |   9 +-
>  arch/arm/include/asm/arch-stm32f7/rcc.h          |  64 -------
>  arch/arm/include/asm/arch-stm32f7/stm32.h        | 119 +++++-------
>  arch/arm/include/asm/arch-stm32f7/stm32_periph.h |   3 +
>  arch/arm/mach-stm32/stm32f7/clock.c              | 227 ++++++++++++++---------
>  arch/arm/mach-stm32/stm32f7/timer.c              |   4 +-
>  board/st/stm32f746-disco/stm32f746-disco.c       |  10 +-
>  drivers/mtd/stm32_flash.c                        |   2 +-
>  drivers/serial/serial_stm32x7.c                  |   4 +-
>  11 files changed, 203 insertions(+), 248 deletions(-)  delete mode 100644 arch/arm/include/asm/arch-stm32f7/rcc.h
> 
> diff --git a/arch/arm/include/asm/arch-stm32f4/stm32.h b/arch/arm/include/asm/arch-stm32f4/stm32.h
> index 6cc1966..b77345a 100644
> --- a/arch/arm/include/asm/arch-stm32f4/stm32.h
> +++ b/arch/arm/include/asm/arch-stm32f4/stm32.h
> @@ -102,7 +102,7 @@ struct stm32_pwr_regs {
>  #define STM32_USART3_BASE	(STM32_APB1PERIPH_BASE + 0x4800)
>  #define STM32_USART6_BASE	(STM32_APB2PERIPH_BASE + 0x1400)
> 
> -#define FLASH_CNTL_BASE		(STM32_AHB1PERIPH_BASE + 0x3C00)
> +#define STM32_FLASH_CNTL_BASE	(STM32_AHB1PERIPH_BASE + 0x3C00)
> 
>  static const u32 sect_sz_kb[CONFIG_SYS_MAX_FLASH_SECT] = {
>  	[0 ... 3] =	16 * 1024,

[...]

> -#define RCC_ENR_GPIO_J_EN		(1 << 9)
> -#define RCC_ENR_GPIO_K_EN		(1 << 10)
> -
> -#endif

Not a good design to delete the reset and clock related stuff contained in one header and moving it to source file/stm32.c

> diff --git a/arch/arm/include/asm/arch-stm32f7/stm32.h b/arch/arm/include/asm/arch-stm32f7/stm32.h
> index de55ae5..efc4fd7 100644
> --- a/arch/arm/include/asm/arch-stm32f7/stm32.h
> +++ b/arch/arm/include/asm/arch-stm32f7/stm32.h
> @@ -9,46 +9,47 @@
>  #define _ASM_ARCH_HARDWARE_H
> 
>  /* STM32F746 */
> -#define ITCM_FLASH_BASE		0x00200000UL
> -#define AXIM_FLASH_BASE		0x08000000UL
> +#define STM32_ITCM_FLASH_BASE	0x00200000UL
> +#define STM32_AXIM_FLASH_BASE	0x08000000UL
> +
> +#define STM32_ITCM_SRAM_BASE	0x00000000UL
> +#define STM32_DTCM_SRAM_BASE	0x20000000UL
> +#define STM32_SRAM1_BASE	0x20010000UL
> +#define STM32_SRAM2_BASE	0x2004C000UL
> +
> +#define STM32_PERIPH_BASE	0x40000000UL
> +
> +#define STM32_APB1_PERIPH_BASE	(STM32_PERIPH_BASE + 0x00000000)
> +#define STM32_APB2_PERIPH_BASE	(STM32_PERIPH_BASE + 0x00010000)
> +#define STM32_AHB1_PERIPH_BASE	(STM32_PERIPH_BASE + 0x00020000)
> +#define STM32_AHB2_PERIPH_BASE	(STM32_PERIPH_BASE + 0x10000000)
> +#define STM32_AHB3_PERIPH_BASE	(STM32_PERIPH_BASE + 0x20000000)
> +
> +#define STM32_TIM2_BASE		(STM32_APB1_PERIPH_BASE + 0x0000)
> +#define STM32_USART2_BASE	(STM32_APB1_PERIPH_BASE + 0x4400)
> +#define STM32_USART3_BASE	(STM32_APB1_PERIPH_BASE + 0x4800)
> +#define STM32_PWR_BASE		(STM32_APB1_PERIPH_BASE + 0x7000)
> +
> +#define STM32_USART1_BASE	(STM32_APB2_PERIPH_BASE + 0x1000)
> +#define STM32_USART6_BASE	(STM32_APB2_PERIPH_BASE + 0x1400)
> +#define STM32_SYSCFG_BASE	(STM32_APB2_PERIPH_BASE + 0x3800)
> +
> +#define STM32_GPIOA_BASE	(STM32_AHB1_PERIPH_BASE + 0x0000)
> +#define STM32_GPIOB_BASE	(STM32_AHB1_PERIPH_BASE + 0x0400)
> +#define STM32_GPIOC_BASE	(STM32_AHB1_PERIPH_BASE + 0x0800)
> +#define STM32_GPIOD_BASE	(STM32_AHB1_PERIPH_BASE + 0x0C00)
> +#define STM32_GPIOE_BASE	(STM32_AHB1_PERIPH_BASE + 0x1000)
> +#define STM32_GPIOF_BASE	(STM32_AHB1_PERIPH_BASE + 0x1400)
> +#define STM32_GPIOG_BASE	(STM32_AHB1_PERIPH_BASE + 0x1800)
> +#define STM32_GPIOH_BASE	(STM32_AHB1_PERIPH_BASE + 0x1C00)
> +#define STM32_GPIOI_BASE	(STM32_AHB1_PERIPH_BASE + 0x2000)
> +#define STM32_GPIOJ_BASE	(STM32_AHB1_PERIPH_BASE + 0x2400)
> +#define STM32_GPIOK_BASE	(STM32_AHB1_PERIPH_BASE + 0x2800)
> +#define STM32_RCC_BASE		(STM32_AHB1_PERIPH_BASE + 0x3800)
> +#define STM32_FLASH_CNTL_BASE	(STM32_AHB1_PERIPH_BASE + 0x3C00)
> +
> +#define STM32_SDRAM_FMC_BASE	(STM32_AHB3_PERIPH_BASE + 0x40000140)
> 
> -#define ITCM_SRAM_BASE		0x00000000UL
> -#define DTCM_SRAM_BASE		0x20000000UL
> -#define SRAM1_BASE		0x20010000UL
> -#define SRAM2_BASE		0x2004C000UL
> -
> -#define PERIPH_BASE		0x40000000UL
> -
> -#define APB1_PERIPH_BASE	(PERIPH_BASE + 0x00000000)
> -#define APB2_PERIPH_BASE	(PERIPH_BASE + 0x00010000)
> -#define AHB1_PERIPH_BASE	(PERIPH_BASE + 0x00020000)
> -#define AHB2_PERIPH_BASE	(PERIPH_BASE + 0x10000000)
> -#define AHB3_PERIPH_BASE	(PERIPH_BASE + 0x20000000)
> -
> -#define TIM2_BASE		(APB1_PERIPH_BASE + 0x0000)
> -#define USART2_BASE		(APB1_PERIPH_BASE + 0x4400)
> -#define USART3_BASE		(APB1_PERIPH_BASE + 0x4800)
> -#define PWR_BASE		(APB1_PERIPH_BASE + 0x7000)
> -
> -#define USART1_BASE		(APB2_PERIPH_BASE + 0x1000)
> -#define USART6_BASE		(APB2_PERIPH_BASE + 0x1400)
> -
> -#define STM32_GPIOA_BASE	(AHB1_PERIPH_BASE + 0x0000)
> -#define STM32_GPIOB_BASE	(AHB1_PERIPH_BASE + 0x0400)
> -#define STM32_GPIOC_BASE	(AHB1_PERIPH_BASE + 0x0800)
> -#define STM32_GPIOD_BASE	(AHB1_PERIPH_BASE + 0x0C00)
> -#define STM32_GPIOE_BASE	(AHB1_PERIPH_BASE + 0x1000)
> -#define STM32_GPIOF_BASE	(AHB1_PERIPH_BASE + 0x1400)
> -#define STM32_GPIOG_BASE	(AHB1_PERIPH_BASE + 0x1800)
> -#define STM32_GPIOH_BASE	(AHB1_PERIPH_BASE + 0x1C00)
> -#define STM32_GPIOI_BASE	(AHB1_PERIPH_BASE + 0x2000)
> -#define STM32_GPIOJ_BASE	(AHB1_PERIPH_BASE + 0x2400)
> -#define STM32_GPIOK_BASE	(AHB1_PERIPH_BASE + 0x2800)
> -#define RCC_BASE		(AHB1_PERIPH_BASE + 0x3800)
> -#define FLASH_CNTL_BASE		(AHB1_PERIPH_BASE + 0x3C00)
> -
> -
> -#define SDRAM_FMC_BASE		(AHB3_PERIPH_BASE + 0x4A0000140)
> 
>  static const u32 sect_sz_kb[CONFIG_SYS_MAX_FLASH_SECT] = {
>  	[0 ... 3] =	32 * 1024,
> @@ -62,43 +63,7 @@ enum clock {
>  	CLOCK_APB1,
>  	CLOCK_APB2
>  };
> -#define STM32_BUS_MASK          0xFFFF0000
> -
> -struct stm32_rcc_regs {
> -	u32 cr;		/* RCC clock control */
> -	u32 pllcfgr;	/* RCC PLL configuration */
> -	u32 cfgr;	/* RCC clock configuration */
> -	u32 cir;	/* RCC clock interrupt */
> -	u32 ahb1rstr;	/* RCC AHB1 peripheral reset */
> -	u32 ahb2rstr;	/* RCC AHB2 peripheral reset */
> -	u32 ahb3rstr;	/* RCC AHB3 peripheral reset */
> -	u32 rsv0;
> -	u32 apb1rstr;	/* RCC APB1 peripheral reset */
> -	u32 apb2rstr;	/* RCC APB2 peripheral reset */
> -	u32 rsv1[2];
> -	u32 ahb1enr;	/* RCC AHB1 peripheral clock enable */
> -	u32 ahb2enr;	/* RCC AHB2 peripheral clock enable */
> -	u32 ahb3enr;	/* RCC AHB3 peripheral clock enable */
> -	u32 rsv2;
> -	u32 apb1enr;	/* RCC APB1 peripheral clock enable */
> -	u32 apb2enr;	/* RCC APB2 peripheral clock enable */
> -	u32 rsv3[2];
> -	u32 ahb1lpenr;	/* RCC AHB1 periph clk enable in low pwr mode */
> -	u32 ahb2lpenr;	/* RCC AHB2 periph clk enable in low pwr mode */
> -	u32 ahb3lpenr;	/* RCC AHB3 periph clk enable in low pwr mode */
> -	u32 rsv4;
> -	u32 apb1lpenr;	/* RCC APB1 periph clk enable in low pwr mode */
> -	u32 apb2lpenr;	/* RCC APB2 periph clk enable in low pwr mode */
> -	u32 rsv5[2];
> -	u32 bdcr;	/* RCC Backup domain control */
> -	u32 csr;	/* RCC clock control & status */
> -	u32 rsv6[2];
> -	u32 sscgr;	/* RCC spread spectrum clock generation */
> -	u32 plli2scfgr;	/* RCC PLLI2S configuration */
> -	u32 pllsaicfgr;
> -	u32 dckcfgr;
> -};
> -#define STM32_RCC		((struct stm32_rcc_regs *)RCC_BASE)
> +#define STM32_BUS_MASK		GENMASK(31, 16)
> 
>  struct stm32_pwr_regs {
>  	u32 cr1;   /* power control register 1 */
> @@ -106,7 +71,7 @@ struct stm32_pwr_regs {
>  	u32 cr2;   /* power control register 2 */
>  	u32 csr2;  /* power control/status register 2 */  };
> -#define STM32_PWR		((struct stm32_pwr_regs *)PWR_BASE)
> +#define STM32_PWR		((struct stm32_pwr_regs *)STM32_PWR_BASE)
> 
>  int configure_clocks(void);
>  unsigned long clock_get(enum clock clck); diff --git a/arch/arm/include/asm/arch-stm32f7/stm32_periph.h
> b/arch/arm/include/asm/arch-stm32f7/stm32_periph.h
> index 38adc4e..9b315a8 100644
> --- a/arch/arm/include/asm/arch-stm32f7/stm32_periph.h
> +++ b/arch/arm/include/asm/arch-stm32f7/stm32_periph.h
> @@ -33,6 +33,9 @@ enum periph_clock {
>  	GPIO_I_CLOCK_CFG,
>  	GPIO_J_CLOCK_CFG,
>  	GPIO_K_CLOCK_CFG,
> +	SYSCFG_CLOCK_CFG,
> +	TIMER2_CLOCK_CFG,
> +	FMC_CLOCK_CFG,

should be in separate patch.

>  };
> 
>  #endif /* __ASM_ARM_ARCH_PERIPH_H */
> diff --git a/arch/arm/mach-stm32/stm32f7/clock.c b/arch/arm/mach-stm32/stm32f7/clock.c
> index 78d22d4..839d928 100644
> --- a/arch/arm/mach-stm32/stm32f7/clock.c
> +++ b/arch/arm/mach-stm32/stm32f7/clock.c
> @@ -7,80 +7,130 @@
> 
>  #include <common.h>
>  #include <asm/io.h>
> -#include <asm/arch/rcc.h>
>  #include <asm/arch/stm32.h>
>  #include <asm/arch/stm32_periph.h>
> 
> -#define RCC_CR_HSION		(1 << 0)
> -#define RCC_CR_HSEON		(1 << 16)
> -#define RCC_CR_HSERDY		(1 << 17)
> -#define RCC_CR_HSEBYP		(1 << 18)
> -#define RCC_CR_CSSON		(1 << 19)
> -#define RCC_CR_PLLON		(1 << 24)
> -#define RCC_CR_PLLRDY		(1 << 25)
> +struct stm32_rcc_regs {
> +	u32 cr;		/* RCC clock control */
> +	u32 pllcfgr;	/* RCC PLL configuration */
> +	u32 cfgr;	/* RCC clock configuration */
> +	u32 cir;	/* RCC clock interrupt */
> +	u32 ahb1rstr;	/* RCC AHB1 peripheral reset */
> +	u32 ahb2rstr;	/* RCC AHB2 peripheral reset */
> +	u32 ahb3rstr;	/* RCC AHB3 peripheral reset */
> +	u32 rsv0;
> +	u32 apb1rstr;	/* RCC APB1 peripheral reset */
> +	u32 apb2rstr;	/* RCC APB2 peripheral reset */
> +	u32 rsv1[2];
> +	u32 ahb1enr;	/* RCC AHB1 peripheral clock enable */
> +	u32 ahb2enr;	/* RCC AHB2 peripheral clock enable */
> +	u32 ahb3enr;	/* RCC AHB3 peripheral clock enable */
> +	u32 rsv2;
> +	u32 apb1enr;	/* RCC APB1 peripheral clock enable */
> +	u32 apb2enr;	/* RCC APB2 peripheral clock enable */
> +	u32 rsv3[2];
> +	u32 ahb1lpenr;	/* RCC AHB1 periph clk enable in low pwr mode */
> +	u32 ahb2lpenr;	/* RCC AHB2 periph clk enable in low pwr mode */
> +	u32 ahb3lpenr;	/* RCC AHB3 periph clk enable in low pwr mode */
> +	u32 rsv4;
> +	u32 apb1lpenr;	/* RCC APB1 periph clk enable in low pwr mode */
> +	u32 apb2lpenr;	/* RCC APB2 periph clk enable in low pwr mode */
> +	u32 rsv5[2];
> +	u32 bdcr;	/* RCC Backup domain control */
> +	u32 csr;	/* RCC clock control & status */
> +	u32 rsv6[2];
> +	u32 sscgr;	/* RCC spread spectrum clock generation */
> +	u32 plli2scfgr;	/* RCC PLLI2S configuration */
> +	u32 pllsaicfgr;	/* PLLSAI configuration */
> +	u32 dckcfgr1;	/* dedicated clocks configuration register 1 */
> +	u32 dckcfgr2;	/* dedicated clocks configuration register 2 */
> +};

Don't think its again good idea to move rcc struct from header to source.

> +#define STM32_RCC		((struct stm32_rcc_regs *)STM32_RCC_BASE)
> 
> -#define RCC_PLLCFGR_PLLM_MASK	0x3F
> -#define RCC_PLLCFGR_PLLN_MASK	0x7FC0
> -#define RCC_PLLCFGR_PLLP_MASK	0x30000
> -#define RCC_PLLCFGR_PLLQ_MASK	0xF000000
> -#define RCC_PLLCFGR_PLLSRC	(1 << 22)

[...]

Cheers,
Vikas


More information about the U-Boot mailing list