On 4/17/2018 12:12 AM, Stephen Boyd wrote:
> Quoting Karthikeyan Ramasubramanian (2018-04-09 12:38:41)
>> 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(-)
>
> This patch needs to update the earlycon section of
> Documentation/admin-guide/kernel-parameters.txt as well.
Thank you for pointing it out. I will update the documentation.
>
>>
>> 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)
>
> Is the cast needed?
Yes. Without that, "set" being a bool gets casted to 1 or 0 and the LHS
is strictly compared against 1 or 0. That in turn leads to the function
polling for the entirety of the timeout.
>
>> + 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);
>> }
> [...]
>> + 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",
>
> Maybe geni_serial instead? Or qcom_geni? earlycon sort of implies serial
> already so it seems redundant. And yes I messed that up for msm_serial
> but it doesn't mean we should repeat the same mistake again.
Ok, I will update to qcom_geni.
>
Regards,
Karthik.
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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
