Re: [PATCH v2 8/8] tty: serial: qcom_geni_serial: Add early console support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux