Re: [PATCH v4 07/39] serial: msm: add debug UART

[Caleb Connolly <caleb.connolly@linaro.org>]

On 20/02/2024 06:08, Sumit Garg wrote:
> On Fri, 16 Feb 2024 at 02:22, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>
>> Introduce support for early debugging. This relies on the previous stage
>> bootloader to initialise the UART clocks, when running with U-Boot as
>> the primary bootloader this feature doesn't work. It will require a way
>> to configure the clocks before the driver model is available.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>> ---
>>  drivers/serial/Kconfig      |  8 ++++++++
>>  drivers/serial/serial_msm.c | 37 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 45 insertions(+)
>>
>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>> index 26460c4e0cab..fbd351a47859 100644
>> --- a/drivers/serial/Kconfig
>> +++ b/drivers/serial/Kconfig
>> @@ -319,6 +319,14 @@ config DEBUG_UART_S5P
>>           will need to provide parameters to make this work. The driver will
>>           be available until the real driver-model serial is running.
>>
>> +config DEBUG_UART_MSM
>> +       bool "Qualcomm QUP UART debug"
>> +       depends on ARCH_SNAPDRAGON
> 
> Since this debug UART only works for chainloaded configuration, can we
> somehow add explicit dependency here? Something like !REMAKE_ELF?

With a small patch (which didn't make it into v4 apparently) the
apq8016_clk_init_uart() function from clock-apq8016 can be adjusted to
just take a base address rather than "struct msm_clk_priv". It can then
be called from debug_uart_init() and allows for debug UART to be used
when U-Boot is running as the first stage.

This is definitely not ideal (although fwiw if the GPLLs were configured
right then this same function could maybe work on QCS404 as well - the
RCGs are at the same physical addresses), but I don't think gating it
behind REMAKE_ELF or something is a great solution here.
> 
> -Sumit
> 
>> +       help
>> +         Select this to enable a debug UART using the serial_msm driver. You
>> +         will need to provide parameters to make this work. The driver will
>> +         be available until the real driver-model serial is running.
>> +
>>  config DEBUG_UART_MSM_GENI
>>         bool "Qualcomm snapdragon"
>>         depends on ARCH_SNAPDRAGON
>> diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
>> index f4d96313b931..44b93bd7ff21 100644
>> --- a/drivers/serial/serial_msm.c
>> +++ b/drivers/serial/serial_msm.c
>> @@ -252,3 +252,40 @@ U_BOOT_DRIVER(serial_msm) = {
>>         .probe = msm_serial_probe,
>>         .ops    = &msm_serial_ops,
>>  };
>> +
>> +#ifdef CONFIG_DEBUG_UART_MSM
>> +
>> +static struct msm_serial_data init_serial_data = {
>> +       .base = CONFIG_VAL(DEBUG_UART_BASE),
>> +       .clk_rate = 7372800,
>> +};
>> +
>> +#include <debug_uart.h>
>> +
>> +/* Uncomment to turn on UART clocks when debugging U-Boot as aboot on MSM8916 */
>> +//int apq8016_clk_init_uart(phys_addr_t gcc_base);
>> +
>> +static inline void _debug_uart_init(void)
>> +{
>> +       /* Uncomment to turn on UART clocks when debugging U-Boot as aboot on MSM8916 */
>> +       //apq8016_clk_init_uart(0x1800000);
>> +       uart_dm_init(&init_serial_data);
>> +}
>> +
>> +static inline void _debug_uart_putc(int ch)
>> +{
>> +       struct msm_serial_data *priv = &init_serial_data;
>> +
>> +       while (!(readl(priv->base + UARTDM_SR) & UARTDM_SR_TX_EMPTY) &&
>> +              !(readl(priv->base + UARTDM_ISR) & UARTDM_ISR_TX_READY))
>> +               ;
>> +
>> +       writel(UARTDM_CR_CMD_RESET_TX_READY, priv->base + UARTDM_CR);
>> +
>> +       writel(1, priv->base + UARTDM_NCF_TX);
>> +       writel(ch, priv->base + UARTDM_TF);
>> +}
>> +
>> +DEBUG_UART_FUNCS
>> +
>> +#endif
>>
>> --
>> 2.43.1
>>

-- 
// Caleb (they/them)