| Message ID | 1384708667-22489-2-git-send-email-eric.nelson@boundarydevices.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
Hi Eric, > Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> > --- > This patch is new in V3 > > arch/arm/include/asm/arch-mx5/sys_proto.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm/include/asm/arch-mx5/sys_proto.h > b/arch/arm/include/asm/arch-mx5/sys_proto.h index 9949ad1..9dad5fc 100644 > --- a/arch/arm/include/asm/arch-mx5/sys_proto.h > +++ b/arch/arm/include/asm/arch-mx5/sys_proto.h > @@ -17,6 +17,10 @@ > > #define is_soc_rev(rev) ((get_cpu_rev() & 0xFF) - rev) > u32 get_cpu_rev(void); > + > +/* returns MXC_CPU_ value */ > +#define cpu_type(rev) (((rev) >> 12)&0xff) Maybe you can implement it the same way as get_cpu_rev() and make it call get_cpu_rev() internally ? Then it'd be a function (with all the typechecking and stuff) and you won't need to pass extra param. Best regards, Marek Vasut
Hi Eric, On 17/11/2013 18:17, Eric Nelson wrote: > Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> > --- > This patch is new in V3 > > arch/arm/include/asm/arch-mx5/sys_proto.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm/include/asm/arch-mx5/sys_proto.h b/arch/arm/include/asm/arch-mx5/sys_proto.h > index 9949ad1..9dad5fc 100644 > --- a/arch/arm/include/asm/arch-mx5/sys_proto.h > +++ b/arch/arm/include/asm/arch-mx5/sys_proto.h > @@ -17,6 +17,10 @@ > > #define is_soc_rev(rev) ((get_cpu_rev() & 0xFF) - rev) > u32 get_cpu_rev(void); > + > +/* returns MXC_CPU_ value */ > +#define cpu_type(rev) (((rev) >> 12)&0xff) > + There is already a get_cpu_type() for other architectures (OMAP). We do not need to reinvent the wheel this time, and it is correct to add get_cpu_type(void) to sys_proto.h. This lets also easier to understand the code because it can be directly derived from the User's Manual: shifting 12 bit in your macro is only because this is done in get_cpu_rev(), not because this is the offset in the i.MX6 register. Best regards, Stefano Babic
Hi Stefano, On 11/18/2013 03:42 AM, Stefano Babic wrote: > Hi Eric, > > On 17/11/2013 18:17, Eric Nelson wrote: >> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> >> --- >> This patch is new in V3 >> >> arch/arm/include/asm/arch-mx5/sys_proto.h | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/arch/arm/include/asm/arch-mx5/sys_proto.h b/arch/arm/include/asm/arch-mx5/sys_proto.h >> index 9949ad1..9dad5fc 100644 >> --- a/arch/arm/include/asm/arch-mx5/sys_proto.h >> +++ b/arch/arm/include/asm/arch-mx5/sys_proto.h >> @@ -17,6 +17,10 @@ >> >> #define is_soc_rev(rev) ((get_cpu_rev() & 0xFF) - rev) >> u32 get_cpu_rev(void); >> + >> +/* returns MXC_CPU_ value */ >> +#define cpu_type(rev) (((rev) >> 12)&0xff) >> + > > There is already a get_cpu_type() for other architectures (OMAP). We do > not need to reinvent the wheel this time, and it is correct to add > get_cpu_type(void) to sys_proto.h. > > This lets also easier to understand the code because it can be directly > derived from the User's Manual: shifting 12 bit in your macro is only > because this is done in get_cpu_rev(), not because this is the offset in > the i.MX6 register. > Okay. I'll re-submit with get_cpu_type(void) implemented imx-common/cpu.c. I still question the fact that we have two header files for i.MX5x and i.MX6x declaring the returns implemented there. It seems that we should have a single header for routines implemented there. Perhaps arch/arm/include/imx-common/cpu.h? Please advise, Eric
Hi Eric, On 19/11/2013 04:25, Eric Nelson wrote: > I still question the fact that we have two header files for i.MX5x > and i.MX6x declaring the returns implemented there. Good point. > > It seems that we should have a single header for routines > implemented there. > > Perhaps arch/arm/include/imx-common/cpu.h? +1 > > Please advise, Nice if you clean up also this point ! Best regards, Stefano
diff --git a/arch/arm/include/asm/arch-mx5/sys_proto.h b/arch/arm/include/asm/arch-mx5/sys_proto.h index 9949ad1..9dad5fc 100644 --- a/arch/arm/include/asm/arch-mx5/sys_proto.h +++ b/arch/arm/include/asm/arch-mx5/sys_proto.h @@ -17,6 +17,10 @@ #define is_soc_rev(rev) ((get_cpu_rev() & 0xFF) - rev) u32 get_cpu_rev(void); + +/* returns MXC_CPU_ value */ +#define cpu_type(rev) (((rev) >> 12)&0xff) + unsigned imx_ddr_size(void); void sdelay(unsigned long); void set_chipselect_size(int const);
Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> --- This patch is new in V3 arch/arm/include/asm/arch-mx5/sys_proto.h | 4 ++++ 1 file changed, 4 insertions(+)