All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] powerpc/zImage: Remove #ifdef in opal.c
@ 2018-03-19  5:13 Oliver O'Halloran
  2018-03-19  5:14 ` [PATCH 2/5] powerpc/zImage: Don't build zlib object files Oliver O'Halloran
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Oliver O'Halloran @ 2018-03-19  5:13 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran

This file is only ever compiled if CONFIG_PPC64_BOOT_WRAPPER is set
so this check is unnecessary.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/boot/opal.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/powerpc/boot/opal.c b/arch/powerpc/boot/opal.c
index 0272570d02de..dfb199ef5b94 100644
--- a/arch/powerpc/boot/opal.c
+++ b/arch/powerpc/boot/opal.c
@@ -13,8 +13,6 @@
 #include <libfdt.h>
 #include "../include/asm/opal-api.h"
 
-#ifdef CONFIG_PPC64_BOOT_WRAPPER
-
 /* Global OPAL struct used by opal-call.S */
 struct opal {
 	u64 base;
@@ -101,9 +99,3 @@ int opal_console_init(void *devp, struct serial_console_data *scdp)
 
 	return 0;
 }
-#else
-int opal_console_init(void *devp, struct serial_console_data *scdp)
-{
-	return -1;
-}
-#endif /* __powerpc64__ */
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 2/5] powerpc/zImage: Don't build zlib object files
  2018-03-19  5:13 [PATCH 1/5] powerpc/zImage: Remove #ifdef in opal.c Oliver O'Halloran
@ 2018-03-19  5:14 ` Oliver O'Halloran
  2018-03-19  5:14 ` [PATCH 3/5] powerpc/zImage: Check compatible at driver init Oliver O'Halloran
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Oliver O'Halloran @ 2018-03-19  5:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran

The required source files are directly #include`ed into
decompress.c when using gzip compression. Building the
separate .o files for the zlib sources isn't required
and can cause linker errors due to the symbols being
defined in decompress.o and the zlib .o files.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
You can stop throwing up now
---
 arch/powerpc/boot/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 26d5d2a5b8e9..f66b9dba99fd 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -108,7 +108,7 @@ $(addprefix $(obj)/,$(libfdt) libfdt-wrapper.o simpleboot.o epapr.o opal.o \
 src-wlib-y := string.S crt0.S stdio.c decompress.c main.c \
 		$(libfdt) libfdt-wrapper.c \
 		ns16550.c serial.c simple_alloc.c div64.S util.S \
-		elf_util.c $(zlib-y) devtree.c stdlib.c \
+		elf_util.c devtree.c stdlib.c \
 		oflib.c ofconsole.c cuboot.c
 
 src-wlib-$(CONFIG_PPC_MPC52XX) += mpc52xx-psc.c
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 3/5] powerpc/zImage: Check compatible at driver init
  2018-03-19  5:13 [PATCH 1/5] powerpc/zImage: Remove #ifdef in opal.c Oliver O'Halloran
  2018-03-19  5:14 ` [PATCH 2/5] powerpc/zImage: Don't build zlib object files Oliver O'Halloran
@ 2018-03-19  5:14 ` Oliver O'Halloran
  2018-03-20  9:19   ` Michael Ellerman
  2018-03-19  5:14 ` [PATCH 4/5] powerpc/zImage: Rework serial driver probing Oliver O'Halloran
  2018-03-19  5:14 ` [PATCH 5/5] powerpc/zImage: Also check for stdout-path Oliver O'Halloran
  3 siblings, 1 reply; 12+ messages in thread
From: Oliver O'Halloran @ 2018-03-19  5:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran

Have each driver's init function check the compatible string
of the node given by the stdout path. This gives the drivers
a more traditional probe-also-inits structure.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/boot/cpm-serial.c  | 2 ++
 arch/powerpc/boot/mpc52xx-psc.c | 3 +++
 arch/powerpc/boot/mpsc.c        | 3 +++
 arch/powerpc/boot/ns16550.c     | 4 ++++
 arch/powerpc/boot/opal.c        | 3 +++
 arch/powerpc/boot/uartlite.c    | 5 +++++
 6 files changed, 20 insertions(+)

diff --git a/arch/powerpc/boot/cpm-serial.c b/arch/powerpc/boot/cpm-serial.c
index dfb56829cace..f6b1cf23946e 100644
--- a/arch/powerpc/boot/cpm-serial.c
+++ b/arch/powerpc/boot/cpm-serial.c
@@ -212,6 +212,8 @@ int cpm_console_init(void *devp, struct serial_console_data *scdp)
 	} else if (dt_is_compatible(devp, "fsl,cpm2-smc-uart")) {
 		is_cpm2 = 1;
 		is_smc = 1;
+	} else if (!dt_is_compatible(devp, "fsl,cpm1-scc-uart")) {
+		return 1; /* not compatible */
 	}
 
 	if (is_smc) {
diff --git a/arch/powerpc/boot/mpc52xx-psc.c b/arch/powerpc/boot/mpc52xx-psc.c
index c2c08633ee35..75470936e661 100644
--- a/arch/powerpc/boot/mpc52xx-psc.c
+++ b/arch/powerpc/boot/mpc52xx-psc.c
@@ -52,6 +52,9 @@ static unsigned char psc_getc(void)
 
 int mpc5200_psc_console_init(void *devp, struct serial_console_data *scdp)
 {
+	if (!dt_is_compatible(devp, "fsl,mpc5200-psc-uart"))
+		return 1;
+
 	/* Get the base address of the psc registers */
 	if (dt_get_virtual_reg(devp, &psc, 1) < 1)
 		return -1;
diff --git a/arch/powerpc/boot/mpsc.c b/arch/powerpc/boot/mpsc.c
index 425ad88cce8d..59e23e886440 100644
--- a/arch/powerpc/boot/mpsc.c
+++ b/arch/powerpc/boot/mpsc.c
@@ -128,6 +128,9 @@ int mpsc_console_init(void *devp, struct serial_console_data *scdp)
 	int n, reg_set;
 	volatile char *sdma_base;
 
+	if (!dt_is_compatible(devp, "marvell,mv64360-mpsc"))
+		return 1;
+
 	n = getprop(devp, "virtual-reg", &v, sizeof(v));
 	if (n != sizeof(v))
 		goto err_out;
diff --git a/arch/powerpc/boot/ns16550.c b/arch/powerpc/boot/ns16550.c
index b0da4466d419..0e7bd5284255 100644
--- a/arch/powerpc/boot/ns16550.c
+++ b/arch/powerpc/boot/ns16550.c
@@ -58,6 +58,10 @@ int ns16550_console_init(void *devp, struct serial_console_data *scdp)
 	int n;
 	u32 reg_offset;
 
+	if (!dt_is_compatible(devp, "ns16550") &&
+	    !dt_is_compatible(devp, "pnpPNP,501"))
+		return 1;
+
 	if (dt_get_virtual_reg(devp, (void **)&reg_base, 1) < 1)
 		return -1;
 
diff --git a/arch/powerpc/boot/opal.c b/arch/powerpc/boot/opal.c
index dfb199ef5b94..0e92537536b9 100644
--- a/arch/powerpc/boot/opal.c
+++ b/arch/powerpc/boot/opal.c
@@ -83,6 +83,9 @@ static void opal_init(void)
 
 int opal_console_init(void *devp, struct serial_console_data *scdp)
 {
+	if (!dt_is_compatible(devp, "ibm,opal-console-raw"))
+		return 1;
+
 	opal_init();
 
 	if (devp) {
diff --git a/arch/powerpc/boot/uartlite.c b/arch/powerpc/boot/uartlite.c
index 46bed69b4169..fc66a5fcea66 100644
--- a/arch/powerpc/boot/uartlite.c
+++ b/arch/powerpc/boot/uartlite.c
@@ -62,6 +62,11 @@ int uartlite_console_init(void *devp, struct serial_console_data *scdp)
 	int n;
 	unsigned long reg_phys;
 
+
+	if (!dt_is_compatible(devp, "xlnx,opb-uartlite-1.00.b") &&
+	    !dt_is_compatible(devp, "xlnx,xps-uartlite-1.00.a"))
+		return 1;
+
 	n = getprop(devp, "virtual-reg", &reg_base, sizeof(reg_base));
 	if (n != sizeof(reg_base)) {
 		if (!dt_xlate_reg(devp, 0, &reg_phys, NULL))
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 4/5] powerpc/zImage: Rework serial driver probing
  2018-03-19  5:13 [PATCH 1/5] powerpc/zImage: Remove #ifdef in opal.c Oliver O'Halloran
  2018-03-19  5:14 ` [PATCH 2/5] powerpc/zImage: Don't build zlib object files Oliver O'Halloran
  2018-03-19  5:14 ` [PATCH 3/5] powerpc/zImage: Check compatible at driver init Oliver O'Halloran
@ 2018-03-19  5:14 ` Oliver O'Halloran
  2018-03-20  7:50   ` kbuild test robot
                     ` (2 more replies)
  2018-03-19  5:14 ` [PATCH 5/5] powerpc/zImage: Also check for stdout-path Oliver O'Halloran
  3 siblings, 3 replies; 12+ messages in thread
From: Oliver O'Halloran @ 2018-03-19  5:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran

Rather than checking the compatible string in serial_driver_init()
we call into the driver's init function and wait for a driver to
inidicate it bound to the device by returning zero.

The pointers to each driver probe functions are stored in the
".serialdrv" section of the zImage, similar to how initcalls and
whatnot are handled inside Linux. This structure allows us to
conditionally compile specific driver files based on the KConfig
settings. This is needed because we don't have access to the
KConfig #defines in the zImage source files (it's a giant #include
headache) so conditional compilation is the only way to eliminate
unnecessary code.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/boot/cpm-serial.c      |  1 +
 arch/powerpc/boot/mpc52xx-psc.c     |  1 +
 arch/powerpc/boot/mpsc.c            |  1 +
 arch/powerpc/boot/ns16550.c         |  2 ++
 arch/powerpc/boot/opal.c            |  1 +
 arch/powerpc/boot/ops.h             | 16 +++++++++------
 arch/powerpc/boot/serial.c          | 41 +++++++++++--------------------------
 arch/powerpc/boot/uartlite.c        |  1 +
 arch/powerpc/boot/wrapper           |  2 +-
 arch/powerpc/boot/zImage.coff.lds.S |  4 ++++
 arch/powerpc/boot/zImage.lds.S      |  8 ++++++++
 11 files changed, 42 insertions(+), 36 deletions(-)

diff --git a/arch/powerpc/boot/cpm-serial.c b/arch/powerpc/boot/cpm-serial.c
index f6b1cf23946e..5ebf3dfd810a 100644
--- a/arch/powerpc/boot/cpm-serial.c
+++ b/arch/powerpc/boot/cpm-serial.c
@@ -295,3 +295,4 @@ int cpm_console_init(void *devp, struct serial_console_data *scdp)
 
 	return 0;
 }
+SERIAL_DRIVER(cpm_console_init);
diff --git a/arch/powerpc/boot/mpc52xx-psc.c b/arch/powerpc/boot/mpc52xx-psc.c
index 75470936e661..c09e82eaf006 100644
--- a/arch/powerpc/boot/mpc52xx-psc.c
+++ b/arch/powerpc/boot/mpc52xx-psc.c
@@ -66,3 +66,4 @@ int mpc5200_psc_console_init(void *devp, struct serial_console_data *scdp)
 
 	return 0;
 }
+SERIAL_DRIVER(mpc5200_psc_console_init);
diff --git a/arch/powerpc/boot/mpsc.c b/arch/powerpc/boot/mpsc.c
index 59e23e886440..2d0a72522b3c 100644
--- a/arch/powerpc/boot/mpsc.c
+++ b/arch/powerpc/boot/mpsc.c
@@ -170,3 +170,4 @@ int mpsc_console_init(void *devp, struct serial_console_data *scdp)
 err_out:
 	return -1;
 }
+SERIAL_DRIVER(mpsc_console_init);
diff --git a/arch/powerpc/boot/ns16550.c b/arch/powerpc/boot/ns16550.c
index 0e7bd5284255..49e8c299b829 100644
--- a/arch/powerpc/boot/ns16550.c
+++ b/arch/powerpc/boot/ns16550.c
@@ -81,3 +81,5 @@ int ns16550_console_init(void *devp, struct serial_console_data *scdp)
 
 	return 0;
 }
+
+SERIAL_DRIVER(ns16550_console_init);
diff --git a/arch/powerpc/boot/opal.c b/arch/powerpc/boot/opal.c
index 0e92537536b9..dc345ff63422 100644
--- a/arch/powerpc/boot/opal.c
+++ b/arch/powerpc/boot/opal.c
@@ -102,3 +102,4 @@ int opal_console_init(void *devp, struct serial_console_data *scdp)
 
 	return 0;
 }
+SERIAL_DRIVER(opal_console_init);
diff --git a/arch/powerpc/boot/ops.h b/arch/powerpc/boot/ops.h
index fad1862f4b2d..06151ecb2f45 100644
--- a/arch/powerpc/boot/ops.h
+++ b/arch/powerpc/boot/ops.h
@@ -84,13 +84,17 @@ extern struct loader_info loader_info;
 
 void start(void);
 void fdt_init(void *blob);
+
+struct driver_ptr {
+	int (*ptr)(void *node, struct serial_console_data *scdp);
+};
+
+#define SERIAL_DRIVER(init_function) static const struct driver_ptr \
+	__attribute__((__section__(".serialdrv"))) __attribute__((used)) \
+	init_function##_ptr = { .ptr = &init_function, }
+
 int serial_console_init(void);
-int ns16550_console_init(void *devp, struct serial_console_data *scdp);
-int mpsc_console_init(void *devp, struct serial_console_data *scdp);
-int cpm_console_init(void *devp, struct serial_console_data *scdp);
-int mpc5200_psc_console_init(void *devp, struct serial_console_data *scdp);
-int uartlite_console_init(void *devp, struct serial_console_data *scdp);
-int opal_console_init(void *devp, struct serial_console_data *scdp);
+
 void *simple_alloc_init(char *base, unsigned long heap_size,
 			unsigned long granularity, unsigned long max_allocs);
 extern void flush_cache(void *, unsigned long);
diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
index 88955095ec07..a7f9ac9a27b5 100644
--- a/arch/powerpc/boot/serial.c
+++ b/arch/powerpc/boot/serial.c
@@ -105,11 +105,15 @@ static void *serial_get_stdout_devp(void)
 	return NULL;
 }
 
+extern unsigned long _serial_drv_start;
+extern unsigned long _serial_drv_end;
+
 static struct serial_console_data serial_cd;
 
 /* Node's "compatible" property determines which serial driver to use */
 int serial_console_init(void)
 {
+	struct driver_ptr *start, *end, *probe;
 	void *devp;
 	int rc = -1;
 
@@ -117,35 +121,14 @@ int serial_console_init(void)
 	if (devp == NULL)
 		goto err_out;
 
-	if (dt_is_compatible(devp, "ns16550") ||
-	    dt_is_compatible(devp, "pnpPNP,501"))
-		rc = ns16550_console_init(devp, &serial_cd);
-#ifdef CONFIG_EMBEDDED6xx
-	else if (dt_is_compatible(devp, "marvell,mv64360-mpsc"))
-		rc = mpsc_console_init(devp, &serial_cd);
-#endif
-#ifdef CONFIG_CPM
-	else if (dt_is_compatible(devp, "fsl,cpm1-scc-uart") ||
-	         dt_is_compatible(devp, "fsl,cpm1-smc-uart") ||
-	         dt_is_compatible(devp, "fsl,cpm2-scc-uart") ||
-	         dt_is_compatible(devp, "fsl,cpm2-smc-uart"))
-		rc = cpm_console_init(devp, &serial_cd);
-#endif
-#ifdef CONFIG_PPC_MPC52XX
-	else if (dt_is_compatible(devp, "fsl,mpc5200-psc-uart"))
-		rc = mpc5200_psc_console_init(devp, &serial_cd);
-#endif
-#ifdef CONFIG_XILINX_VIRTEX
-	else if (dt_is_compatible(devp, "xlnx,opb-uartlite-1.00.b") ||
-		 dt_is_compatible(devp, "xlnx,xps-uartlite-1.00.a"))
-		rc = uartlite_console_init(devp, &serial_cd);
-#endif
-#ifdef CONFIG_PPC64_BOOT_WRAPPER
-	else if (dt_is_compatible(devp, "ibm,opal-console-raw"))
-		rc = opal_console_init(devp, &serial_cd);
-#endif
-
-	/* Add other serial console driver calls here */
+	start = (void *) &_serial_drv_start;
+	end = (void *) &_serial_drv_end;
+
+	for (probe = start; probe < end; probe++) {
+		rc = probe->ptr(devp, &serial_cd);
+		if (rc <= 0)
+			break;
+	}
 
 	if (!rc) {
 		console_ops.open = serial_open;
diff --git a/arch/powerpc/boot/uartlite.c b/arch/powerpc/boot/uartlite.c
index fc66a5fcea66..92d4f00bf626 100644
--- a/arch/powerpc/boot/uartlite.c
+++ b/arch/powerpc/boot/uartlite.c
@@ -82,3 +82,4 @@ int uartlite_console_init(void *devp, struct serial_console_data *scdp)
 	scdp->close = NULL;
 	return 0;
 }
+SERIAL_DRIVER(uartlite_console_init);
diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index 76fe3ccfd381..b5c887ae729d 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -469,7 +469,7 @@ if [ "$platform" != "miboot" ]; then
     fi
 #link everything
     ${CROSS}ld -m $format -T $lds $text_start $pie $nodl -o "$ofile" \
-	$platformo $tmp $object/wrapper.a
+	$platformo $tmp --whole-archive $object/wrapper.a
     rm $tmp
 fi
 
diff --git a/arch/powerpc/boot/zImage.coff.lds.S b/arch/powerpc/boot/zImage.coff.lds.S
index 117951295117..d667fe3f5999 100644
--- a/arch/powerpc/boot/zImage.coff.lds.S
+++ b/arch/powerpc/boot/zImage.coff.lds.S
@@ -20,6 +20,10 @@ SECTIONS
     *(.sdata*)
     *(.got2)
 
+    _serial_drv_start = .;
+    *(.serialdrv)
+    _serial_drv_end = .;
+
     _dtb_start = .;
     *(.kernel:dtb)
     _dtb_end = .;
diff --git a/arch/powerpc/boot/zImage.lds.S b/arch/powerpc/boot/zImage.lds.S
index 4ac1e36edfe7..9dda24bcee00 100644
--- a/arch/powerpc/boot/zImage.lds.S
+++ b/arch/powerpc/boot/zImage.lds.S
@@ -45,6 +45,14 @@ SECTIONS
   }
 
   . = ALIGN(8);
+  .serialdrv :
+  {
+    _serial_drv_start = .;
+    KEEP(*(.serialdrv));
+    _serial_drv_end = .;
+  }
+
+  . = ALIGN(8);
   .kernel:dtb :
   {
     _dtb_start = .;
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 5/5] powerpc/zImage: Also check for stdout-path
  2018-03-19  5:13 [PATCH 1/5] powerpc/zImage: Remove #ifdef in opal.c Oliver O'Halloran
                   ` (2 preceding siblings ...)
  2018-03-19  5:14 ` [PATCH 4/5] powerpc/zImage: Rework serial driver probing Oliver O'Halloran
@ 2018-03-19  5:14 ` Oliver O'Halloran
  2018-12-23 13:28   ` [5/5] " Michael Ellerman
  3 siblings, 1 reply; 12+ messages in thread
From: Oliver O'Halloran @ 2018-03-19  5:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran

The /chosen/linux,stdout-path is "deprecated" in favour of
/chosen/stdout-path so we should be checking for both.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/boot/serial.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
index a7f9ac9a27b5..9ef1ed35ae62 100644
--- a/arch/powerpc/boot/serial.c
+++ b/arch/powerpc/boot/serial.c
@@ -92,7 +92,8 @@ static void *serial_get_stdout_devp(void)
 	if (devp == NULL)
 		goto err_out;
 
-	if (getprop(devp, "linux,stdout-path", path, MAX_PATH_LEN) > 0) {
+	if (getprop(devp, "linux,stdout-path", path, MAX_PATH_LEN) > 0 ||
+		getprop(devp, "stdout-path", path, MAX_PATH_LEN) > 0) {
 		devp = finddevice(path);
 		if (devp == NULL)
 			goto err_out;
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] powerpc/zImage: Rework serial driver probing
  2018-03-19  5:14 ` [PATCH 4/5] powerpc/zImage: Rework serial driver probing Oliver O'Halloran
