Re: [U-Boot] [PATCH v3 3/9] ARM: stm32: cleanup stm32f7 files

2016-12-05 Thread Vikas MANOCHA
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

Re: [U-Boot] [PATCH v3 3/9] ARM: stm32: cleanup stm32f7 files

2016-12-05 Thread Michael Kurz


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


Re: [U-Boot] [PATCH v3 3/9] ARM: stm32: cleanup stm32f7 files

2016-12-01 Thread Vikas MANOCHA
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


Re: [U-Boot] [PATCH v3 3/9] ARM: stm32: cleanup stm32f7 files

2016-12-01 Thread Joe Hershberger
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?

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


Re: [U-Boot] [PATCH v3 3/9] ARM: stm32: cleanup stm32f7 files

2016-12-01 Thread Vikas MANOCHA
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).

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


Re: [U-Boot] [PATCH v3 3/9] ARM: stm32: cleanup stm32f7 files

2016-12-01 Thread Joe Hershberger
On Thu, Dec 1, 2016 at 12:18 PM, vikas  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?

> rcc struct shouldn't be in for stm32f7 in first place, the last patch which 
> added it went unnoticed.
>
>>
>> Signed-off-by: Michael Kurz 
>>
>> ---
>>
>> 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


Re: [U-Boot] [PATCH v3 3/9] ARM: stm32: cleanup stm32f7 files

2016-12-01 Thread vikas
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.

rcc struct shouldn't be in for stm32f7 in first place, the last patch which 
added it went unnoticed.

> 
> Signed-off-by: Michael Kurz 
> 
> ---
> 
> 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)
> 

[...]

> 
>  struct stm32_rcc_regs {
> u32 cr; /* RCC clock control */
> @@ -95,8 +96,9 @@ struct stm32_rcc_regs {
> u32 rsv6[2];
> u32 sscgr;  /* RCC spread spectrum clock generation */
> u32 plli2scfgr; /* RCC PLLI2S configuration */
> -   u32 pllsaicfgr;
> -   u32 dckcfgr;
> +   u32 pllsaicfgr; /* PLLSAI configuration */
> +   u32 dckcfgr;/* dedicated clocks configuration register */
> +   u32 dckcfgr2;   /* dedicated clocks configuration register */

not a cleanup as the title of the patch suggest.

>  };
>  #define STM32_RCC  ((struct stm32_rcc_regs *)RCC_BASE)
> 
> diff --git a/arch/arm/mach-stm32/stm32f7/clock.c 
> b/arch/arm/mach-stm32/stm32f7/clock.c
> index 78d22d4..8091c74 100644
> --- a/arch/arm/mach-stm32/stm32f7/clock.c
> +++ b/arch/arm/mach-stm32/stm32f7/clock.c
> @@ -11,76 +11,50 @@
>  #include 
>  #include 
> 
> -#define RCC_CR_HSION   (1 << 0)

[...]

> +#define APB_PSC_4  0x5
> +#define APB_PSC_8  0x6
> +#define APB_PSC_16 0x7
> 
>  #if !defined(CONFIG_STM32_HSE_HZ)
>  #error "CONFIG_STM32_HSE_HZ not defined!"
> @@ -243,40 +217,40 @@ void clock_setup(int peripheral)
>  {
> switch (peripheral) {
> case USART1_CLOCK_CFG:
> -   setbits_le32(RCC_BASE + RCC_APB2ENR, RCC_ENR_USART1EN);
> +   setbits_le32(_RCC->apb2enr, RCC_APB2ENR_USART1EN);

lets keep macro for the reason stated above.

> break;
> case GPIO_A_CLOCK_CFG:
> -   setbits_le32(RCC_BASE + RCC_AHB1ENR, RCC_ENR_GPIO_A_EN);
> +   setbits_le32(_RCC->ahb1enr, RCC_AHB1ENR_GPIO_A_EN);

DITTO, same for rest of the changes below.

Cheers,
Vikas

> break;
> case GPIO_B_CLOCK_CFG:
> -   setbits_le32(RCC_BASE + RCC_AHB1ENR, RCC_ENR_GPIO_B_EN);
> +   setbits_le32(_RCC->ahb1enr, RCC_AHB1ENR_GPIO_B_EN);
> break;
> case GPIO_C_CLOCK_CFG:
> -   setbits_le32(RCC_BASE + RCC_AHB1ENR, RCC_ENR_GPIO_C_EN);
> +   setbits_le32(_RCC->ahb1enr, RCC_AHB1ENR_GPIO_C_EN);
> break;
> case GPIO_D_CLOCK_CFG:
> -   setbits_le32(RCC_BASE + RCC_AHB1ENR, RCC_ENR_GPIO_D_EN);
> +   setbits_le32(_RCC->ahb1enr, RCC_AHB1ENR_GPIO_D_EN);
> break;
> case GPIO_E_CLOCK_CFG:
> -   setbits_le32(RCC_BASE + RCC_AHB1ENR, RCC_ENR_GPIO_E_EN);
> +   setbits_le32(_RCC->ahb1enr, RCC_AHB1ENR_GPIO_E_EN);
> break;
> case GPIO_F_CLOCK_CFG:
> -   setbits_le32(RCC_BASE + RCC_AHB1ENR, RCC_ENR_GPIO_F_EN);
> +   setbits_le32(_RCC->ahb1enr, RCC_AHB1ENR_GPIO_F_EN);
> break;
> case GPIO_G_CLOCK_CFG:
> -   setbits_le32(RCC_BASE + RCC_AHB1ENR, RCC_ENR_GPIO_G_EN);
> +   setbits_le32(_RCC->ahb1enr, RCC_AHB1ENR_GPIO_G_EN);
> break;
> case GPIO_H_CLOCK_CFG:
> -   setbits_le32(RCC_BASE + RCC_AHB1ENR, RCC_ENR_GPIO_H_EN);
> + 

Re: [U-Boot] [PATCH v3 3/9] ARM: stm32: cleanup stm32f7 files

2016-11-30 Thread Joe Hershberger
On Thu, Nov 24, 2016 at 1:10 PM, Michael Kurz  wrote:
> Cleanup stm32f7 files:
> - use BIT macro
> - use GENMASK macro
> - use rcc struct instead of macro additions
>
> Signed-off-by: Michael Kurz 

Reviewed-by: Joe Hershberger 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot