From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Chou <thomas@wytron.com.tw>
Date: Wed, 16 Dec 2015 08:25:12 +0800
Subject: [U-Boot] [PATCH 2/3] serial: uartlite: Add support for debug
	console
In-Reply-To: <56703405.109@xilinx.com>
References: <cover.1449834851.git.michal.simek@xilinx.com>
 <94a336e70a9b7682e914d917ebedc29a4447fa3e.1449834851.git.michal.simek@xilinx.com>
 <566EC0D2.80108@wytron.com.tw> <566EE533.4070708@xilinx.com>
 <566F9083.7090007@wytron.com.tw> <56703405.109@xilinx.com>
Message-ID: <5670AF68.9010309@wytron.com.tw>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Michal,

On 2015?12?15? 23:38, Michal Simek wrote:
> Hi,
>
> On 15.12.2015 05:01, Thomas Chou wrote:
>> Hi Michal,
>>
>> On 2015?12?14? 23:50, Michal Simek wrote:
>>> Hi,
>>>
>>> On 14.12.2015 14:14, Thomas Chou wrote:
>>>> Hi Michal,
>>>>
>>>> On 2015?12?11? 19:54, Michal Simek wrote:
>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>> ---
>>>>>
>>>>>     drivers/serial/Kconfig            |  7 +++++++
>>>>>     drivers/serial/serial_xuartlite.c | 23 +++++++++++++++++++++++
>>>>>     2 files changed, 30 insertions(+)
>>>>>
>>>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>>>>> index 1fc287ee98ec..f1e221799b81 100644
>>>>> --- a/drivers/serial/Kconfig
>>>>> +++ b/drivers/serial/Kconfig
>>>>> @@ -92,6 +92,13 @@ 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_UARTLITE
>>>>> +    bool "Xilinx Uartlite"
>>>>> +    help
>>>>> +      Select this to enable a debug UART using the serial_uartlite
>>>>> 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_ZYNQ
>>>>>         bool "Xilinx Zynq"
>>>>>         help
>>>>> diff --git a/drivers/serial/serial_xuartlite.c
>>>>> b/drivers/serial/serial_xuartlite.c
>>>>> index 10089f5a34b5..fe87b515d902 100644
>>>>> --- a/drivers/serial/serial_xuartlite.c
>>>>> +++ b/drivers/serial/serial_xuartlite.c
>>>>> @@ -120,3 +120,26 @@ U_BOOT_DRIVER(serial_uartlite) = {
>>>>>         .ops    = &uartlite_serial_ops,
>>>>>         .flags = DM_FLAG_PRE_RELOC,
>>>>>     };
>>>>> +
>>>>> +#ifdef CONFIG_DEBUG_UART_UARTLITE
>>>>
>>>> Better move the "#include <debug_uart.h>" here.
>>>
>>> This is the patch I sent some days ago about removing it from this
>>> location and it was Reviewed twice.
>>> http://lists.denx.de/pipermail/u-boot/2015-December/236341.html
>>>
>>>>
>>
>> This is because commit 42800ffa7997 ("arm: zynq: Move serial driver to
>> driver model") added the extra #include <debug_uart.h>. It is this one
>> should be removed. The original one in commit c54c0a4c1c74 ("arm: zynq:
>> Support the debug UART") is good. The wrong one was removed.
>
> I tend to have headers in the same location for the whole file
> It will be easier to include this file when CONFIG_DEBUG_UART is enabled.
> But no problem to move it. It is not causing any difference for me.
>
>
>>>>> +void _debug_uart_init(void)
>>>>
>>>> Please add "static inline" to void _debug_uart_init(void).
>>>
>>> Ok. Will fix this for uartlite and zynq uart in follow up patch.
>>>
>>>
>>>>> +{
>>>>> +    struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
>>>>> +
>>>>> +    out_be32(&regs->control, 0);
>>>>> +    out_be32(&regs->control, ULITE_CONTROL_RST_RX |
>>>>> ULITE_CONTROL_RST_TX);
>>>>> +    in_be32(&regs->control);
>>>>> +}
>>>>> +
>>>>> +static inline void _debug_uart_putc(int ch)
>>>>> +{
>>>>> +    struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
>>>>> +
>>>>> +    while (in_be32(&regs->status) & SR_TX_FIFO_FULL)
>>>>> +        WATCHDOG_RESET();
>>>>
>>>> WATCHDOG_RESET() is not really needed for debug serial output.
>>>
>>> TBH I don't think so. There could be watchdog running from early
>>> bootloader and needs to be service even for debugging purpose.
>>>
>>
>> Unless the serial input clock is dead, the serial shift out in UART
>> won't take so long to trigger watchdog.
>
> Uartlite driver is also the part of MDM which is console over jtag which
> can take some time.
> But TBH if you like, I will remove it in v2.
>

Now I understand. Either is fine. Thanks for the explanation.

Best regards,
Thomas