[U-Boot] [PATCH v2 0/7] RPi: Properly handle dynamic serial configuration

[U-Boot] [PATCH v2 0/7] RPi: Properly handle dynamic serial configuration

[Alexander Graf]

The RPi has proprietary firmware that can be configured (using config.txt)
to expose either the PL11, Mini-UART or no serial device to the UART pins
on the GPIO pin bar of the RPi.

So far we've only half-heartedly dealt with that, with lost of heuristics
that ended up falling apart at times. For example the RPi3 CM uses PL011
when serial is enabled in config.txt, but we disabled PL11 support for BCM2837
because the RPi3 uses the Mini-UART with enable_uart=1 is set in config.txt.

This patch set always enables both serial outputs and determines at probe
time whether a serial device is actually muxed to the UART pins on the board.
Only in that case, it will be probed and thus used for in- and output in
U-Boot.

With this patch set applied, I have successfully used the same U-Boot binary
with CONFIG_OF_BOARD=y and a RPi firmware provided device tree with both
enable_uart=1 and without on both a RPi3 and RPi3 CM.

This patch set depends on the patch set "Rpi: Add support for second sd host
controller".

v1 -> v2:

  - Make search logic easier to follow

Alexander Graf (7):
  serial: Use next serial device if probing fails
  serial: Allow boards to determine whether a serial device is usable
  rpi: Remove runtime disabling support for serial
  serial: bcm283x_mu: Remove support for post-init disabling
  rpi: Properly detect which serial device is active
  rpi: Determine PL011/Mini-UART availability at runtime
  rpi: Force skip_init on serial devices

 arch/arm/mach-bcm283x/include/mach/gpio.h    |  1 -
 board/raspberrypi/rpi/rpi.c                  | 91 +++++++++++++++++++---------
 drivers/gpio/bcm2835_gpio.c                  |  2 +-
 drivers/serial/serial-uclass.c               | 36 +++++++++--
 drivers/serial/serial_bcm283x_mu.c           | 20 +-----
 drivers/serial/serial_pl01x.c                |  2 +-
 include/configs/rpi.h                        |  5 +-
 include/dm/platform_data/serial_bcm283x_mu.h |  1 -
 include/serial.h                             | 11 ++++
 9 files changed, 108 insertions(+), 61 deletions(-)

-- 
2.12.3

[U-Boot] [PATCH v2 1/7] serial: Use next serial device if probing fails

[Alexander Graf]

Currently our serial device search chokes on the fact that the serial
probe function could fail. If it does, instead of searching for the next
usable serial device, it just quits.

This patch changes the fallback logic so that even when a serial device
was not probed correctly, we just try the next ones until we find one that
works.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - Make search logic easier to follow
---
 drivers/serial/serial-uclass.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 2e5116f7ce..68ca2d09d1 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -74,6 +74,7 @@ static void serial_find_console_or_panic(void)
 {
 	const void *blob = gd->fdt_blob;
 	struct udevice *dev;
+	int ret;
 
 	if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
 		uclass_first_device(UCLASS_SERIAL, &dev);
@@ -104,8 +105,8 @@ static void serial_find_console_or_panic(void)
 		 * from 1!).
 		 *
 		 * Failing that, get the device with sequence number 0, or in
-		 * extremis just the first serial device we can find. But we
-		 * insist on having a console (even if it is silent).
+		 * extremis just the first working serial device we can find.
+		 * But we insist on having a console (even if it is silent).
 		 */
 #ifdef CONFIG_CONS_INDEX
 #define INDEX (CONFIG_CONS_INDEX - 1)
@@ -113,10 +114,22 @@ static void serial_find_console_or_panic(void)
 #define INDEX 0
 #endif
 		if (!uclass_get_device_by_seq(UCLASS_SERIAL, INDEX, &dev) ||
-		    !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) ||
-		    (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) {
-			gd->cur_serial_dev = dev;
-			return;
+		    !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) {
+			if (dev->flags & DM_FLAG_ACTIVATED) {
+				gd->cur_serial_dev = dev;
+				return;
+			}
+		}
+
+		/* Search for any working device */
+		for (ret = uclass_first_device_check(UCLASS_SERIAL, &dev);
+		     dev;
+		     ret = uclass_next_device_check(&dev)) {
+			if (!ret) {
+				/* Device did succeed probing */
+				gd->cur_serial_dev = dev;
+				return;
+			}
 		}
 #undef INDEX
 	}
-- 
2.12.3

[U-Boot] [PATCH v2 2/7] serial: Allow boards to determine whether a serial device is usable

[Alexander Graf]

On some boards, serial devices may or may not be muxed properly to actual
pins, depending on firmware configuration. To determine whether we should
use a serial device for U-Boot in-/output, we need to check whether it
is muxed properly.

This is something only the board file can do, so let's expose a weak
function that a board can override to explicitly allow or disallow
usage of certain serial devices.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 drivers/serial/serial-uclass.c | 11 +++++++++++
 include/serial.h               | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 68ca2d09d1..ecd64f8e86 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -366,6 +366,16 @@ static int on_baudrate(const char *name, const char *value, enum env_op op,
 U_BOOT_ENV_CALLBACK(baudrate, on_baudrate);
 
 #if CONFIG_IS_ENABLED(SERIAL_PRESENT)
+__weak int board_check_serial(struct udevice *dev)
+{
+	return 0;
+}
+
+static int serial_pre_probe(struct udevice *dev)
+{
+	return board_check_serial(dev);
+}
+
 static int serial_post_probe(struct udevice *dev)
 {
 	struct dm_serial_ops *ops = serial_get_ops(dev);
@@ -438,6 +448,7 @@ UCLASS_DRIVER(serial) = {
 	.name		= "serial",
 	.flags		= DM_UC_FLAG_SEQ_ALIAS,
 	.post_probe	= serial_post_probe,
+	.pre_probe	= serial_pre_probe,
 	.pre_remove	= serial_pre_remove,
 	.per_device_auto_alloc_size = sizeof(struct serial_dev_priv),
 };
diff --git a/include/serial.h b/include/serial.h
index d87f01082a..221b3e1402 100644
--- a/include/serial.h
+++ b/include/serial.h
@@ -207,4 +207,15 @@ void sh_serial_initialize(void);
 void uartlite_serial_initialize(void);
 void zynq_serial_initialize(void);
 
+/**
+ * board_check_serial() - Determine whether a serial device works
+ *
+ * This is a board callback that allows boards to override whether a serial
+ * device is usable. By default, all devices are declared usable.
+ *
+ * @dev: Device pointer
+ * @return 0 if the device is usable, !0 otherwise
+ */
+int board_check_serial(struct udevice *dev);
+
 #endif
-- 
2.12.3

[U-Boot] [PATCH v2 3/7] rpi: Remove runtime disabling support for serial

[Alexander Graf]

We are switching to a model where our board file can directly fail probing
of serial devices when they're not usable, so remove the current runtime
hack we have.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/arm/mach-bcm283x/include/mach/gpio.h |  1 -
 board/raspberrypi/rpi/rpi.c               | 43 -------------------------------
 drivers/gpio/bcm2835_gpio.c               |  2 +-
 3 files changed, 1 insertion(+), 45 deletions(-)

diff --git a/arch/arm/mach-bcm283x/include/mach/gpio.h b/arch/arm/mach-bcm283x/include/mach/gpio.h
index 751594d09f..1bcb5846ca 100644
--- a/arch/arm/mach-bcm283x/include/mach/gpio.h
+++ b/arch/arm/mach-bcm283x/include/mach/gpio.h
@@ -61,7 +61,6 @@ struct bcm2835_gpio_platdata {
 	unsigned long base;
 };
 
-int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned gpio);
 void bcm2835_gpio_set_pinmux(struct udevice *dev, int handle);
 
 #endif /* _BCM2835_GPIO_H_ */
diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 3b7a54f519..a96d5d8952 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -419,54 +419,11 @@ static void get_board_rev(void)
 	printf("RPI %s (0x%x)\n", model->name, revision);
 }
 
-#ifndef CONFIG_PL01X_SERIAL
-static bool rpi_is_serial_active(void)
-{
-	int serial_gpio = 15;
-	struct udevice *dev;
-
-	/*
-	 * The RPi3 disables the mini uart by default. The easiest way to find
-	 * out whether it is available is to check if the RX pin is muxed.
-	 */
-
-	if (uclass_first_device(UCLASS_GPIO, &dev) || !dev)
-		return true;
-
-	if (bcm2835_gpio_get_func_id(dev, serial_gpio) != BCM2835_GPIO_ALT5)
-		return false;
-
-	return true;
-}
-
-/* Disable mini-UART I/O if it's not pinmuxed to our pins.
- * The firmware only enables it if explicitly done in config.txt: enable_uart=1
- */
-static void rpi_disable_inactive_uart(void)
-{
-	struct udevice *dev;
-	struct bcm283x_mu_serial_platdata *plat;
-
-	if (uclass_get_device_by_driver(UCLASS_SERIAL,
-					DM_GET_DRIVER(serial_bcm283x_mu),
-					&dev) || !dev)
-		return;
-
-	if (!rpi_is_serial_active()) {
-		plat = dev_get_platdata(dev);
-		plat->disabled = true;
-	}
-}
-#endif
-
 int board_init(void)
 {
 #ifdef CONFIG_HW_WATCHDOG
 	hw_watchdog_init();
 #endif
-#ifndef CONFIG_PL01X_SERIAL
-	rpi_disable_inactive_uart();
-#endif
 
 	get_board_rev();
 
diff --git a/drivers/gpio/bcm2835_gpio.c b/drivers/gpio/bcm2835_gpio.c
index 209cbed9e6..3edd90ea97 100644
--- a/drivers/gpio/bcm2835_gpio.c
+++ b/drivers/gpio/bcm2835_gpio.c
@@ -73,7 +73,7 @@ static int bcm2835_gpio_set_value(struct udevice *dev, unsigned gpio,
 	return 0;
 }
 
-int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned gpio)
+static int bcm2835_gpio_get_func_id(struct udevice *dev, unsigned gpio)
 {
 	struct bcm2835_gpios *gpios = dev_get_priv(dev);
 	u32 val;
-- 
2.12.3

[U-Boot] [PATCH v2 4/7] serial: bcm283x_mu: Remove support for post-init disabling

[Alexander Graf]

We are switching to a model where a serial device doesn't even get probed when
it's not muxed properly, so we don't need device specific disabling
functionality anymore.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 drivers/serial/serial_bcm283x_mu.c           | 18 +-----------------
 include/dm/platform_data/serial_bcm283x_mu.h |  1 -
 2 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/serial/serial_bcm283x_mu.c b/drivers/serial/serial_bcm283x_mu.c
index 41c26b3d93..7ce990b9b8 100644
--- a/drivers/serial/serial_bcm283x_mu.c
+++ b/drivers/serial/serial_bcm283x_mu.c
@@ -59,7 +59,7 @@ static int bcm283x_mu_serial_setbrg(struct udevice *dev, int baudrate)
 	struct bcm283x_mu_regs *regs = priv->regs;
 	u32 divider;
 
-	if (plat->disabled || plat->skip_init)
+	if (plat->skip_init)
 		return 0;
 
 	divider = plat->clock / (baudrate * 8);
@@ -75,9 +75,6 @@ static int bcm283x_mu_serial_probe(struct udevice *dev)
 	struct bcm283x_mu_serial_platdata *plat = dev_get_platdata(dev);
 	struct bcm283x_mu_priv *priv = dev_get_priv(dev);
 
-	if (plat->disabled)
-		return -ENODEV;
-
 	priv->regs = (struct bcm283x_mu_regs *)plat->base;
 
 	return 0;
@@ -85,14 +82,10 @@ static int bcm283x_mu_serial_probe(struct udevice *dev)
 
 static int bcm283x_mu_serial_getc(struct udevice *dev)
 {
-	struct bcm283x_mu_serial_platdata *plat = dev_get_platdata(dev);
 	struct bcm283x_mu_priv *priv = dev_get_priv(dev);
 	struct bcm283x_mu_regs *regs = priv->regs;
 	u32 data;
 
-	if (plat->disabled)
-		return -EAGAIN;
-
 	/* Wait until there is data in the FIFO */
 	if (!(readl(&regs->lsr) & BCM283X_MU_LSR_RX_READY))
 		return -EAGAIN;
@@ -104,13 +97,9 @@ static int bcm283x_mu_serial_getc(struct udevice *dev)
 
 static int bcm283x_mu_serial_putc(struct udevice *dev, const char data)
 {
-	struct bcm283x_mu_serial_platdata *plat = dev_get_platdata(dev);
 	struct bcm283x_mu_priv *priv = dev_get_priv(dev);
 	struct bcm283x_mu_regs *regs = priv->regs;
 
-	if (plat->disabled)
-		return 0;
-
 	/* Wait until there is space in the FIFO */
 	if (!(readl(&regs->lsr) & BCM283X_MU_LSR_TX_EMPTY))
 		return -EAGAIN;
@@ -123,14 +112,10 @@ static int bcm283x_mu_serial_putc(struct udevice *dev, const char data)
 
 static int bcm283x_mu_serial_pending(struct udevice *dev, bool input)
 {
-	struct bcm283x_mu_serial_platdata *plat = dev_get_platdata(dev);
 	struct bcm283x_mu_priv *priv = dev_get_priv(dev);
 	struct bcm283x_mu_regs *regs = priv->regs;
 	unsigned int lsr;
 
-	if (plat->disabled)
-		return 0;
-
 	lsr = readl(&regs->lsr);
 
 	if (input) {
@@ -168,7 +153,6 @@ static int bcm283x_mu_serial_ofdata_to_platdata(struct udevice *dev)
 				     1);
 	plat->skip_init = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev),
 	                                  "skip-init");
-	plat->disabled = false;
 	return 0;
 }
 #endif
diff --git a/include/dm/platform_data/serial_bcm283x_mu.h b/include/dm/platform_data/serial_bcm283x_mu.h
index c47d3c0e60..57ae6adc05 100644
--- a/include/dm/platform_data/serial_bcm283x_mu.h
+++ b/include/dm/platform_data/serial_bcm283x_mu.h
@@ -19,7 +19,6 @@ struct bcm283x_mu_serial_platdata {
 	unsigned long base;
 	unsigned int clock;
 	bool skip_init;
-	bool disabled;
 };
 
 #endif
-- 
2.12.3

[U-Boot] [PATCH v2 5/7] rpi: Properly detect which serial device is active

[Alexander Graf]

Now that we have all infrastructure in place to dynamically determine whether
a serial device is actually usable (read: routed to user accessible pins), we
can wire it up to the board.

This patch adds support to determine whether the pl011 or mini-uart or no serial
is routed to the UART RX/TX pins on the Raspberry Pi family of boards.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 board/raspberrypi/rpi/rpi.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index a96d5d8952..b0cdad70f7 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -24,9 +24,16 @@
 #include <asm/armv8/mmu.h>
 #endif
 #include <watchdog.h>
+#include <asm/io.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
+/*
+ * This is the GPIO pin that the user facing UART RX line is attached to.
+ * We use this pin to determine which serial device is available.
+ */
+#define BCM2835_GPIO_RX		15
+
 /* From lowlevel_init.S */
 extern unsigned long fw_dtb_pointer;
 
@@ -419,6 +426,68 @@ static void get_board_rev(void)
 	printf("RPI %s (0x%x)\n", model->name, revision);
 }
 
