Hi Michael,

> -----Original Message-----
> From: Michael Kurz [mailto:michi.k...@gmail.com]
> Sent: Monday, December 05, 2016 9:21 AM
> To: Vikas MANOCHA <vikas.mano...@st.com>
> Cc: Joe Hershberger <joe.hershber...@gmail.com>; 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
> 
> 
> 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.

Ok, let's do it this way.

> 
> I ordered a stm32f429-disco board so I could change (and test the changes) 
> for both boards once it arrives.

We don’t need to wait for f429, f7 rcc is independent to stm32f429 as of now. 
We can merge it with f4 as next step in another patch.

Cheers,
Vikas

> 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

Reply via email to