[PATCH 0/4] [RFT] cleanup /chosen/linux,stdout-path warning

[PATCH 0/4] [RFT] cleanup /chosen/linux,stdout-path warning

[Andrew Jones]

On arm we get

"Unable to find /proc/device-tree//chosen/linux,stdout-path, printing
from purgatory is diabled"

when loading a kexec kernel. There are three problems with this warning.

 1) two slashes after device-tree
 2) we should be also checking (and firstly) for /chosen/stdout-path,
    as linux,stdout-path is the deprecated node name. We may actually
    have the former on arm, allowing us to avoid this warning all together.
 3) "disabled" has a typo in it

This series addresses these three problems with the last three patches,
and the first patch removes a compiler warning.

I've added the RFT as these patches touch code shared with other
architectures, namely powerpc, but I only tested on arm.

Please keep me on CC, as I'm not subscribed to the kexec mailing list.

Thanks,
drew

Andrew Jones (4):
  uImage: cleanup some warnings
  kexec/fs2dt: s/diabled/disabled/
  kexec/fs2dt: cleanup pathname
  kexec/fs2dt: check for /chosen/stdout-path first

 kexec/arch/arm/kexec-uImage-arm.c |  3 ++-
 kexec/arch/ppc/kexec-uImage-ppc.c |  5 +++--
 kexec/arch/sh/kexec-uImage-sh.c   |  3 ++-
 kexec/fs2dt.c                     | 26 ++++++++++++++------------
 4 files changed, 21 insertions(+), 16 deletions(-)

-- 
2.4.3


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 1/4] uImage: cleanup some warnings

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 kexec/arch/arm/kexec-uImage-arm.c | 3 ++-
 kexec/arch/ppc/kexec-uImage-ppc.c | 5 +++--
 kexec/arch/sh/kexec-uImage-sh.c   | 3 ++-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/kexec/arch/arm/kexec-uImage-arm.c b/kexec/arch/arm/kexec-uImage-arm.c