+/*
+ * We may get called before the device model is initialized, so we can not
+ * rely on the GPIO driver.
+ */
+int get_func_id(unsigned gpio)
+{
+	u32 val;
+	u32 node;
+	u32 *gpfsel;
+	fdt_addr_t addr;
+	fdt_size_t size;
+
+	node = fdt_node_offset_by_compatible(gd->fdt_blob, -1, "brcm,bcm2835-gpio");
+	if (node < 0)
+		return -EINVAL;
+
+	addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob, node, "reg",
+						  0, &size, true);
+	gpfsel = (void*)addr;
+
+	val = readl(&gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
+
+	return (val >> BCM2835_GPIO_FSEL_SHIFT(gpio) & BCM2835_GPIO_FSEL_MASK);
+}
+
+
+/*
+ * The RPi has 2 serial ports: A PL011 based one and the mini-uart.
+ * Depending on firmware configuration, either can be configured to either
+ * nothing, the wifi adapter or serial output.
+ *
+ * We only want to use the serial port that is user facing to not
+ * end up with a potentially unresponsive serial port. Due to this
+ * we need to check whether the serial device is actually connected
+ * to the UART RX/TX pins on the RPi GPIO pin bar.
+ *
+ * We only allow U-Boot to instantiate the serial driver for the serial
+ * device that is muxed correctly.
+ */
+int board_check_serial(struct udevice *dev)
+{
+	int func;
+
+	printf("Checking serial %s\n", dev->name);
+
+	if (device_is_compatible(dev, "arm,pl011")) {
+		func = BCM2835_GPIO_ALT0;
+	} else if (device_is_compatible(dev, "brcm,bcm2835-aux-uart")) {
+		func = BCM2835_GPIO_ALT5;
+	} else {
+		return 0;
+	}
+
+	if (get_func_id(BCM2835_GPIO_RX) != func) {
+		printf("Disabling serial %s\n", dev->name);
+		return -ENODEV;
+	}
+
+	printf("Enabling serial %s\n", dev->name);
+	return 0;
+}
+
 int board_init(void)
 {
 #ifdef CONFIG_HW_WATCHDOG
-- 
2.12.3

[U-Boot] [PATCH v2 6/7] rpi: Determine PL011/Mini-UART availability at runtime

[Alexander Graf]

Firmware on the Raspberry Pi family of devices can dynamically configure either
the PL011, Mini-UART or no device at all to be routed to the user accessible
UART pins.

That means we need to always include both drivers, because we can never be sure
which of the two serial devices firmware actually chooses to use.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 include/configs/rpi.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/configs/rpi.h b/include/configs/rpi.h
index cab8661779..2c84cf9a49 100644
--- a/include/configs/rpi.h
+++ b/include/configs/rpi.h
@@ -75,12 +75,9 @@
 #define CONFIG_MISC_INIT_R
 #endif
 
-/* Console UART */
-#if defined (CONFIG_BCM2837) || defined(CONFIG_TARGET_RPI_0_W)
+/* Console UART, can be configured dynamically in config.txt */
 #define CONFIG_BCM283X_MU_SERIAL
-#else
 #define CONFIG_PL01X_SERIAL
-#endif
 
 /* Console configuration */
 #define CONFIG_SYS_CBSIZE		1024
-- 
2.12.3

[U-Boot] [PATCH v2 7/7] rpi: Force skip_init on serial devices

[Alexander Graf]

The serial devices on the raspberry pi are based on clocks we can't easily
read and influence in U-Boot. However, the serial devices are always already
properly set up when coming up, so all we need to do is leave them alone.

The way to do that is to specify "skip-init" in device tree usually, but
if we set CONFIG_OF_BOARD to get the device tree from the RPi firmware,
that does not have skip-init properly set.

So instead we just force it in board specific code. That way serial devices
also work fine when skip-init is not passed explicitly in DT.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 board/raspberrypi/rpi/rpi.c        | 7 +++++++
 drivers/serial/serial_bcm283x_mu.c | 2 +-
 drivers/serial/serial_pl01x.c      | 2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index b0cdad70f7..ce1286a53a 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -20,6 +20,7 @@
 #include <asm/arch/sdhci.h>
 #include <asm/global_data.h>
 #include <dm/platform_data/serial_bcm283x_mu.h>
+#include <dm/platform_data/serial_pl01x.h>
 #ifdef CONFIG_ARM64
 #include <asm/armv8/mmu.h>
 #endif
@@ -472,9 +473,15 @@ int board_check_serial(struct udevice *dev)
 	printf("Checking serial %s\n", dev->name);
 
 	if (device_is_compatible(dev, "arm,pl011")) {
+		struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
+
 		func = BCM2835_GPIO_ALT0;
+		plat->skip_init = true;
 	} else if (device_is_compatible(dev, "brcm,bcm2835-aux-uart")) {
+		struct bcm283x_mu_serial_platdata *plat = dev_get_platdata(dev);
+
 		func = BCM2835_GPIO_ALT5;
+		plat->skip_init = true;
 	} else {
 		return 0;
 	}
diff --git a/drivers/serial/serial_bcm283x_mu.c b/drivers/serial/serial_bcm283x_mu.c
index 7ce990b9b8..8a7ca75d4a 100644
--- a/drivers/serial/serial_bcm283x_mu.c
+++ b/drivers/serial/serial_bcm283x_mu.c
@@ -151,7 +151,7 @@ static int bcm283x_mu_serial_ofdata_to_platdata(struct udevice *dev)
 	plat->base = addr;
 	plat->clock = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), "clock",
 				     1);
