Hi Weijie, On 5 November 2018 at 19:48, Weijie Gao <weijie....@mediatek.com> wrote: > On Mon, 2018-11-05 at 16:37 +0800, Ryder Lee wrote: >> On Mon, 2018-11-05 at 10:20 +0800, Ryder Lee wrote: >> > On Sat, 2018-11-03 at 00:09 -0600, Simon Glass wrote: >> > > Hi Ryder, >> > > >> > > On 2 November 2018 at 09:15, Ryder Lee <ryder....@mediatek.com> wrote: >> > > > This patch adds an extra operation in ns16550.c to suuport MediaTek >> > > > SoCs as we have a highspeed register which influences the calcualtion >> > > > of the divisor. >> > > > >> > > > Note that we don't support the baudrate greater than 115200 currently. >> > > > >> > > > Signed-off-by: Ryder Lee <ryder....@mediatek.com> >> > > > Tested-by: Matthias Brugger <matthias....@gmail.com> >> > > > Reviewed-by: Simon Glass <s...@chromium.org> >> > > > --- >> > > > drivers/serial/ns16550.c | 10 ++++++++++ >> > > > 1 file changed, 10 insertions(+) >> > > >> > > Actually it seems to me that we should have a compatible string for >> > > this and deal with this at run-time? >> > > >> > > Is that easy to do here? >> > > >> > >> > How about this: >> > >> > uart0: serial@11002000 { >> > compatible = "ns16550a"; >> > .... >> > mediatek,highspeed = <0>; >> > .... >> > >> > Ryder >> > >> >> Sorry, I didn't get it right. I will add a compatible string in >> ns16550_serial_ids[] for MTK chips. >> >> Ryder >> >> > > Hi Simon, > > We have tried the compatible string, but it made the ns16550 driver more > complicated. > > To use the compatible string we have to add a new field in > ns16550_platdata, and change the flow of ns16550_serial_probe. > Moreover, it's totally useless for debug uart. > > At present using a macro is the easiest way here.
It might be easier but it is not correct. We should not have platform-specific #ifdefs in a common driver. Here's what I think you should do: 1. Create your own separate driver which calls into the code of this one 2. Add a 'quirks' flag word to the platform data and refactor the code to deal with your quirk, or add a separate field like you say 3. For the debug uart, put it in your separate driver, if you cannot easily change _debug_uart_init(). I suppose we could have a CONFIG_DEBUG_UART_QUIRKS flag wod? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot