[PATCH v2 2/3] ep93xx: added chip revision reading function

[Mika Westerberg]

On Sat, Mar 20, 2010 at 02:42:26PM -0500, H Hartley Sweeten wrote:
> On Saturday, March 20, 2010 11:31 AM, Mika Westerberg wrote:
> > On Sat, Mar 20, 2010 at 06:07:50PM +0000, Martin Guy wrote:
> >> On 3/18/10, Mika Westerberg <mika.westerberg at iki.fi> wrote:
> >>> Added a new function: ep93xx_chip_revision() which reads chip revision from the
> >>>  sysconfig register.
> >>>
> >>>  diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
> >>>  index 90fb591..07572bb 100644
> >>>  --- a/arch/arm/mach-ep93xx/core.c
> >>>  +++ b/arch/arm/mach-ep93xx/core.c
> >>>  @@ -222,6 +222,20 @@ void ep93xx_devcfg_set_clear(unsigned int set_bits, unsigned int clear_bits)
> >>>   }
> >>>   EXPORT_SYMBOL(ep93xx_devcfg_set_clear);
> >>>
> >>>  +/**
> >>>  + * ep93xx_chip_revision() - returns the EP93xx chip revision
> >>>  + *
> >>>  + * See <mach/platform.h> for more information.
> >>>  + */
> >>>  +unsigned int ep93xx_chip_revision(void)
> >>>  +{
> >>>  +       unsigned int v;
> >>>  +
> >>>  +       v = __raw_readl(EP93XX_SYSCON_SYSCFG);
> >>>  +       v &= EP93XX_SYSCON_SYSCFG_REV_MASK;
> >>>  +       v >>= EP93XX_SYSCON_SYSCFG_REV_SHIFT;
> >>>  +       return v;
> >>>  +}
> >> 
> >>   The chip revision is normally placed in the global unsigned int
> >> "system_rev", which is also reported by /proc/cpuinfo.
> >
> > Is system_rev same as cpu chip revision? I think it should contain
> > board revision number or something similar.
> 
> The system_rev != the chip revision.
> 
> >>   It normally seems to be set from the atags passed by the bootloader
> >> but uboot on this platform doesn't provide that, so it remains 0.
> >
> > Neither my platform (uses redboot):
> >
> > Hardware        : Technologic Systems TS-72xx SBC
> > Revision        : 0000
> > Serial          : 0000000000000000
> >
> >>   The same applies to the very similar variables system_serial_low and
> >> system_serial_high (which also remain 0 on this u-boot platform).
> >> Shall we also set these from the hardware on ep93xx during thus giving
> >> them non-zero values in /proc/cpuinfo too?
> >>   As to where...? The other platforms that set these variables from
> >> hardware queries do it in the board-specific files' MACHINE_START
> >> function, but we'd have to do this in every board file, or have some
> >> comon routine and modify every mach-ep93xx board file. The only common
> >> routine all ep93xx board inits seem to have is ep93xx_init_devices.
> >> This seems the least-resistance place to do this, but has nothing to
> >> do with initlalizing devices of course. Is making another function in
> >> toe core file and modifying every board's init function preferable?
> >> 
> >> Also, I have the simpler
> >>     system_rev = *((unsigned int *)EP93XX_SYSCON_CHIP_ID) >> 28;
> >>     system_serial_low = *((unsigned int *)EP93XX_SECURITY_UNIQID);
> >> but maybe explicit readl and use of long defines are preferable.
> >
> > I'm not the best guy to answer these. Maybe Hartley Sweeten or Ryan
> > Mallon have some ideas about this. I didn't even know that such
> > variable exists :)
> 
> This is not a good idea.
> 
> system_rev, system_serial_high, and system_serial_low are all passed to
> the kernel by the bootloader as ATAGs.  If your bootloader doesn't pass
> the ATAGs it should be fixed.  Overriding the variables breaks what they
> are intended for.

Ok, thanks for clarification. It seems that in my platform, the bootloader
doesn't handle these correctly.

> I have some patches for Redboot but Cirrus still has not sent in the FSF
> paperwork to transfer the ep93xx port to eCos.  With those patches and
> the /proc/cpuinfo extension patches I mentioned previously I get the
> following on my system:
> 
> / # cat /proc/cpuinfo
> Processor       : ARM920T rev 0 (v4l)
> BogoMIPS        : 99.53
> Features        : swp half thumb crunch
> CPU implementer : 0x41
> CPU architecture: 4T
> CPU variant     : 0x1
> CPU part        : 0x920
> CPU revision    : 0
> 
> Hardware        : Vision Engraving Systems EP9307
> Revision        : 0001
> Serial          : 4943410027260017
> 
> System Type     : iMARC
> 
> Unique ID       : 921040f8
> Maverick Key    : 921040f85b6f0e7b76eb23d24309b3 OK
> Silicon Rev     : E2
> Watchdog        : active
> Reset duration  : active
> Boot mode       : normal async internal 16-bit
> / #

Cool! You should definitely repost your patches.

Regards,
MW