-	plat->skip_init = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev),
+	plat->skip_init |= fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev),
 	                                  "skip-init");
 	return 0;
 }
diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
index 4ec0f29c42..f957eddc07 100644
--- a/drivers/serial/serial_pl01x.c
+++ b/drivers/serial/serial_pl01x.c
@@ -357,7 +357,7 @@ static int pl01x_serial_ofdata_to_platdata(struct udevice *dev)
 	plat->clock = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), "clock",
 				     1);
 	plat->type = dev_get_driver_data(dev);
-	plat->skip_init = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev),
+	plat->skip_init |= fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev),
 	                                  "skip-init");
 	return 0;
 }
-- 
2.12.3

[U-Boot] [PATCH v2 1/7] serial: Use next serial device if probing fails

[Heinrich Schuchardt]

On 01/17/2018 09:54 AM, Alexander Graf wrote:
> Currently our serial device search chokes on the fact that the serial
> probe function could fail. If it does, instead of searching for the next
> usable serial device, it just quits.
> 
> This patch changes the fallback logic so that even when a serial device
> was not probed correctly, we just try the next ones until we find one that
> works.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> v1 -> v2:
> 
>   - Make search logic easier to follow

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

[U-Boot] [PATCH v2 1/7] serial: Use next serial device if probing fails

[Simon Glass]

On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
> Currently our serial device search chokes on the fact that the serial
> probe function could fail. If it does, instead of searching for the next
> usable serial device, it just quits.
>
> This patch changes the fallback logic so that even when a serial device
> was not probed correctly, we just try the next ones until we find one that
> works.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
>
> ---
>
> v1 -> v2:
>
>   - Make search logic easier to follow
> ---
>  drivers/serial/serial-uclass.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

The fallback logic is getting more complicated. How come the DT does
not specify the correct console?

Regards,
Simon

[U-Boot] [PATCH v2 2/7] serial: Allow boards to determine whether a serial device is usable

[Simon Glass]

Hi Alex,

On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
> On some boards, serial devices may or may not be muxed properly to actual
> pins, depending on firmware configuration. To determine whether we should
> use a serial device for U-Boot in-/output, we need to check whether it
> is muxed properly.
>
> This is something only the board file can do, so let's expose a weak
> function that a board can override to explicitly allow or disallow
> usage of certain serial devices.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  drivers/serial/serial-uclass.c | 11 +++++++++++
>  include/serial.h               | 11 +++++++++++
>  2 files changed, 22 insertions(+)
>

Can we please figure out how handle this in the serial driver / driver
model itself? I want to avoid weak functions with driver model.

Regards,
Simon

[U-Boot] [PATCH v2 7/7] rpi: Force skip_init on serial devices

[Simon Glass]

Hi Alex,

On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
> The serial devices on the raspberry pi are based on clocks we can't easily
> read and influence in U-Boot. However, the serial devices are always already
> properly set up when coming up, so all we need to do is leave them alone.
>
> The way to do that is to specify "skip-init" in device tree usually, but
> if we set CONFIG_OF_BOARD to get the device tree from the RPi firmware,
> that does not have skip-init properly set.
>
> So instead we just force it in board specific code. That way serial devices
> also work fine when skip-init is not passed explicitly in DT.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  board/raspberrypi/rpi/rpi.c        | 7 +++++++
>  drivers/serial/serial_bcm283x_mu.c | 2 +-
>  drivers/serial/serial_pl01x.c      | 2 +-
>  3 files changed, 9 insertions(+), 2 deletions(-)

Would you mind converting these drivers to livetree before adding these patches?

Regards,
Simon

[U-Boot] [PATCH v2 5/7] rpi: Properly detect which serial device is active

[Simon Glass]

Hi Alex,

On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
> Now that we have all infrastructure in place to dynamically determine whether
> a serial device is actually usable (read: routed to user accessible pins), we
> can wire it up to the board.
>
> This patch adds support to determine whether the pl011 or mini-uart or no serial
> is routed to the UART RX/TX pins on the Raspberry Pi family of boards.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  board/raspberrypi/rpi/rpi.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>
> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> index a96d5d8952..b0cdad70f7 100644
> --- a/board/raspberrypi/rpi/rpi.c
> +++ b/board/raspberrypi/rpi/rpi.c
> @@ -24,9 +24,16 @@
>  #include <asm/armv8/mmu.h>
>  #endif
>  #include <watchdog.h>
> +#include <asm/io.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +/*
> + * This is the GPIO pin that the user facing UART RX line is attached to.
> + * We use this pin to determine which serial device is available.
> + */
> +#define BCM2835_GPIO_RX                15
> +
>  /* From lowlevel_init.S */
>  extern unsigned long fw_dtb_pointer;
>
> @@ -419,6 +426,68 @@ static void get_board_rev(void)
>         printf("RPI %s (0x%x)\n", model->name, revision);
>  }
>
> +/*
> + * We may get called before the device model is initialized, so we can not
> + * rely on the GPIO driver.
> + */
> +int get_func_id(unsigned gpio)
> +{
> +       u32 val;
> +       u32 node;
> +       u32 *gpfsel;
> +       fdt_addr_t addr;
> +       fdt_size_t size;
> +
> +       node = fdt_node_offset_by_compatible(gd->fdt_blob, -1, "brcm,bcm2835-gpio");
> +       if (node < 0)
> +               return -EINVAL;
> +
> +       addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob, node, "reg",
> +                                                 0, &size, true);
> +       gpfsel = (void*)addr;
> +
> +       val = readl(&gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
> +
> +       return (val >> BCM2835_GPIO_FSEL_SHIFT(gpio) & BCM2835_GPIO_FSEL_MASK);
> +}

