[U-Boot] [PATCH] board: rpi4: fix instantiating PL011 driver

Matthias Brugger matthias.bgg at gmail.com
Fri Nov 8 17:23:36 UTC 2019



On 27/09/2019 07:39, Mark Dapoz wrote:
> The bcm283x PL011 serial driver is hard coded to allow connections only
> for UART0.  This prevents using any of the other UARTs as the U-boot
> console.
> 
> The bcm283x serial driver is updated to allow connections to any of the
> PL011 devices.  The initialization logic has been updated as follows:
> 
>     - a GPIO pinlist is extracted from the device tree for the device
>     - each pin is compared against a function assignment table
>     - if the pin is found in the table and the currently assigned GPIO
>       function doesn't match, the device is unavailable
>     - if the function matches the table entry or the pin is not listed
>       the table, the device is available
> 
> The function assignment table contains entries for UART0 to ensure it
> is multiplexed to GPIO pins 14 and 15.
> 

Which is what we already check right now, correct?

> This logic allows GPIO 14 and 15 to be multiplexed between the miniUART
> and the PL011 UART by the board's loader.
> 

Maybe I'm too tired, but I don't see where this is done. I can just see that you
changed the function to check if PL011 uses GPIO15 as ALT0 to a different
function which checks the same. Can you please explain.

> An error in the default clock rate has also been fixed.  If the device
> tree entry for the serial device doesn't specify a "clocks" property,
> the default frequency of 1Hz was used.  The default has been changed
> to 48MHz.

If doesn't that mean that we don't have a valid DTS?
If this still makes sense then please split this out into a separate patch.

Regards,
Matthias

