On Thu, 24 Nov 2016, Vikas MANOCHA wrote:
Hi Vikas,

Hi Michael,

-----Original Message-----
From: Michael Kurz [mailto:michi.k...@gmail.com]
Sent: Friday, November 04, 2016 12:21 PM
To: u-boot@lists.denx.de
Cc: Michael Kurz <michi.k...@gmail.com>; Kamil Lulko <kamil.lu...@gmail.com>; 
Toshifumi NISHINAGA
<tnishinaga....@gmail.com>; Vadzim Dambrouski <pftb...@gmail.com>; Albert Aribaud 
<albert.u.b...@aribaud.net>; Vikas
MANOCHA <vikas.mano...@st.com>; Simon Glass <s...@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.


I see your point. I'll remove it in v3.

- remove double constants

Signed-off-by: Michael Kurz <michi.k...@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


I thought I read somewhere in the docs to only add a header file when needed. As the rcc.h header file was only included in the clock.c file I thought it would be the right thing to move it into the source file. I agree to keep it though.

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.


I split this into a separate patch in v3.

 };

 #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.


I'll keep it in the header in v3.

+#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


Thanks for your comments!

Regards,
Michael
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to