[U-Boot] [PATCH] serial: 16550: Add JZ47xx support

[Marek Vasut]

On 05/26/2016 12:31 AM, Daniel Schwierzeck wrote:
> 
> 
> Am 26.05.2016 um 00:21 schrieb Marek Vasut:
>> On 05/26/2016 12:17 AM, Daniel Schwierzeck wrote:
>>>
>>>
>>> Am 25.05.2016 um 02:19 schrieb Marek Vasut:
>>>> The Ingenic JZ47xx requires special bit (UART_EN) set in FCR register
>>>> in order to work at all. Add this special case handling into the driver.
>>>>
>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>> Cc: Tom Rini <trini at konsulko.com>
>>>> Cc: Simon Glass <sjg at chromium.org>
>>>> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>>>> Cc: Paul Burton <paul.burton at imgtec.com>
>>>> ---
>>>>  drivers/serial/ns16550.c | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
>>>> index 30ba0aa..1323881 100644
>>>> --- a/drivers/serial/ns16550.c
>>>> +++ b/drivers/serial/ns16550.c
>>>> @@ -50,6 +50,14 @@ DECLARE_GLOBAL_DATA_PTR;
>>>>  #endif
>>>>  #endif
>>>>  
>>>> +#ifdef CONFIG_ARCH_JZ47XX
>>>> +#undef UART_FCRVAL
>>>> +/* Ingenic JZ47xx SoCs require that a 'UART Module Enable' bit be set */
>>>> +#define UART_FCR_UME		0x10
>>>> +#define UART_FCRVAL (UART_FCR_FIFO_EN | UART_FCR_RXSR |	\
>>>> +		     UART_FCR_TXSR | UART_FCR_UME)
>>>> +#endif
>>>
>>> I think this could be added as DT property
>>
>> Not for SPL, which has 14 kiB size limit and it is itching to overflow.
>> I am literally counting bytes in the SPL and removing slop from
>> structures to make it fit, just barely. With the USB loader, I can
>> brutalize the SPL into having extremely rudimentary UART support now
>> (like printch() being the most advanced output mechanism, but you can
>> only use it three times, otherwise the code won't fit and the board is
>> eaten by demons) and this is where this patch comes into play.
>>
>> So yes, for full u-boot, this _should_ be part of DT. For SPL, please apply.
>>
> 
> ok, but wouldn't it be better to introduce an option like
> CONFIG_SYS_NS16550_UME instead of using the SoC-specific
> CONFIG_ARCH_JZ47XX. This driver is messed up enough ;)

I was undecided between this (like the IER) and adding new ifdef (like
SOC_KEYSTONE). Whichever way is fine with me. Yeah, the driver is
repugnant for sure.

>>>> +
>>>>  #ifndef CONFIG_SYS_NS16550_IER
>>>>  #define CONFIG_SYS_NS16550_IER  0x00
>>>>  #endif /* CONFIG_SYS_NS16550_IER */
>>>>
>>>
>>
>>
> 


-- 
Best regards,
Marek Vasut