fdt: Pass the device serial number through devicetree

PATCH Superseded Paul Kocialkowski 2 participants 5 messages May 21, 2015 – May 24, 2015

Share & tools

Paul Kocialkowski

author

2015-05-21 09:27 UTC #0 lore raw

PATCH v2 1/1 fdt: Pass the device serial number through devicetree

Before device-tree, the device serial number used to be passed to the kernel using ATAGs (on ARM). This is now deprecated and all the handover to the kernel should now be done using device-tree. Thus, this passes the serial-number property to the kernel using the serial-number property of the root node, as expected by the kernel.

The serial number is a string that somewhat represents the device's serial number. It might come from some form of storage (e.g. an eeprom) and be programmed at factory-time by the manufacturer or come from identification bits available in e.g. the SoC.

Signed-off-by Paul Kocialkowski <[email protected]>

---
 common/fdt_support.c              | 25 +++++++++++++++++++++++++
 common/image-fdt.c                |  4 ++++
 doc/device-tree-bindings/root.txt |  4 ++++
 include/fdt_support.h             |  1 +
 4 files changed, 34 insertions(+)
 create mode 100644 doc/device-tree-bindings/root.txt

common/fdt_support.c +25 -0

index 9e50148..10648b5 100644
@@ -194,6 +194,31 @@ static inline int fdt_setprop_uxx(void *fdt, int nodeoffset, const char *name,
 		return fdt_setprop_u32(fdt, nodeoffset, name, (uint32_t)val);
 }
 
