On Mon, Apr 09, 2018 at 01:38:41PM -0600, Karthikeyan Ramasubramanian wrote:
> Add early console support in Qualcomm Technologies Inc., GENI based
> UART controller.
>
> Signed-off-by: Karthikeyan Ramasubramanian <kramasub@xxxxxxxxxxxxxx>
> Signed-off-by: Girish Mahadevan <girishm@xxxxxxxxxxxxxx>
> ---
> drivers/tty/serial/qcom_geni_serial.c | 85 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 83 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index e40d4a4..2ce83a57 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -196,8 +196,18 @@ static bool qcom_geni_serial_poll_bit(struct uart_port *uport,
> timeout_us = ((fifo_bits * USEC_PER_SEC) / baud) + 500;
> }
>
> - return !readl_poll_timeout_atomic(uport->membase + offset, reg,
> - (bool)(reg & field) == set, 10, timeout_us);
> + /*
> + * Use custom implementation instead of readl_poll_atomic since ktimer
> + * is not ready at the time of early console.
> + */
> + while (timeout_us) {
> + reg = readl_relaxed(uport->membase + offset);
> + if ((bool)(reg & field) == set)
> + return true;
> + udelay(10);
> + timeout_us -= 10;
> + }
> + return false;
> }
>
> static void qcom_geni_serial_setup_tx(struct uart_port *uport, u32 xmit_size)
> @@ -944,6 +954,77 @@ static int __init qcom_geni_console_setup(struct console *co, char *options)
> return uart_set_options(uport, co, baud, parity, bits, flow);
> }
>
> +static void qcom_geni_serial_earlycon_write(struct console *con,
> + const char *s, unsigned int n)
> +{
> + struct earlycon_device *dev = con->data;
> +
> + __qcom_geni_serial_console_write(&dev->port, s, n);
> +}
> +
> +static int __init qcom_geni_serial_earlycon_setup(struct earlycon_device *dev,
> + const char *opt)
> +{
> + struct uart_port *uport = &dev->port;
> + u32 tx_trans_cfg;
> + u32 tx_parity_cfg = 0; /* Disable Tx Parity */
> + u32 rx_trans_cfg = 0;
> + u32 rx_parity_cfg = 0; /* Disable Rx Parity */
> + u32 stop_bit_len = 0; /* Default stop bit length - 1 bit */
> + u32 bits_per_char;
> + u32 ser_clk_cfg;
> + u32 baud = 115200;
My understanding is that the baudrate should either be specified in
the earlycon parmameter or left as set up by the bootloader:
"If baud rate is not specified, the serial port must already be setup
and configured."
Documentation/admin-guide/kernel-parameters.txt
> + unsigned int clk_div;
> + unsigned long clk_rate;
> + struct geni_se se;
> +
> + if (!uport->membase)
> + return -EINVAL;
> +
> + memset(&se, 0, sizeof(se));
> + se.base = uport->membase;
> + if (geni_se_read_proto(&se) != GENI_SE_UART)
> + return -ENXIO;
Declaring se on the stack and only initializing se.base is ugly, it
makes the assumption that geni_se_read_proto() does not use other
members of the struct, which is brittle.
Possible alternatives could be to duplicate the code of
geni_se_read_proto() here (also not ideal, though it's only a few
lines) or add a geni_se_read_proto_xyz(void __iomem *base) which is
then also called by geni_se_read_proto().
> +
> + /*
> + * Ignore Flow control.
> + * n = 8.
> + */
> + tx_trans_cfg = UART_CTS_MASK;
> + bits_per_char = BITS_PER_BYTE;
> +
> + clk_rate = get_clk_div_rate(baud, &clk_div);
> + if (!clk_rate)
> + return -EINVAL;
Here the parent clock rate and the corresponding divider are
calculated, however later only the divider is programmed, apparently
with the assumption that the calculated parent clock rate has been
configured by the bootloader.
I was told that at this stage the parent clock can't be
reconfigured. If we effectively rely on the bootloader to do part of
the initialization, shouldn't we then directly assume that the BL
initializes the hardware and only do the minimum to integrate it
with the kernel?
> + ser_clk_cfg = SER_CLK_EN | (clk_div << CLK_DIV_SHFT);
> + /*
> + * Make an unconditional cancel on the main sequencer to reset
> + * it else we could end up in data loss scenarios.
> + */
> + qcom_geni_serial_poll_tx_done(uport);
> + qcom_geni_serial_abort_rx(uport);
> + geni_se_config_packing(&se, BITS_PER_BYTE, 1, false, true, false);
> + geni_se_init(&se, DEF_FIFO_DEPTH_WORDS / 2, DEF_FIFO_DEPTH_WORDS - 2);
> + geni_se_select_mode(&se, GENI_SE_FIFO);
> +
> + writel_relaxed(tx_trans_cfg, uport->membase + SE_UART_TX_TRANS_CFG);
> + writel_relaxed(tx_parity_cfg, uport->membase + SE_UART_TX_PARITY_CFG);
> + writel_relaxed(rx_trans_cfg, uport->membase + SE_UART_RX_TRANS_CFG);
> + writel_relaxed(rx_parity_cfg, uport->membase + SE_UART_RX_PARITY_CFG);
> + writel_relaxed(bits_per_char, uport->membase + SE_UART_TX_WORD_LEN);
> + writel_relaxed(bits_per_char, uport->membase + SE_UART_RX_WORD_LEN);
> + writel_relaxed(stop_bit_len, uport->membase + SE_UART_TX_STOP_BIT_LEN);
> + writel_relaxed(ser_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG);
> + writel_relaxed(ser_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG);
> +
> + dev->con->write = qcom_geni_serial_earlycon_write;
> + dev->con->setup = NULL;
> + return 0;
> +}
> +OF_EARLYCON_DECLARE(qcom_serial, "qcom,geni-debug-uart",
> + qcom_geni_serial_earlycon_setup);
> +
> static int __init console_register(struct uart_driver *drv)
> {
> return uart_register_driver(drv);
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
