From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Kocialkowski Date: Fri, 22 May 2015 12:55:27 +0200 Subject: [U-Boot] [PATCH v2] fdt: Pass the device serial number through devicetree In-Reply-To: References: <1432200423-4092-1-git-send-email-contact@paulk.fr> Message-ID: <1432292127.2510.37.camel@collins> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Le jeudi 21 mai 2015 ? 14:32 -0600, Simon Glass a ?crit : > Hi Paul, > > On 21 May 2015 at 03:27, Paul Kocialkowski wrote: > > 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 > > Reviewed-by: Simon Glass > > But please see a nit below. > > > --- > > 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 > > > > diff --git a/common/fdt_support.c b/common/fdt_support.c > > index 9e50148..10648b5 100644 > > --- a/common/fdt_support.c > > +++ b/common/fdt_support.c > > @@ -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) > > { > > diff --git a/common/image-fdt.c b/common/image-fdt.c > > index 7e2da7b..80e3e63 100644 > > --- a/common/image-fdt.c > > +++ b/common/image-fdt.c > > @@ -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; > > diff --git a/doc/device-tree-bindings/root.txt b/doc/device-tree-bindings/root.txt > > new file mode 100644 > > index 0000000..001ccf3 > > --- /dev/null > > +++ b/doc/device-tree-bindings/root.txt > > @@ -0,0 +1,4 @@ > > +The root node > > + > > +Optional properties: > > + - serial-number : a string representing the device's serial number > > diff --git a/include/fdt_support.h b/include/fdt_support.h > > index 5d4f28d..56185c9 100644 > > --- a/include/fdt_support.h > > +++ b/include/fdt_support.h > > @@ -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. > > 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: