On Wednesday 18 June 2014 22:46:19 Rob Herring wrote: > On Wed, Jun 18, 2014 at 2:22 PM, Pali Rohár wrote: > > On Wednesday 18 June 2014 21:07:35 Rob Herring wrote: > >> On Wed, Jun 18, 2014 at 11:54 AM, Pali Rohár > > > > wrote: > >> > Machine name from board description is some generic name > >> > on DT kernel. DT provides machine name property which is > >> > specific for board, so use it instead generic one when > >> > possible. > >> > > >> > Signed-off-by: Pali Rohár > >> > --- > >> > > >> > arch/arm/kernel/setup.c | 7 +++++-- > >> > 1 file changed, 5 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/arch/arm/kernel/setup.c > >> > b/arch/arm/kernel/setup.c index 8a16ee5..fbc7b4f 100644 > >> > --- a/arch/arm/kernel/setup.c > >> > +++ b/arch/arm/kernel/setup.c > >> > @@ -875,10 +875,13 @@ void __init setup_arch(char > >> > **cmdline_p) > >> > > >> > setup_processor(); > >> > mdesc = setup_machine_fdt(__atags_pointer); > >> > > >> > - if (!mdesc) > >> > + if (mdesc) > >> > + machine_name = > >> > of_flat_dt_get_machine_name(); + else > >> > > >> > mdesc = > >> > setup_machine_tags(__atags_pointer, > >> > __machine_arch_type); > >> > > >> > machine_desc = mdesc; > >> > > >> > - machine_name = mdesc->name; > >> > + if (!machine_name) > >> > + machine_name = mdesc->name; > >> > > >> > if (mdesc->reboot_mode != REBOOT_HARD) > >> > > >> > reboot_mode = mdesc->reboot_mode; > >> > >> I did a similar patch previously[1]. Like my original > >> patch, your patch unconditionally changes the name which > >> could be considered part of the userspace ABI. It's > >> arguably not good practice for userspace to depend on the > >> name, but there are likely cases that do. So I think this > >> needs to be optional and only use the DT name if the > >> machine desc name is NULL. > >> > >> So something like the below and then change the machine > >> descriptors you care about. The default generic DT machine > >> desc should definitely be changed. > >> > >> I had a follow-up discussion with Grant about his concerns > >> in the thread about knowing which machine desc used to > >> boot. He said he was okay if just the machine desc address > >> is printed out. > >> > >> > >> diff --git a/arch/arm/kernel/setup.c > >> b/arch/arm/kernel/setup.c index 50e198c..1479250 100644 > >> --- a/arch/arm/kernel/setup.c > >> +++ b/arch/arm/kernel/setup.c > >> @@ -887,7 +887,7 @@ void __init setup_arch(char > >> **cmdline_p) > >> > >> if (!mdesc) > >> > >> mdesc = setup_machine_tags(__atags_pointer, > >> > >> __machine_arch_type); > >> > >> machine_desc = mdesc; > >> > >> - machine_name = mdesc->name; > >> + machine_name = mdesc->name ? mdesc->name : > >> of_flat_dt_get_machine_name(); > >> > >> if (mdesc->reboot_mode != REBOOT_HARD) > >> > >> reboot_mode = mdesc->reboot_mode; > >> > >> [1] > >> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-> >> No vember/208878.html > > > > Hi, > > > > now legacy board code for Nokia N900 (RX-51) is migrating to > > DT kernel and there is problem with info which > > /proc/cpuinfo reports > > Thanks for providing your motivation which was missing from > the commit message. > > > New DT kernel (3.16-rc1) reports: > > > > # busybox cat /proc/cpuinfo > > processor : 0 > > model name : ARMv7 Processor rev 3 (v7l) > > Features : swp half thumb fastmult vfp edsp thumbee > > neon vfpv3 tls vfpd32 > > CPU implementer : 0x41 > > CPU architecture: 7 > > CPU variant : 0x2 > > CPU part : 0xc08 > > CPU revision : 3 > > > > Hardware : Generic OMAP3 (Flattened Device Tree) > > Revision : 0000 > > Serial : 0000000000000000 > > > > But legacy board code kernel reports: > > > > # busybox cat /proc/cpuinfo > > processor : 0 > > model name : ARMv7 Processor rev 3 (v7l) > > Features : swp half thumb fastmult vfp edsp thumbee > > neon vfpv3 tls vfpd32 > > CPU implementer : 0x41 > > CPU architecture: 7 > > CPU variant : 0x2 > > CPU part : 0xc08 > > CPU revision : 3 > > > > Hardware : Nokia RX-51 board > > Revision : 0012 > > Serial : 0000000000000000 > > > > Basically in DT kernel is missing Hardware, Revision and > > probably also Serial key. (Now I used only qemu for testing > > which set serial key to 0). All these informations is used > > by userspace applications which determinate how to behave. > > It is somewhat fragile to expect the name in the DT to match > the old name from the kernel. As your patch to n900 dts > shows, we'd probably have to update every dts file to make > them match. While I think we should work to remove this > string from the kernel, userspace depending on the DT model > string is a bad idea. For example, if you had some platform > with multiple OEMs just rebranding the same base design, they > may all want to put their own model string into each product. > The true h/w id is the compatible string. > > Serial number is easy. There is already a standard although > not widely used property for it with "/serial-number". You > just need to wire it up to cpuinfo. > There is no "/serial-number" property in kernel: $ ls -l -a /sys/bus/soc/devices/soc0/ drwxr-xr-x 3 0 0 0 Jan 1 00:00 . drwxr-xr-x 18 0 0 0 Jan 1 00:00 .. -r--r--r-- 1 0 0 4096 Jan 1 00:00 family -r--r--r-- 1 0 0 4096 Jan 1 00:00 machine drwxr-xr-x 2 0 0 0 Jan 1 00:00 power -r--r--r-- 1 0 0 4096 Jan 1 00:00 revision lrwxrwxrwx 1 0 0 0 Jan 1 00:00 subsystem -> ../../bus/soc -r--r--r-- 1 0 0 4096 Jan 1 00:00 type -rw-r--r-- 1 0 0 4096 Jan 1 00:00 uevent > > So without this patch DT migration for Nokia N900 cannot be > > done without breaking userspace - which is not > > acceptable... > > I agree, but I would like to come up with something that > prevents future dependence on this string. > > > Also I still did not know why DT kernel does not report > > Revision number which is passed by bootloader via atags. > > Any idea? > > Probably because no one cared until now and revision info for > every SOC is different. I would like to see revision info set > in the DT in a standard way and remove the various SOC > specific kernel implementations. > > Rob -- Pali Rohár pali.rohar@gmail.com