Hi Vikas,
On Thu, 1 Dec 2016, Vikas MANOCHA wrote:
Thanks Joe for your feedback,
-----Original Message-----
From: Joe Hershberger [mailto:joe.hershber...@gmail.com]
Sent: Thursday, December 01, 2016 11:13 AM
To: Vikas MANOCHA <vikas.mano...@st.com>
Cc: Michael Kurz <michi.k...@gmail.com>; u-boot@lists.denx.de; Toshifumi NISHINAGA
<tnishinaga....@gmail.com>; Albert
Aribaud <albert.u.b...@aribaud.net>; Joe Hershberger <joe.hershber...@ni.com>; Kamil
Lulko <kamil.lu...@gmail.com>
Subject: Re: [U-Boot] [PATCH v3 3/9] ARM: stm32: cleanup stm32f7 files
On Thu, Dec 1, 2016 at 1:09 PM, Vikas MANOCHA <vikas.mano...@st.com> wrote:
Hi Joe,
-----Original Message-----
From: Joe Hershberger [mailto:joe.hershber...@gmail.com]
Sent: Thursday, December 01, 2016 10:42 AM
To: Vikas MANOCHA <vikas.mano...@st.com>
Cc: Michael Kurz <michi.k...@gmail.com>; u-boot@lists.denx.de;
Toshifumi NISHINAGA <tnishinaga....@gmail.com>; Albert Aribaud
<albert.u.b...@aribaud.net>; Joe Hershberger
<joe.hershber...@ni.com>; Kamil Lulko <kamil.lu...@gmail.com>
Subject: Re: [U-Boot] [PATCH v3 3/9] ARM: stm32: cleanup stm32f7
files
On Thu, Dec 1, 2016 at 12:18 PM, vikas <vikas.mano...@st.com> wrote:
Hi Michael,
On 11/24/2016 11:10 AM, Michael Kurz wrote:
Cleanup stm32f7 files:
- use BIT macro
- use GENMASK macro
good.
- use rcc struct instead of macro additions
Macro definitions are better than struct to make rcc compatible
throughout the stm32f7 family in case of additional registers and
also to reuse it for stm32f4. At present we cant use same rcc struct
for stm32f4 and stm32f7 because of two additional registers in stm32f7.
So keep the macros for rcc, we would move it for both stm32f7 and stm32f4.
Just 2 extra regs, or they change the position of the existing regs?
only extra registers (infact only 1 extra register, not 2).
If just extra, then use the struct. Who cares if there is an extra reg that you
don't use?
The extra register is to select the clock source for peripherals like u(s)arts,
i2c etc. I don’t want to neglect it.
But again, today its one, tomorrow there would be more. Off-course the
locations and definitions might change (unlikey) but macros provide better
compatibility.
My first choice is also structures as it makes debugging/maintenance easier but
it not scalable. Do you agree ?
Another approach could be to have another additional struct like "rcc_ext_f7" pointing to
end of "rcc_common" struct. What's your opinion about it ?
Cheers,
Vikas
In my opinion split the struct in two structs like you suggested would be
the best solution. But i think that is mostly based on personal
preference.
I ordered a stm32f429-disco board so I could change (and test the changes)
for both boards once it arrives. Or i can revert the changes and use the
macros. I'm don't have any strong preference to either one.
Regards,
Michael
rcc struct shouldn't be in for stm32f7 in first place, the last patch which
added it went unnoticed.
Signed-off-by: Michael Kurz <michi.k...@gmail.com>
---
Changes in v3:
- Removed 'prefix all constants with STM32_'
- Reverted move of header into source file (rcc.h -> clock.c)
Changes in v2:
- Add cleanup patch
arch/arm/include/asm/arch-stm32f7/fmc.h | 6 +-
arch/arm/include/asm/arch-stm32f7/gpt.h | 6 +-
arch/arm/include/asm/arch-stm32f7/rcc.h | 50 ++++++----
arch/arm/include/asm/arch-stm32f7/stm32.h | 8 +-
arch/arm/mach-stm32/stm32f7/clock.c | 154 ++++++++++++-----------------
board/st/stm32f746-disco/stm32f746-disco.c | 7 +-
6 files changed, 107 insertions(+), 124 deletions(-)
diff --git a/arch/arm/include/asm/arch-stm32f7/fmc.h
b/arch/arm/include/asm/arch-stm32f7/fmc.h
index 7dd5077..d61a86f 100644
--- a/arch/arm/include/asm/arch-stm32f7/fmc.h
+++ b/arch/arm/include/asm/arch-stm32f7/fmc.h
@@ -58,12 +58,12 @@ struct stm32_fmc_regs {
#define FMC_SDCMR_MODE_SELFREFRESH 5
#define FMC_SDCMR_MODE_POWERDOWN 6
-#define FMC_SDCMR_BANK_1 (1 << 4)
-#define FMC_SDCMR_BANK_2 (1 << 3)
+#define FMC_SDCMR_BANK_1 BIT(4)
+#define FMC_SDCMR_BANK_2 BIT(3)
[...]
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
