Patchwork [v2,2/2] ARM: mxs: Pass the system revision

login
register
mail settings
Submitter Fabio Estevam
Date May 31, 2013, 10:42 p.m.
Message ID <1370040127-15072-2-git-send-email-festevam@gmail.com>
Download mbox | patch
Permalink /patch/248034/
State New
Headers show

Comments

Fabio Estevam - May 31, 2013, 10:42 p.m.
From: Fabio Estevam <fabio.estevam@freescale.com>

Some mxs userspace tools, such as multimedia plugins and kobs-ng (tool used to 
burn boot images to NAND) rely on the 'Revision' field reported by 
'/proc/cpuinfo'.

Provide a mechanism to pass such information, so that now we can get:

$ cat /proc/cpuinfo                                             
processor       : 0                                                             
model name      : ARM926EJ-S rev 5 (v5l)                                        
BogoMIPS        : 226.09                                                        
Features        : swp half fastmult edsp java                                   
CPU implementer : 0x41                                                          
CPU architecture: 5TEJ                                                          
CPU variant     : 0x0                                                           
CPU part        : 0x926                                                         
CPU revision    : 5                                                             
                                                                                
Hardware        : Freescale i.MX28 Evaluation Kit                               
Revision        : 28012                                                         
Serial          : 0000000000000000 

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> 
---
Changes since v1:
- No changes

 arch/arm/mach-mxs/mach-mxs.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
Michael Heimpold - June 1, 2013, 12:19 p.m.
Hi Fabio,

Am Freitag, 31. Mai 2013, 19:42:07 schrieb Fabio Estevam:
>...                                                                                 
> Hardware        : Freescale i.MX28 Evaluation Kit                               
> Revision        : 28012                                                         
> ...
> +	system_rev = (cputype | mxs_get_cpu_rev());
> +}

I think it would be difficult to use system_rev for this purpose as some
boards I know about are using this field to pass a board/PCB revision from
U-Boot to the kernel.
After spending a short look through documentation in kernel and U-Boot
I did not find any hint whether this was the indended usage of this field.

I would rather argue about introducing a new line
"SoC revision: ..." as this is the revision you want to pass to user-space.

The lines "Revision" and "Serial" should IMHO left
alone for usage by board vendors.

BR, Michael
Fabio Estevam - June 1, 2013, 2:58 p.m.
Hi Michael,

On Sat, Jun 1, 2013 at 9:19 AM, Michael Heimpold <mhei@heimpold.de> wrote:

> I think it would be difficult to use system_rev for this purpose as some
> boards I know about are using this field to pass a board/PCB revision from
> U-Boot to the kernel.

Can you please provide some real examples?

For mx28 I see no boards passing ATAGS from bootloader to the kernel.

If you look at FSL kernel code the system revision is hardcoded in the
board file.

For mx5/mx6 boards we do pass ATAGS from bootloader to kernel in the
same format it was used here.

For example: on mx53 it will be 0x53112 (where 0x53 means mx53, 1 is
the board revision and 12 means TO1.2)

> After spending a short look through documentation in kernel and U-Boot
> I did not find any hint whether this was the indended usage of this field.
>
> I would rather argue about introducing a new line
> "SoC revision: ..." as this is the revision you want to pass to user-space.
>
> The lines "Revision" and "Serial" should IMHO left
> alone for usage by board vendors.

As I mentioned in the commit log, the only reason we are doing this
here is to allow a mainline kernel to run som mxs applications like
Gstreamer plugins and kobs-ng.

Regards,

Fabio Estevam
Michael Heimpold - June 1, 2013, 10:11 p.m.
Hi Fabio,

Am Samstag, 1. Juni 2013, 11:58:18 schrieb Fabio Estevam:
> Can you please provide some real examples?

Please have a look at e.g. mach-orion5x/dns323-setup.c or grep for
system_rev in arch/arm.
An out-of-vanilla-tree example is a board I've worked on:
https://github.com/iequalize/linux-mxs/blob/v2.6.35.14-openwrt-fsl-tq-ieq/arch/arm/mach-mx28/board-homebox.c
In arm/mach-omap2/board-omap3touchbook.c I'm not able to tell whether
the use system_rev to encode a PCB revision or a CPU revision.

> If you look at FSL kernel code the system revision is hardcoded in the
> board file.
> For mx5/mx6 boards we do pass ATAGS from bootloader to kernel in the
> same format it was used here.

