ARM: /proc/cpuinfo: Use DT machine name when possible

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.

Quoted 4 lines · Pali Roh?r

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.

http://archive.arm.linux.org.uk/lurker/message/20130726.132850
.53d47576.en.html

"If userspace wants to get at the DT information about a platform, we already have ways that can happen already - we export the DT stuff so that kexec's tools can get at it."

Quoted 6 lines · On Wednesday 18 June 2014 21:01:09 Russell King - ARM Linux wrote:

Quoted 4 lines · Pali Roh?r

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.

Quoted 6 lines

http://archive.arm.linux.org.uk/lurker/message/20130726.132850
.53d47576.en.html

"If userspace wants to get at the DT information about a platform, we already have ways that can happen already - we export the DT stuff so that kexec's tools can get at it."

Userspace application does not know that kernel using DT. And also it does not want to get DT information. Only board/machine name. So existing applications stop working after migration to DT. And because legacy board boot code (without DT) is going to be removed for ARM in near future this will permanently break existing applications.

And sometimes it is even not possible to fix userspace application if is closed source - binary only.

Quoted 15 lines · Pali Roh?r

Quoted 6 lines · On Wednesday 18 June 2014 21:01:09 Russell King - ARM Linux wrote:

Quoted 4 lines · Pali Roh?r

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.

Quoted 6 lines

http://archive.arm.linux.org.uk/lurker/message/20130726.132850
.53d47576.en.html

"If userspace wants to get at the DT information about a platform, we already have ways that can happen already - we export the DT stuff so that kexec's tools can get at it."

Quoted 6 lines

Userspace application does not know that kernel using DT. And also it does not want to get DT information. Only board/machine name. So existing applications stop working after migration to DT. And because legacy board boot code (without DT) is going to be removed for ARM in near future this will permanently break existing applications.

We're already breaking the userspace API through moving to DT, because all the device names in /sys/devices are changing. Userspace is going to have to cope with change as we move towards DT. This is just another aspect of moving towards DT, and one which userspace is going to have to deal with.

Quoted 2 lines

And sometimes it is even not possible to fix userspace application if is closed source - binary only.

And why do we care about closed source?

If we listened to this argument, then we wouldn't ever be able to change anything in procfs or sysfs.

Quoted 15 lines · Pali Roh?r

Quoted 6 lines · On Wednesday 18 June 2014 21:01:09 Russell King - ARM Linux wrote:

Quoted 4 lines · Pali Roh?r

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.

Quoted 6 lines

http://archive.arm.linux.org.uk/lurker/message/20130726.132850
.53d47576.en.html

"If userspace wants to get at the DT information about a platform, we already have ways that can happen already - we export the DT stuff so that kexec's tools can get at it."

Quoted 6 lines

Userspace application does not know that kernel using DT. And also it does not want to get DT information. Only board/machine name. So existing applications stop working after migration to DT. And because legacy board boot code (without DT) is going to be removed for ARM in near future this will permanently break existing applications.

We're already breaking the userspace API through moving to DT, because all the device names in /sys/devices are changing. Userspace is going to have to cope with change as we move towards DT. This is just another aspect of moving towards DT, and one which userspace is going to have to deal with.

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 <[email protected]>

---
 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

arch/arm/kernel/setup.c +1 -1

index 8a16ee5..fbc7b4f 100644
@@ -875,10 +875,13 @@ void __init setup_arch(char **cmdline_p)
 
        setup_processor();
        mdesc = setup_machine_fdt(__atags_pointer);
-       if (!mdesc)
+       if (mdesc)

On Wed, Jun 18, 2014 at 11:54 AM, Pali Roh?r

Quoted 31 lines

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 <[email protected]>

---

 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/arm/kernel/setup.c index 8a16ee5..fbc7b4f 100644

arch/arm/kernel/setup.c +2 -1

@@ -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;

arch/arm/kernel/setup.c

Quoted 7 lines

arch/arm/kernel/setup.c +2 -1

-       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.

arch/arm/kernel/setup.c +1 -1

index 50e198c..1479250 100644
@@ -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

So without this patch DT migration for Nokia N900 cannot be done without breaking userspace - which is not acceptable...

Also I still did not know why DT kernel does not report Revision number which is passed by bootloader via atags. Any idea?

Quoted 4 lines · Pali Roh?r

So without this patch DT migration for Nokia N900 cannot be done without breaking userspace - which is not acceptable...

Quoted 3 lines

Also I still did not know why DT kernel does not report Revision number which is passed by bootloader via atags. Any idea?