@ 2018-03-20  7:50   ` kbuild test robot
  2018-03-20  8:04   ` kbuild test robot
  2018-03-20  9:20   ` Michael Ellerman
  2 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2018-03-20  7:50 UTC (permalink / raw)
  To: Oliver O'Halloran; +Cc: kbuild-all, linuxppc-dev, Oliver O'Halloran

[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]

Hi Oliver,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180319]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Oliver-O-Halloran/powerpc-zImage-Remove-ifdef-in-opal-c/20180320-023705
config: powerpc64-alldefconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc64 

All errors (new ones prefixed by >>):

   arch/powerpc/boot/wrapper.a(crt0.o): In function `_zimage_start_opd':
>> (.text+0x0): multiple definition of `_zimage_start_opd'
   arch/powerpc/boot/crt0.o:(.text+0x0): first defined here
   arch/powerpc/boot/wrapper.a(crt0.o): In function `_zimage_start':
>> (.text+0x24): multiple definition of `_zimage_start_lib'
   arch/powerpc/boot/crt0.o:(.text+0x24): first defined here

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 8347 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] powerpc/zImage: Rework serial driver probing
  2018-03-19  5:14 ` [PATCH 4/5] powerpc/zImage: Rework serial driver probing Oliver O'Halloran
  2018-03-20  7:50   ` kbuild test robot
@ 2018-03-20  8:04   ` kbuild test robot
  2018-03-20  9:20   ` Michael Ellerman
  2 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2018-03-20  8:04 UTC (permalink / raw)
  To: Oliver O'Halloran; +Cc: kbuild-all, linuxppc-dev, Oliver O'Halloran

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

Hi Oliver,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180319]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Oliver-O-Halloran/powerpc-zImage-Remove-ifdef-in-opal-c/20180320-023705
config: powerpc-ppc64_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

>> arch/powerpc/boot/wrapper.a(serial.o):(.got2+0x1c): undefined reference to `_serial_drv_start'
>> arch/powerpc/boot/wrapper.a(serial.o):(.got2+0x24): undefined reference to `_serial_drv_end'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24150 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] powerpc/zImage: Check compatible at driver init
  2018-03-19  5:14 ` [PATCH 3/5] powerpc/zImage: Check compatible at driver init Oliver O'Halloran
@ 2018-03-20  9:19   ` Michael Ellerman
  2018-03-20 12:56     ` Oliver
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Ellerman @ 2018-03-20  9:19 UTC (permalink / raw)
  To: Oliver O'Halloran, linuxppc-dev; +Cc: Oliver O'Halloran

Oliver O'Halloran <oohall@gmail.com> writes:
> diff --git a/arch/powerpc/boot/mpc52xx-psc.c b/arch/powerpc/boot/mpc52xx-psc.c
> index c2c08633ee35..75470936e661 100644
> --- a/arch/powerpc/boot/mpc52xx-psc.c
> +++ b/arch/powerpc/boot/mpc52xx-psc.c
> @@ -52,6 +52,9 @@ static unsigned char psc_getc(void)
>  
>  int mpc5200_psc_console_init(void *devp, struct serial_console_data *scdp)
>  {
> +	if (!dt_is_compatible(devp, "fsl,mpc5200-psc-uart"))
> +		return 1;

Returning 1 raises the question of whether this is a bool-style function
that returns 1 on success and 0 on failure - or a UNIX style function
that returns 0 for sucesss and 1 on failure.

Can we return ENODEV, or just -1 instead to make it clearer?

cheers

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] powerpc/zImage: Rework serial driver probing
  2018-03-19  5:14 ` [PATCH 4/5] powerpc/zImage: Rework serial driver probing Oliver O'Halloran
  2018-03-20  7:50   ` kbuild test robot
  2018-03-20  8:04   ` kbuild test robot
@ 2018-03-20  9:20   ` Michael Ellerman
  2018-03-20 13:05     ` Oliver
  2 siblings, 1 reply; 12+ messages in thread
From: Michael Ellerman @ 2018-03-20  9:20 UTC (permalink / raw)
  To: Oliver O'Halloran, linuxppc-dev; +Cc: Oliver O'Halloran

Oliver O'Halloran <oohall@gmail.com> writes:

> Rather than checking the compatible string in serial_driver_init()
> we call into the driver's init function and wait for a driver to
> inidicate it bound to the device by returning zero.
>
> The pointers to each driver probe functions are stored in the
> ".serialdrv" section of the zImage, similar to how initcalls and
> whatnot are handled inside Linux. This structure allows us to
> conditionally compile specific driver files based on the KConfig
> settings. This is needed because we don't have access to the
> KConfig #defines in the zImage source files (it's a giant #include
> headache) so conditional compilation is the only way to eliminate
> unnecessary code.

Did you actually get the config.h include working? Or was it such a big
headache that it doesn't even work?

I'm pretty happy with this patch regardless, but it would be simpler and
less bug prone if the CONFIG_ symbols were usable in the boot code.

cheers

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] powerpc/zImage: Check compatible at driver init
  2018-03-20  9:19   ` Michael Ellerman
@ 2018-03-20 12:56     ` Oliver
  0 siblings, 0 replies; 12+ messages in thread
From: Oliver @ 2018-03-20 12:56 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev

On Tue, Mar 20, 2018 at 8:19 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Oliver O'Halloran <oohall@gmail.com> writes:
>> diff --git a/arch/powerpc/boot/mpc52xx-psc.c b/arch/powerpc/boot/mpc52xx-psc.c
>> index c2c08633ee35..75470936e661 100644
>> --- a/arch/powerpc/boot/mpc52xx-psc.c
>> +++ b/arch/powerpc/boot/mpc52xx-psc.c
>> @@ -52,6 +52,9 @@ static unsigned char psc_getc(void)
>>
>>  int mpc5200_psc_console_init(void *devp, struct serial_console_data *scdp)
>>  {
>> +     if (!dt_is_compatible(devp, "fsl,mpc5200-psc-uart"))
>> +             return 1;
>
> Returning 1 raises the question of whether this is a bool-style function
> that returns 1 on success and 0 on failure - or a UNIX style function
> that returns 0 for sucesss and 1 on failure.
>
> Can we return ENODEV, or just -1 instead to make it clearer?

I figured we would want to differentiate between a mis-matched driver
and any other error type. There's no existing definition of ENODEV and
friends in the wrapper so I just used positive 1 as a half-assed
workaround. I don't have particularly strong feelings about it so
adding an explicit ENODEV would make things a bit cleaner. I'll fix it
on the respin.

>
> cheers

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] powerpc/zImage: Rework serial driver probing
  2018-03-20  9:20   ` Michael Ellerman