index 03c2f4ddca7b0..285883eb9779d 100644
--- a/kexec/arch/arm/kexec-uImage-arm.c
+++ b/kexec/arch/arm/kexec-uImage-arm.c
@@ -11,7 +11,8 @@
 
 int uImage_arm_probe(const char *buf, off_t len)
 {
-	return uImage_probe_kernel(buf, len, IH_ARCH_ARM);
+	return uImage_probe_kernel((const unsigned char *)buf,
+				   len, IH_ARCH_ARM);
 }
 
 int uImage_arm_load(int argc, char **argv, const char *buf, off_t len,
diff --git a/kexec/arch/ppc/kexec-uImage-ppc.c b/kexec/arch/ppc/kexec-uImage-ppc.c
index c89a1a77926d2..30be9d9cb5817 100644
--- a/kexec/arch/ppc/kexec-uImage-ppc.c
+++ b/kexec/arch/ppc/kexec-uImage-ppc.c
@@ -78,7 +78,8 @@ char *slurp_ramdisk_ppc(const char *filename, off_t *r_size)
 	
 int uImage_ppc_probe(const char *buf, off_t len)
 {
-	return uImage_probe_kernel(buf, len, IH_ARCH_PPC);
+	return uImage_probe_kernel((const unsigned char *)buf,
+				   len, IH_ARCH_PPC);
 }
 
 static int ppc_load_bare_bits(int argc, char **argv, const char *buf,
@@ -316,7 +317,7 @@ int uImage_ppc_load(int argc, char **argv, const char *buf, off_t len,
 	struct Image_info img;
 	int ret;
 
-	ret = uImage_load(buf, len, &img);
+	ret = uImage_load((const unsigned char *)buf, len, &img);
 	if (ret)
 		return ret;
 
diff --git a/kexec/arch/sh/kexec-uImage-sh.c b/kexec/arch/sh/kexec-uImage-sh.c
index 130f12ca94317..28c8f2fde3a35 100644
--- a/kexec/arch/sh/kexec-uImage-sh.c
+++ b/kexec/arch/sh/kexec-uImage-sh.c
@@ -13,7 +13,8 @@
 
 int uImage_sh_probe(const char *buf, off_t len)
 {
-	return uImage_probe_kernel(buf, len, IH_ARCH_SH);
+	return uImage_probe_kernel((const unsigned char *)buf,
+				   len, IH_ARCH_SH);
 }
 
 int uImage_sh_load(int argc, char **argv, const char *buf, off_t len,
-- 
2.4.3


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/4] uImage: cleanup some warnings

[Simon Horman]

I don't see any value in masking warnings with casts.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/4] uImage: cleanup some warnings

[Andrew Jones]

On Tue, Nov 24, 2015 at 04:00:26PM +0900, Simon Horman wrote:
> I don't see any value in masking warnings with casts.

I assume you mean that you would prefer to either change the callers' type
or uImage's API in order to avoid the need for casts. Choosing to keep
warnings wouldn't make sense; they add noise making it difficult to spot
warnings that point out real problems, and you can't ever turn on -Werror.

In this case I think casts are the right solution. We shouldn't change the
uImage API, as unsigned char is the type generally used for an arbitrary
data stream. The callers could possibly be changed, but they're also free
to use whatever type they want, signed char may well be what they want.
Indeed, the fact that each caller (architecture) has its own wrapper
around the uImage API calls allows for this very thing. Furthermore, an
explicit cast acknowledges that the two types were both chosen by design.
If this patch is wrong, then the warning has done its job and we need to
change the callers.

Thanks,
drew

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 2/4] kexec/fs2dt: s/diabled/disabled/

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 kexec/fs2dt.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kexec/fs2dt.c b/kexec/fs2dt.c
index 0ea785ff4cf80..9392e85d6d5f3 100644
--- a/kexec/fs2dt.c
+++ b/kexec/fs2dt.c
@@ -659,12 +659,12 @@ static void putnode(void)
 		snprintf(filename, MAXPATH, "%slinux,stdout-path", pathname);
 		fd = open(filename, O_RDONLY);
 		if (fd == -1) {
-			printf("Unable to find %s, printing from purgatory is diabled\n",
+			printf("Unable to find %s, printing from purgatory is disabled\n",
 														filename);
 			goto no_debug;
 		}
 		if (fstat(fd, &statbuf)) {
-			printf("Unable to stat %s, printing from purgatory is diabled\n",
+			printf("Unable to stat %s, printing from purgatory is disabled\n",
 														filename);
 			close(fd);
 			goto no_debug;
@@ -680,19 +680,19 @@ static void putnode(void)
 		result = read(fd, buff, statbuf.st_size);
 		close(fd);
 		if (result <= 0) {
-			printf("Unable to read %s, printing from purgatory is diabled\n",
+			printf("Unable to read %s, printing from purgatory is disabled\n",
 														filename);
 			goto no_debug;
 		}
 		snprintf(filename, MAXPATH, "/proc/device-tree/%s/compatible", buff);
 		fd = open(filename, O_RDONLY);
 		if (fd == -1) {
-			printf("Unable to find %s printing from purgatory is diabled\n",
+			printf("Unable to find %s printing from purgatory is disabled\n",
 														filename);
 			goto no_debug;
 		}
 		if (fstat(fd, &statbuf)) {
-			printf("Unable to stat %s printing from purgatory is diabled\n",
+			printf("Unable to stat %s printing from purgatory is disabled\n",
 														filename);
 			close(fd);
 			goto no_debug;
-- 
2.4.3


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/4] kexec/fs2dt: s/diabled/disabled/

[Simon Horman]

Thanks, applied.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 3/4] kexec/fs2dt: cleanup pathname

[Andrew Jones]

putnode() will add the trailing '/', avoid having two. Also
pathstart is unused, get rid of it.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 kexec/fs2dt.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kexec/fs2dt.c b/kexec/fs2dt.c
index 9392e85d6d5f3..affee57b9bb62 100644
--- a/kexec/fs2dt.c
+++ b/kexec/fs2dt.c
@@ -39,7 +39,7 @@
 #define MEMRESERVE 256		/* max number of reserved memory blocks */
 #define MEM_RANGE_CHUNK_SZ 2048 /* Initial num dwords for mem ranges */
 
-static char pathname[MAXPATH], *pathstart;
+static char pathname[MAXPATH];
 static char propnames[NAMESPACE] = { 0 };
 static unsigned *dt_base, *dt;
 static unsigned int dt_cur_size;
@@ -800,9 +800,7 @@ static void add_boot_block(char **bufp, off_t *sizep)
 
 void create_flatten_tree(char **bufp, off_t *sizep, const char *cmdline)
 {
-	strcpy(pathname, "/proc/device-tree/");
-
-	pathstart = pathname + strlen(pathname);
+	strcpy(pathname, "/proc/device-tree");
 
 	dt_cur_size = INIT_TREE_WORDS;
 	dt_base = malloc(dt_cur_size*4);
-- 
2.4.3


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 3/4] kexec/fs2dt: cleanup pathname

[Simon Horman]

On Fri, Nov 20, 2015 at 12:31:53PM -0500, Andrew Jones wrote:
> putnode() will add the trailing '/', avoid having two. Also
> pathstart is unused, get rid of it.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Thanks, applied.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 4/4] kexec/fs2dt: check for /chosen/stdout-path first

[Andrew Jones]

Check /chosen/stdout-path first, as linux,stdout-path is deprecated.
I don't know how the ppc64:my_debug thing works, but on arm the warning
"Unable to find /proc/device-tree//chosen/linux,stdout-path, printing
from purgatory is diabled" is output when loading a kexec kernel. This
patch at least suppresses that when /chosen/stdout-path exists, and
maybe it even enables printing from purgatory?

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 kexec/fs2dt.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kexec/fs2dt.c b/kexec/fs2dt.c
index affee57b9bb62..b3c209b871a15 100644
--- a/kexec/fs2dt.c
+++ b/kexec/fs2dt.c
@@ -656,12 +656,16 @@ static void putnode(void)
 		 * code can print 'I'm in purgatory' message. Currently only
 		 * pseries/hvcterminal is supported.
 		 */
-		snprintf(filename, MAXPATH, "%slinux,stdout-path", pathname);
+		snprintf(filename, MAXPATH, "%sstdout-path", pathname);
 		fd = open(filename, O_RDONLY);
 		if (fd == -1) {
-			printf("Unable to find %s, printing from purgatory is disabled\n",
-														filename);
-			goto no_debug;
+			snprintf(filename, MAXPATH, "%slinux,stdout-path", pathname);
+			fd = open(filename, O_RDONLY);
+			if (fd == -1) {
+				printf("Unable to find %s[linux,]stdout-path, printing from purgatory is disabled\n",
+														pathname);
+				goto no_debug;
+			}
 		}
 		if (fstat(fd, &statbuf)) {
 			printf("Unable to stat %s, printing from purgatory is disabled\n",
-- 
2.4.3


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/4] kexec/fs2dt: check for /chosen/stdout-path first

[Simon Horman]

On Fri, Nov 20, 2015 at 12:31:54PM -0500, Andrew Jones wrote:
> Check /chosen/stdout-path first, as linux,stdout-path is deprecated.
> I don't know how the ppc64:my_debug thing works, but on arm the warning
> "Unable to find /proc/device-tree//chosen/linux,stdout-path, printing
> from purgatory is diabled" is output when loading a kexec kernel. This
> patch at least suppresses that when /chosen/stdout-path exists, and
> maybe it even enables printing from purgatory?
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Thanks, applied.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec