| Message ID | 1370040127-15072-2-git-send-email-festevam@gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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();