It was a concious decision by the DT people to exclude most of the ATAGs which provide that information. Only memory information and command line ATAGs are merged into DT. Every other tag is ignored (as there's no place for it in DT.)

So how to read Revision and Serial number in DT based kernel?

Quoted 2 lines · Pali Roh?r

So how to read Revision and Serial number in DT based kernel?

$ cat /sys/bus/soc/devices/soc0/soc_id
i.MX6SX

$ cat /sys/bus/soc/devices/soc0/revision
1.0

$ cat /sys/bus/soc/devices/soc0/machine
Freescale i.MX6 SoloX SDB Board

Quoted 1 line · On Wednesday 18 June 2014 21:07:35 Rob Herring wrote:

On Wed, Jun 18, 2014 at 11:54 AM, Pali Roh?r

Quoted 73 lines · <[email protected]> wrote:

Quoted 31 lines

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 <[email protected]>

---

 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/arm/kernel/setup.c index 8a16ee5..fbc7b4f 100644

arch/arm/kernel/setup.c +2 -1

@@ -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;

arch/arm/kernel/setup.c

Quoted 7 lines

arch/arm/kernel/setup.c +2 -1

-       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.

arch/arm/kernel/setup.c +1 -1

index 50e198c..1479250 100644
@@ -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;

Quoted 3 lines

[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

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.

So without this patch DT migration for Nokia N900 cannot be done without breaking userspace - which is not acceptable...

Also I still did not know why DT kernel does not report Revision number which is passed by bootloader via atags. Any idea?

Quoted 2 lines · Pali Roh?r

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.

From what I've seen, almost everyone else sets this to zero in their

Quoted 4 lines · Rob Herring

Quoted 2 lines · Pali Roh?r

Also I still did not know why DT kernel does not report Revision number which is passed by bootloader via atags. Any idea?

Quoted 4 lines

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.

Except... that's not what it is. What that revision field was originally invented for was the Netwinder to indicate the _platform_ revision.

From what I've seen, almost everyone else sets this to zero in their boot loaders - it is /very/ rarely used. However, I think OMAP (ab)uses it by putting the SoC revision into it at kernel boot time. That's not what it is supposed to be used for.

Others have already solved the problem of exporting SoC specific information, such as SoC name, SoC revision, etc, if only people would use it - drivers/base/soc.c. This gives machine, family, soc_id and SoC revision information in a standard place - it /might/ have been a good idea if the creation of that also contained documentation for what was expected in each of the fields, rather than leaving it open...

Quoted 5 lines · Russell King - ARM Linux

Quoted 4 lines · Rob Herring

Quoted 2 lines · Pali Roh?r

Also I still did not know why DT kernel does not report Revision number which is passed by bootloader via atags. Any idea?

Quoted 7 lines

Quoted 4 lines

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.

Except... that's not what it is. What that revision field was originally invented for was the Netwinder to indicate the _platform_ revision.

Okay. DT describes the platform, so having a top-level revision in the DT could be similar, but...

Quoted 1 line

Quoted 4 lines

From what I've seen, almost everyone else sets this to zero in their boot loaders - it is /very/ rarely used. However, I think OMAP (ab)uses it by putting the SoC revision into it at kernel boot time. That's not what it is supposed to be used for.

it could suffer the same abuse as the ATAG.

The problem with soc-device is it is optional and at the whim of the platform to add. Adding it also causes the the platform devices to change paths because people make the soc device the bus parent.

Sysfs
paths to devices are not considered part of the ABI, but still this is a silly reason to change the path. If we want soc-device to be used, then it should always exist and have a default version.

On Wed, Jun 18, 2014 at 2:22 PM, Pali Roh?r

Quoted 85 lines · On Wednesday 18 June 2014 21:07:35 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 11:54 AM, Pali Roh?r

Quoted 81 lines · <[email protected]> wrote:

Quoted 32 lines

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 <[email protected]>

---

 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/arm/kernel/setup.c index 8a16ee5..fbc7b4f 100644

arch/arm/kernel/setup.c +2 -1

@@ -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;

arch/arm/kernel/setup.c

Quoted 7 lines

arch/arm/kernel/setup.c +2 -1

-       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.

(quoted diff) +0 -0

b/arch/arm/kernel/setup.c index 50e198c..1479250 100644

arch/arm/kernel/setup.c +0 -0

@@ -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;

arch/arm/kernel/setup.c +1 -1

-       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;

Quoted 8 lines

Quoted 2 lines

[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.

Quoted 30 lines

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

Quoted 8 lines

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.

Quoted 3 lines

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.

Quoted 3 lines

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

Quoted 1 line · On Wednesday 18 June 2014 22:46:19 Rob Herring wrote:

On Wed, Jun 18, 2014 at 2:22 PM, Pali Roh?r

Quoted 9 lines · <[email protected]> wrote:

Quoted 9 lines · On Wednesday 18 June 2014 21:07:35 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 11:54 AM, Pali Roh?r

Quoted 5 lines · <[email protected]> wrote:

Quoted 5 lines

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.

Quoted 15 lines

Quoted 4 lines

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.

Quoted 4 lines

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 ..

(quoted diff) +0 -2

-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

On Mon, Nov 24, 2014 at 4:19 PM, Pali Roh?r

Quoted 12 lines · On Wednesday 18 June 2014 22:46:19 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 2:22 PM, Pali Roh?r

Quoted 8 lines · <[email protected]> wrote:

Quoted 8 lines · On Wednesday 18 June 2014 21:07:35 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 11:54 AM, Pali Roh?r

Quoted 4 lines · <[email protected]> wrote:

Quoted 4 lines

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.

[...]

Quoted 23 lines

Quoted 17 lines

Quoted 5 lines

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.

Quoted 3 lines

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:

Quoted 12 lines

$ 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

Wrong place. If you put /serial-number in your DT, then it will be in /proc/device-tree/serial-number.

If you want to wire that up to /proc/cpuinfo, I would be fine with that.

Rob

Quoted 1 line · On Thursday 04 December 2014 01:33:08 Rob Herring wrote:

On Mon, Nov 24, 2014 at 4:19 PM, Pali Roh?r

Quoted 57 lines · <[email protected]> wrote:

Quoted 12 lines · On Wednesday 18 June 2014 22:46:19 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 2:22 PM, Pali Roh?r

Quoted 8 lines · <[email protected]> wrote:

Quoted 8 lines · On Wednesday 18 June 2014 21:07:35 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 11:54 AM, Pali Roh?r

Quoted 4 lines · <[email protected]> wrote:

Quoted 4 lines

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.

[...]

Quoted 23 lines

Quoted 17 lines

Quoted 5 lines

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.

Quoted 3 lines

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:

Quoted 12 lines

$ 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

Wrong place. If you put /serial-number in your DT, then it will be in /proc/device-tree/serial-number.

If you want to wire that up to /proc/cpuinfo, I would be fine with that.

Quoted 1 line

Rob

I cannot hardcode it into DT, because it is set by bootloader. And bootloader tell it to kernel via ATAGs structure. Bootloader is closed and proprietary and cannot be replaced (there is no documentation for hw init).

On Wed, Dec 3, 2014 at 6:48 PM, Pali Roh?r

Quoted 65 lines · On Thursday 04 December 2014 01:33:08 Rob Herring wrote:

Quoted 1 line

On Mon, Nov 24, 2014 at 4:19 PM, Pali Roh?r

Quoted 61 lines · <[email protected]> wrote:

Quoted 12 lines · On Wednesday 18 June 2014 22:46:19 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 2:22 PM, Pali Roh?r

Quoted 8 lines · <[email protected]> wrote:

Quoted 8 lines · On Wednesday 18 June 2014 21:07:35 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 11:54 AM, Pali Roh?r

Quoted 4 lines · <[email protected]> wrote:

Quoted 4 lines

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.

[...]

Quoted 34 lines

Quoted 17 lines

Quoted 5 lines

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.

Quoted 4 lines

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

Quoted 5 lines

         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

Wrong place. If you put /serial-number in your DT, then it will be in /proc/device-tree/serial-number.

If you want to wire that up to /proc/cpuinfo, I would be fine with that.

Quoted 6 lines

Quoted 1 line

Rob

I cannot hardcode it into DT, because it is set by bootloader. And bootloader tell it to kernel via ATAGs structure. Bootloader is closed and proprietary and cannot be replaced (there is no documentation for hw init).

Either the bootloader can put the serial number into the DTB instead of ATAGs or the kernel decompressor can copy it from ATAGs to DTB like is done already for other ATAGs. I presume you cannot update the bootloader and will need the latter.

Rob

Quoted 1 line · On Thursday 04 December 2014 17:49:40 Rob Herring wrote:

On Wed, Dec 3, 2014 at 6:48 PM, Pali Roh?r

Quoted 77 lines · <[email protected]> wrote:

Quoted 65 lines · On Thursday 04 December 2014 01:33:08 Rob Herring wrote:

Quoted 1 line

On Mon, Nov 24, 2014 at 4:19 PM, Pali Roh?r

Quoted 61 lines · <[email protected]> wrote:

Quoted 12 lines · On Wednesday 18 June 2014 22:46:19 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 2:22 PM, Pali Roh?r

Quoted 8 lines · <[email protected]> wrote:

Quoted 8 lines · On Wednesday 18 June 2014 21:07:35 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 11:54 AM, Pali Roh?r

Quoted 4 lines · <[email protected]> wrote:

Quoted 4 lines

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.

[...]

Quoted 34 lines

Quoted 17 lines

Quoted 5 lines

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.

Quoted 4 lines

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

Quoted 5 lines

         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

Wrong place. If you put /serial-number in your DT, then it will be in /proc/device-tree/serial-number.

If you want to wire that up to /proc/cpuinfo, I would be fine with that.

Quoted 6 lines

Quoted 1 line

Rob

I cannot hardcode it into DT, because it is set by bootloader. And bootloader tell it to kernel via ATAGs structure. Bootloader is closed and proprietary and cannot be replaced (there is no documentation for hw init).

Either the bootloader can put the serial number into the DTB instead of ATAGs or the kernel decompressor can copy it from ATAGs to DTB like is done already for other ATAGs. I presume you cannot update the bootloader and will need the latter.

Quoted 1 line

Rob

Rob, thanks for info! Btw, theoretically I can update bootloader if someone write it. So in practise I need to use second option.

Can you tell me which CONFIG_ needs to be enabled for kernel decompressor (so it copy some ATAGs) and where to start looking at code (for adding new code for copying other ATAGs)?

Ok, here is patch which set Revision field (global variable system_rev) in /proc/cpuinfo from DT revision property:

arch/arm/kernel/devtree.c +10 -2

index 11c54de..9946c1b 100644
@@ -19,6 +19,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/smp.h>
+#include <linux/libfdt_env.h>
 
 #include <asm/cputype.h>
 #include <asm/setup.h>

@@ -26,6 +27,7 @@
 #include <asm/smp_plat.h>
 #include <asm/mach/arch.h>
 #include <asm/mach-types.h>
+#include <asm/system_info.h>
 
 
 #ifdef CONFIG_SMP

@@ -204,6 +206,8 @@ static const void * __init arch_get_next_mach(const char *const **match)
 const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
 {
        const struct machine_desc *mdesc, *mdesc_best = NULL;
+       unsigned long dt_root;
+       const u32 *prop;
 
 #ifdef CONFIG_ARCH_MULTIPLATFORM
        DT_MACHINE_START(GENERIC_DT, "Generic DT based system")

@@ -215,17 +219,16 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
        if (!dt_phys || !early_init_dt_verify(phys_to_virt(dt_phys)))
                return NULL;
 
+       dt_root = of_get_flat_dt_root();
        mdesc = of_flat_dt_match_machine(mdesc_best, arch_get_next_mach);
 
        if (!mdesc) {
                const char *prop;
                int size;
-               unsigned long dt_root;
 
                early_print("\nError: unrecognized/unsupported "
                            "device tree compatible list:\n[ ");
 
-               dt_root = of_get_flat_dt_root();
                prop = of_get_flat_dt_prop(dt_root, "compatible", &size);
                while (size > 0) {
                        early_print("'%s' ", prop);

@@ -246,5 +249,10 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
        /* Change machine number to match the mdesc we're using */
        __machine_arch_type = mdesc->nr;
 
+       /* Set system revision from DT */
+       prop = of_get_flat_dt_prop(dt_root, "revision", NULL);
+       if (prop)
+               system_rev = fdt32_to_cpu(*prop);
+
        return mdesc;
 }

And here is patch which convert ATAG revision into DT revision property and append it into DT in decompress code:

arch/arm/boot/compressed/atags_to_fdt.c +2 -0

index 9448aa0..e7e1cc9 100644
@@ -171,6 +171,8 @@ int atags_to_fdt(void *atag_list, void *fdt, int total_space)
                                        cpu_to_fdt32(atag->u.mem.size);
                        }
 
+               } else if (atag->hdr.tag == ATAG_REVISION) {
+                       setprop_cell(fdt, "/", "revision", atag->u.revision.rev);
                } else if (atag->hdr.tag == ATAG_INITRD2) {
                        uint32_t initrd_start, initrd_size;
                        initrd_start = atag->u.initrd.start;

I tested it on DT booted Nokia N900 and it is working, in /proc/cpuinfo is revision from ATAG.

I do not know which DT property to use for storing HW Revision, so if "/revision" is not correct one, let me know what to use instead. Also you can add my Signed-off-by for both patches.

On Mon, Jan 26, 2015 at 1:09 PM, Pali Roh?r

Quoted 119 lines

Ok, here is patch which set Revision field (global variable system_rev) in /proc/cpuinfo from DT revision property:

(quoted diff) +0 -0

b/arch/arm/kernel/devtree.c index 11c54de..9946c1b 100644

arch/arm/kernel/devtree.c +1 -0

@@ -19,6 +19,7 @@
 
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/smp.h>
 
+#include <linux/libfdt_env.h>
 

#include <asm/cputype.h>
#include <asm/setup.h>

arch/arm/kernel/devtree.c +1 -0

@@ -26,6 +27,7 @@
 
 #include <asm/smp_plat.h>
 #include <asm/mach/arch.h>
 #include <asm/mach-types.h>
 
+#include <asm/system_info.h>
 

#ifdef CONFIG_SMP

arch/arm/kernel/devtree.c +5 -2

@@ -204,6 +206,8 @@ static const void * __init
 arch_get_next_mach(const char *const **match)
 
 const struct machine_desc * __init
 setup_machine_fdt(unsigned int dt_phys) {
 
        const struct machine_desc *mdesc, *mdesc_best =
        NULL;
 
+       unsigned long dt_root;
+       const u32 *prop;
 
 #ifdef CONFIG_ARCH_MULTIPLATFORM
 
        DT_MACHINE_START(GENERIC_DT, "Generic DT based
        system")

@@ -215,17 +219,16 @@ const struct machine_desc * __init
 setup_machine_fdt(unsigned int dt_phys)
 
        if (!dt_phys ||
        !early_init_dt_verify(phys_to_virt(dt_phys)))
 
                return NULL;
 
+       dt_root = of_get_flat_dt_root();
 
        mdesc = of_flat_dt_match_machine(mdesc_best,
        arch_get_next_mach);
 
        if (!mdesc) {
 
                const char *prop;
                int size;
 
-               unsigned long dt_root;
 
                early_print("\nError:
                unrecognized/unsupported "
 
                            "device tree compatible list:\n[
                            ");
 
-               dt_root = of_get_flat_dt_root();
 
                prop = of_get_flat_dt_prop(dt_root,
                "compatible", &size); while (size > 0) {
 
                        early_print("'%s' ", prop);

@@ -246,5 +249,10 @@ const struct machine_desc * __init
 setup_machine_fdt(unsigned int dt_phys)
 
        /* Change machine number to match the mdesc we're
        using */ __machine_arch_type = mdesc->nr;
 
+       /* Set system revision from DT */
+       prop = of_get_flat_dt_prop(dt_root, "revision",

NULL); + if (prop)

arch/arm/kernel/devtree.c +2 -0

+               system_rev = fdt32_to_cpu(*prop);
+
 
        return mdesc;
 
 }

And here is patch which convert ATAG revision into DT revision property and append it into DT in decompress code:

(quoted diff) +0 -0

b/arch/arm/boot/compressed/atags_to_fdt.c index
9448aa0..e7e1cc9 100644

arch/arm/boot/compressed/atags_to_fdt.c +2 -0

@@ -171,6 +171,8 @@ int atags_to_fdt(void *atag_list, void
 *fdt, int total_space)
 
                                        cpu_to_fdt32(atag->u
                                        .mem.size);
 
                        }
 
+               } else if (atag->hdr.tag == ATAG_REVISION) {
+                       setprop_cell(fdt, "/", "revision",

atag->u.revision.rev);

                } else if (atag->hdr.tag == ATAG_INITRD2) {
                
                        uint32_t initrd_start, initrd_size;
                        initrd_start = atag->u.initrd.start;

Quoted 7 lines

I tested it on DT booted Nokia N900 and it is working, in /proc/cpuinfo is revision from ATAG.

I do not know which DT property to use for storing HW Revision, so if "/revision" is not correct one, let me know what to use instead. Also you can add my Signed-off-by for both patches.

It is the correct property, but /revision in DT is a string.

You need to add your own sign-off.

Rob

Quoted 1 line · On Monday 26 January 2015 21:22:27 Rob Herring wrote:

On Mon, Jan 26, 2015 at 1:09 PM, Pali Roh?r

Quoted 2 lines · <[email protected]> wrote:

Quoted 2 lines

Ok, here is patch which set Revision field (global variable system_rev) in /proc/cpuinfo from DT revision property:

Quoted 5 lines

Quoted 4 lines

I do not know which DT property to use for storing HW Revision, so if "/revision" is not correct one, let me know what to use instead. Also you can add my Signed-off-by for both patches.

Quoted 5 lines

It is the correct property, but /revision in DT is a string.

You need to add your own sign-off.

Rob

Any documentation which says that /revision is string?

If it is really string, how to store ATAG_REVISON (number) to string? dec or hex?

lOn Fri, Jan 30, 2015 at 8:14 AM, Pali Roh?r

Quoted 8 lines · On Monday 26 January 2015 21:22:27 Rob Herring wrote:

Quoted 1 line

On Mon, Jan 26, 2015 at 1:09 PM, Pali Roh?r

Quoted 4 lines · <[email protected]> wrote:

Quoted 4 lines

Ok, here is patch which set Revision field (global variable

system_rev) in /proc/cpuinfo from DT revision property:

[...]

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

I do not know which DT property to use for storing HW Revision, so if "/revision" is not correct one, let me know what to use instead. Also you can add my Signed-off-by for both patches.

Quoted 8 lines

Quoted 6 lines

It is the correct property, but /revision in DT is a string.

You need to add your own sign-off.

Rob

Any documentation which says that /revision is string?

Sorry, I was confusing this with serial-number, so I guess a number is fine here.

Quoted 2 lines

If it is really string, how to store ATAG_REVISON (number) to string? dec or hex?

However /proc/cpuinfo displays it would be fine if you stay with a string.

Rob

This patch adds support for DT "/revision" and convert ATAG_REVISION to DT.

Pali Roh?r (2):
arm: devtree: Set system_rev from DT revision
arm: boot: convert ATAG_REVISION to DT revision field

Quoted 5 lines · Pali Roh?r

This patch adds support for DT "/revision" and convert ATAG_REVISION to DT.

Pali Roh?r (2):
arm: devtree: Set system_rev from DT revision
arm: boot: convert ATAG_REVISION to DT revision field

Acked-by Pavel Machek <[email protected]>

Quoted 7 lines · Pavel Machek

Quoted 5 lines · Pali Roh?r

This patch adds support for DT "/revision" and convert ATAG_REVISION to DT.

Pali Roh?r (2):
arm: devtree: Set system_rev from DT revision
arm: boot: convert ATAG_REVISION to DT revision field

Quoted 1 line

Acked-by Pavel Machek <[email protected]>

Are these queued somewhere now? Sounds like this is the last pending issue for n900 to use legacy user space with current mainline kernels, so I'd like to get these in so we can get closer to making omap3 boot in device tree only mode.

In any case, I'm currently not accepting /any/ patches into my tree at present; I'm chasing a horrid instability on one of my test platforms which is making it impossible to tell whether any particular change or changes in my tree is responsible or not for it. It doesn't seem to be a hardware failure (if it was, I'd simply take the board out of the nightly test system.)

It's quite literally taking hours to figure out what's going on - I've been on this for about 12 hours now and still not much closer to knowing what's causing it (other than I know that -rc1 plus my queue seems to be fine, -rc3 plus my queue is definitely broken, -rc3 alone is fine. So something I'm already carrying seems to be responsible, but each time I identify a particular patch and drop _just_ that change, I find that the problem is back when I try my queue minus the bad changes. With a test cycle time of 20+ minutes (due to the number of boots required to make certain of a dependable result), this is /really/ slow progress.

Right now, I can't be positively sure that /anything/ I have already merged isn't a factor in causing this problem, so I don't want to augment my tree with any additional patches which would upset my ability to move about in the tree, and get reproducable results from repeated testing. To merge something else will probably mean I'll have to start again from the beginning and the last 12 hours spent testing will have been wasted.

Your publicly visible tree contains only a few mundane patches. Is that the tree you're testing with? If no then could you publish it for others to have a look and test?

Quoted 8 lines · Tony Lindgren

Quoted 7 lines · Pavel Machek

Quoted 5 lines · Pali Roh?r

This patch adds support for DT "/revision" and convert ATAG_REVISION to DT.

Pali Roh?r (2):
arm: devtree: Set system_rev from DT revision
arm: boot: convert ATAG_REVISION to DT revision field

Quoted 6 lines

Quoted 1 line

Acked-by Pavel Machek <[email protected]>

Are these queued somewhere now? Sounds like this is the last pending issue for n900 to use legacy user space with current mainline kernels, so I'd like to get these in so we can get closer to making omap3 boot in device tree only mode.

Not that I know of. As everyone is aware, patches need to be in my patch system if they want me to apply them - which would've been especially important as I was away from kernel stuff for a week at the start of March (for medical reasons) and I can't be expected to track the status of stuff which is buried behind 1000+ extra mails.

In any case, I'm currently not accepting /any/ patches into my tree at present; I'm chasing a horrid instability on one of my test platforms which is making it impossible to tell whether any particular change or changes in my tree is responsible or not for it. It doesn't seem to be a hardware failure (if it was, I'd simply take the board out of the nightly test system.)

It's quite literally taking hours to figure out what's going on - I've been on this for about 12 hours now and still not much closer to knowing what's causing it (other than I know that -rc1 plus my queue seems to be fine, -rc3 plus my queue is definitely broken, -rc3 alone is fine. So something I'm already carrying seems to be responsible, but each time I identify a particular patch and drop _just_ that change, I find that the problem is back when I try my queue minus the bad changes. With a test cycle time of 20+ minutes (due to the number of boots required to make certain of a dependable result), this is /really/ slow progress.

Right now, I can't be positively sure that /anything/ I have already merged isn't a factor in causing this problem, so I don't want to augment my tree with any additional patches which would upset my ability to move about in the tree, and get reproducable results from repeated testing. To merge something else will probably mean I'll have to start again from the beginning and the last 12 hours spent testing will have been wasted.

Quoted 16 lines · Russell King - ARM Linux

Quoted 14 lines · Tony Lindgren

Quoted 8 lines · Pavel Machek

Quoted 5 lines · Pali Roh?r

This patch adds support for DT "/revision" and convert ATAG_REVISION to DT.

Pali Roh?r (2):
arm: devtree: Set system_rev from DT revision
arm: boot: convert ATAG_REVISION to DT revision field

Acked-by Pavel Machek <[email protected]>

Are these queued somewhere now? Sounds like this is the last pending issue for n900 to use legacy user space with current mainline kernels, so I'd like to get these in so we can get closer to making omap3 boot in device tree only mode.

Quoted 5 lines

Not that I know of. As everyone is aware, patches need to be in my patch system if they want me to apply them - which would've been especially important as I was away from kernel stuff for a week at the start of March (for medical reasons) and I can't be expected to track the status of stuff which is buried behind 1000+ extra mails.

Pali, care to upload these two patches to Russell's patch tracking system if no other comments?

Quoted 29 lines · Tony Lindgren

* Russell King - ARM Linux <[email protected]> [150316 09:15]:

Quoted 26 lines

Quoted 10 lines · Tony Lindgren

Quoted 9 lines · Pavel Machek

Quoted 7 lines · Pali Roh?r

This patch adds support for DT "/revision" and
convert ATAG_REVISION to DT.

Pali Roh?r (2):
arm: devtree: Set system_rev from DT revision
arm: boot: convert ATAG_REVISION to DT revision
field

Quoted 7 lines

Quoted 1 line

Acked-by Pavel Machek <[email protected]>

Are these queued somewhere now? Sounds like this is the last pending issue for n900 to use legacy user space with current mainline kernels, so I'd like to get these in so we can get closer to making omap3 boot in device tree only mode.

Not that I know of. As everyone is aware, patches need to be in my patch system if they want me to apply them - which would've been especially important as I was away from kernel stuff for a week at the start of March (for medical reasons) and I can't be expected to track the status of stuff which is buried behind 1000+ extra mails.

Quoted 2 lines

Pali, care to upload these two patches to Russell's patch tracking system if no other comments?

If it's done soon, I should be able to send them to Linus fairly quickly, since this problem I'm grappling with is being seen by others who have better tools to be able to investigate the problem (and hence they're in a better position to investigate what's going on.)

Quoted 33 lines · On Monday 16 March 2015 20:59:04 Russell King - ARM Linux wrote:

Quoted 31 lines · Tony Lindgren

* Russell King - ARM Linux <[email protected]> [150316 09:15]:

Quoted 20 lines

Quoted 10 lines · Tony Lindgren

Quoted 9 lines · Pavel Machek

Quoted 7 lines · Pali Roh?r

This patch adds support for DT "/revision" and
convert ATAG_REVISION to DT.

Pali Roh?r (2):
arm: devtree: Set system_rev from DT revision
arm: boot: convert ATAG_REVISION to DT revision
field

Quoted 7 lines

Quoted 1 line

Acked-by Pavel Machek <[email protected]>

Are these queued somewhere now? Sounds like this is the last pending issue for n900 to use legacy user space with current mainline kernels, so I'd like to get these in so we can get closer to making omap3 boot in device tree only mode.

Quoted 6 lines

Not that I know of. As everyone is aware, patches need to be in my patch system if they want me to apply them - which would've been especially important as I was away from kernel stuff for a week at the start of March (for medical reasons) and I can't be expected to track the status of stuff which is buried behind 1000+ extra mails.

Pali, care to upload these two patches to Russell's patch tracking system if no other comments?

Quoted 5 lines

If it's done soon, I should be able to send them to Linus fairly quickly, since this problem I'm grappling with is being seen by others who have better tools to be able to investigate the problem (and hence they're in a better position to investigate what's going on.)

I have no idea how to upload those patches into tracking system. Right now I do not have enough time for testing any patches...

Tony, or somebody else, if you have free time, can you upload (my) patches to that needed tracking system?

arch/arm/kernel/devtree.c +0 -0

index 11c54de..9946c1b 100644

arch/arm/kernel/devtree.c +2 -0

@@ -204,6 +206,8 @@ static const void * __init arch_get_next_mach(const char *const **match)
 const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
 {
 	const struct machine_desc *mdesc, *mdesc_best = NULL;
+	unsigned long dt_root;
+	const u32 *prop;
 
 #ifdef CONFIG_ARCH_MULTIPLATFORM
 	DT_MACHINE_START(GENERIC_DT, "Generic DT based system")

arch/arm/kernel/devtree.c +5 -2

 		int size;
-		unsigned long dt_root;
 
 		early_print("\nError: unrecognized/unsupported "
 			    "device tree compatible list:\n[ ");
 
-		dt_root = of_get_flat_dt_root();
 		prop = of_get_flat_dt_prop(dt_root, "compatible", &size);
 		while (size > 0) {
 			early_print("'%s' ", prop);

@@ -246,5 +249,10 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
 	/* Change machine number to match the mdesc we're using */
 	__machine_arch_type = mdesc->nr;
 
+	/* Set system revision from DT */
+	prop = of_get_flat_dt_prop(dt_root, "revision", NULL);
+	if (prop)
+		system_rev = fdt32_to_cpu(*prop);
+
 	return mdesc;
 }

Am 26.01.2015 um 20:09 schrieb Pali Roh?r:

Quoted 4 lines

(quoted diff) +0 -0

b/arch/arm/kernel/devtree.c index 11c54de..9946c1b 100644

arch/arm/kernel/devtree.c +0 -0

[...]

Quoted 28 lines

arch/arm/kernel/devtree.c +3 -0

@@ -204,6 +206,8 @@ static const void * __init
 arch_get_next_mach(const char *const **match)
 
 const struct machine_desc * __init
 setup_machine_fdt(unsigned int dt_phys) {
 
 	const struct machine_desc *mdesc, *mdesc_best = NULL;
 
+	unsigned long dt_root;
+	const u32 *prop;
 
 #ifdef CONFIG_ARCH_MULTIPLATFORM
 
 	DT_MACHINE_START(GENERIC_DT, "Generic DT based system")

@@ -215,17 +219,16 @@ const struct machine_desc * __init
 setup_machine_fdt(unsigned int dt_phys)
 
 	if (!dt_phys ||
 	!early_init_dt_verify(phys_to_virt(dt_phys)))
 
 		return NULL;
 
+	dt_root = of_get_flat_dt_root();
 
 	mdesc = of_flat_dt_match_machine(mdesc_best,
 	arch_get_next_mach);
 

arch/arm/kernel/devtree.c const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)

Quoted 3 lines

arch/arm/kernel/devtree.c +0 -0

 	if (!mdesc) {
 
 		const char *prop;

Probably the use of two differently typed variables of name "prop" in this function is not intentional?

Regards,
Andreas

Quoted 4 lines

arch/arm/kernel/devtree.c +0 -1

 		int size;
 
-		unsigned long dt_root;

arch/arm/kernel/devtree.c const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)

Quoted 7 lines

arch/arm/kernel/devtree.c +0 -1

 		early_print("\nError: unrecognized/unsupported "
 
 			    "device tree compatible list:\n[ ");
 
-		dt_root = of_get_flat_dt_root();
 
 		prop = of_get_flat_dt_prop(dt_root, "compatible", 

Quoted 19 lines

arch/arm/kernel/devtree.c +5 -0

 		while (size > 0) {
 
 			early_print("'%s' ", prop);

@@ -246,5 +249,10 @@ const struct machine_desc * __init
 setup_machine_fdt(unsigned int dt_phys)
 
 	/* Change machine number to match the mdesc we're using */
 	__machine_arch_type = mdesc->nr;
 
+	/* Set system revision from DT */
+	prop = of_get_flat_dt_prop(dt_root, "revision", NULL);
+	if (prop)
+		system_rev = fdt32_to_cpu(*prop);
+

return mdesc;

}

[snip]

This patch will cause that decompressor store full ATAG structure into DT tree ("/atags"):

arch/arm/boot/compressed/atags_to_fdt.c +5 -1

index e7e1cc9..1975d7c 100644
@@ -112,7 +112,7 @@ int atags_to_fdt(void *atag_list, void *fdt, int total_space)
         * address and size for each bank */
        uint32_t mem_reg_property[2 * 2 * NR_BANKS];
        int memcount = 0;
-       int ret, memsize;
+       int ret, memsize, atag_size;
 
        /* make sure we've got an aligned pointer */
        if ((u32)atag_list & 0x3)

@@ -184,6 +184,10 @@ int atags_to_fdt(void *atag_list, void *fdt, int total_space)
                }
        }
 
+       /* include the terminating ATAG_NONE */
+       atag_size = (char *)atag - (char *)atag_list + sizeof(struct tag_header);
+       setprop(fdt, "/", "atags", atag_list, atag_size);
+
        if (memcount) {
                setprop(fdt, "/memory", "reg", mem_reg_property,
                        4 * memcount * memsize);

And this patch will export ATAG structure from DT tree ("/atags") into /proc/atags file:

arch/arm/kernel/devtree.c +6 -0

index 9946c1b..0f249a3 100644
@@ -29,6 +29,7 @@
 #include <asm/mach-types.h>
 #include <asm/system_info.h>
 
+#include "atags.h"
 
 #ifdef CONFIG_SMP
 extern struct of_cpu_method __cpu_method_of_table[];

@@ -254,5 +255,10 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
        if (prop)
                system_rev = fdt32_to_cpu(*prop);
 
+       /* Save atags */
+       prop = of_get_flat_dt_prop(dt_root, "atags", NULL);
+       if (prop)
+               save_atags((void *)prop);
+
        return mdesc;
 }

Some userspace applications needs access to ATAG structure where can be stored some information passed from bootloader to kernel. Example is Nokia N900 device and NOLO bootloader which provides information about bootreason (device was started by power button or by alarm or restarted...) and bootmode (normal mode or device update mode).

Quoted 13 lines · Pali Roh?r

This patch will cause that decompressor store full ATAG structure into DT tree ("/atags"):

arch/arm/boot/compressed/atags_to_fdt.c +4 -0

+       /* include the terminating ATAG_NONE */
+       atag_size = (char *)atag - (char *)atag_list + sizeof(struct tag_header);
+       setprop(fdt, "/", "atags", atag_list, atag_size);
+
        if (memcount) {
                setprop(fdt, "/memory", "reg", mem_reg_property,
                        4 * memcount * memsize);

Quoted 1 line

And this patch will export ATAG structure from DT tree ("/atags") into /proc/atags file:

Please properly send your patches.

Quoted 4 lines

Some userspace applications needs access to ATAG structure where can be stored some information passed from bootloader to kernel. Example is Nokia N900 device and NOLO bootloader which provides information about bootreason (device was started by power button or by alarm or restarted...) and bootmode (normal mode or device update mode).

This goes in the commit message.

These would be non-standard fields which are not upstream. I don't know that we care in that case...

Quoted 14 lines · Rob Herring

Quoted 13 lines · Pali Roh?r

This patch will cause that decompressor store full ATAG structure into DT tree ("/atags"):

arch/arm/boot/compressed/atags_to_fdt.c +4 -0

+       /* include the terminating ATAG_NONE */
+       atag_size = (char *)atag - (char *)atag_list + sizeof(struct tag_header);
+       setprop(fdt, "/", "atags", atag_list, atag_size);
+
        if (memcount) {
                setprop(fdt, "/memory", "reg", mem_reg_property,
                        4 * memcount * memsize);

Quoted 3 lines

Quoted 1 line

And this patch will export ATAG structure from DT tree ("/atags") into /proc/atags file:

Please properly send your patches.

Actually, when sending patches for discussion, this is format easier to read.

Quoted 5 lines

Quoted 4 lines

Some userspace applications needs access to ATAG structure where can be stored some information passed from bootloader to kernel. Example is Nokia N900 device and NOLO bootloader which provides information about bootreason (device was started by power button or by alarm or restarted...) and bootmode (normal mode or device update mode).

Quoted 4 lines

This goes in the commit message.

These would be non-standard fields which are not upstream. I don't know that we care in that case...

Other devices are going to care about boot reason, too, and we might as well be compatible...

Quoted 10 lines · Pavel Machek

Quoted 6 lines · Rob Herring

On Mon, Jan 26, 2015 at 1:16 PM, Pali Roh?r <[email protected]> wrote:

This goes in the commit message.

These would be non-standard fields which are not upstream. I don't know that we care in that case...

Other devices are going to care about boot reason, too, and we might as well be compatible...

What other devices? Where is bootreason in the list of ATAGS:

#define ATAG_MEM        0x54410002
#define ATAG_VIDEOTEXT  0x54410003
#define ATAG_RAMDISK    0x54410004
#define ATAG_INITRD     0x54410005
#define ATAG_INITRD2    0x54420005
#define ATAG_SERIAL     0x54410006
#define ATAG_REVISION   0x54410007
#define ATAG_VIDEOLFB   0x54410008
#define ATAG_CMDLINE    0x54410009
#define ATAG_ACORN      0x41000101
#define ATAG_MEMCLK     0x41000402

On Tue, Jan 27, 2015 at 7:21 AM, Pavel Machek <[email protected]>

Quoted 2 lines · Rob Herring

Quoted 1 line

On Mon, Jan 26, 2015 at 1:16 PM, Pali Roh?r

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

This patch will cause that decompressor store full ATAG structure into DT tree ("/atags"):

(quoted diff) +2 -0

+       /* include the terminating ATAG_NONE */
+       atag_size = (char *)atag - (char *)atag_list +

sizeof(struct tag_header); + setprop(fdt, "/",
"atags", atag_list, atag_size); +

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

        if (memcount) {
        
                setprop(fdt, "/memory", "reg",
                mem_reg_property,
                
                        4 * memcount * memsize);

And this patch will export ATAG structure from DT tree

Quoted 4 lines

Quoted 1 line

Please properly send your patches.

Actually, when sending patches for discussion, this is format easier to read.

Some people might prefer them as attachments too...

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines

Some userspace applications needs access to ATAG structure where can be stored some information passed from bootloader to kernel. Example is Nokia N900 device and NOLO bootloader which provides information about bootreason (device was started by power button or by alarm or restarted...) and bootmode (normal mode or device update mode).

Quoted 7 lines

Quoted 4 lines

This goes in the commit message.

These would be non-standard fields which are not upstream. I don't know that we care in that case...

Other devices are going to care about boot reason, too, and we might as well be compatible...

What other devices? Where is bootreason in the list of ATAGS:

#define ATAG_MEM        0x54410002
#define ATAG_VIDEOTEXT  0x54410003
#define ATAG_RAMDISK    0x54410004
#define ATAG_INITRD     0x54410005
#define ATAG_INITRD2    0x54420005
#define ATAG_SERIAL     0x54410006
#define ATAG_REVISION   0x54410007
#define ATAG_VIDEOLFB   0x54410008
#define ATAG_CMDLINE    0x54410009
#define ATAG_ACORN      0x41000101
#define ATAG_MEMCLK     0x41000402

Rob

Quoted 14 lines · On Tuesday 27 January 2015 15:16:45 Rob Herring wrote:

What other devices? Where is bootreason in the list of ATAGS:

#define ATAG_MEM        0x54410002
#define ATAG_VIDEOTEXT  0x54410003
#define ATAG_RAMDISK    0x54410004
#define ATAG_INITRD     0x54410005
#define ATAG_INITRD2    0x54420005
#define ATAG_SERIAL     0x54410006
#define ATAG_REVISION   0x54410007
#define ATAG_VIDEOLFB   0x54410008
#define ATAG_CMDLINE    0x54410009
#define ATAG_ACORN      0x41000101
#define ATAG_MEMCLK     0x41000402

Quoted 1 line

Rob

Each device is using own proprietary atag (or other information) to pass bootreason from bootloader to kernel. No standard way :-(

I think this kind of information (how was board/computer started) can be useful also for other architectures. E.g. on laptop you would like to know if if was started by RTC, power button, WakeOnLan, another ACPI event, rebooted machine, watchdog, etc... And scripts can act depending on this event (when by RTC, you need to run some planned job, when by watchdog reset you should check what caused that reason...).

Quoted 15 lines · Pali Roh?r

Quoted 14 lines · On Tuesday 27 January 2015 15:16:45 Rob Herring wrote:

What other devices? Where is bootreason in the list of ATAGS:

#define ATAG_MEM        0x54410002
#define ATAG_VIDEOTEXT  0x54410003
#define ATAG_RAMDISK    0x54410004
#define ATAG_INITRD     0x54410005
#define ATAG_INITRD2    0x54420005
#define ATAG_SERIAL     0x54410006
#define ATAG_REVISION   0x54410007
#define ATAG_VIDEOLFB   0x54410008
#define ATAG_CMDLINE    0x54410009
#define ATAG_ACORN      0x41000101
#define ATAG_MEMCLK     0x41000402

Quoted 4 lines

Quoted 1 line

Rob

Each device is using own proprietary atag (or other information) to pass bootreason from bootloader to kernel. No standard way :-(

The reason that happens is because people refuse to discuss their requirements here - just like people refuse to report userspace API regressions to kernel people. This rather pisses me off, because it creates all sorts of horrid per-platform yuck.

We _could_ (and have in the past) turned round and refused to support these kinds of hacks - which IMHO is quite a reasonable stance to
take: the message we should be sending is "if you wish to design new methods without discussing it with us, we reserve the right not to support them in mainline kernels; please discuss with us your requirements."

Each time that we accept one of these hacks, we're sending a message that says "it's okay to work in this crappy way".

Yes, I realise that the N900 has little in the way of support, and we can't exert that kind of back pressure (since there's no one to direct that onto to effect any change) so I guess we just have to live with it.

Quoted 7 lines

I think this kind of information (how was board/computer started) can be useful also for other architectures. E.g. on laptop you would like to know if if was started by RTC, power button, WakeOnLan, another ACPI event, rebooted machine, watchdog, etc... And scripts can act depending on this event (when by RTC, you need to run some planned job, when by watchdog reset you should check what caused that reason...).

Quoted 16 lines · Russell King - ARM Linux

Quoted 15 lines · Pali Roh?r

Quoted 14 lines · On Tuesday 27 January 2015 15:16:45 Rob Herring wrote:

What other devices? Where is bootreason in the list of ATAGS:

#define ATAG_MEM        0x54410002
#define ATAG_VIDEOTEXT  0x54410003
#define ATAG_RAMDISK    0x54410004
#define ATAG_INITRD     0x54410005
#define ATAG_INITRD2    0x54420005
#define ATAG_SERIAL     0x54410006
#define ATAG_REVISION   0x54410007
#define ATAG_VIDEOLFB   0x54410008
#define ATAG_CMDLINE    0x54410009
#define ATAG_ACORN      0x41000101
#define ATAG_MEMCLK     0x41000402

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

Rob

Each device is using own proprietary atag (or other information) to pass bootreason from bootloader to kernel. No standard way :-(

So that's what Pavel was alluding to.

Quoted 12 lines

The reason that happens is because people refuse to discuss their requirements here - just like people refuse to report userspace API regressions to kernel people. This rather pisses me off, because it creates all sorts of horrid per-platform yuck.

We _could_ (and have in the past) turned round and refused to support these kinds of hacks - which IMHO is quite a reasonable stance to
take: the message we should be sending is "if you wish to design new methods without discussing it with us, we reserve the right not to support them in mainline kernels; please discuss with us your requirements."

Quoted 6 lines

Each time that we accept one of these hacks, we're sending a message that says "it's okay to work in this crappy way".

Yes, I realise that the N900 has little in the way of support, and we can't exert that kind of back pressure (since there's no one to direct that onto to effect any change) so I guess we just have to live with it.

If the method is: "let's pass non-standard ATAGs around and have ad-hoc user space code interpret it in some arbitrary way" then it's a complete abomination.

Quoted 7 lines

Quoted 7 lines

I think this kind of information (how was board/computer started) can be useful also for other architectures. E.g. on laptop you would like to know if if was started by RTC, power button, WakeOnLan, another ACPI event, rebooted machine, watchdog, etc... And scripts can act depending on this event (when by RTC, you need to run some planned job, when by watchdog reset you should check what caused that reason...).

Useful when properly designed and generic enough to be shared.

I'd suggest a DT property be proposed for that purpose if it doesn't already exist. That at least has a chance to be generically useful.

Quoted 58 lines · Nicolas Pitre

Quoted 16 lines · Russell King - ARM Linux

Quoted 15 lines · Pali Roh?r

Quoted 14 lines · On Tuesday 27 January 2015 15:16:45 Rob Herring wrote:

What other devices? Where is bootreason in the list of ATAGS:

#define ATAG_MEM        0x54410002
#define ATAG_VIDEOTEXT  0x54410003
#define ATAG_RAMDISK    0x54410004
#define ATAG_INITRD     0x54410005
#define ATAG_INITRD2    0x54420005
#define ATAG_SERIAL     0x54410006
#define ATAG_REVISION   0x54410007
#define ATAG_VIDEOLFB   0x54410008
#define ATAG_CMDLINE    0x54410009
#define ATAG_ACORN      0x41000101
#define ATAG_MEMCLK     0x41000402

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

Rob

Each device is using own proprietary atag (or other information) to pass bootreason from bootloader to kernel. No standard way :-(

So that's what Pavel was alluding to.

Quoted 12 lines

The reason that happens is because people refuse to discuss their requirements here - just like people refuse to report userspace API regressions to kernel people. This rather pisses me off, because it creates all sorts of horrid per-platform yuck.

We _could_ (and have in the past) turned round and refused to support these kinds of hacks - which IMHO is quite a reasonable stance to
take: the message we should be sending is "if you wish to design new methods without discussing it with us, we reserve the right not to support them in mainline kernels; please discuss with us your requirements."

Quoted 6 lines

Each time that we accept one of these hacks, we're sending a message that says "it's okay to work in this crappy way".

Yes, I realise that the N900 has little in the way of support, and we can't exert that kind of back pressure (since there's no one to direct that onto to effect any change) so I guess we just have to live with it.

If the method is: "let's pass non-standard ATAGs around and have ad-hoc user space code interpret it in some arbitrary way" then it's a complete abomination.

Quoted 7 lines

Quoted 7 lines

I think this kind of information (how was board/computer started) can be useful also for other architectures. E.g. on laptop you would like to know if if was started by RTC, power button, WakeOnLan, another ACPI event, rebooted machine, watchdog, etc... And scripts can act depending on this event (when by RTC, you need to run some planned job, when by watchdog reset you should check what caused that reason...).

Useful when properly designed and generic enough to be shared.

Quoted 2 lines

I'd suggest a DT property be proposed for that purpose if it doesn't already exist. That at least has a chance to be generically useful.

What this means is that we have to further augment the atags-to-dt code in the decompressor with the platform specific ATAGs to parse this information and merge it into the appended DT before passing the resulting DT blob to the kernel.

Or we pass both the ATAGs and wrapped DT to the kernel when both exist, and let the kernel deal with it in a much saner environment than the restricted decompressor environment.

Or we /could/ say that mainline never supported it, and we aren't going to add support for "new" tags to the mainline kernel. It wouldn't be a regression, because mainline hasn't ever supported them (that's been said before about such things on other platforms.)

However, there's another consideration to be had here before we can say
that: how many people out there want to run a mainline (or even an updated kernel derived from mainline) on this device? If there's a sizable following for the device wanting updated support, then it's something we really need to consider supporting inspite of our disappointment with Nokia inventing these "proprietary" tags.

If there is indeed a sizable following for the device then someone should figure out how to cope with upstream kernels, either through the installation of some intermediate bootloader that can talk FDT directly, or via a shim layer. None of that has to be performed by nor linked with the kernel binary, nor maintained in the kernel tree. And it's not even that hard to do: we have precedents on other platforms with crappy bootloaders where this just works.

Quoted 3 lines · Russell King - ARM Linux

Or we pass both the ATAGs and wrapped DT to the kernel when both exist, and let the kernel deal with it in a much saner environment than the restricted decompressor environment.

... which would be a step backward in the context of the transition to DT we accepted. People will have even less of an incentive to fix their stuff.

Quoted 6 lines

However, there's another consideration to be had here before we can say
that: how many people out there want to run a mainline (or even an updated kernel derived from mainline) on this device? If there's a sizable following for the device wanting updated support, then it's something we really need to consider supporting inspite of our disappointment with Nokia inventing these "proprietary" tags.

If there is indeed a sizable following for the device then someone should figure out how to cope with upstream kernels, either through the installation of some intermediate bootloader that can talk FDT directly, or via a shim layer. None of that has to be performed by nor linked with the kernel binary, nor maintained in the kernel tree. And it's not even that hard to do: we have precedents on other platforms with crappy bootloaders where this just works.

Quoted 8 lines · Nicolas Pitre

Quoted 3 lines · Russell King - ARM Linux

Or we pass both the ATAGs and wrapped DT to the kernel when both exist, and let the kernel deal with it in a much saner environment than the restricted decompressor environment.

... which would be a step backward in the context of the transition to DT we accepted. People will have even less of an incentive to fix their stuff.

Where's the people fixing their stuff today?

I think your position is something of an idealist rather than a realist - the reality is that five years down the line of DT, the platforms which have not converted are now *never* going to convert, irrespective of how much "incentive" we may think we should apply to the situation.

Quoted 7 lines

If there is indeed a sizable following for the device then someone should figure out how to cope with upstream kernels, either through the installation of some intermediate bootloader that can talk FDT directly, or via a shim layer. None of that has to be performed by nor linked with the kernel binary, nor maintained in the kernel tree. And it's not even that hard to do: we have precedents on other platforms with crappy bootloaders where this just works.

That's a rediculous position if you want something which "just works" on as much hardware as possible, which is, after all, what the single zImage project is all about.

To be pro single-zImage, and anti popular hardware is quite contradictory.

You only have to look at x86 for this: just because ACPI came along does not mean that the Linux kernel started demanding that ACPI is the only way to describe the world. Linux continues to directly support all the old boot protocols that it did since the days of i386, such as the e820 memory interface and doesn't convert these to an ACPI table just because it can.

Quoted 11 lines · Russell King - ARM Linux

Quoted 8 lines · Nicolas Pitre

Quoted 3 lines · Russell King - ARM Linux

Or we pass both the ATAGs and wrapped DT to the kernel when both exist, and let the kernel deal with it in a much saner environment than the restricted decompressor environment.

... which would be a step backward in the context of the transition to DT we accepted. People will have even less of an incentive to fix their stuff.

Where's the people fixing their stuff today?

At least some people in this thread are, otherwise they'd still be away hacking on their own.

Quoted 5 lines

I think your position is something of an idealist rather than a realist - the reality is that five years down the line of DT, the platforms which have not converted are now *never* going to convert, irrespective of how much "incentive" we may think we should apply to the situation.

Don't get me wrong. I'm not expecting those platforms to do native DT booting ever. However "faking" DT booting with already proven solutions that don't bastardize the kernel further should be encouraged.

Quoted 11 lines

Quoted 7 lines

If there is indeed a sizable following for the device then someone should figure out how to cope with upstream kernels, either through the installation of some intermediate bootloader that can talk FDT directly, or via a shim layer. None of that has to be performed by nor linked with the kernel binary, nor maintained in the kernel tree. And it's not even that hard to do: we have precedents on other platforms with crappy bootloaders where this just works.

That's a rediculous position if you want something which "just works" on as much hardware as possible, which is, after all, what the single zImage project is all about.

Agreed.

Quoted 1 line

To be pro single-zImage, and anti popular hardware is quite contradictory.

Indeed. I'm not against popular hardware.

Quoted 6 lines

You only have to look at x86 for this: just because ACPI came along does not mean that the Linux kernel started demanding that ACPI is the only way to describe the world. Linux continues to directly support all the old boot protocols that it did since the days of i386, such as the e820 memory interface and doesn't convert these to an ACPI table just because it can.

Booting from a floppy boot sector is no longer supported though.

But that's besides the point. If someone needs a 5-line addition to atags_to_fdt.c in order to boot some old stuff then let's just do it and move on. I'll happily ACK the patch. The code is there and it is not going away soon.

However, if something more involved is needed, is platform specific and is likely to require about as many lines in the kernel than it would need in some externat shim then the shim solution should be encouraged instead. After all that's why LILO, Syslinux and Grub were created on
X86: to Supplement the crappy PC BIOS boot interface. And if the hardware is popular, then finding a motivated hacker to do it shouldn't be too hard, right?

In other words, what prevents someone from creating, say, a custom minimal Barebox version that sits on top of the existing N900 bootloader? Wouldn't that provide a much better user experience?

Nicolas

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 3 lines

In other words, what prevents someone from creating, say, a custom minimal Barebox version that sits on top of the existing N900 bootloader? Wouldn't that provide a much better user experience?

I do agree with Nicolas

If I can get my hand on a phone I?ll put barebox on it

Hi!

Quoted 4 lines

Quoted 3 lines

In other words, what prevents someone from creating, say, a custom minimal Barebox version that sits on top of the existing N900 bootloader? Wouldn't that provide a much better user experience?

Quoted 3 lines

I do agree with Nicolas

If I can get my hand on a phone I?ll put barebox on it

Grab it in the nearest "second hand cellphones" shop, and have fun! It should be less than $70.

Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Quoted 11 lines · Pavel Machek

Hi!

Quoted 6 lines

Quoted 3 lines

In other words, what prevents someone from creating, say, a custom minimal Barebox version that sits on top of the existing N900 bootloader? Wouldn't that provide a much better user experience?

I do agree with Nicolas

Quoted 4 lines

Quoted 1 line

If I can get my hand on a phone I?ll put barebox on it

Grab it in the nearest "second hand cellphones" shop, and have fun! It should be less than $70.

will see if I can found one in Shanghai at the second hand market

Does it exist a qemu emulation?

In other words, what prevents someone from creating, say, a custom minimal Barebox version that sits on top of the existing N900 bootloader? Wouldn't that provide a much better user experience?

In other words, what prevents someone from creating, say, a custom minimal Barebox version that sits on top of the existing N900 bootloader? Wouldn't that provide a much better user experience?

Quoted 9 lines

Quoted 4 lines

Useful when properly designed and generic enough to be shared.

I'd suggest a DT property be proposed for that purpose if it doesn't already exist. That at least has a chance to be generically useful.

What this means is that we have to further augment the atags-to-dt code in the decompressor with the platform specific ATAGs to parse this information and merge it into the appended DT before passing the resulting DT blob to the kernel.

That would be only 2 new lines of code in the best case.

We _could_ (and have in the past) turned round and refused to support these kinds of hacks - which IMHO is quite a reasonable stance to
take: the message we should be sending is "if you wish to design new methods without discussing it with us, we reserve the right not to support them in mainline kernels; please discuss with us your requirements."

Each time that we accept one of these hacks, we're sending a message that says "it's okay to work in this crappy way".

Yes, I realise that the N900 has little in the way of support, and we can't exert that kind of back pressure (since there's no one to direct that onto to effect any change) so I guess we just have to live with it.

Quoted 7 lines

I think this kind of information (how was board/computer started) can be useful also for other architectures. E.g. on laptop you would like to know if if was started by RTC, power button, WakeOnLan, another ACPI event, rebooted machine, watchdog, etc... And scripts can act depending on this event (when by RTC, you need to run some planned job, when by watchdog reset you should check what caused that reason...).

There is a standard way to get the boot information already: look at the watchdog API:

#define WDIOC_GETBOOTSTATUS _IOR(WATCHDOG_IOCTL_BASE, 2, int)

which uses the WDIOF_* flags to indicate the last boot reason. It probably isn't as flexible as some may desire, but it should provide at least the "watchdog rebooted us" vs "over temperature" vs some other boot reason.

The other thing to consider is whether we have a way to know what the boot reason was, and what we should do if we do not have a way of supporting some of the boot reasons. For example, if we have support for RTC alarm based booting, but no way to actually tell if the boot was caused by the RTC alarm triggering.

* Russell King - ARM Linux <[email protected]> [150127

Quoted 11 lines

We _could_ (and have in the past) turned round and refused to support these kinds of hacks - which IMHO is quite a reasonable stance to take: the message we should be sending is "if you wish to design new methods without discussing it with us, we reserve the right not to support them in mainline kernels; please discuss with us your requirements."

Each time that we accept one of these hacks, we're sending a message that says "it's okay to work in this crappy way".

Quoted 4 lines

Yes, I realise that the N900 has little in the way of support, and we can't exert that kind of back pressure (since there's no one to direct that onto to effect any change) so I guess we just have to live with it.

I believe after N900 Nokia dropped the custom ATAGs and used the kernel cmdline instead. And most of the n900 custom ATAGs are not even needed any longer.

The ATAG_REVISION is a standard feature that we should support naturally. I don't think we should add any custom ATAGs, except maybe for the bootreason.

Quoted 20 lines

Quoted 8 lines

I think this kind of information (how was board/computer started) can be useful also for other architectures. E.g. on laptop you would like to know if if was started by RTC, power button, WakeOnLan, another ACPI event, rebooted machine, watchdog, etc... And scripts can act depending on this event (when by RTC, you need to run some planned job, when by watchdog reset you should check what caused that reason...).

There is a standard way to get the boot information already:
look at the watchdog API:

#define WDIOC_GETBOOTSTATUS _IOR(WATCHDOG_IOCTL_BASE, 2,
int)

which uses the WDIOF_* flags to indicate the last boot reason. It probably isn't as flexible as some may desire, but it should provide at least the "watchdog rebooted us" vs "over temperature" vs some other boot reason.

Quoted 6 lines

The other thing to consider is whether we have a way to know what the boot reason was, and what we should do if we do not have a way of supporting some of the boot reasons. For example, if we have support for RTC alarm based booting, but no way to actually tell if the boot was caused by the RTC alarm triggering.

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Of course the problem is that the signed bootloader on n900 cannot be modified so the pass through u-boot would have to translate the custom ATAG for bootreason into a kernel cmdline..

But it may actually make sense to add the bootreason ATAGs, it seems quite generic to me.

AFAIK, the other n900 ATAGs can be just ignored.

Regards,

Tony

Quoted 5 lines · On Wednesday 28 January 2015 01:50:33 Tony Lindgren wrote:

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Not truth for N900. Bootreason depends on PRM_RSTST omap register, state of vbat charger pins, time how long was power key pressed, R&D data stored in CAL partition and other undocumented registers for omap HS devices. I already tried to implement at least some subset of it in userspace (or kernel), but it is impossible because NOLO bootloader clear status of PRM_RSTST register.

There is also copy of PRM_RSTST register stored at address 0x4020FFB8 (tracing data) but that address is rewritten (probably by kernel), so we really cannot implement reading bootreason in kernel.

But in early stage in uboot it is possible to read 0x4020FFB8 address and get some part of bootreason. But still PRM_RSTST is not enough!

I would be happy if DT kernel can export /proc/atags file with ATAGs passed by bootloader. It would be enough for me. In userspace I can parse content and do what is needed.

Quoted 20 lines · Pali Roh?r

Quoted 5 lines · On Wednesday 28 January 2015 01:50:33 Tony Lindgren wrote:

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Not truth for N900. Bootreason depends on PRM_RSTST omap register, state of vbat charger pins, time how long was power key pressed, R&D data stored in CAL partition and other undocumented registers for omap HS devices. I already tried to implement at least some subset of it in userspace (or kernel), but it is impossible because NOLO bootloader clear status of PRM_RSTST register.

There is also copy of PRM_RSTST register stored at address 0x4020FFB8 (tracing data) but that address is rewritten (probably by kernel), so we really cannot implement reading bootreason in kernel.

Quoted 7 lines

But in early stage in uboot it is possible to read 0x4020FFB8 address and get some part of bootreason. But still PRM_RSTST is not enough!

I would be happy if DT kernel can export /proc/atags file with ATAGs passed by bootloader. It would be enough for me. In userspace I can parse content and do what is needed.

What about defining a DT boot reason property instead?
Maybe it already exists? If not, it's something that could certainly be generically used on other platforms too.

Converting the special ATAG into its standard DT equivalent would then be trivial and much cleaner overall.

Quoted 29 lines · Nicolas Pitre

Quoted 19 lines · Pali Roh?r

Quoted 4 lines · On Wednesday 28 January 2015 01:50:33 Tony Lindgren wrote:

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Not truth for N900. Bootreason depends on PRM_RSTST omap register, state of vbat charger pins, time how long was power key pressed, R&D data stored in CAL partition and other undocumented registers for omap HS devices. I already tried to implement at least some subset of it in userspace (or kernel), but it is impossible because NOLO bootloader clear status of PRM_RSTST register.

There is also copy of PRM_RSTST register stored at address 0x4020FFB8 (tracing data) but that address is rewritten (probably by kernel), so we really cannot implement reading bootreason in kernel.

Quoted 8 lines

But in early stage in uboot it is possible to read 0x4020FFB8 address and get some part of bootreason. But still PRM_RSTST is not enough!

I would be happy if DT kernel can export /proc/atags file with ATAGs passed by bootloader. It would be enough for me. In userspace I can parse content and do what is needed.

Quoted 6 lines

What about defining a DT boot reason property instead? Maybe it already exists? If not, it's something that could certainly be generically used on other platforms too.

Converting the special ATAG into its standard DT equivalent would then be trivial and much cleaner overall.

Sounds good to me as then we don't have to add any legacy custom Nokia specific atag. And it won't prevent us from adding a generic ATAG_BOOTREASON if really needed.

Regards,

Tony

Quoted 37 lines · On Wednesday 28 January 2015 16:39:13 Tony Lindgren wrote:

Quoted 35 lines · Nicolas Pitre

Quoted 19 lines · Pali Roh?r

Quoted 4 lines · On Wednesday 28 January 2015 01:50:33 Tony Lindgren wrote:

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Not truth for N900. Bootreason depends on PRM_RSTST omap register, state of vbat charger pins, time how long was power key pressed, R&D data stored in CAL partition and other undocumented registers for omap HS devices. I already tried to implement at least some subset of it in userspace (or kernel), but it is impossible because NOLO bootloader clear status of PRM_RSTST register.

There is also copy of PRM_RSTST register stored at address 0x4020FFB8 (tracing data) but that address is rewritten (probably by kernel), so we really cannot implement reading bootreason in kernel.

Quoted 8 lines

But in early stage in uboot it is possible to read 0x4020FFB8 address and get some part of bootreason. But still PRM_RSTST is not enough!

I would be happy if DT kernel can export /proc/atags file with ATAGs passed by bootloader. It would be enough for me. In userspace I can parse content and do what is needed.

What about defining a DT boot reason property instead? Maybe it already exists? If not, it's something that could certainly be generically used on other platforms too.

Converting the special ATAG into its standard DT equivalent would then be trivial and much cleaner overall.

Quoted 3 lines

Sounds good to me as then we don't have to add any legacy custom Nokia specific atag. And it won't prevent us from adding a generic ATAG_BOOTREASON if really needed.

And what would new atag ATAG_BOOTREASON solve for Nokia N900? Nothing.

Quoted 42 lines · Pali Roh?r

Quoted 40 lines · On Wednesday 28 January 2015 16:39:13 Tony Lindgren wrote:

Quoted 29 lines · Nicolas Pitre

Quoted 19 lines · Pali Roh?r

Quoted 4 lines · On Wednesday 28 January 2015 01:50:33 Tony Lindgren wrote:

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Not truth for N900. Bootreason depends on PRM_RSTST omap register, state of vbat charger pins, time how long was power key pressed, R&D data stored in CAL partition and other undocumented registers for omap HS devices. I already tried to implement at least some subset of it in userspace (or kernel), but it is impossible because NOLO bootloader clear status of PRM_RSTST register.

There is also copy of PRM_RSTST register stored at address 0x4020FFB8 (tracing data) but that address is rewritten (probably by kernel), so we really cannot implement reading bootreason in kernel.

Quoted 8 lines

But in early stage in uboot it is possible to read 0x4020FFB8 address and get some part of bootreason. But still PRM_RSTST is not enough!

I would be happy if DT kernel can export /proc/atags file with ATAGs passed by bootloader. It would be enough for me. In userspace I can parse content and do what is needed.

Quoted 6 lines

What about defining a DT boot reason property instead? Maybe it already exists? If not, it's something that could certainly be generically used on other platforms too.

Converting the special ATAG into its standard DT equivalent would then be trivial and much cleaner overall.

Sounds good to me as then we don't have to add any legacy custom Nokia specific atag. And it won't prevent us from adding a generic ATAG_BOOTREASON if really needed.

Quoted 2 lines

And what would new atag ATAG_BOOTREASON solve for Nokia N900? Nothing.

Right, so probably no need to add it then :) But what Nico is saying we can translate the Nokia custom bootreason to a standard DT property if I'm reading right.

Quoted 24 lines · Pali Roh?r

Quoted 5 lines · On Wednesday 28 January 2015 01:50:33 Tony Lindgren wrote:

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Not truth for N900. Bootreason depends on PRM_RSTST omap register, state of vbat charger pins, time how long was power key pressed, R&D data stored in CAL partition and other undocumented registers for omap HS devices. I already tried to implement at least some subset of it in userspace (or kernel), but it is impossible because NOLO bootloader clear status of PRM_RSTST register.

There is also copy of PRM_RSTST register stored at address 0x4020FFB8 (tracing data) but that address is rewritten (probably by kernel), so we really cannot implement reading bootreason in kernel.

But in early stage in uboot it is possible to read 0x4020FFB8 address and get some part of bootreason. But still PRM_RSTST is not enough!

Quoted 3 lines

I would be happy if DT kernel can export /proc/atags file with ATAGs passed by bootloader. It would be enough for me. In userspace I can parse content and do what is needed.

What about defining a DT boot reason property instead?
Maybe it already exists? If not, it's something that could certainly be generically used on other platforms too.

I'm fine with that, but we just need to have a standard kernel userspace interface in addition to something like /proc/device-tree/bootreason. Perhaps this can be the default implementation for the watchdog dev. Someday when we decide DT is crap and have a new boot interface, we'll have people relying on /proc/device-tree. I hope to be retired when that happens...

Quoted 6 lines · Rob Herring

I'm fine with that, but we just need to have a standard kernel userspace interface in addition to something like /proc/device-tree/bootreason. Perhaps this can be the default implementation for the watchdog dev. Someday when we decide DT is crap and have a new boot interface, we'll have people relying on /proc/device-tree. I hope to be retired when that happens...

Anyone who thinks that DT can be replaced in the same way that we made the mistake with ATAGs would really need their head examined.

As you point out, removing DT removes the /proc/device-tree/ sub-tree. Whether we like it or not, that is a userspace API, one which we have users of already. That pretty much means that we can't remove DT for existing platforms or any platform we have now converted to DT.

Quoted 8 lines · Russell King - ARM Linux

Quoted 6 lines · Rob Herring

I'm fine with that, but we just need to have a standard kernel userspace interface in addition to something like /proc/device-tree/bootreason. Perhaps this can be the default implementation for the watchdog dev. Someday when we decide DT is crap and have a new boot interface, we'll have people relying on /proc/device-tree. I hope to be retired when that happens...

Quoted 7 lines

Anyone who thinks that DT can be replaced in the same way that we made the mistake with ATAGs would really need their head examined.

As you point out, removing DT removes the /proc/device-tree/ sub-tree. Whether we like it or not, that is a userspace API, one which we have users of already. That pretty much means that we can't remove DT for existing platforms or any platform we have now converted to DT.

<ok, I'll go there!>

... and for platforms that can also be booted via ACPI? If we have to convert the ACPI tables into a device-tree purely for /proc/device-tree, then we may as well boot with the thing too :)

Seriously though, I don't see how we can maintain this directory for ACPI, regardless of whether or not it's ABI.

Quoted 8 lines · Russell King - ARM Linux

Quoted 6 lines · Rob Herring

I'm fine with that, but we just need to have a standard kernel userspace interface in addition to something like /proc/device-tree/bootreason. Perhaps this can be the default implementation for the watchdog dev. Someday when we decide DT is crap and have a new boot interface, we'll have people relying on /proc/device-tree. I hope to be retired when that happens...

Quoted 7 lines

Anyone who thinks that DT can be replaced in the same way that we made the mistake with ATAGs would really need their head examined.

As you point out, removing DT removes the /proc/device-tree/ sub-tree. Whether we like it or not, that is a userspace API, one which we have users of already. That pretty much means that we can't remove DT for existing platforms or any platform we have now converted to DT.

<ok, I'll go there!>

... and for platforms that can also be booted via ACPI? If we have to convert the ACPI tables into a device-tree purely for /proc/device-tree, then we may as well boot with the thing too :)

Seriously though, I don't see how we can maintain this directory for ACPI, regardless of whether or not it's ABI.

On Wed, Jan 28, 2015 at 8:33 AM, Nicolas Pitre

Quoted 24 lines · Pali Roh?r

Quoted 23 lines · On Wednesday 28 January 2015 01:50:33 Tony Lindgren wrote:

Quoted 4 lines

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Not truth for N900. Bootreason depends on PRM_RSTST omap register, state of vbat charger pins, time how long was power key pressed, R&D data stored in CAL partition and other undocumented registers for omap HS devices. I already tried to implement at least some subset of it in userspace (or kernel), but it is impossible because NOLO bootloader clear status of PRM_RSTST register.

There is also copy of PRM_RSTST register stored at address 0x4020FFB8 (tracing data) but that address is rewritten (probably by kernel), so we really cannot implement reading bootreason in kernel.

But in early stage in uboot it is possible to read 0x4020FFB8 address and get some part of bootreason. But still PRM_RSTST is not enough!

Quoted 8 lines

Quoted 4 lines

I would be happy if DT kernel can export /proc/atags file with ATAGs passed by bootloader. It would be enough for me. In userspace I can parse content and do what is needed.

What about defining a DT boot reason property instead? Maybe it already exists? If not, it's something that could certainly be generically used on other platforms too.

I'm fine with that, but we just need to have a standard kernel userspace interface in addition to something like /proc/device-tree/bootreason. Perhaps this can be the default implementation for the watchdog dev. Someday when we decide DT is crap and have a new boot interface, we'll have people relying on /proc/device-tree. I hope to be retired when that happens...

Rob

Quoted 26 lines · Nicolas Pitre

Quoted 24 lines · Pali Roh?r

Quoted 5 lines · On Wednesday 28 January 2015 01:50:33 Tony Lindgren wrote:

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Not truth for N900. Bootreason depends on PRM_RSTST omap register, state of vbat charger pins, time how long was power key pressed, R&D data stored in CAL partition and other undocumented registers for omap HS devices. I already tried to implement at least some subset of it in userspace (or kernel), but it is impossible because NOLO bootloader clear status of PRM_RSTST register.

There is also copy of PRM_RSTST register stored at address 0x4020FFB8 (tracing data) but that address is rewritten (probably by kernel), so we really cannot implement reading bootreason in kernel.

But in early stage in uboot it is possible to read 0x4020FFB8 address and get some part of bootreason. But still PRM_RSTST is not enough!

Quoted 7 lines

Quoted 3 lines

I would be happy if DT kernel can export /proc/atags file with ATAGs passed by bootloader. It would be enough for me. In userspace I can parse content and do what is needed.

What about defining a DT boot reason property instead?
Maybe it already exists? If not, it's something that could certainly be generically used on other platforms too.

I'm fine with that, but we just need to have a standard kernel userspace interface in addition to something like /proc/device-tree/bootreason. Perhaps this can be the default implementation for the watchdog dev. Someday when we decide DT is crap and have a new boot interface, we'll have people relying on /proc/device-tree. I hope to be retired when that happens?

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 4 lines

On Jan 28, 2015, at 11:57 PM, Rob Herring
<[email protected]> wrote:

On Wed, Jan 28, 2015 at 8:33 AM, Nicolas Pitre

Quoted 33 lines

Quoted 24 lines · Pali Roh?r

Quoted 23 lines · On Wednesday 28 January 2015 01:50:33 Tony Lindgren wrote:

Quoted 4 lines

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Not truth for N900. Bootreason depends on PRM_RSTST omap register, state of vbat charger pins, time how long was power key pressed, R&D data stored in CAL partition and other undocumented registers for omap HS devices. I already tried to implement at least some subset of it in userspace (or kernel), but it is impossible because NOLO bootloader clear status of PRM_RSTST register.

There is also copy of PRM_RSTST register stored at address 0x4020FFB8 (tracing data) but that address is rewritten (probably by kernel), so we really cannot implement reading bootreason in kernel.

But in early stage in uboot it is possible to read 0x4020FFB8 address and get some part of bootreason. But still PRM_RSTST is not enough!

Quoted 8 lines

Quoted 4 lines

I would be happy if DT kernel can export /proc/atags file with ATAGs passed by bootloader. It would be enough for me. In userspace I can parse content and do what is needed.

What about defining a DT boot reason property instead? Maybe it already exists? If not, it's something that could certainly be generically used on other platforms too.

Quoted 7 lines

I'm fine with that, but we just need to have a standard kernel userspace interface in addition to something like /proc/device-tree/bootreason. Perhaps this can be the default implementation for the watchdog dev. Someday when we decide DT is crap and have a new boot interface, we'll have people relying on /proc/device-tree. I hope to be retired when that happens?

but if we try to do this generic, where will you store the boot mode

I mean where the SoC boot from

useful to for the Userspace to known where is the bootloader in case of multi boot mode

Best Regards,
J.

Quoted 2 lines

Rob

Quoted 26 lines · Nicolas Pitre

Quoted 24 lines · Pali Roh?r

Quoted 5 lines · On Wednesday 28 January 2015 01:50:33 Tony Lindgren wrote:

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Not truth for N900. Bootreason depends on PRM_RSTST omap register, state of vbat charger pins, time how long was power key pressed, R&D data stored in CAL partition and other undocumented registers for omap HS devices. I already tried to implement at least some subset of it in userspace (or kernel), but it is impossible because NOLO bootloader clear status of PRM_RSTST register.

There is also copy of PRM_RSTST register stored at address 0x4020FFB8 (tracing data) but that address is rewritten (probably by kernel), so we really cannot implement reading bootreason in kernel.

But in early stage in uboot it is possible to read 0x4020FFB8 address and get some part of bootreason. But still PRM_RSTST is not enough!

Quoted 7 lines

Quoted 3 lines

I would be happy if DT kernel can export /proc/atags file with ATAGs passed by bootloader. It would be enough for me. In userspace I can parse content and do what is needed.

What about defining a DT boot reason property instead?
Maybe it already exists? If not, it's something that could certainly be generically used on other platforms too.

I'm fine with that, but we just need to have a standard kernel userspace interface in addition to something like /proc/device-tree/bootreason. Perhaps this can be the default implementation for the watchdog dev. Someday when we decide DT is crap and have a new boot interface, we'll have people relying on /proc/device-tree. I hope to be retired when that happens...

Quoted 1 line

/proc/device-tree. I hope to be retired when that happens...

Huh. Who thought putting device-tree into /proc was good idea? That should have gone to /sys, as it is not process related...

Quoted 1 line · On Wednesday 28 January 2015 01:50:33 Tony Lindgren wrote:

* Russell King - ARM Linux <[email protected]> [150127

09:51]:

Quoted 11 lines

Quoted 11 lines

We _could_ (and have in the past) turned round and refused to support these kinds of hacks - which IMHO is quite a reasonable stance to take: the message we should be sending is "if you wish to design new methods without discussing it with us, we reserve the right not to support them in mainline kernels; please discuss with us your requirements."

Each time that we accept one of these hacks, we're sending a message that says "it's okay to work in this crappy way".

Quoted 9 lines

Quoted 4 lines

Yes, I realise that the N900 has little in the way of support, and we can't exert that kind of back pressure (since there's no one to direct that onto to effect any change) so I guess we just have to live with it.

I believe after N900 Nokia dropped the custom ATAGs and used the kernel cmdline instead. And most of the n900 custom ATAGs are not even needed any longer.

Yes, almost all N900 ATAGs are static and are already hardcoded into kernel or DT file.

Basically there are 4 non static values which are used:

1. ATAG_REVISION

2. ATAG_OMAP
2.1 OMAP_TAG_BOOT_REASON --> boot reason

2.2 OMAP_TAG_VERSION ("nolo") --> for bootloader version

2.3 OMAP_TAG_VERSION ("boot-mode") --> "normal" or ?update"

ATAG_OMAP is non standard and contains sub-atags.

bootloader version is static now (as Nokia does not develop it anymore), but boot reason and boot mode are set by bootloader and are needed for userspace. boot mode tells init system/userspace if to start normal OS or only small subset for flashing.

Quoted 31 lines

The ATAG_REVISION is a standard feature that we should support naturally. I don't think we should add any custom ATAGs, except maybe for the bootreason.

Quoted 20 lines

Quoted 8 lines

I think this kind of information (how was board/computer started) can be useful also for other architectures. E.g. on laptop you would like to know if if was started by RTC, power button, WakeOnLan, another ACPI event, rebooted machine, watchdog, etc... And scripts can act depending on this event (when by RTC, you need to run some planned job, when by watchdog reset you should check what caused that reason...).

There is a standard way to get the boot information already:
look at the watchdog API:

#define WDIOC_GETBOOTSTATUS _IOR(WATCHDOG_IOCTL_BASE, 2,
int)

which uses the WDIOF_* flags to indicate the last boot reason. It probably isn't as flexible as some may desire, but it should provide at least the "watchdog rebooted us" vs "over temperature" vs some other boot reason.

Quoted 6 lines

The other thing to consider is whether we have a way to know what the boot reason was, and what we should do if we do not have a way of supporting some of the boot reasons. For example, if we have support for RTC alarm based booting, but no way to actually tell if the boot was caused by the RTC alarm triggering.

Quoted 5 lines

On omaps, the bootrom passes the bootreason in r1 to the bootloader that can do whatever it wants with it. We could maybe pass it in the kernel cmdline to the watchdog driver for user space?

Not truth for N900. Bootreason depends on PRM_RSTST omap register, state of vbat charger pins, time how long was power key pressed, R&D data stored in CAL partition and other undocumented registers for omap HS devices. I already tried to implement at least some subset of it in userspace (or kernel), but it is impossible because NOLO bootloader clear status of PRM_RSTST register.

There is also copy of PRM_RSTST register stored at address 0x4020FFB8 (tracing data) but that address is rewritten (probably by kernel), so we really cannot implement reading bootreason in kernel.

But in early stage in uboot it is possible to read 0x4020FFB8 address and get some part of bootreason. But still PRM_RSTST is not enough!

I would be happy if DT kernel can export /proc/atags file with ATAGs passed by bootloader. It would be enough for me. In userspace I can parse content and do what is needed.

In non DT kernel file /proc/atags is always exported.

Quoted 8 lines

Of course the problem is that the signed bootloader on n900 cannot be modified so the pass through u-boot would have to translate the custom ATAG for bootreason into a kernel cmdline..

But it may actually make sense to add the bootreason ATAGs, it seems quite generic to me.

Which bootreason atag? Invent new? Or use above big ATAG_OMAP structure? Inventing new does not solve anything because all developers does not boot kernel for debugging from uboot -- but directly.

Quoted 5 lines

AFAIK, the other n900 ATAGs can be just ignored.

Regards,

Tony

Signature · 6 lines

Pali Roh?r
pali.rohar at gmail.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 2 lines · Rob Herring

These would be non-standard fields which are not upstream. I don't know that we care in that case...

Other devices are going to care about boot reason, too, and we might as well be compatible...

This patch will cause that decompressor store full ATAG structure into DT tree ("/atags"):

Quoted 4 lines · Pali Roh?r

This patch will cause that decompressor store full ATAG structure into

DT tree ("/atags"):

How about something a little more radical.

Rather than trying to squeeze various ATAGs into DT, why don't we add a standard ATAG to contain the DT and pass that through into the kernel. This is IMHO how we _should_ have done the ATAG compatibility from the start.

That means we could get rid of most of the libfdt in the decompressor, and instead resolve the differences in the kernel.

On Wed, Dec 3, 2014 at 6:48 PM, Pali Roh?r

Quoted 65 lines · On Thursday 04 December 2014 01:33:08 Rob Herring wrote:

Quoted 1 line

On Mon, Nov 24, 2014 at 4:19 PM, Pali Roh?r

Quoted 61 lines · <[email protected]> wrote:

Quoted 12 lines · On Wednesday 18 June 2014 22:46:19 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 2:22 PM, Pali Roh?r

Quoted 8 lines · <[email protected]> wrote:

Quoted 8 lines · On Wednesday 18 June 2014 21:07:35 Rob Herring wrote:

Quoted 1 line

On Wed, Jun 18, 2014 at 11:54 AM, Pali Roh?r

Quoted 4 lines · <[email protected]> wrote:

Quoted 4 lines

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.

[...]

Quoted 34 lines

Quoted 17 lines

Quoted 5 lines

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.

Quoted 4 lines

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

Quoted 5 lines

         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

Wrong place. If you put /serial-number in your DT, then it will be in /proc/device-tree/serial-number.

If you want to wire that up to /proc/cpuinfo, I would be fine with that.

Quoted 6 lines

Quoted 1 line

Rob

I cannot hardcode it into DT, because it is set by bootloader. And bootloader tell it to kernel via ATAGs structure. Bootloader is closed and proprietary and cannot be replaced (there is no documentation for hw init).

Either the bootloader can put the serial number into the DTB instead of ATAGs or the kernel decompressor can copy it from ATAGs to DTB like is done already for other ATAGs. I presume you cannot update the bootloader and will need the latter.

Rob

Review

Quoted 13 lines · Pali Roh?r

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 ..

(quoted diff) +0 -2

-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

(quoted diff) +0 -2

-r--r--r--    1 0        0             4096 Jan  1 00:00 type
-rw-r--r--    1 0        0             4096 Jan  1 00:00 uevent

Wrong place. If you put /serial-number in your DT, then it will be in /proc/device-tree/serial-number.

If you want to wire that up to /proc/cpuinfo, I would be fine with that.

So without this patch DT migration for Nokia N900 cannot be done without breaking userspace - which is not acceptable...

Hi,

Quoted 2 lines · Pali Roh?r

So without this patch DT migration for Nokia N900 cannot be done without breaking userspace - which is not acceptable...

I guess the fact is that the Nokia N900 proprietary userspace has never worked properly with the mainline kernel without severe patching, so you cannot really talk about any regression. If you cannot fix those programs, you could still probably emulate the old Nokia kernel's /proc/cpuinfo by providing some text file and bind mounting it, no?

Hi,

Quoted 3 lines · Pali Roh?r

So without this patch DT migration for Nokia N900 cannot be done without breaking userspace - which is not acceptable...

I guess the fact is that the Nokia N900 proprietary userspace has never worked properly with the mainline kernel without severe patching, so you cannot really talk about any regression. If you cannot fix those programs, you could still probably emulate the old Nokia kernel's /proc/cpuinfo by providing some text file and bind mounting it, no?

A.

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 <[email protected]>

---
 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

arch/arm/kernel/setup.c +1 -1

index 8a16ee5..fbc7b4f 100644
@@ -875,10 +875,13 @@ void __init setup_arch(char **cmdline_p)
 
 	setup_processor();
 	mdesc = setup_machine_fdt(__atags_pointer);
-	if (!mdesc)
+	if (mdesc)

Quoted 27 lines · On Wednesday 18 June 2014 18:54:24 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 <[email protected]>

---
 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

arch/arm/kernel/setup.c +5 -2

index 8a16ee5..fbc7b4f 100644
@@ -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;
 

arch/arm/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 887

Quoted 2 lines

arch/arm/kernel/setup.c +0 -0

 	if (mdesc->reboot_mode != REBOOT_HARD)
 		reboot_mode = mdesc->reboot_mode;

So, do you really want to break userspace which reading file /proc/cpuinfo (after migration from boardcode --> DT)?

I still do not see reason for that. And only this one file is problematic...

Quoted 17 lines · Pali Roh?r

arch/arm/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 887

Quoted 12 lines · On Wednesday 18 June 2014 18:54:24 Pali Roh?r wrote:

arch/arm/kernel/setup.c +3 -1

-	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;

arch/arm/kernel/setup.c +2 -1

-	machine_name = mdesc->name;
+	if (!machine_name)
+		machine_name = mdesc->name;
 
 	if (mdesc->reboot_mode != REBOOT_HARD)
 		reboot_mode = mdesc->reboot_mode;

So, do you really want to break userspace which reading file /proc/cpuinfo (after migration from boardcode --> DT)?

Quoted 2 lines

I still do not see reason for that. And only this one file is problematic...

Sorry, I just don't give a damn about your whinging about this. I've made the situation perfectly clear. Your patch will not be accepted.

Quoted 22 lines · Russell King - ARM Linux

Quoted 21 lines · Pali Roh?r

Quoted 8 lines · On Wednesday 18 June 2014 18:54:24 Pali Roh?r wrote:

arch/arm/kernel/setup.c +3 -1

-	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;

arch/arm/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 887

Quoted 7 lines

arch/arm/kernel/setup.c +2 -1

-	machine_name = mdesc->name;
+	if (!machine_name)
+		machine_name = mdesc->name;
 
 	if (mdesc->reboot_mode != REBOOT_HARD)
 
 		reboot_mode = mdesc->reboot_mode;

So, do you really want to break userspace which reading file /proc/cpuinfo (after migration from boardcode --> DT)?

Quoted 6 lines

Quoted 2 lines

I still do not see reason for that. And only this one file is problematic...

Sorry, I just don't give a damn about your whinging about this. I've made the situation perfectly clear. Your patch will not be accepted.

Linus made it pretty clear that regressions are not accepted.

You are an arm maintainer and there is regression in n900 that broke userspace.

How do you solve it?
Pavel

Quoted 27 lines · On Wednesday 18 June 2014 18:54:24 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 <[email protected]>

---
 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

arch/arm/kernel/setup.c +5 -2

index 8a16ee5..fbc7b4f 100644
@@ -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;
 

arch/arm/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 887

Quoted 2 lines

arch/arm/kernel/setup.c +0 -0

 	if (mdesc->reboot_mode != REBOOT_HARD)
 		reboot_mode = mdesc->reboot_mode;

So, do you really want to break userspace which reading file /proc/cpuinfo (after migration from boardcode --> DT)?

Quoted 34 lines · Pali Roh?r

Quoted 33 lines · On Wednesday 18 June 2014 18:54:24 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 <[email protected]>

---

 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/arm/kernel/setup.c index 8a16ee5..fbc7b4f 100644

arch/arm/kernel/setup.c +5 -2

@@ -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;

Quoted 6 lines

arch/arm/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 887

Quoted 3 lines

arch/arm/kernel/setup.c +0 -0

 	if (mdesc->reboot_mode != REBOOT_HARD)
 
 		reboot_mode = mdesc->reboot_mode;

So, do you really want to break userspace which reading file /proc/cpuinfo (after migration from boardcode --> DT)?

You have no guarantee model name in the DT == the name in a board file anyhow, and trying to force that is wrong. So further to Russell's reply, I must NAK this from a DT perspective.

Realistically your userspace is already broken if relying on such things. You built something that only ever worked for a particular arbitrary string. So it was already broken for every other board, and there was never any guarantee that new boards where your userspace could have worked would share the same name.

You're trying to fix the wrong side of the equation.

Mark.

Quoted 54 lines · On Friday 05 September 2014 15:45:42 Mark Rutland wrote:

Quoted 34 lines · Pali Roh?r

Quoted 33 lines · On Wednesday 18 June 2014 18:54:24 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 <[email protected]>

---

 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/arm/kernel/setup.c index 8a16ee5..fbc7b4f 100644

arch/arm/kernel/setup.c +5 -2

@@ -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;

Quoted 6 lines

arch/arm/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 887

Quoted 3 lines

arch/arm/kernel/setup.c +0 -0

 	if (mdesc->reboot_mode != REBOOT_HARD)
 
 		reboot_mode = mdesc->reboot_mode;

So, do you really want to break userspace which reading file /proc/cpuinfo (after migration from boardcode --> DT)?

You have no guarantee model name in the DT == the name in a board file anyhow, and trying to force that is wrong. So further to Russell's reply, I must NAK this from a DT perspective.

Realistically your userspace is already broken if relying on such things. You built something that only ever worked for a particular arbitrary string. So it was already broken for every other board, and there was never any guarantee that new boards where your userspace could have worked would share the same name.

Quoted 3 lines

You're trying to fix the wrong side of the equation.

Mark.

So what is your suggestion for identifing board (name/type) which will work with any kernel (and will not be broken again by kernel later)?

Quoted 60 lines · Pali Roh?r

Quoted 58 lines · On Friday 05 September 2014 15:45:42 Mark Rutland wrote:

Quoted 34 lines · Pali Roh?r

Quoted 33 lines · On Wednesday 18 June 2014 18:54:24 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 <[email protected]>

---

 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/arm/kernel/setup.c index 8a16ee5..fbc7b4f 100644 --- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c

(quoted diff) +5 -2

@@ -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;

Quoted 7 lines

arch/arm/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 887

Quoted 3 lines

arch/arm/kernel/setup.c +0 -0

 	if (mdesc->reboot_mode != REBOOT_HARD)
 
 		reboot_mode = mdesc->reboot_mode;

So, do you really want to break userspace which reading file /proc/cpuinfo (after migration from boardcode --> DT)?

You have no guarantee model name in the DT == the name in a board file anyhow, and trying to force that is wrong. So further to Russell's reply, I must NAK this from a DT perspective.

Realistically your userspace is already broken if relying on such things. You built something that only ever worked for a particular arbitrary string. So it was already broken for every other board, and there was never any guarantee that new boards where your userspace could have worked would share the same name.

You're trying to fix the wrong side of the equation.

Mark.

Quoted 3 lines

So what is your suggestion for identifing board (name/type) which will work with any kernel (and will not be broken again by kernel later)?

Without knowing your use case I have no idea if that's even a sane thing to do.

Mark.

So what is your suggestion for identifing board (name/type) which will work with any kernel (and will not be broken again by kernel later)?

On Fri, Sep 5, 2014 at 10:52 AM, Pali Roh?r

Quoted 3 lines

So what is your suggestion for identifing board (name/type) which will work with any kernel (and will not be broken again by kernel later)?

$ cat /sys/bus/soc/devices/soc0/machine
Freescale i.MX6 Quad SABRE Smart Device Board

Quoted 54 lines · Pali Roh?r

Quoted 34 lines · On Wednesday 18 June 2014 18:54:24 Pali Roh?r wrote:

Quoted 33 lines

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 <[email protected]>

---

 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/arm/kernel/setup.c index 8a16ee5..fbc7b4f 100644

arch/arm/kernel/setup.c +5 -2

@@ -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;

Quoted 6 lines

arch/arm/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 887

Quoted 3 lines

arch/arm/kernel/setup.c +0 -0

 	if (mdesc->reboot_mode != REBOOT_HARD)
 
 		reboot_mode = mdesc->reboot_mode;

So, do you really want to break userspace which reading file /proc/cpuinfo (after migration from boardcode --> DT)?

You have no guarantee model name in the DT == the name in a board file anyhow, and trying to force that is wrong. So further to Russell's reply, I must NAK this from a DT perspective.

Realistically your userspace is already broken if relying on such things. You built something that only ever worked for a particular arbitrary string. So it was already broken for every other board, and there was never any guarantee that new boards where your userspace could have worked would share the same name.

Quoted 1 line

You're trying to fix the wrong side of the equation.

So what is your suggestion for identifing board (name/type) which will work with any kernel (and will not be broken again by kernel later)?

Am 05.09.2014 15:52, schrieb Pali Roh?r:

Quoted 56 lines · On Friday 05 September 2014 15:45:42 Mark Rutland wrote:

Quoted 55 lines · Pali Roh?r

Quoted 34 lines · On Wednesday 18 June 2014 18:54:24 Pali Roh?r wrote:

Quoted 33 lines

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 <[email protected]>

---

 arch/arm/kernel/setup.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/arm/kernel/setup.c index 8a16ee5..fbc7b4f 100644

arch/arm/kernel/setup.c +5 -2

@@ -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;

Quoted 7 lines

arch/arm/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 887

Quoted 3 lines

arch/arm/kernel/setup.c +0 -0

 	if (mdesc->reboot_mode != REBOOT_HARD)
 
 		reboot_mode = mdesc->reboot_mode;

So, do you really want to break userspace which reading file /proc/cpuinfo (after migration from boardcode --> DT)?

You have no guarantee model name in the DT == the name in a board file anyhow, and trying to force that is wrong. So further to Russell's reply, I must NAK this from a DT perspective.

Realistically your userspace is already broken if relying on such things. You built something that only ever worked for a particular arbitrary string. So it was already broken for every other board, and there was never any guarantee that new boards where your userspace could have worked would share the same name.

Quoted 5 lines

Quoted 1 line

You're trying to fix the wrong side of the equation.

So what is your suggestion for identifing board (name/type) which will work with any kernel (and will not be broken again by kernel later)?

/proc/device-tree/compatible should give you a nul-separated list of compatible strings for the machine. Ideally they're even documented under Documentation/devicetree/bindings/arm/.

But as Mark said, depending on what you are actually trying to distinguish in userspace, there may be better ways.

Regards,
Andreas