| Message ID | 1417417556-23946-4-git-send-email-nobuhiro.iwamatsu.yj@renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
Dear Nobuhiro, In message <1417417556-23946-4-git-send-email-nobuhiro.iwamatsu.yj@renesas.com> you wrote: > Before a kernel boots, GPIO, SYS-DMAC, QSPI and MSIOF clock > is halted. > > Signed-off-by: Hisashi Nakamura <hisashi.nakamura.ak@renesas.com> > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com> The data structures and the code are all repeated for this patch and the following patches: [PATCH 4/7] arm: rmobile: lager: Halt clock prior to booting kernel [PATCH 5/7] arm: rmobile: koelsch: Halt clock prior to booting kernel [U-Boot] [PATCH 6/7] arm: rmobile: alt: Halt clock prior to booting kernel [PATCH 7/7] arm: rmobile: gose: Halt clock prior to booting kernel Can you please move the code to a common place so we have it only once? > +} mstptbl[] = { > + [0] = { SMSTPCR0, 0x00640801, 0x00400001, > + RMSTPCR0, 0x00640801, 0x00000000 }, > + [1] = { SMSTPCR1, 0xDB6E9BDF, 0x00000000, > + RMSTPCR1, 0xDB6E9BDF, 0x00000000 }, > + [2] = { SMSTPCR2, 0x300DA1FC, 0x000CA120, > + RMSTPCR2, 0x300DA1FC, 0x00000000 }, > + [3] = { SMSTPCR3, 0xF08CF831, 0x00000000, > + RMSTPCR3, 0xF08CF831, 0x00000000 }, > + [4] = { SMSTPCR4, 0x80000184, 0x00000180, > + RMSTPCR4, 0x80000184, 0x00000000 }, > + [5] = { SMSTPCR5, 0x44C00046, 0x00000000, > + RMSTPCR5, 0x44C00046, 0x00000000 }, > + [7] = { SMSTPCR7, 0x07F30718, 0x00200000, > + RMSTPCR7, 0x07F30718, 0x00000000 }, > + [8] = { SMSTPCR8, 0x01F0FF84, 0x00000000, > + RMSTPCR8, 0x01F0FF84, 0x00000000 }, > + [9] = { SMSTPCR9, 0xF5979FCF, 0x00021F80, > + RMSTPCR9, 0xF5979FCF, 0x00001F80 }, > + [10] = { SMSTPCR10, 0xFFFEFFE0, 0x00000000, > + RMSTPCR10, 0xFFFEFFE0, 0x00000000 }, > + [11] = { SMSTPCR11, 0x00000000, 0x00000000, > + RMSTPCR11, 0x00000000, 0x00000000 }, > +}; Also, these data look pretty much the same to me, with only minor differences in some bits. If we use some defines instead of the magic numbers this could probably help to see the common part and the differences in the data. We probably don't need board-specific data in the code, and can move this to the configuration files? Thanks. Best regards, Wolfgang Denk
Hi, Thanks for your review. 2014-12-01 16:17 GMT+09:00 Wolfgang Denk <wd@denx.de>: > Dear Nobuhiro, > > In message <1417417556-23946-4-git-send-email-nobuhiro.iwamatsu.yj@renesas.com> you wrote: >> Before a kernel boots, GPIO, SYS-DMAC, QSPI and MSIOF clock >> is halted. >> >> Signed-off-by: Hisashi Nakamura <hisashi.nakamura.ak@renesas.com> >> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com> > > The data structures and the code are all repeated for this patch and > the following patches: > > [PATCH 4/7] arm: rmobile: lager: Halt clock prior to booting kernel > [PATCH 5/7] arm: rmobile: koelsch: Halt clock prior to booting kernel > [U-Boot] [PATCH 6/7] arm: rmobile: alt: Halt clock prior to booting kernel > [PATCH 7/7] arm: rmobile: gose: Halt clock prior to booting kernel > > Can you please move the code to a common place so we have it only > once? Yes. I will updates and resend patches. > >> +} mstptbl[] = { >> + [0] = { SMSTPCR0, 0x00640801, 0x00400001, >> + RMSTPCR0, 0x00640801, 0x00000000 }, >> + [1] = { SMSTPCR1, 0xDB6E9BDF, 0x00000000, >> + RMSTPCR1, 0xDB6E9BDF, 0x00000000 }, >> + [2] = { SMSTPCR2, 0x300DA1FC, 0x000CA120, >> + RMSTPCR2, 0x300DA1FC, 0x00000000 }, >> + [3] = { SMSTPCR3, 0xF08CF831, 0x00000000, >> + RMSTPCR3, 0xF08CF831, 0x00000000 }, >> + [4] = { SMSTPCR4, 0x80000184, 0x00000180, >> + RMSTPCR4, 0x80000184, 0x00000000 }, >> + [5] = { SMSTPCR5, 0x44C00046, 0x00000000, >> + RMSTPCR5, 0x44C00046, 0x00000000 }, >> + [7] = { SMSTPCR7, 0x07F30718, 0x00200000, >> + RMSTPCR7, 0x07F30718, 0x00000000 }, >> + [8] = { SMSTPCR8, 0x01F0FF84, 0x00000000, >> + RMSTPCR8, 0x01F0FF84, 0x00000000 }, >> + [9] = { SMSTPCR9, 0xF5979FCF, 0x00021F80, >> + RMSTPCR9, 0xF5979FCF, 0x00001F80 }, >> + [10] = { SMSTPCR10, 0xFFFEFFE0, 0x00000000, >> + RMSTPCR10, 0xFFFEFFE0, 0x00000000 }, >> + [11] = { SMSTPCR11, 0x00000000, 0x00000000, >> + RMSTPCR11, 0x00000000, 0x00000000 }, >> +}; > > Also, these data look pretty much the same to me, with only minor > differences in some bits. If we use some defines instead of the > magic numbers this could probably help to see the common part and the > differences in the data. We probably don't need board-specific data in > the code, and can move this to the configuration files? Yes. I will move setting data to config files. > > Thanks. > > Best regards, > > Wolfgang Denk > Best regards, Nobuhiro
diff --git a/board/renesas/lager/lager.c b/board/renesas/lager/lager.c index 47cf51b..077a78f 100644 --- a/board/renesas/lager/lager.c +++ b/board/renesas/lager/lager.c @@ -67,10 +67,49 @@ int board_early_init_f(void) return 0; } +static struct mstp_ctl { + u32 s_addr; + u32 s_dis; + u32 s_ena; + u32 r_addr; + u32 r_dis; + u32 r_ena; +} mstptbl[] = { + [0] = { SMSTPCR0, 0x00640801, 0x00400001, + RMSTPCR0, 0x00640801, 0x00000000 }, + [1] = { SMSTPCR1, 0xDB6E9BDF, 0x00000000, + RMSTPCR1, 0xDB6E9BDF, 0x00000000 }, + [2] = { SMSTPCR2, 0x300DA1FC, 0x000CA120, + RMSTPCR2, 0x300DA1FC, 0x00000000 }, + [3] = { SMSTPCR3, 0xF08CF831, 0x00000000, + RMSTPCR3, 0xF08CF831, 0x00000000 }, + [4] = { SMSTPCR4, 0x80000184, 0x00000180, + RMSTPCR4, 0x80000184, 0x00000000 }, + [5] = { SMSTPCR5, 0x44C00046, 0x00000000, + RMSTPCR5, 0x44C00046, 0x00000000 }, + [7] = { SMSTPCR7, 0x07F30718, 0x00200000, + RMSTPCR7, 0x07F30718, 0x00000000 }, + [8] = { SMSTPCR8, 0x01F0FF84, 0x00000000, + RMSTPCR8, 0x01F0FF84, 0x00000000 }, + [9] = { SMSTPCR9, 0xF5979FCF, 0x00021F80, + RMSTPCR9, 0xF5979FCF, 0x00001F80 }, + [10] = { SMSTPCR10, 0xFFFEFFE0, 0x00000000, + RMSTPCR10, 0xFFFEFFE0, 0x00000000 }, + [11] = { SMSTPCR11, 0x00000000, 0x00000000, + RMSTPCR11, 0x00000000, 0x00000000 }, +}; + void arch_preboot_os(void) { - /* Disable TMU0 */ - mstp_setbits_le32(MSTPSR1, SMSTPCR1, TMU0_MSTP125); + int i; + + /* Stop all module clock */ + for (i = 0; i < ARRAY_SIZE(mstptbl); i++) { + mstp_setclrbits_le32(mstptbl[i].s_addr, mstptbl[i].s_addr, + mstptbl[i].s_dis, mstptbl[i].s_ena); + mstp_setclrbits_le32(mstptbl[i].r_addr, mstptbl[i].r_addr, + mstptbl[i].r_dis, mstptbl[i].r_ena); + } } DECLARE_GLOBAL_DATA_PTR;