Message ID | b3700528b88bf715d4401f7e31817a8daeb2b9c5.1541474544.git.ryder.lee@mediatek.com |
---|---|
State | Superseded |
Delegated to: | Tom Rini |
Headers | show |
Series | Add U-Boot support for MediaTek SoCs - MT7623n & MT7629 | expand |
Hi Ryder, On 6 November 2018 at 01:47, Ryder Lee <ryder.lee@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.lee@mediatek.com> > Tested-by: Matthias Brugger <matthias.bgg@gmail.com> > Reviewed-by: Simon Glass <sjg@chromium.org> > --- > Changes since v4: None > > @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. > > What do you think? > --- > drivers/serial/ns16550.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) Please see my reply to the other thread. We should not have arch-specific code in generic drivers. Sorry for not replying earlier on that. Regards, Simon
On Tue, 2018-11-13 at 11:53 -0800, Simon Glass wrote: > Hi Ryder, > > On 6 November 2018 at 01:47, Ryder Lee <ryder.lee@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.lee@mediatek.com> > > Tested-by: Matthias Brugger <matthias.bgg@gmail.com> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > --- > > Changes since v4: None > > > > @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. > > > > What do you think? > > --- > > drivers/serial/ns16550.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > Please see my reply to the other thread. We should not have > arch-specific code in generic drivers. > > Sorry for not replying earlier on that. > OK. BTW, can you (or Tom) please take the other patches through your tree? Ryder
Hi Ryder, On 13 November 2018 at 17:32, Ryder Lee <ryder.lee@mediatek.com> wrote: > On Tue, 2018-11-13 at 11:53 -0800, Simon Glass wrote: >> Hi Ryder, >> >> On 6 November 2018 at 01:47, Ryder Lee <ryder.lee@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.lee@mediatek.com> >> > Tested-by: Matthias Brugger <matthias.bgg@gmail.com> >> > Reviewed-by: Simon Glass <sjg@chromium.org> >> > --- >> > Changes since v4: None >> > >> > @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. >> > >> > What do you think? >> > --- >> > drivers/serial/ns16550.c | 10 ++++++++++ >> > 1 file changed, 10 insertions(+) >> >> Please see my reply to the other thread. We should not have >> arch-specific code in generic drivers. >> >> Sorry for not replying earlier on that. >> > > OK. > > BTW, can you (or Tom) please take the other patches through your tree? I think this is more Tom's domain, but I'm happy to do this if Tom wants me to. Regards, Simon
On Thu, Nov 15, 2018 at 11:59:34AM -0800, Simon Glass wrote: > Hi Ryder, > > On 13 November 2018 at 17:32, Ryder Lee <ryder.lee@mediatek.com> wrote: > > On Tue, 2018-11-13 at 11:53 -0800, Simon Glass wrote: > >> Hi Ryder, > >> > >> On 6 November 2018 at 01:47, Ryder Lee <ryder.lee@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.lee@mediatek.com> > >> > Tested-by: Matthias Brugger <matthias.bgg@gmail.com> > >> > Reviewed-by: Simon Glass <sjg@chromium.org> > >> > --- > >> > Changes since v4: None > >> > > >> > @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. > >> > > >> > What do you think? > >> > --- > >> > drivers/serial/ns16550.c | 10 ++++++++++ > >> > 1 file changed, 10 insertions(+) > >> > >> Please see my reply to the other thread. We should not have > >> arch-specific code in generic drivers. > >> > >> Sorry for not replying earlier on that. > >> > > > > OK. > > > > BTW, can you (or Tom) please take the other patches through your tree? > > I think this is more Tom's domain, but I'm happy to do this if Tom wants me to. I'll be picking up the whole series once everyone seems happy with it, thanks!
On Fri, 2018-11-16 at 09:30 -0500, Tom Rini wrote: > On Thu, Nov 15, 2018 at 11:59:34AM -0800, Simon Glass wrote: > > Hi Ryder, > > > > On 13 November 2018 at 17:32, Ryder Lee <ryder.lee@mediatek.com> wrote: > > > On Tue, 2018-11-13 at 11:53 -0800, Simon Glass wrote: > > >> Hi Ryder, > > >> > > >> On 6 November 2018 at 01:47, Ryder Lee <ryder.lee@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.lee@mediatek.com> > > >> > Tested-by: Matthias Brugger <matthias.bgg@gmail.com> > > >> > Reviewed-by: Simon Glass <sjg@chromium.org> > > >> > --- > > >> > Changes since v4: None > > >> > > > >> > @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. > > >> > > > >> > What do you think? > > >> > --- > > >> > drivers/serial/ns16550.c | 10 ++++++++++ > > >> > 1 file changed, 10 insertions(+) > > >> > > >> Please see my reply to the other thread. We should not have > > >> arch-specific code in generic drivers. > > >> > > >> Sorry for not replying earlier on that. > > >> > > > > > > OK. > > > > > > BTW, can you (or Tom) please take the other patches through your tree? > > > > I think this is more Tom's domain, but I'm happy to do this if Tom wants me to. > > I'll be picking up the whole series once everyone seems happy with it, > thanks! > Hi Tom, We have uploaded the v5 patch series, which introduce a new serial driver for MTK's high speed uart. The v4 patch is obsolete. Best Regards, Weijie
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index f9041aa..f5410af 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -148,6 +148,13 @@ int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate) static void NS16550_setbrg(NS16550_t com_port, int baud_divisor) { +#ifdef CONFIG_ARCH_MEDIATEK + /* + * MediaTek UARTs has an extra highspeed register. + * We need to clear it if baudrate <= 115200. + */ + serial_out(0, &com_port->reg9); +#endif serial_out(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr); serial_out(baud_divisor & 0xff, &com_port->dll); serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm); @@ -261,6 +268,9 @@ static inline void _debug_uart_init(void) serial_dout(&com_port->mcr, UART_MCRVAL); serial_dout(&com_port->fcr, UART_FCR_DEFVAL); +#ifdef CONFIG_ARCH_MEDIATEK + serial_dout(&com_port->reg9, 0); +#endif serial_dout(&com_port->lcr, UART_LCR_BKSE | UART_LCRVAL); serial_dout(&com_port->dll, baud_divisor & 0xff); serial_dout(&com_port->dlm, (baud_divisor >> 8) & 0xff);