Ick, this should be done in the GPIO driver and use gpio_get_function().

Regards,
Simon

[U-Boot] [PATCH v2 1/7] serial: Use next serial device if probing fails

[Alexander Graf]

On 17.01.18 20:34, Simon Glass wrote:
> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>> Currently our serial device search chokes on the fact that the serial
>> probe function could fail. If it does, instead of searching for the next
>> usable serial device, it just quits.
>>
>> This patch changes the fallback logic so that even when a serial device
>> was not probed correctly, we just try the next ones until we find one that
>> works.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>
>> ---
>>
>> v1 -> v2:
>>
>>   - Make search logic easier to follow
>> ---
>>  drivers/serial/serial-uclass.c | 25 +++++++++++++++++++------
>>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> The fallback logic is getting more complicated. How come the DT does
> not specify the correct console?

By default the DT on the RPi does not specify any serial console.
However, users have grown accustomed to the fact that there is indeed
one :).

I guess it all boils down to the fact that Linux is terrible when it
comes to console selection. 99.9% of users that want a serial console
and have a graphical screen want to have *both* available as first class
citizen consoles. But Linux just can't do that, so we have a lot of
crude hacks all over the place.


Alex

[U-Boot] [PATCH v2 2/7] serial: Allow boards to determine whether a serial device is usable

[Alexander Graf]

On 17.01.18 20:39, Simon Glass wrote:
> Hi Alex,
> 
> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>> On some boards, serial devices may or may not be muxed properly to actual
>> pins, depending on firmware configuration. To determine whether we should
>> use a serial device for U-Boot in-/output, we need to check whether it
>> is muxed properly.
>>
>> This is something only the board file can do, so let's expose a weak
>> function that a board can override to explicitly allow or disallow
>> usage of certain serial devices.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  drivers/serial/serial-uclass.c | 11 +++++++++++
>>  include/serial.h               | 11 +++++++++++
>>  2 files changed, 22 insertions(+)
>>
> 
> Can we please figure out how handle this in the serial driver / driver
> model itself? I want to avoid weak functions with driver model.

I'm very happy to see suggestions :). The reason I went with the weak
function is really because I couldn't think of anything better.


Alex

[U-Boot] [PATCH v2 5/7] rpi: Properly detect which serial device is active

[Alexander Graf]

On 17.01.18 20:46, Simon Glass wrote:
> Hi Alex,
> 
> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>> Now that we have all infrastructure in place to dynamically determine whether
>> a serial device is actually usable (read: routed to user accessible pins), we
>> can wire it up to the board.
>>
>> This patch adds support to determine whether the pl011 or mini-uart or no serial
>> is routed to the UART RX/TX pins on the Raspberry Pi family of boards.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  board/raspberrypi/rpi/rpi.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 69 insertions(+)
>>
>> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
>> index a96d5d8952..b0cdad70f7 100644
>> --- a/board/raspberrypi/rpi/rpi.c
>> +++ b/board/raspberrypi/rpi/rpi.c
>> @@ -24,9 +24,16 @@
>>  #include <asm/armv8/mmu.h>
>>  #endif
>>  #include <watchdog.h>
>> +#include <asm/io.h>
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> +/*
>> + * This is the GPIO pin that the user facing UART RX line is attached to.
>> + * We use this pin to determine which serial device is available.
>> + */
>> +#define BCM2835_GPIO_RX                15
>> +
>>  /* From lowlevel_init.S */
>>  extern unsigned long fw_dtb_pointer;
>>
>> @@ -419,6 +426,68 @@ static void get_board_rev(void)
>>         printf("RPI %s (0x%x)\n", model->name, revision);
>>  }
>>
>> +/*
>> + * We may get called before the device model is initialized, so we can not
>> + * rely on the GPIO driver.
>> + */
>> +int get_func_id(unsigned gpio)
>> +{
>> +       u32 val;
>> +       u32 node;
>> +       u32 *gpfsel;
>> +       fdt_addr_t addr;
>> +       fdt_size_t size;
>> +
>> +       node = fdt_node_offset_by_compatible(gd->fdt_blob, -1, "brcm,bcm2835-gpio");
>> +       if (node < 0)
>> +               return -EINVAL;
>> +
>> +       addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob, node, "reg",
>> +                                                 0, &size, true);
>> +       gpfsel = (void*)addr;
>> +
>> +       val = readl(&gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
>> +
>> +       return (val >> BCM2835_GPIO_FSEL_SHIFT(gpio) & BCM2835_GPIO_FSEL_MASK);
>> +}
> 
> Ick, this should be done in the GPIO driver and use gpio_get_function().

Yes, but what if users specify the serial device to be pre-reloc and
don't do that for the GPIO (or really pinctrl) one? Then we'd not have
the driver around to determine whether serial is active, right?


Alex