> 
> The configuration for the Raspberry Pi 4 has been updated to include
> the PL011 driver.
> 
> To select an alternate UART as the U-boot console, the stdout-path can
> be used as follows:
> 
>       chosen {
>              stdout-path = "/soc/serial at 7e201a00:115200";
>              }
> 
> Signed-off-by: Mark Dapoz <Mark.Dapoz at windriver.com>
> ---
> 
>  configs/rpi_4_32b_defconfig           |   1 +
>  configs/rpi_4_defconfig               |   1 +
>  drivers/serial/serial_bcm283x_pl011.c | 150 ++++++++++++++++++++++++++++++----
>  3 files changed, 136 insertions(+), 16 deletions(-)
> 
> diff --git a/configs/rpi_4_32b_defconfig b/configs/rpi_4_32b_defconfig
> index dc69690..cd66b0b 100644
> --- a/configs/rpi_4_32b_defconfig
> +++ b/configs/rpi_4_32b_defconfig
> @@ -31,3 +31,4 @@ CONFIG_SYS_WHITE_ON_BLACK=y
>  CONFIG_CONSOLE_SCROLL_LINES=10
>  CONFIG_PHYS_TO_BUS=y
>  CONFIG_OF_LIBFDT_OVERLAY=y
> +CONFIG_BCM283X_PL011_SERIAL=y
> diff --git a/configs/rpi_4_defconfig b/configs/rpi_4_defconfig
> index 2d63197..9f862c4 100644
> --- a/configs/rpi_4_defconfig
> +++ b/configs/rpi_4_defconfig
> @@ -31,3 +31,4 @@ CONFIG_SYS_WHITE_ON_BLACK=y
>  CONFIG_CONSOLE_SCROLL_LINES=10
>  CONFIG_PHYS_TO_BUS=y
>  CONFIG_OF_LIBFDT_OVERLAY=y
> +CONFIG_BCM283X_PL011_SERIAL=y
> diff --git a/drivers/serial/serial_bcm283x_pl011.c
> b/drivers/serial/serial_bcm283x_pl011.c
> index 2527bb8..75b7ed5 100644
> --- a/drivers/serial/serial_bcm283x_pl011.c
> +++ b/drivers/serial/serial_bcm283x_pl011.c
> @@ -11,24 +11,142 @@
>  #include <serial.h>
>  #include "serial_pl01x_internal.h"
> 
> +#define    UART0_GPIO_RX    15
> +#define    UART0_GPIO_TX    14
> +
> +/* required pin vs function list for the PL011 UARTs */
> +
> +static const struct pin_func_map {
> +    int pin;    /* GPIO pin number */
> +    int function;    /* GPIO function */
> +} uart_pin_functions[] = {
> +    { .pin = UART0_GPIO_TX, .function = BCM2835_GPIO_ALT0 }, /* TXD0 */
> +    { .pin = UART0_GPIO_RX, .function = BCM2835_GPIO_ALT0 }, /* RXD0 */
> +};
> +
> +static const int num_uart_pin_functions =
> +         sizeof(uart_pin_functions) / sizeof(struct pin_func_map);
> +
> +/*
> + * Check if the pin/function assignment is valid
> + *
> + * The pin/function assignment is validated against the PL011 function
> + * table to determine if the current configuration is valid for a
> + * PL011 UART.
> + *
> + * @return true if the assignment is valid, false if not
> + */
> +static bool bcm383x_check_gpio_mapping(int pin, int function)
> +{
> +    int i;
> +
> +    /* iterate through the list checking the pin assignment */
> +
> +    for (i = 0; i < num_uart_pin_functions; i++) {
> +        if (uart_pin_functions[i].pin == pin) {
> +            /* pin assignment matches, check for a valid function */
> +
> +            if (uart_pin_functions[i].function == function)
> +                return true;
> +            else
> +                return false;
> +        }
> +    }
> +
> +    /* no pin match found, the mapping is unrestricted */
> +
> +    return true;
> +}
> +
>  /*
> - * Check if this serial device is muxed
> + * Check if this serial device is available and muxed correctly
>   *
> - * The serial device will only work properly if it has been muxed to the serial
> - * pins by firmware. Check whether that happened here.
> + * The uart0 serial device will only work properly if it has been muxed to
> + * the serial pins by firmware. Check if the device is uart0 and if it is
> + * is the muxing correct.
>   *
> - * @return true if serial device is muxed, false if not
> + * @return true if serial device is usable, false if not
>   */
> -static bool bcm283x_is_serial_muxed(void)
> +static bool bcm383x_check_serial_availability(struct udevice *dev)
>  {
> -    int serial_gpio = 15;
> -    struct udevice *dev;
> +    const void *fdt = gd->fdt_blob;
> +    const fdt32_t *cell;
> +    struct udevice *pin_dev;
> +    u32 phandle;
> +    int pin_func;
> +    int pin;
> +    int offset = 0;
> +    int size;
> +    int len;
> +    int i;
> +
> +    /* get the offset for this device's pinctrl-0 phandle */
> +
> +    cell = fdt_getprop(fdt, dev_of_offset(dev), "pinctrl-0", &size);
> +    if (!cell)
> +        return false;
> +
> +    /* find the offset of the device's pinctrl-0 node */
> +
> +    while (size) {
> +        phandle = fdt32_to_cpu(*cell++);
> +        size -= sizeof(*cell);
> +
> +        offset = fdt_node_offset_by_phandle(fdt, phandle);
> +        if (offset < 0) {
> +            /* bad offset, not found */
> 
> -    if (uclass_first_device(UCLASS_PINCTRL, &dev) || !dev)
> +            return false;
> +        }
> +    }
> +
> +    /* check to make sure we found the pinctrl-0 node */
> +
> +    if (offset == 0)
>          return false;
> 
> -    if (pinctrl_get_gpio_mux(dev, 0, serial_gpio) != BCM2835_GPIO_ALT0)
> +    /* get the bcrm,pins property */
> +
> +    cell = fdt_getprop(fdt, offset, "brcm,pins", &len);
> +
> +    if (!cell) {
> +        /* no pin property, signal as unavailable */
> +
>          return false;
> +    }
> +
> +    /* find the pinctrl device */
> +
> +    if (uclass_first_device(UCLASS_PINCTRL, &pin_dev) || !pin_dev)
> +        return false;
> +
> +    /* if the pin list is empty, assume this is uart0 */
> +
> +    if (len < sizeof(int)) {
> +        /* check that the Tx GPIO pin is muxed for the PL011 */
> +
> +        pin_func = pinctrl_get_gpio_mux(pin_dev, 0, UART0_GPIO_TX);
> +        return bcm383x_check_gpio_mapping(UART0_GPIO_TX, pin_func);
> +    }
> +
> +    /* check the pin list for a valid PL011 mapping */
> +
> +    for (i = 0; i < len / sizeof(int); i++) {
> +        /* get the function assigned to this pin */
> +
> +        pin = fdt32_to_cpu(cell[i]);
> +        pin_func = pinctrl_get_gpio_mux(pin_dev, 0, pin);
> +
> +        /* check the pin/function assignment */
> +
> +        if (bcm383x_check_gpio_mapping(pin, pin_func) == false) {
> +            /* any failed pin checks makes the device unavailable */
> +
> +            return false;
> +        }
> +    }
> +
> +    /* all checks pass, indicate device is available */
> 
>      return true;
>  }
> @@ -38,19 +156,19 @@ static int bcm283x_pl011_serial_ofdata_to_platdata(struct
> udevice *dev)
>      struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
>      int ret;
> 
> -    /* Don't spawn the device if it's not muxed */
> -    if (!bcm283x_is_serial_muxed())
> +    /* don't spawn the device if it's not usable */
> +
> +    if (bcm383x_check_serial_availability(dev) == false)
>          return -ENODEV;
> 
>      ret = pl01x_serial_ofdata_to_platdata(dev);
>      if (ret)
>          return ret;
> 
> -    /*
> -     * TODO: Reinitialization doesn't always work for now, just skip
> -     *       init always - we know we're already initialized
> -     */
> -    plat->skip_init = true;
> +    /* default clock to 48MHz if the device tree didn't specificy a freq */
> +
> +    if (plat->clock == 1)
> +        plat->clock = 48000000;
> 
>      return 0;
>  }


More information about the U-Boot mailing list