I see it. But I still wonder whether this is the right direction to pass the
SoC revision to user-space. And I feel uncomfortable when we're
mixing it with a board revision. I would prefer dedicated lines in /proc/cpuinfo.
Is there a policy about adding line which would prohibit this?

> As I mentioned in the commit log, the only reason we are doing this
> here is to allow a mainline kernel to run som mxs applications like
> Gstreamer plugins and kobs-ng.

I only know the kobs-ng util but I got the point: these tools (only) look at the
revision line and cannot be used with a mainline kernel at the moment
as a vanilla kernel don't pass the information required by these tools.

However, the revision field is arm platform specific and should be used with
the same purpose over all mach types. Interpreting it for different SoCs
in a different manner is IMO not a good idea.

So my question are:
What was/is the real intension for system_rev?
Is passing a PCB revision via ATAG obsoleted by DT?

Regards,
Michael
Shawn Guo - June 3, 2013, 1:51 a.m.
On Sun, Jun 02, 2013 at 12:11:16AM +0200, Michael Heimpold wrote:
> Hi Fabio,
> 
> Am Samstag, 1. Juni 2013, 11:58:18 schrieb Fabio Estevam:
> > Can you please provide some real examples?
> 
> Please have a look at e.g. mach-orion5x/dns323-setup.c or grep for
> system_rev in arch/arm.
> An out-of-vanilla-tree example is a board I've worked on:
> https://github.com/iequalize/linux-mxs/blob/v2.6.35.14-openwrt-fsl-tq-ieq/arch/arm/mach-mx28/board-homebox.c
> In arm/mach-omap2/board-omap3touchbook.c I'm not able to tell whether
> the use system_rev to encode a PCB revision or a CPU revision.
> 
> > If you look at FSL kernel code the system revision is hardcoded in the
> > board file.
> > For mx5/mx6 boards we do pass ATAGS from bootloader to kernel in the
> > same format it was used here.
> 
> I see it. But I still wonder whether this is the right direction to pass the
> SoC revision to user-space. And I feel uncomfortable when we're
> mixing it with a board revision. I would prefer dedicated lines in /proc/cpuinfo.
> Is there a policy about adding line which would prohibit this?
> 
> > As I mentioned in the commit log, the only reason we are doing this
> > here is to allow a mainline kernel to run som mxs applications like
> > Gstreamer plugins and kobs-ng.
> 
> I only know the kobs-ng util but I got the point: these tools (only) look at the
> revision line and cannot be used with a mainline kernel at the moment
> as a vanilla kernel don't pass the information required by these tools.
> 
> However, the revision field is arm platform specific and should be used with
> the same purpose over all mach types. Interpreting it for different SoCs
> in a different manner is IMO not a good idea.

+1

Now I feel strongly that we should use soc infrastructure
(drivers/base/soc.c) to export soc ID and revision to user space.  Maybe
it's time to change those incompatible user applications.

Shawn

Patch

diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
index 462fa14..0d7e1eb 100644
--- a/arch/arm/mach-mxs/mach-mxs.c
+++ b/arch/arm/mach-mxs/mach-mxs.c
@@ -29,6 +29,7 @@ 
 #include <asm/mach/map.h>
 #include <asm/mach/time.h>
 #include <asm/system_misc.h>
+#include <asm/system_info.h>
 
 #include "pm.h"
 
@@ -442,12 +443,26 @@  static void mxs_print_silicon_rev(const char *cpu, int srev)
 				cpu, (srev >> 4) & 0xf, srev & 0xf);
 }
 
+static void mxs_pass_sysrev(void)
+{
+	int cputype = 0;
+
+	if (strcmp(mxs_get_cpu_type(), "MX23"))
+		cputype = 0x23000;
+
+	if (strcmp(mxs_get_cpu_type(), "MX28"))
+		cputype = 0x28000;
+
+	system_rev = (cputype | mxs_get_cpu_rev());
+}
+
 static void __init mxs_machine_init(void)
 {
 	const char *cpu_char = mxs_get_cpu_type();
 	int cpu_rev = mxs_get_cpu_rev();
 
 	mxs_print_silicon_rev(cpu_char, cpu_rev);
+	mxs_pass_sysrev();
 
 	if (of_machine_is_compatible("fsl,imx28-evk"))
 		imx28_evk_init();