Hi Marek, On 25 May 2016 at 16:35, Marek Vasut <ma...@denx.de> wrote: > 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 <ma...@denx.de> >>>>> Cc: Tom Rini <tr...@konsulko.com> >>>>> Cc: Simon Glass <s...@chromium.org> >>>>> Cc: Daniel Schwierzeck <daniel.schwierz...@gmail.com> >>>>> Cc: Paul Burton <paul.bur...@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 */ >>>>>
That way seems better to me. You should be able to add your UME flag as a Kconfig for this driver, in drivers/serial/Kconfig, which defaults to 0. It would be good to keep out board-specific stuff from this file, as you did with OMAP1510. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