[U-Boot] [PATCH v2 7/7] rpi: Force skip_init on serial devices

[Alexander Graf]

On 17.01.18 20:40, Simon Glass wrote:
> Hi Alex,
> 
> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>> The serial devices on the raspberry pi are based on clocks we can't easily
>> read and influence in U-Boot. However, the serial devices are always already
>> properly set up when coming up, so all we need to do is leave them alone.
>>
>> The way to do that is to specify "skip-init" in device tree usually, but
>> if we set CONFIG_OF_BOARD to get the device tree from the RPi firmware,
>> that does not have skip-init properly set.
>>
>> So instead we just force it in board specific code. That way serial devices
>> also work fine when skip-init is not passed explicitly in DT.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  board/raspberrypi/rpi/rpi.c        | 7 +++++++
>>  drivers/serial/serial_bcm283x_mu.c | 2 +-
>>  drivers/serial/serial_pl01x.c      | 2 +-
>>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> Would you mind converting these drivers to livetree before adding these patches?

Uh, why? I don't quite see how it's related - and I'm not sure I'll make
the rc1 cut-off even with just the pile I have right now :)


Alex

[U-Boot] [PATCH v2 2/7] serial: Allow boards to determine whether a serial device is usable

[Simon Glass]

Hi Alex,

On 17 January 2018 at 15:03, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 17.01.18 20:39, Simon Glass wrote:
>> Hi Alex,
>>
>> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>>> On some boards, serial devices may or may not be muxed properly to actual
>>> pins, depending on firmware configuration. To determine whether we should
>>> use a serial device for U-Boot in-/output, we need to check whether it
>>> is muxed properly.
>>>
>>> This is something only the board file can do, so let's expose a weak
>>> function that a board can override to explicitly allow or disallow
>>> usage of certain serial devices.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>>  drivers/serial/serial-uclass.c | 11 +++++++++++
>>>  include/serial.h               | 11 +++++++++++
>>>  2 files changed, 22 insertions(+)
>>>
>>
>> Can we please figure out how handle this in the serial driver / driver
>> model itself? I want to avoid weak functions with driver model.
>
> I'm very happy to see suggestions :). The reason I went with the weak
> function is really because I couldn't think of anything better.

The serial driver is proprietary so you should be able to put a call
into the pinctrl driver from that. The pinctrl driver can return the
state of a pin - maybe get_gpio_mux() or a new pinmux_get()?

Regards,
Simon

[U-Boot] [PATCH v2 2/7] serial: Allow boards to determine whether a serial device is usable

[Alexander Graf]

On 17.01.18 23:11, Simon Glass wrote:
> Hi Alex,
> 
> On 17 January 2018 at 15:03, Alexander Graf <agraf@suse.de> wrote:
>>
>>
>> On 17.01.18 20:39, Simon Glass wrote:
>>> Hi Alex,
>>>
>>> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>>>> On some boards, serial devices may or may not be muxed properly to actual
>>>> pins, depending on firmware configuration. To determine whether we should
>>>> use a serial device for U-Boot in-/output, we need to check whether it
>>>> is muxed properly.
>>>>
>>>> This is something only the board file can do, so let's expose a weak
>>>> function that a board can override to explicitly allow or disallow
>>>> usage of certain serial devices.
>>>>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> ---
>>>>  drivers/serial/serial-uclass.c | 11 +++++++++++
>>>>  include/serial.h               | 11 +++++++++++
>>>>  2 files changed, 22 insertions(+)
>>>>
>>>
>>> Can we please figure out how handle this in the serial driver / driver
>>> model itself? I want to avoid weak functions with driver model.
>>
>> I'm very happy to see suggestions :). The reason I went with the weak
>> function is really because I couldn't think of anything better.
> 
> The serial driver is proprietary so you should be able to put a call
> into the pinctrl driver from that. The pinctrl driver can return the
> state of a pin - maybe get_gpio_mux() or a new pinmux_get()?

The SoC has 2 serial drivers: a proprietary one and a pl011. Would you
think it's ok to put an architecture specific hack into the generic
pl011 code with an #ifdef?


Alex

[U-Boot] [PATCH v2 2/7] serial: Allow boards to determine whether a serial device is usable

[Simon Glass]

Hi Alex,

On 17 January 2018 at 14:37, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 17.01.18 23:11, Simon Glass wrote:
>> Hi Alex,
>>
>> On 17 January 2018 at 15:03, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>
>>> On 17.01.18 20:39, Simon Glass wrote:
>>>> Hi Alex,
>>>>
>>>> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>>>>> On some boards, serial devices may or may not be muxed properly to actual
>>>>> pins, depending on firmware configuration. To determine whether we should
>>>>> use a serial device for U-Boot in-/output, we need to check whether it
>>>>> is muxed properly.
>>>>>
>>>>> This is something only the board file can do, so let's expose a weak
>>>>> function that a board can override to explicitly allow or disallow
>>>>> usage of certain serial devices.
>>>>>
>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>> ---
>>>>>  drivers/serial/serial-uclass.c | 11 +++++++++++
>>>>>  include/serial.h               | 11 +++++++++++
>>>>>  2 files changed, 22 insertions(+)
>>>>>
>>>>
>>>> Can we please figure out how handle this in the serial driver / driver
>>>> model itself? I want to avoid weak functions with driver model.
>>>
>>> I'm very happy to see suggestions :). The reason I went with the weak
>>> function is really because I couldn't think of anything better.
>>
>> The serial driver is proprietary so you should be able to put a call
>> into the pinctrl driver from that. The pinctrl driver can return the
>> state of a pin - maybe get_gpio_mux() or a new pinmux_get()?
>
> The SoC has 2 serial drivers: a proprietary one and a pl011. Would you
> think it's ok to put an architecture specific hack into the generic
> pl011 code with an #ifdef?