+int fdt_root(void *fdt)
+{
+	char *serial;
+	int err;
+
+	err = fdt_check_header(fdt);
+	if (err < 0) {
+		printf("fdt_root: %s\n", fdt_strerror(err));
+		return err;
+	}
+
+	serial = getenv("serial#");
+	if (serial) {
+		err = fdt_setprop(fdt, 0, "serial-number", serial,
+				  strlen(serial) + 1);
+
+		if (err < 0) {
+			printf("WARNING: could not set serial-number %s.\n",
+			       fdt_strerror(err));
+			return err;
+		}
+	}
+
+	return 0;
+}
 
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end)
 {

common/image-fdt.c +4 -0

index 7e2da7b..80e3e63 100644
@@ -471,6 +471,10 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob,
 	int ret = -EPERM;
 	int fdt_ret;
 
+	if (fdt_root(blob) < 0) {
+		printf("ERROR: root node setup failed\n");
+		goto err;
+	}
 	if (fdt_chosen(blob) < 0) {
 		printf("ERROR: /chosen node create failed\n");
 		goto err;

doc/device-tree-bindings/root.txt new file +4 -0

new file mode 100644
index 0000000..001ccf3
@@ -0,0 +1,4 @@
+The root node
+
+Optional properties:
+  - serial-number : a string representing the device's serial number

include/fdt_support.h +1 -0

index 5d4f28d..56185c9 100644
@@ -16,6 +16,7 @@ u32 fdt_getprop_u32_default_node(const void *fdt, int off, int cell,
 				const char *prop, const u32 dflt);
 u32 fdt_getprop_u32_default(const void *fdt, const char *path,
 				const char *prop, const u32 dflt);
+int fdt_root(void *fdt);
 int fdt_chosen(void *fdt);
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,

Signature · 1 line

1.9.1

Simon Glass

2015-05-21 20:32 UTC #1 lore raw

PATCH v2 1/1 fdt: Pass the device serial number through devicetree

Hi Paul,

Quoted 6 lines · Paul Kocialkowski

Before device-tree, the device serial number used to be passed to the kernel using ATAGs (on ARM). This is now deprecated and all the handover to the kernel should now be done using device-tree. Thus, this passes the serial-number property to the kernel using the serial-number property of the root node, as expected by the kernel.

Quoted 6 lines

The serial number is a string that somewhat represents the device's serial number. It might come from some form of storage (e.g. an eeprom) and be programmed at factory-time by the manufacturer or come from identification bits available in e.g. the SoC.
Signed-off-by Paul Kocialkowski <[email protected]>

Reviewed-by Simon Glass <[email protected]>

But please see a nit below.

Quoted 74 lines

---
 common/fdt_support.c              | 25 +++++++++++++++++++++++++
 common/image-fdt.c                |  4 ++++
 doc/device-tree-bindings/root.txt |  4 ++++
 include/fdt_support.h             |  1 +
 4 files changed, 34 insertions(+)
 create mode 100644 doc/device-tree-bindings/root.txt

common/fdt_support.c +25 -0

index 9e50148..10648b5 100644
@@ -194,6 +194,31 @@ static inline int fdt_setprop_uxx(void *fdt, int nodeoffset, const char *name,
                return fdt_setprop_u32(fdt, nodeoffset, name, (uint32_t)val);
 }
 
+int fdt_root(void *fdt)
+{
+       char *serial;
+       int err;
+
+       err = fdt_check_header(fdt);
+       if (err < 0) {
+               printf("fdt_root: %s\n", fdt_strerror(err));
+               return err;
+       }
+
+       serial = getenv("serial#");
+       if (serial) {
+               err = fdt_setprop(fdt, 0, "serial-number", serial,
+                                 strlen(serial) + 1);
+
+               if (err < 0) {
+                       printf("WARNING: could not set serial-number %s.\n",
+                              fdt_strerror(err));
+                       return err;
+               }
+       }
+
+       return 0;
+}
 
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end)
 {

common/image-fdt.c +4 -0

index 7e2da7b..80e3e63 100644
@@ -471,6 +471,10 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob,
        int ret = -EPERM;
        int fdt_ret;
 
+       if (fdt_root(blob) < 0) {
+               printf("ERROR: root node setup failed\n");
+               goto err;
+       }
        if (fdt_chosen(blob) < 0) {
                printf("ERROR: /chosen node create failed\n");
                goto err;

doc/device-tree-bindings/root.txt new file +4 -0

new file mode 100644
index 0000000..001ccf3
@@ -0,0 +1,4 @@
+The root node
+
+Optional properties:
+  - serial-number : a string representing the device's serial number

include/fdt_support.h +0 -0

index 5d4f28d..56185c9 100644

include/fdt_support.h u32 fdt_getprop_u32_default_node(const void *fdt, int off, int cell, · line 19

Quoted 4 lines

include/fdt_support.h +1 -0

@@ -16,6 +16,7 @@ u32 fdt_getprop_u32_default_node(const void *fdt, int off, int cell,
                                const char *prop, const u32 dflt);
 u32 fdt_getprop_u32_default(const void *fdt, const char *path,
                                const char *prop, const u32 dflt);
+int fdt_root(void *fdt);

Please can you add a comment for this in the standard style?

Quoted 6 lines

include/fdt_support.h +0 -1

 int fdt_chosen(void *fdt);
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,
--

1.9.1

Regards,
Simon

Paul Kocialkowski

author

2015-05-22 10:55 UTC #2 lore raw

PATCH v2 1/1 fdt: Pass the device serial number through devicetree

Le jeudi 21 mai 2015 ? 14:32 -0600, Simon Glass a ?crit :

Quoted 92 lines

Hi Paul,

Quoted 6 lines · Paul Kocialkowski

Before device-tree, the device serial number used to be passed to the kernel using ATAGs (on ARM). This is now deprecated and all the handover to the kernel should now be done using device-tree. Thus, this passes the serial-number property to the kernel using the serial-number property of the root node, as expected by the kernel.

Quoted 6 lines

The serial number is a string that somewhat represents the device's serial number. It might come from some form of storage (e.g. an eeprom) and be programmed at factory-time by the manufacturer or come from identification bits available in e.g. the SoC.
Signed-off-by Paul Kocialkowski <[email protected]>

Reviewed-by Simon Glass <[email protected]>

But please see a nit below.

Quoted 72 lines

---
 common/fdt_support.c              | 25 +++++++++++++++++++++++++
 common/image-fdt.c                |  4 ++++
 doc/device-tree-bindings/root.txt |  4 ++++
 include/fdt_support.h             |  1 +
 4 files changed, 34 insertions(+)
 create mode 100644 doc/device-tree-bindings/root.txt

common/fdt_support.c +25 -0

index 9e50148..10648b5 100644
@@ -194,6 +194,31 @@ static inline int fdt_setprop_uxx(void *fdt, int nodeoffset, const char *name,
                return fdt_setprop_u32(fdt, nodeoffset, name, (uint32_t)val);
 }
 
+int fdt_root(void *fdt)
+{
+       char *serial;
+       int err;
+
+       err = fdt_check_header(fdt);
+       if (err < 0) {
+               printf("fdt_root: %s\n", fdt_strerror(err));
+               return err;
+       }
+
+       serial = getenv("serial#");
+       if (serial) {
+               err = fdt_setprop(fdt, 0, "serial-number", serial,
+                                 strlen(serial) + 1);
+
+               if (err < 0) {
+                       printf("WARNING: could not set serial-number %s.\n",
+                              fdt_strerror(err));
+                       return err;
+               }
+       }
+
+       return 0;
+}
 
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end)
 {

common/image-fdt.c +4 -0

index 7e2da7b..80e3e63 100644
@@ -471,6 +471,10 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob,
        int ret = -EPERM;
        int fdt_ret;
 
+       if (fdt_root(blob) < 0) {
+               printf("ERROR: root node setup failed\n");
+               goto err;
+       }
        if (fdt_chosen(blob) < 0) {
                printf("ERROR: /chosen node create failed\n");
                goto err;

doc/device-tree-bindings/root.txt new file +4 -0

new file mode 100644
index 0000000..001ccf3
@@ -0,0 +1,4 @@
+The root node
+
+Optional properties:
+  - serial-number : a string representing the device's serial number

include/fdt_support.h +0 -1

index 5d4f28d..56185c9 100644
--- a/include/fdt_support.h

Quoted 8 lines

include/fdt_support.h u32 fdt_getprop_u32_default_node(const void *fdt, int off, int cell, · line 19
Quoted 6 lines
+++ b/include/fdt_support.h
include/fdt_support.h +1 -0
@@ -16,6 +16,7 @@ u32 fdt_getprop_u32_default_node(const void *fdt, int off, int cell,
                                const char *prop, const u32 dflt);
 u32 fdt_getprop_u32_default(const void *fdt, const char *path,
                                const char *prop, const u32 dflt);
+int fdt_root(void *fdt);
Please can you add a comment for this in the standard style?

As far as I can see, fdt_root plays a similar role to fdt_chosen and fdt_initrd, all of which are defined in fdt_support.c and used in image-fdt.c's image_setup_libfdt.

Their prototypes are defined in fdt_support.h and neither fdt_chosen nor fdt_initrd have such a comment, so I didn't think it was very consistent to add one for fdt_root when writing the patch.

Now if you think it's worth adding such a comment for the sake of documentation, I don't object to it but it still leaves me with a feeling of inconsistency with regard to other similar prototypes.

Quoted 9 lines

Quoted 6 lines

include/fdt_support.h +0 -1

 int fdt_chosen(void *fdt);
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,
--

1.9.1

Regards,
Simon

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150522/3ca6f70d/attachment.sig>

Simon Glass

2015-05-22 16:12 UTC #3 lore raw

PATCH v2 1/1 fdt: Pass the device serial number through devicetree

Hi Paul,

Quoted 107 lines · Paul Kocialkowski

Le jeudi 21 mai 2015 ? 14:32 -0600, Simon Glass a ?crit :

Quoted 92 lines

Hi Paul,

Quoted 6 lines · Paul Kocialkowski

Before device-tree, the device serial number used to be passed to the kernel using ATAGs (on ARM). This is now deprecated and all the handover to the kernel should now be done using device-tree. Thus, this passes the serial-number property to the kernel using the serial-number property of the root node, as expected by the kernel.

Quoted 6 lines

The serial number is a string that somewhat represents the device's serial number. It might come from some form of storage (e.g. an eeprom) and be programmed at factory-time by the manufacturer or come from identification bits available in e.g. the SoC.
Signed-off-by Paul Kocialkowski <[email protected]>

Reviewed-by Simon Glass <[email protected]>

But please see a nit below.

Quoted 72 lines

---
 common/fdt_support.c              | 25 +++++++++++++++++++++++++
 common/image-fdt.c                |  4 ++++
 doc/device-tree-bindings/root.txt |  4 ++++
 include/fdt_support.h             |  1 +
 4 files changed, 34 insertions(+)
 create mode 100644 doc/device-tree-bindings/root.txt

common/fdt_support.c +25 -0

index 9e50148..10648b5 100644
@@ -194,6 +194,31 @@ static inline int fdt_setprop_uxx(void *fdt, int nodeoffset, const char *name,
                return fdt_setprop_u32(fdt, nodeoffset, name, (uint32_t)val);
 }
 
+int fdt_root(void *fdt)
+{
+       char *serial;
+       int err;
+
+       err = fdt_check_header(fdt);
+       if (err < 0) {
+               printf("fdt_root: %s\n", fdt_strerror(err));
+               return err;
+       }
+
+       serial = getenv("serial#");
+       if (serial) {
+               err = fdt_setprop(fdt, 0, "serial-number", serial,
+                                 strlen(serial) + 1);
+
+               if (err < 0) {
+                       printf("WARNING: could not set serial-number %s.\n",
+                              fdt_strerror(err));
+                       return err;
+               }
+       }
+
+       return 0;
+}
 
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end)
 {

common/image-fdt.c +4 -0

index 7e2da7b..80e3e63 100644
@@ -471,6 +471,10 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob,
        int ret = -EPERM;
        int fdt_ret;
 
+       if (fdt_root(blob) < 0) {
+               printf("ERROR: root node setup failed\n");
+               goto err;
+       }
        if (fdt_chosen(blob) < 0) {
                printf("ERROR: /chosen node create failed\n");
                goto err;

doc/device-tree-bindings/root.txt new file +4 -0

new file mode 100644
index 0000000..001ccf3
@@ -0,0 +1,4 @@
+The root node
+
+Optional properties:
+  - serial-number : a string representing the device's serial number

include/fdt_support.h +0 -1

index 5d4f28d..56185c9 100644
--- a/include/fdt_support.h

Quoted 8 lines

include/fdt_support.h u32 fdt_getprop_u32_default_node(const void *fdt, int off, int cell, · line 19
Quoted 6 lines
+++ b/include/fdt_support.h
include/fdt_support.h +1 -0
@@ -16,6 +16,7 @@ u32 fdt_getprop_u32_default_node(const void *fdt, int off, int cell,
                                const char *prop, const u32 dflt);
 u32 fdt_getprop_u32_default(const void *fdt, const char *path,
                                const char *prop, const u32 dflt);
+int fdt_root(void *fdt);
Please can you add a comment for this in the standard style?

As far as I can see, fdt_root plays a similar role to fdt_chosen and fdt_initrd, all of which are defined in fdt_support.c and used in image-fdt.c's image_setup_libfdt.

Quoted 7 lines

Their prototypes are defined in fdt_support.h and neither fdt_chosen nor fdt_initrd have such a comment, so I didn't think it was very consistent to add one for fdt_root when writing the patch.
Now if you think it's worth adding such a comment for the sake of documentation, I don't object to it but it still leaves me with a feeling of inconsistency with regard to other similar prototypes.

You want uncommented code for consistency?? :-)

If you are concerned, you could add a new patch that adds comments for some other functions in fdt_support.h. That would be welcome. But at least, please add comments for new code.

The problem with that is that no one is going to go through and comment all these APIs. Generally the only way the code quality improves is when people add new features or refactor the code for some reason.

Quoted 7 lines

include/fdt_support.h +0 -1

 int fdt_chosen(void *fdt);
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,
--

1.9.1

Regards,
Simon

Paul Kocialkowski

author

2015-05-24 10:03 UTC #4 lore raw

PATCH v2 1/1 fdt: Pass the device serial number through devicetree

Hi Simon,

[?]

Quoted 24 lines

Quoted 16 lines
Quoted 11 lines
include/fdt_support.h u32 fdt_getprop_u32_default_node(const void *fdt, int off, int cell, · line 19
Quoted 9 lines
include/fdt_support.h +1 -0
index 5d4f28d..56185c9 100644
@@ -16,6 +16,7 @@ u32 fdt_getprop_u32_default_node(const void *fdt, int off, int cell,
                                const char *prop, const u32 dflt);
 u32 fdt_getprop_u32_default(const void *fdt, const char *path,
                                const char *prop, const u32 dflt);
+int fdt_root(void *fdt);
Please can you add a comment for this in the standard style?
As far as I can see, fdt_root plays a similar role to fdt_chosen and fdt_initrd, all of which are defined in fdt_support.c and used in image-fdt.c's image_setup_libfdt.
Quoted 7 lines
Their prototypes are defined in fdt_support.h and neither fdt_chosen nor fdt_initrd have such a comment, so I didn't think it was very consistent to add one for fdt_root when writing the patch.
Now if you think it's worth adding such a comment for the sake of documentation, I don't object to it but it still leaves me with a feeling of inconsistency with regard to other similar prototypes.

Quoted 5 lines

You want uncommented code for consistency?? :-)
If you are concerned, you could add a new patch that adds comments for some other functions in fdt_support.h. That would be welcome. But at least, please add comments for new code.

Fair enough, I've just sent out another patch that adds (minimalistic) documentation for all 3 functions.

Quoted 4 lines

The problem with that is that no one is going to go through and comment all these APIs. Generally the only way the code quality improves is when people add new features or refactor the code for some reason.

I understand.

Thanks for the review!

Quoted 9 lines

Quoted 6 lines

include/fdt_support.h +0 -1

 int fdt_chosen(void *fdt);
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,
--

1.9.1

Regards,
Simon