[v2,5/7] rpi: Properly detect which serial device is active

Message ID 20180117085458.27293-6-agraf@suse.de
State New
Headers show
Series
  • RPi: Properly handle dynamic serial configuration
Related show

Commit Message

Alexander Graf Jan. 17, 2018, 8:54 a.m.
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(+)

Comments

Simon Glass Jan. 17, 2018, 7:46 p.m. | #1
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
Alexander Graf Jan. 17, 2018, 10:05 p.m. | #2
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
Simon Glass Jan. 17, 2018, 11:18 p.m. | #3
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

Patch

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