| Message ID | 1325817712-4573-2-git-send-email-shawn.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Jan 06, 2012 at 10:41:51AM +0800, Shawn Guo wrote: > Both imx23 and imx28 have CHIPID register at address 0x8001c310, which > can be used to identify the SoC. This patch changes cpu_is_xxx and > __arch_decomp_setup to use this CHIPID register than machine type to > detect the chip between imx23 and imx28, so that we do not need to > change these functions whenever a new board/machine gets added. > > Suggested-by: Wolfram Sang <w.sang@pengutronix.de> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> [...] > + > +/* > + * MXS CPU types > + */ > +#define CHIPID (MXS_IO_ADDRESS(MXS_DIGCTL_BASE_ADDR) + 0x310) Hmm, in general I don't like accessing single registers this way. Since we only read, it might work for now, but I think it will turn ugly the more we use from DIGCTL (which is annoying to abstract, yes). 0x310 should not be hardcoded. CHIPID could be MXS_CHIPID. > diff --git a/arch/arm/mach-mxs/include/mach/uncompress.h b/arch/arm/mach-mxs/include/mach/uncompress.h > index 6777674..a8de26d 100644 > --- a/arch/arm/mach-mxs/include/mach/uncompress.h > +++ b/arch/arm/mach-mxs/include/mach/uncompress.h > @@ -55,16 +55,17 @@ static inline void flush(void) > > #define MX23_DUART_BASE_ADDR 0x80070000 > #define MX28_DUART_BASE_ADDR 0x80074000 > +#define MXS_DIGCTL_CHIPID 0x8001c310 > > static inline void __arch_decomp_setup(unsigned long arch_id) > { > - switch (arch_id) { > - case MACH_TYPE_MX23EVK: > + u16 chipid = (*(volatile unsigned long *) MXS_DIGCTL_CHIPID) >> 16; Can't we use the cpu_is-functions here? Regards, Wolfram
On Fri, Jan 06, 2012 at 12:40:29PM +0100, Wolfram Sang wrote: > On Fri, Jan 06, 2012 at 10:41:51AM +0800, Shawn Guo wrote: > > Both imx23 and imx28 have CHIPID register at address 0x8001c310, which > > can be used to identify the SoC. This patch changes cpu_is_xxx and > > __arch_decomp_setup to use this CHIPID register than machine type to > > detect the chip between imx23 and imx28, so that we do not need to > > change these functions whenever a new board/machine gets added. > > > > Suggested-by: Wolfram Sang <w.sang@pengutronix.de> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > [...] > > > + > > +/* > > + * MXS CPU types > > + */ > > +#define CHIPID (MXS_IO_ADDRESS(MXS_DIGCTL_BASE_ADDR) + 0x310) > > Hmm, in general I don't like accessing single registers this way. Since > we only read, it might work for now, but I think it will turn ugly the > more we use from DIGCTL (which is annoying to abstract, yes). > I agree with you. But I do not really want to start writing a digctl.c just from this patch series. > 0x310 should not be hardcoded. CHIPID could be MXS_CHIPID. > I thought about those when I wrote the code, and decided not to bother, since they are very really limited in mxs.h. But now I think I should bother, because your comments are here. > > diff --git a/arch/arm/mach-mxs/include/mach/uncompress.h b/arch/arm/mach-mxs/include/mach/uncompress.h > > index 6777674..a8de26d 100644 > > --- a/arch/arm/mach-mxs/include/mach/uncompress.h > > +++ b/arch/arm/mach-mxs/include/mach/uncompress.h > > @@ -55,16 +55,17 @@ static inline void flush(void) > > > > #define MX23_DUART_BASE_ADDR 0x80070000 > > #define MX28_DUART_BASE_ADDR 0x80074000 > > +#define MXS_DIGCTL_CHIPID 0x8001c310 > > > > static inline void __arch_decomp_setup(unsigned long arch_id) > > { > > - switch (arch_id) { > > - case MACH_TYPE_MX23EVK: > > + u16 chipid = (*(volatile unsigned long *) MXS_DIGCTL_CHIPID) >> 16; > > Can't we use the cpu_is-functions here? > No. We are running with MMU off here.
> > 0x310 should not be hardcoded. CHIPID could be MXS_CHIPID. > > > I thought about those when I wrote the code, and decided not to bother, > since they are very really limited in mxs.h. But now I think I should > bother, because your comments are here. :) I think 0x310 should be defined in digctl.h (which was introduced by Aisheng IIRC). Then, we at least know which functionality from DIGCTL is used currently. > > Can't we use the cpu_is-functions here? > > > No. We are running with MMU off here. Right, thanks, missed that. Thanks, Wolfram
diff --git a/arch/arm/mach-mxs/include/mach/mxs.h b/arch/arm/mach-mxs/include/mach/mxs.h index bde5f66..cf39c3b 100644 --- a/arch/arm/mach-mxs/include/mach/mxs.h +++ b/arch/arm/mach-mxs/include/mach/mxs.h @@ -26,19 +26,6 @@ #include <mach/hardware.h> /* - * MXS CPU types - */ -#define cpu_is_mx23() ( \ - machine_is_mx23evk() || \ - machine_is_stmp378x() || \ - 0) -#define cpu_is_mx28() ( \ - machine_is_mx28evk() || \ - machine_is_m28evk() || \ - machine_is_tx28() || \ - 0) - -/* * IO addresses common to MXS-based */ #define MXS_IO_BASE_ADDR 0x80000000 @@ -109,6 +96,21 @@ static inline void __mxs_togl(u32 mask, void __iomem *reg) { __raw_writel(mask, reg + MXS_TOG_ADDR); } + +/* + * MXS CPU types + */ +#define CHIPID (MXS_IO_ADDRESS(MXS_DIGCTL_BASE_ADDR) + 0x310) + +static inline int cpu_is_mx23(void) +{ + return ((__raw_readl(CHIPID) >> 16) == 0x3780); +} + +static inline int cpu_is_mx28(void) +{ + return ((__raw_readl(CHIPID) >> 16) == 0x2800); +} #endif #endif /* __MACH_MXS_H__ */ diff --git a/arch/arm/mach-mxs/include/mach/uncompress.h b/arch/arm/mach-mxs/include/mach/uncompress.h index 6777674..a8de26d 100644 --- a/arch/arm/mach-mxs/include/mach/uncompress.h +++ b/arch/arm/mach-mxs/include/mach/uncompress.h @@ -55,16 +55,17 @@ static inline void flush(void) #define MX23_DUART_BASE_ADDR 0x80070000 #define MX28_DUART_BASE_ADDR 0x80074000 +#define MXS_DIGCTL_CHIPID 0x8001c310 static inline void __arch_decomp_setup(unsigned long arch_id) { - switch (arch_id) { - case MACH_TYPE_MX23EVK: + u16 chipid = (*(volatile unsigned long *) MXS_DIGCTL_CHIPID) >> 16; + + switch (chipid) { + case 0x3780: mxs_duart_base = MX23_DUART_BASE_ADDR; break; - case MACH_TYPE_MX28EVK: - case MACH_TYPE_M28EVK: - case MACH_TYPE_TX28: + case 0x2800: mxs_duart_base = MX28_DUART_BASE_ADDR; break; default:
Both imx23 and imx28 have CHIPID register at address 0x8001c310, which can be used to identify the SoC. This patch changes cpu_is_xxx and __arch_decomp_setup to use this CHIPID register than machine type to detect the chip between imx23 and imx28, so that we do not need to change these functions whenever a new board/machine gets added. Suggested-by: Wolfram Sang <w.sang@pengutronix.de> Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- arch/arm/mach-mxs/include/mach/mxs.h | 28 ++++++++++++++------------ arch/arm/mach-mxs/include/mach/uncompress.h | 11 +++++---- 2 files changed, 21 insertions(+), 18 deletions(-)