@ 2018-03-20 13:05     ` Oliver
  0 siblings, 0 replies; 12+ messages in thread
From: Oliver @ 2018-03-20 13:05 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev

On Tue, Mar 20, 2018 at 8:20 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Oliver O'Halloran <oohall@gmail.com> writes:
>
>> Rather than checking the compatible string in serial_driver_init()
>> we call into the driver's init function and wait for a driver to
>> inidicate it bound to the device by returning zero.
>>
>> The pointers to each driver probe functions are stored in the
>> ".serialdrv" section of the zImage, similar to how initcalls and
>> whatnot are handled inside Linux. This structure allows us to
>> conditionally compile specific driver files based on the KConfig
>> settings. This is needed because we don't have access to the
>> KConfig #defines in the zImage source files (it's a giant #include
>> headache) so conditional compilation is the only way to eliminate
>> unnecessary code.
>
> Did you actually get the config.h include working? Or was it such a big
> headache that it doesn't even work?

I had a half-hearted go at it and ran into some problems with include
paths. The usual workaround for that is copying the files into the
build directory and using sed scripts to replace the <> include with a
"" include, so I said screw it and went with this instead. I'm not too
suprised about there being a few linker errors. I tested a few
different defconfigs before posting, but I figured either your CI or
the 0day bot would catch the rest ;)

> I'm pretty happy with this patch regardless, but it would be simpler and
> less bug prone if the CONFIG_ symbols were usable in the boot code.
>
> cheers

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [5/5] powerpc/zImage: Also check for stdout-path
  2018-03-19  5:14 ` [PATCH 5/5] powerpc/zImage: Also check for stdout-path Oliver O'Halloran
