| Message ID | 1435631776-9733-1-git-send-email-Peng.Fan@freescale.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
Hi Stefano, On Fri, Jul 10, 2015 at 10:30:59AM +0200, Stefano Babic wrote: >Hi Peng, > >in the title "dummpy" instead of "dummy". Thanks pointing this out. > >On 30/06/2015 04:36, Peng Fan wrote: >> Add dummy cpu type MXC_CPU_MX6QP/DP. > >Anyway, why is it dummy ? It matches a real SOC, only the check is done >in another way. Just like MXC_CPU_MX6Q and MXC_CPU_MX6D. MXC_CPU_MX6D is a dummy id, MXC_CPU_MX6Q is the real id. Same MXC_CPU_MX6QP/DP are also dummy id. Since I want to print correct CPU info, so I use this way, but not change arch/arm/imx-common/cpu.c. > >> Since i.MX6QP use a revision 2, but with >> cpu type i.MX6Q, we need the MXC_CPU_MX6QP and to decrease major with 1 to >> let print_cpuinfo print the correct info. > >I understand well the code, not so well the text here. Maybe does it >help simply to write that MX6 with MAJOR_LOWER >= 2 are plus ? > Only for 6QP/DP. Not for all MX6 with MAJOR_LOWER >= 1. >> >> This patch also fix is_mx6dqp(), since get_cpu_rev can return MXC_CPU_MX6QP >> and MXC_CPU_MX6DP, we should use: >> (is_cpu_type(MXC_CPU_MX6QP) || is_cpu_type(MXC_CPU_MX6DP)). >> >> Signed-off-by: Peng Fan <Peng.Fan@freescale.com> >> --- >> >> Changes v4: >> Address Fabio's comments, Change Quad-Plus to Dual-Plus for i.MX6DP. >> >> Changes v3: >> New patch >> This patch is to make print_cpuinfo display correct cpu info, also fix >> is_mx6dqp >> >> Changes v2: >> none >> >> arch/arm/cpu/armv7/mx6/soc.c | 11 +++++++++-- >> arch/arm/imx-common/cpu.c | 4 ++++ >> arch/arm/include/asm/arch-imx/cpu.h | 2 ++ >> arch/arm/include/asm/arch-mx6/sys_proto.h | 4 +--- >> 4 files changed, 16 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c >> index 29de624..d3a3b2e 100644 >> --- a/arch/arm/cpu/armv7/mx6/soc.c >> +++ b/arch/arm/cpu/armv7/mx6/soc.c >> @@ -62,12 +62,12 @@ u32 get_cpu_rev(void) >> struct anatop_regs *anatop = (struct anatop_regs *)ANATOP_BASE_ADDR; >> u32 reg = readl(&anatop->digprog_sololite); >> u32 type = ((reg >> 16) & 0xff); >> - u32 major; >> + u32 major, cfg = 0; >> >> if (type != MXC_CPU_MX6SL) { >> reg = readl(&anatop->digprog); >> struct scu_regs *scu = (struct scu_regs *)SCU_BASE_ADDR; >> - u32 cfg = readl(&scu->config) & 3; >> + cfg = readl(&scu->config) & 3; >> type = ((reg >> 16) & 0xff); >> if (type == MXC_CPU_MX6DL) { >> if (!cfg) >> @@ -81,6 +81,13 @@ u32 get_cpu_rev(void) >> >> } >> major = ((reg >> 8) & 0xff); >> + if ((major >= 1) && > >Everything fine, but I have not understood this line, please help me. >major is the revision number and should be at least 2 for a QP or DP. >But you check that it can be >=, that is revision 1.x is accepted as >Plus. Or am I wrong ? To i.MX6, MAJOR_LOWER is from 0,1,2... maybe larger. I have no knowledge whether major_lower with 2,3,4... is also called DQPlus. 6QP/DP is major_lower >= 1, major_lower 0 is for 6DQ. Now ">= 1" can work for 6QP/DP, just check "== 1" may not a good idea. Is this clear to explain why this patch? The reason for this patch is to print correct cpuinfo: printf("CPU: Freescale i.MX%s rev%d.%d", get_imx_type((cpurev & 0xFF000) >> 12), (cpurev & 0x000F0) >> 4, (cpurev & 0x0000F) >> 0); As Fabio's comments, should print i.MX6QP 1.0, but i.MX6Q rev2.0. > >> + ((type == MXC_CPU_MX6Q) || (type == MXC_CPU_MX6D))) { >> + major--; >> + type = MXC_CPU_MX6QP; >> + if (cfg == 1) >> + type = MXC_CPU_MX6DP; >> + } >> reg &= 0xff; /* mx6 silicon revision */ >> return (type << 12) | (reg + (0x10 * (major + 1))); >> } >> diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c >> index 5e56cfe..096d22e 100644 >> --- a/arch/arm/imx-common/cpu.c >> +++ b/arch/arm/imx-common/cpu.c >> @@ -122,6 +122,10 @@ unsigned imx_ddr_size(void) >> const char *get_imx_type(u32 imxtype) >> { >> switch (imxtype) { >> + case MXC_CPU_MX6QP: >> + return "6QP"; /* Quad-Plus version of the mx6 */ >> + case MXC_CPU_MX6DP: >> + return "6DP"; /* Dual-Plus version of the mx6 */ >> case MXC_CPU_MX6Q: >> return "6Q"; /* Quad-core version of the mx6 */ >> case MXC_CPU_MX6D: >> diff --git a/arch/arm/include/asm/arch-imx/cpu.h b/arch/arm/include/asm/arch-imx/cpu.h >> index 4715f4e..99e0e32 100644 >> --- a/arch/arm/include/asm/arch-imx/cpu.h >> +++ b/arch/arm/include/asm/arch-imx/cpu.h >> @@ -12,6 +12,8 @@ >> #define MXC_CPU_MX6Q 0x63 >> #define MXC_CPU_MX6D 0x64 >> #define MXC_CPU_MX6SOLO 0x65 /* dummy ID */ >> +#define MXC_CPU_MX6DP 0x68 >> +#define MXC_CPU_MX6QP 0x69 >> >> #define CS0_128 0 >> #define CS0_64M_CS1_64M 1 >> diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h >> index 28c77a4..eee8ca8 100644 >> --- a/arch/arm/include/asm/arch-mx6/sys_proto.h >> +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h >> @@ -30,9 +30,7 @@ const char *get_imx_type(u32 imxtype); >> unsigned imx_ddr_size(void); >> void set_chipselect_size(int const); >> >> -#define is_mx6dqp() ((is_cpu_type(MXC_CPU_MX6Q) || \ >> - is_cpu_type(MXC_CPU_MX6D)) && \ >> - (soc_rev() >= CHIP_REV_2_0)) >> +#define is_mx6dqp() (is_cpu_type(MXC_CPU_MX6QP) || is_cpu_type(MXC_CPU_MX6DP)) >> >> /* >> * Initializes on-chip ethernet controllers. >> > > >Best regards, >Stefano Babic > >-- >===================================================================== >DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de >===================================================================== Regards, Peng.
Hi Peng, in the title "dummpy" instead of "dummy". On 30/06/2015 04:36, Peng Fan wrote: > Add dummy cpu type MXC_CPU_MX6QP/DP. Anyway, why is it dummy ? It matches a real SOC, only the check is done in another way. > Since i.MX6QP use a revision 2, but with > cpu type i.MX6Q, we need the MXC_CPU_MX6QP and to decrease major with 1 to > let print_cpuinfo print the correct info. I understand well the code, not so well the text here. Maybe does it help simply to write that MX6 with MAJOR_LOWER >= 2 are plus ? > > This patch also fix is_mx6dqp(), since get_cpu_rev can return MXC_CPU_MX6QP > and MXC_CPU_MX6DP, we should use: > (is_cpu_type(MXC_CPU_MX6QP) || is_cpu_type(MXC_CPU_MX6DP)). > > Signed-off-by: Peng Fan <Peng.Fan@freescale.com> > --- > > Changes v4: > Address Fabio's comments, Change Quad-Plus to Dual-Plus for i.MX6DP. > > Changes v3: > New patch > This patch is to make print_cpuinfo display correct cpu info, also fix > is_mx6dqp > > Changes v2: > none > > arch/arm/cpu/armv7/mx6/soc.c | 11 +++++++++-- > arch/arm/imx-common/cpu.c | 4 ++++ > arch/arm/include/asm/arch-imx/cpu.h | 2 ++ > arch/arm/include/asm/arch-mx6/sys_proto.h | 4 +--- > 4 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c > index 29de624..d3a3b2e 100644 > --- a/arch/arm/cpu/armv7/mx6/soc.c > +++ b/arch/arm/cpu/armv7/mx6/soc.c > @@ -62,12 +62,12 @@ u32 get_cpu_rev(void) > struct anatop_regs *anatop = (struct anatop_regs *)ANATOP_BASE_ADDR; > u32 reg = readl(&anatop->digprog_sololite); > u32 type = ((reg >> 16) & 0xff); > - u32 major; > + u32 major, cfg = 0; > > if (type != MXC_CPU_MX6SL) { > reg = readl(&anatop->digprog); > struct scu_regs *scu = (struct scu_regs *)SCU_BASE_ADDR; > - u32 cfg = readl(&scu->config) & 3; > + cfg = readl(&scu->config) & 3; > type = ((reg >> 16) & 0xff); > if (type == MXC_CPU_MX6DL) { > if (!cfg) > @@ -81,6 +81,13 @@ u32 get_cpu_rev(void) > > } > major = ((reg >> 8) & 0xff); > + if ((major >= 1) && Everything fine, but I have not understood this line, please help me. major is the revision number and should be at least 2 for a QP or DP. But you check that it can be >=, that is revision 1.x is accepted as Plus. Or am I wrong ? > + ((type == MXC_CPU_MX6Q) || (type == MXC_CPU_MX6D))) { > + major--; > + type = MXC_CPU_MX6QP; > + if (cfg == 1) > + type = MXC_CPU_MX6DP; > + } > reg &= 0xff; /* mx6 silicon revision */ > return (type << 12) | (reg + (0x10 * (major + 1))); > } > diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c > index 5e56cfe..096d22e 100644 > --- a/arch/arm/imx-common/cpu.c > +++ b/arch/arm/imx-common/cpu.c > @@ -122,6 +122,10 @@ unsigned imx_ddr_size(void) > const char *get_imx_type(u32 imxtype) > { > switch (imxtype) { > + case MXC_CPU_MX6QP: > + return "6QP"; /* Quad-Plus version of the mx6 */ > + case MXC_CPU_MX6DP: > + return "6DP"; /* Dual-Plus version of the mx6 */ > case MXC_CPU_MX6Q: > return "6Q"; /* Quad-core version of the mx6 */ > case MXC_CPU_MX6D: > diff --git a/arch/arm/include/asm/arch-imx/cpu.h b/arch/arm/include/asm/arch-imx/cpu.h > index 4715f4e..99e0e32 100644 > --- a/arch/arm/include/asm/arch-imx/cpu.h > +++ b/arch/arm/include/asm/arch-imx/cpu.h > @@ -12,6 +12,8 @@ > #define MXC_CPU_MX6Q 0x63 > #define MXC_CPU_MX6D 0x64 > #define MXC_CPU_MX6SOLO 0x65 /* dummy ID */ > +#define MXC_CPU_MX6DP 0x68 > +#define MXC_CPU_MX6QP 0x69 > > #define CS0_128 0 > #define CS0_64M_CS1_64M 1 > diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h > index 28c77a4..eee8ca8 100644 > --- a/arch/arm/include/asm/arch-mx6/sys_proto.h > +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h > @@ -30,9 +30,7 @@ const char *get_imx_type(u32 imxtype); > unsigned imx_ddr_size(void); > void set_chipselect_size(int const); > > -#define is_mx6dqp() ((is_cpu_type(MXC_CPU_MX6Q) || \ > - is_cpu_type(MXC_CPU_MX6D)) && \ > - (soc_rev() >= CHIP_REV_2_0)) > +#define is_mx6dqp() (is_cpu_type(MXC_CPU_MX6QP) || is_cpu_type(MXC_CPU_MX6DP)) > > /* > * Initializes on-chip ethernet controllers. > Best regards, Stefano Babic
Hi Peng, On 10/07/2015 10:06, Peng Fan wrote: >> Anyway, why is it dummy ? It matches a real SOC, only the check is done >> in another way. > > Just like MXC_CPU_MX6Q and MXC_CPU_MX6D. MXC_CPU_MX6D is a dummy id, > MXC_CPU_MX6Q is the real id. Same MXC_CPU_MX6QP/DP are also dummy id. ok, that is what you meant, understood. It is only that value is not exactly what we read from DIGIPROG register. The title and commit message let me think that "the cpu type " is dummy, that is it does not exist, while the CPU-ID is only built with a formula instead of getting the value from the register. IMHO it was enough you simply say "add CPU type for 6QP/DP", dropping the first part of the commit message that is misleading. > Since I want to print correct CPU info, so I use this way, but not > change arch/arm/imx-common/cpu.c. This is ok, agree. >> Everything fine, but I have not understood this line, please help me. >> major is the revision number and should be at least 2 for a QP or DP. >> But you check that it can be >=, that is revision 1.x is accepted as >> Plus. Or am I wrong ? > > To i.MX6, MAJOR_LOWER is from 0,1,2... maybe larger. > I have no knowledge whether major_lower with 2,3,4... is also called DQPlus. > 6QP/DP is major_lower >= 1, major_lower 0 is for 6DQ. ok, thanks - this is clear now. > Now ">= 1" can work > for 6QP/DP, just check "== 1" may not a good idea. > > Is this clear to explain why this patch? yes, it is ok. I was missing that even major_lower = 1 is a Plus. Fine with me. > > The reason for this patch is to print correct cpuinfo: > > printf("CPU: Freescale i.MX%s rev%d.%d", > get_imx_type((cpurev & 0xFF000) >> 12), > (cpurev & 0x000F0) >> 4, > (cpurev & 0x0000F) >> 0); > > As Fabio's comments, should print i.MX6QP 1.0, but i.MX6Q rev2.0. ok Apart interpretation of the commit message, patch is ok for me. Acked-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c index 29de624..d3a3b2e 100644 --- a/arch/arm/cpu/armv7/mx6/soc.c +++ b/arch/arm/cpu/armv7/mx6/soc.c @@ -62,12 +62,12 @@ u32 get_cpu_rev(void) struct anatop_regs *anatop = (struct anatop_regs *)ANATOP_BASE_ADDR; u32 reg = readl(&anatop->digprog_sololite); u32 type = ((reg >> 16) & 0xff); - u32 major; + u32 major, cfg = 0; if (type != MXC_CPU_MX6SL) { reg = readl(&anatop->digprog); struct scu_regs *scu = (struct scu_regs *)SCU_BASE_ADDR; - u32 cfg = readl(&scu->config) & 3; + cfg = readl(&scu->config) & 3; type = ((reg >> 16) & 0xff); if (type == MXC_CPU_MX6DL) { if (!cfg) @@ -81,6 +81,13 @@ u32 get_cpu_rev(void) } major = ((reg >> 8) & 0xff); + if ((major >= 1) && + ((type == MXC_CPU_MX6Q) || (type == MXC_CPU_MX6D))) { + major--; + type = MXC_CPU_MX6QP; + if (cfg == 1) + type = MXC_CPU_MX6DP; + } reg &= 0xff; /* mx6 silicon revision */ return (type << 12) | (reg + (0x10 * (major + 1))); } diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c index 5e56cfe..096d22e 100644 --- a/arch/arm/imx-common/cpu.c +++ b/arch/arm/imx-common/cpu.c @@ -122,6 +122,10 @@ unsigned imx_ddr_size(void) const char *get_imx_type(u32 imxtype) { switch (imxtype) { + case MXC_CPU_MX6QP: + return "6QP"; /* Quad-Plus version of the mx6 */ + case MXC_CPU_MX6DP: + return "6DP"; /* Dual-Plus version of the mx6 */ case MXC_CPU_MX6Q: return "6Q"; /* Quad-core version of the mx6 */ case MXC_CPU_MX6D: diff --git a/arch/arm/include/asm/arch-imx/cpu.h b/arch/arm/include/asm/arch-imx/cpu.h index 4715f4e..99e0e32 100644 --- a/arch/arm/include/asm/arch-imx/cpu.h +++ b/arch/arm/include/asm/arch-imx/cpu.h @@ -12,6 +12,8 @@ #define MXC_CPU_MX6Q 0x63 #define MXC_CPU_MX6D 0x64 #define MXC_CPU_MX6SOLO 0x65 /* dummy ID */ +#define MXC_CPU_MX6DP 0x68 +#define MXC_CPU_MX6QP 0x69 #define CS0_128 0 #define CS0_64M_CS1_64M 1 diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h index 28c77a4..eee8ca8 100644 --- a/arch/arm/include/asm/arch-mx6/sys_proto.h +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h @@ -30,9 +30,7 @@ const char *get_imx_type(u32 imxtype); unsigned imx_ddr_size(void); void set_chipselect_size(int const); -#define is_mx6dqp() ((is_cpu_type(MXC_CPU_MX6Q) || \ - is_cpu_type(MXC_CPU_MX6D)) && \ - (soc_rev() >= CHIP_REV_2_0)) +#define is_mx6dqp() (is_cpu_type(MXC_CPU_MX6QP) || is_cpu_type(MXC_CPU_MX6DP)) /* * Initializes on-chip ethernet controllers.
Add dummy cpu type MXC_CPU_MX6QP/DP. Since i.MX6QP use a revision 2, but with cpu type i.MX6Q, we need the MXC_CPU_MX6QP and to decrease major with 1 to let print_cpuinfo print the correct info. This patch also fix is_mx6dqp(), since get_cpu_rev can return MXC_CPU_MX6QP and MXC_CPU_MX6DP, we should use: (is_cpu_type(MXC_CPU_MX6QP) || is_cpu_type(MXC_CPU_MX6DP)). Signed-off-by: Peng Fan <Peng.Fan@freescale.com> --- Changes v4: Address Fabio's comments, Change Quad-Plus to Dual-Plus for i.MX6DP. Changes v3: New patch This patch is to make print_cpuinfo display correct cpu info, also fix is_mx6dqp Changes v2: none arch/arm/cpu/armv7/mx6/soc.c | 11 +++++++++-- arch/arm/imx-common/cpu.c | 4 ++++ arch/arm/include/asm/arch-imx/cpu.h | 2 ++ arch/arm/include/asm/arch-mx6/sys_proto.h | 4 +--- 4 files changed, 16 insertions(+), 5 deletions(-)