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