@ 2018-12-23 13:28   ` Michael Ellerman
  0 siblings, 0 replies; 12+ messages in thread
From: Michael Ellerman @ 2018-12-23 13:28 UTC (permalink / raw)
  To: Oliver O'Halloran, linuxppc-dev; +Cc: Oliver O'Halloran

On Mon, 2018-03-19 at 05:14:03 UTC, Oliver O'Halloran wrote:
> The /chosen/linux,stdout-path is "deprecated" in favour of
> /chosen/stdout-path so we should be checking for both.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/9bbc7e4ce47ecefa142fcba55eef77

cheers

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-12-23 14:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-19  5:13 [PATCH 1/5] powerpc/zImage: Remove #ifdef in opal.c Oliver O'Halloran
2018-03-19  5:14 ` [PATCH 2/5] powerpc/zImage: Don't build zlib object files Oliver O'Halloran
2018-03-19  5:14 ` [PATCH 3/5] powerpc/zImage: Check compatible at driver init Oliver O'Halloran
2018-03-20  9:19   ` Michael Ellerman
2018-03-20 12:56     ` Oliver
2018-03-19  5:14 ` [PATCH 4/5] powerpc/zImage: Rework serial driver probing Oliver O'Halloran
2018-03-20  7:50   ` kbuild test robot
2018-03-20  8:04   ` kbuild test robot
2018-03-20  9:20   ` Michael Ellerman
2018-03-20 13:05     ` Oliver
2018-03-19  5:14 ` [PATCH 5/5] powerpc/zImage: Also check for stdout-path Oliver O'Halloran
2018-12-23 13:28   ` [5/5] " Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.