Normally in this situation we create an outer driver which uses the
other one - see serial_rockchip.c for example. That way the base
driver doesn't have to know about arch-specific things.

Regards,
Simon

[U-Boot] [PATCH v2 5/7] rpi: Properly detect which serial device is active

[Simon Glass]

Hi Alex,

On 17 January 2018 at 15:05, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 17.01.18 20:46, Simon Glass wrote:
>> Hi Alex,
>>
>> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>>> Now that we have all infrastructure in place to dynamically determine whether
>>> a serial device is actually usable (read: routed to user accessible pins), we
>>> can wire it up to the board.
>>>
>>> This patch adds support to determine whether the pl011 or mini-uart or no serial
>>> is routed to the UART RX/TX pins on the Raspberry Pi family of boards.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>>  board/raspberrypi/rpi/rpi.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 69 insertions(+)
>>>
>>> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
>>> index a96d5d8952..b0cdad70f7 100644
>>> --- a/board/raspberrypi/rpi/rpi.c
>>> +++ b/board/raspberrypi/rpi/rpi.c
>>> @@ -24,9 +24,16 @@
>>>  #include <asm/armv8/mmu.h>
>>>  #endif
>>>  #include <watchdog.h>
>>> +#include <asm/io.h>
>>>
>>>  DECLARE_GLOBAL_DATA_PTR;
>>>
>>> +/*
>>> + * This is the GPIO pin that the user facing UART RX line is attached to.
>>> + * We use this pin to determine which serial device is available.
>>> + */
>>> +#define BCM2835_GPIO_RX                15
>>> +
>>>  /* From lowlevel_init.S */
>>>  extern unsigned long fw_dtb_pointer;
>>>
>>> @@ -419,6 +426,68 @@ static void get_board_rev(void)
>>>         printf("RPI %s (0x%x)\n", model->name, revision);
>>>  }
>>>
>>> +/*
>>> + * We may get called before the device model is initialized, so we can not
>>> + * rely on the GPIO driver.
>>> + */
>>> +int get_func_id(unsigned gpio)
>>> +{
>>> +       u32 val;
>>> +       u32 node;
>>> +       u32 *gpfsel;
>>> +       fdt_addr_t addr;
>>> +       fdt_size_t size;
>>> +
>>> +       node = fdt_node_offset_by_compatible(gd->fdt_blob, -1, "brcm,bcm2835-gpio");
>>> +       if (node < 0)
>>> +               return -EINVAL;
>>> +
>>> +       addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob, node, "reg",
>>> +                                                 0, &size, true);
>>> +       gpfsel = (void*)addr;
>>> +
>>> +       val = readl(&gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
>>> +
>>> +       return (val >> BCM2835_GPIO_FSEL_SHIFT(gpio) & BCM2835_GPIO_FSEL_MASK);
>>> +}
>>
>> Ick, this should be done in the GPIO driver and use gpio_get_function().
>
> Yes, but what if users specify the serial device to be pre-reloc and
> don't do that for the GPIO (or really pinctrl) one? Then we'd not have
> the driver around to determine whether serial is active, right?

We have control of that in the DT so just need to set it up correctly.
I suppose worst case you could add the pre-reloc flag to each driver's
decl instead of in DT. But I'm not sure why the DT would be wrong.

Regards,
Simon

[U-Boot] [PATCH v2 7/7] rpi: Force skip_init on serial devices

[Simon Glass]

Hi Alex,

On 17 January 2018 at 15:08, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 17.01.18 20:40, Simon Glass wrote:
>> Hi Alex,
>>
>> On 17 January 2018 at 00:54, Alexander Graf <agraf@suse.de> wrote:
>>> The serial devices on the raspberry pi are based on clocks we can't easily
>>> read and influence in U-Boot. However, the serial devices are always already
>>> properly set up when coming up, so all we need to do is leave them alone.
>>>
>>> The way to do that is to specify "skip-init" in device tree usually, but
>>> if we set CONFIG_OF_BOARD to get the device tree from the RPi firmware,
>>> that does not have skip-init properly set.
>>>
>>> So instead we just force it in board specific code. That way serial devices
>>> also work fine when skip-init is not passed explicitly in DT.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>>  board/raspberrypi/rpi/rpi.c        | 7 +++++++
>>>  drivers/serial/serial_bcm283x_mu.c | 2 +-
>>>  drivers/serial/serial_pl01x.c      | 2 +-
>>>  3 files changed, 9 insertions(+), 2 deletions(-)
>>
>> Would you mind converting these drivers to livetree before adding these patches?
>
> Uh, why? I don't quite see how it's related - and I'm not sure I'll make
> the rc1 cut-off even with just the pile I have right now :)

I'm trying to make sure that people that touch drivers convert them,
since otherwise it won't get done. I hope we can accept v2/v3 patches
which just miss rc1.

Regards,
Simon