| Message ID | 20211221071634.25980-4-yu.tu@amlogic.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | the UART driver compatible with the Amlogic Meson S4 SoC | expand |
On Tue, Dec 21, 2021 at 03:16:34PM +0800, Yu Tu wrote: > The S4 SoC on-chip UART uses a 12M clock as the clock source for > calculating the baud rate of the UART. But previously, chips used 24M or > other clock sources. So add this change. The specific clock source is > determined by chip design. > > Signed-off-by: Yu Tu <yu.tu@amlogic.com> > --- > drivers/tty/serial/meson_uart.c | 62 +++++++++++++++++++++++++++++---- > 1 file changed, 55 insertions(+), 7 deletions(-) > > diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c > index 69450a461c48..557c24d954a2 100644 > --- a/drivers/tty/serial/meson_uart.c > +++ b/drivers/tty/serial/meson_uart.c > @@ -19,6 +19,7 @@ > #include <linux/serial_core.h> > #include <linux/tty.h> > #include <linux/tty_flip.h> > +#include <linux/of_device.h> > > /* Register offsets */ > #define AML_UART_WFIFO 0x00 > @@ -68,6 +69,8 @@ > #define AML_UART_BAUD_MASK 0x7fffff > #define AML_UART_BAUD_USE BIT(23) > #define AML_UART_BAUD_XTAL BIT(24) > +#define AML_UART_BAUD_XTAL_TICK BIT(26) > +#define AML_UART_BAUD_XTAL_DIV2 BIT(27) > > #define AML_UART_PORT_NUM 12 > #define AML_UART_PORT_OFFSET 6 > @@ -80,6 +83,11 @@ static struct uart_driver meson_uart_driver; > > static struct uart_port *meson_ports[AML_UART_PORT_NUM]; > > +struct meson_uart_data { > + /*A clock source that calculates baud rates*/ Please use spaces in your comments. > + unsigned int xtal_tick_en; What is "_en" for? "enabled"? Spell it out please. And why an unsigned int for a boolean flag? > +}; > + > static void meson_uart_set_mctrl(struct uart_port *port, unsigned int mctrl) > { > } > @@ -294,16 +302,29 @@ static int meson_uart_startup(struct uart_port *port) > > static void meson_uart_change_speed(struct uart_port *port, unsigned long baud) > { > + struct meson_uart_data *uart_data = port->private_data; > u32 val; > > while (!meson_uart_tx_empty(port)) > cpu_relax(); > > + val = readl_relaxed(port->membase + AML_UART_REG5); > + val &= ~AML_UART_BAUD_MASK; > + > if (port->uartclk == 24000000) { > - val = ((port->uartclk / 3) / baud) - 1; > - val |= AML_UART_BAUD_XTAL; > + if (uart_data->xtal_tick_en) { > + val = (port->uartclk / 2 + baud / 2) / baud - 1; > + val |= (AML_UART_BAUD_XTAL | AML_UART_BAUD_XTAL_DIV2); > + } else { > + val = ((port->uartclk / 3) + baud / 2) / baud - 1; > + val &= (~(AML_UART_BAUD_XTAL_TICK | > + AML_UART_BAUD_XTAL_DIV2)); > + val |= AML_UART_BAUD_XTAL; > + } > } else { > val = ((port->uartclk * 10 / (baud * 4) + 5) / 10) - 1; > + val &= (~(AML_UART_BAUD_XTAL | AML_UART_BAUD_XTAL_TICK | > + AML_UART_BAUD_XTAL_DIV2)); > } > val |= AML_UART_BAUD_USE; > writel(val, port->membase + AML_UART_REG5); > @@ -714,6 +735,7 @@ static int meson_uart_probe(struct platform_device *pdev) > { > struct resource *res_mem, *res_irq; > struct uart_port *port; > + struct meson_uart_data *uart_data; > int ret = 0; > int id = -1; > > @@ -729,6 +751,10 @@ static int meson_uart_probe(struct platform_device *pdev) > } > } > > + uart_data = of_device_get_match_data(&pdev->dev); > + if (!uart_data) > + return -EINVAL; Wrong spacing. Always use checkpatch.pl on your patches before sending them out. And did you just break existing systems? Do you know if all older ones will still work with that call? > + > if (pdev->id < 0 || pdev->id >= AML_UART_PORT_NUM) > return -EINVAL; > > @@ -770,6 +796,7 @@ static int meson_uart_probe(struct platform_device *pdev) > port->x_char = 0; > port->ops = &meson_uart_ops; > port->fifosize = 64; > + port->private_data = uart_data; > > meson_ports[pdev->id] = port; > platform_set_drvdata(pdev, port); > @@ -798,14 +825,35 @@ static int meson_uart_remove(struct platform_device *pdev) > return 0; > } > > +static const struct meson_uart_data meson_uart_data = { > + .xtal_tick_en = 0, > +}; > + > +static const struct meson_uart_data s4_meson_uart_data = { > + .xtal_tick_en = 1, > +}; As your whole structure just has one bit, why not just use that as the data value, instead of a structure? No need to be complex here at all. thanks, greg k-h
On 2021/12/21 15:34, Greg Kroah-Hartman wrote: > [ EXTERNAL EMAIL ] > > On Tue, Dec 21, 2021 at 03:16:34PM +0800, Yu Tu wrote: >> The S4 SoC on-chip UART uses a 12M clock as the clock source for >> calculating the baud rate of the UART. But previously, chips used 24M or >> other clock sources. So add this change. The specific clock source is >> determined by chip design. >> >> Signed-off-by: Yu Tu <yu.tu@amlogic.com> >> --- >> drivers/tty/serial/meson_uart.c | 62 +++++++++++++++++++++++++++++---- >> 1 file changed, 55 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c >> index 69450a461c48..557c24d954a2 100644 >> --- a/drivers/tty/serial/meson_uart.c >> +++ b/drivers/tty/serial/meson_uart.c >> @@ -19,6 +19,7 @@ >> #include <linux/serial_core.h> >> #include <linux/tty.h> >> #include <linux/tty_flip.h> >> +#include <linux/of_device.h> >> >> /* Register offsets */ >> #define AML_UART_WFIFO 0x00 >> @@ -68,6 +69,8 @@ >> #define AML_UART_BAUD_MASK 0x7fffff >> #define AML_UART_BAUD_USE BIT(23) >> #define AML_UART_BAUD_XTAL BIT(24) >> +#define AML_UART_BAUD_XTAL_TICK BIT(26) >> +#define AML_UART_BAUD_XTAL_DIV2 BIT(27) >> >> #define AML_UART_PORT_NUM 12 >> #define AML_UART_PORT_OFFSET 6 >> @@ -80,6 +83,11 @@ static struct uart_driver meson_uart_driver; >> >> static struct uart_port *meson_ports[AML_UART_PORT_NUM]; >> >> +struct meson_uart_data { >> + /*A clock source that calculates baud rates*/ > > Please use spaces in your comments. I will correct this mistake in the next patch. > >> + unsigned int xtal_tick_en; > > What is "_en" for? > > "enabled"? > > Spell it out please. You're right.I will correct as you suggested. > > And why an unsigned int for a boolean flag? It is my thoughtless, I will correct. > >> +}; >> + >> static void meson_uart_set_mctrl(struct uart_port *port, unsigned int mctrl) >> { >> } >> @@ -294,16 +302,29 @@ static int meson_uart_startup(struct uart_port *port) >> >> static void meson_uart_change_speed(struct uart_port *port, unsigned long baud) >> { >> + struct meson_uart_data *uart_data = port->private_data; >> u32 val; >> >> while (!meson_uart_tx_empty(port)) >> cpu_relax(); >> >> + val = readl_relaxed(port->membase + AML_UART_REG5); >> + val &= ~AML_UART_BAUD_MASK; >> + >> if (port->uartclk == 24000000) { >> - val = ((port->uartclk / 3) / baud) - 1; >> - val |= AML_UART_BAUD_XTAL; >> + if (uart_data->xtal_tick_en) { >> + val = (port->uartclk / 2 + baud / 2) / baud - 1; >> + val |= (AML_UART_BAUD_XTAL | AML_UART_BAUD_XTAL_DIV2); >> + } else { >> + val = ((port->uartclk / 3) + baud / 2) / baud - 1; >> + val &= (~(AML_UART_BAUD_XTAL_TICK | >> + AML_UART_BAUD_XTAL_DIV2)); >> + val |= AML_UART_BAUD_XTAL; >> + } >> } else { >> val = ((port->uartclk * 10 / (baud * 4) + 5) / 10) - 1; >> + val &= (~(AML_UART_BAUD_XTAL | AML_UART_BAUD_XTAL_TICK | >> + AML_UART_BAUD_XTAL_DIV2)); >> } >> val |= AML_UART_BAUD_USE; >> writel(val, port->membase + AML_UART_REG5); >> @@ -714,6 +735,7 @@ static int meson_uart_probe(struct platform_device *pdev) >> { >> struct resource *res_mem, *res_irq; >> struct uart_port *port; >> + struct meson_uart_data *uart_data; >> int ret = 0; >> int id = -1; >> >> @@ -729,6 +751,10 @@ static int meson_uart_probe(struct platform_device *pdev) >> } >> } >> >> + uart_data = of_device_get_match_data(&pdev->dev); >> + if (!uart_data) >> + return -EINVAL; > > Wrong spacing. > > Always use checkpatch.pl on your patches before sending them out. Sorry, this is a rookie mistake.But I did check it locally before sending it. I will follow your advice strictly later. > > And did you just break existing systems? Do you know if all older ones > will still work with that call? > It does affect older systems, but the new and older baud rates are not the same. I checked the documents before I made any changes. So this change is compatible with the older. >> + >> if (pdev->id < 0 || pdev->id >= AML_UART_PORT_NUM) >> return -EINVAL; >> >> @@ -770,6 +796,7 @@ static int meson_uart_probe(struct platform_device *pdev) >> port->x_char = 0; >> port->ops = &meson_uart_ops; >> port->fifosize = 64; >> + port->private_data = uart_data; >> >> meson_ports[pdev->id] = port; >> platform_set_drvdata(pdev, port); >> @@ -798,14 +825,35 @@ static int meson_uart_remove(struct platform_device *pdev) >> return 0; >> } >> >> +static const struct meson_uart_data meson_uart_data = { >> + .xtal_tick_en = 0, >> +}; >> + >> +static const struct meson_uart_data s4_meson_uart_data = { >> + .xtal_tick_en = 1, >> +}; > > As your whole structure just has one bit, why not just use that as the > data value, instead of a structure? No need to be complex here at all. > It is my thoughtless, I will correct. > thanks, > > greg k-h >
Hello, On Tue, Dec 21, 2021 at 8:17 AM Yu Tu <yu.tu@amlogic.com> wrote: > > The S4 SoC on-chip UART uses a 12M clock as the clock source for > calculating the baud rate of the UART. But previously, chips used 24M or > other clock sources. So add this change. The specific clock source is > determined by chip design. Does the new S4 SoC use an external 12MHz XTAL or does it use a 24MHz XTAL? If there's still a 24MHz XTAL then I think this description is not correct - at least based on how I understand the UART controller. SoCs up to GXL and GXM had an internal divide-by-3 (clock divider) in the UART controller IP and an external 24MHz XTAL. This was not configurable, so the clock for all baud-rates had to be derived from an 8MHz (24MHz divided by 3) clock. With the A311D (G12B, which is still using an external 24MHz XTAL) SoC the UART controller gained two new bits - with configurable dividers - according to the public datasheets: UART_EE_A_REG5[26]: - 0x0: divide the input clock by 3 (meaning: this internally works with an 8MHz clock) - 0x1: use the input clock directly without further division (meaning: this internally work with an 24MHz clock) UART_EE_A_REG5[27]: - 0x0: use the clock as configured in UART_EE_A_REG5[26] - 0x1: divide the input clock by 2 (meaning: this internally works with an 12MHz clock) While writing this email I did some investigation and found that UART_EE_A_REG5[26] is used in the vendor kernel even for GXL and GXM SoCs. So this probably has been introduced with the GXL generation (and thus is missing on GXBB and earlier SoCs). Also UART_EE_A_REG5[27] seems to have been introduced with the G12A generation of SoCs (not surprising since G12A and G12B peripherals are very similar). Does the UART controller not work with divide-by-3 (as we have it today) or are these configurable dividers to reduce jitter? Best regards, Martin
Hi Martin, Thank you very much for your reply. On 2021/12/25 1:25, Martin Blumenstingl wrote: > [ EXTERNAL EMAIL ] > > Hello, > > On Tue, Dec 21, 2021 at 8:17 AM Yu Tu <yu.tu@amlogic.com> wrote: >> >> The S4 SoC on-chip UART uses a 12M clock as the clock source for >> calculating the baud rate of the UART. But previously, chips used 24M or >> other clock sources. So add this change. The specific clock source is >> determined by chip design. > Does the new S4 SoC use an external 12MHz XTAL or does it use a 24MHz XTAL? > If there's still a 24MHz XTAL then I think this description is not > correct - at least based on how I understand the UART controller. > The S4 SoC uses 12MHz(UART_EE_A_REG5[27]=0x1,the bit is set in romcode). This register description is the same as the G12A and G12B you know. > SoCs up to GXL and GXM had an internal divide-by-3 (clock divider) in > the UART controller IP and an external 24MHz XTAL. > This was not configurable, so the clock for all baud-rates had to be > derived from an 8MHz (24MHz divided by 3) clock. > > With the A311D (G12B, which is still using an external 24MHz XTAL) SoC > the UART controller gained two new bits - with configurable dividers - > according to the public datasheets: > UART_EE_A_REG5[26]: > - 0x0: divide the input clock by 3 (meaning: this internally works > with an 8MHz clock) > - 0x1: use the input clock directly without further division (meaning: > this internally work with an 24MHz clock) > UART_EE_A_REG5[27]: > - 0x0: use the clock as configured in UART_EE_A_REG5[26] > - 0x1: divide the input clock by 2 (meaning: this internally works > with an 12MHz clock) > > While writing this email I did some investigation and found that > UART_EE_A_REG5[26] is used in the vendor kernel even for GXL and GXM > SoCs. > So this probably has been introduced with the GXL generation (and thus > is missing on GXBB and earlier SoCs). > Also UART_EE_A_REG5[27] seems to have been introduced with the G12A > generation of SoCs (not surprising since G12A and G12B peripherals are > very similar). > > Does the UART controller not work with divide-by-3 (as we have it > today) or are these configurable dividers to reduce jitter? > The UART controller can work with divide-by-3. The chip history as you described above, the current reason for using 12MHz clock is really what you call reduce jitter. The UART mainly connects to Bluetooth and uses typical baud rates of 2Mhz, 3MHz and 4MHz, so 12MHz is used as the clock source. > > Best regards, > Martin >
On Mon 27 Dec 2021 at 14:56, Yu Tu <yu.tu@amlogic.com> wrote: > Hi Martin, > Thank you very much for your reply. > > On 2021/12/25 1:25, Martin Blumenstingl wrote: >> [ EXTERNAL EMAIL ] >> Hello, >> On Tue, Dec 21, 2021 at 8:17 AM Yu Tu <yu.tu@amlogic.com> wrote: >>> >>> The S4 SoC on-chip UART uses a 12M clock as the clock source for >>> calculating the baud rate of the UART. But previously, chips used 24M or >>> other clock sources. So add this change. The specific clock source is >>> determined by chip design. >> Does the new S4 SoC use an external 12MHz XTAL or does it use a 24MHz XTAL? >> If there's still a 24MHz XTAL then I think this description is not >> correct - at least based on how I understand the UART controller. >> > The S4 SoC uses 12MHz(UART_EE_A_REG5[27]=0x1,the bit is set in > romcode). This register description is the same as the G12A and G12B you > know. > >> SoCs up to GXL and GXM had an internal divide-by-3 (clock divider) in >> the UART controller IP and an external 24MHz XTAL. >> This was not configurable, so the clock for all baud-rates had to be >> derived from an 8MHz (24MHz divided by 3) clock. >> With the A311D (G12B, which is still using an external 24MHz XTAL) SoC >> the UART controller gained two new bits - with configurable dividers - >> according to the public datasheets: >> UART_EE_A_REG5[26]: >> - 0x0: divide the input clock by 3 (meaning: this internally works >> with an 8MHz clock) >> - 0x1: use the input clock directly without further division (meaning: >> this internally work with an 24MHz clock) >> UART_EE_A_REG5[27]: >> - 0x0: use the clock as configured in UART_EE_A_REG5[26] >> - 0x1: divide the input clock by 2 (meaning: this internally works >> with an 12MHz clock) >> While writing this email I did some investigation and found that >> UART_EE_A_REG5[26] is used in the vendor kernel even for GXL and GXM >> SoCs. >> So this probably has been introduced with the GXL generation (and thus >> is missing on GXBB and earlier SoCs). >> Also UART_EE_A_REG5[27] seems to have been introduced with the G12A >> generation of SoCs (not surprising since G12A and G12B peripherals are >> very similar). >> Does the UART controller not work with divide-by-3 (as we have it >> today) or are these configurable dividers to reduce jitter? >> > The UART controller can work with divide-by-3. > The chip history as you described above, the current reason for using 12MHz > clock is really what you call reduce jitter. The UART mainly connects to > Bluetooth and uses typical baud rates of 2Mhz, 3MHz and 4MHz, so 12MHz is > used as the clock source. Looks to me that the clock divider above should be modelled properly with CCF. If you wish the initial Romcode setting to remain untouched, then don't put CLK_SET_RATE_PARENT to stop rate propagation. CCF will figure out what the internal rate is. You don't need to device tree data if things are done properly >> Best regards, >> Martin >>
Hello, On Mon, Dec 27, 2021 at 7:56 AM Yu Tu <yu.tu@amlogic.com> wrote: [...] > > Does the new S4 SoC use an external 12MHz XTAL or does it use a 24MHz XTAL? > > If there's still a 24MHz XTAL then I think this description is not > > correct - at least based on how I understand the UART controller. > > > The S4 SoC uses 12MHz(UART_EE_A_REG5[27]=0x1,the bit is set in romcode). > This register description is the same as the G12A and G12B you know. Thank you for this explanation! So the problem is that we're not touching bit 26 and bit 27 - and with the updated romcode you would not get any serial output since the divider is calculated off the wrong clock. I agree with Jerome that we shouldn't put a flag in device-tree. Also I did some experimenting with Jerome's idea to implement the clocks using CCF (common clock framework), see the attached patches. It was a bit tricky because some initial clean-ups were needed in the serial driver. Note: I have only briefly tested this on a 32-bit Meson8m2 SoC, see my attached patches and the clk_summary debugfs output. In fact, I expect that there are some issues with at least one of the patches as the whole bit 26 and bit 27 code is untested. Do you see any problems with this patch? Could you try to implement CCF support with the idea from the attached patches (you don't need to re-use them, I just wrote them to make it clearer in our discussion what we're talking about). Best regards, Martin # cat /sys/kernel/debug/clk/clk_summary enable prepare protect duty hardware clock count count count rate accuracy phase cycle enable ------------------------------------------------------------------------------------------------------- [...] xtal 6 6 2 24000000 0 0 50000 Y [...] c81004c0.serial#xtal_div3 0 0 0 8000000 0 0 50000 Y [...] fixed_pll_dco 1 1 0 2550000000 0 0 50000 Y fixed_pll 1 1 0 2550000000 0 0 50000 Y [...] fclk_div3_div 1 1 0 850000000 0 0 50000 Y fclk_div3 2 2 0 850000000 0 0 50000 Y [...] mpeg_clk_sel 1 1 0 850000000 0 0 50000 Y mpeg_clk_div 1 1 0 141666667 0 0 50000 Y clk81 17 20 0 141666667 0 0 50000 Y [...] c81004c0.serial#clk81_div4 1 1 0 35416666 0 0 50000 Y c81004c0.serial#use_xtal 1 1 0 35416666 0 0 50000 Y c81004c0.serial#baud_div 1 1 0 115364 0 0 50000 Y
Hi Martin and Jerome， Thank you very much for your reply. I have learned a lot from your communication. On 2021/12/28 4:04, Martin Blumenstingl wrote: > [ EXTERNAL EMAIL ] > > Hello, > > On Mon, Dec 27, 2021 at 7:56 AM Yu Tu <yu.tu@amlogic.com> wrote: > [...] >>> Does the new S4 SoC use an external 12MHz XTAL or does it use a 24MHz XTAL? >>> If there's still a 24MHz XTAL then I think this description is not >>> correct - at least based on how I understand the UART controller. >>> >> The S4 SoC uses 12MHz(UART_EE_A_REG5[27]=0x1,the bit is set in romcode). >> This register description is the same as the G12A and G12B you know. > Thank you for this explanation! > So the problem is that we're not touching bit 26 and bit 27 - and with > the updated romcode you would not get any serial output since the > divider is calculated off the wrong clock. > > I agree with Jerome that we shouldn't put a flag in device-tree. > > Also I did some experimenting with Jerome's idea to implement the > clocks using CCF (common clock framework), see the attached patches. > It was a bit tricky because some initial clean-ups were needed in the > serial driver. > Note: I have only briefly tested this on a 32-bit Meson8m2 SoC, see my > attached patches and the clk_summary debugfs output. > In fact, I expect that there are some issues with at least one of the > patches as the whole bit 26 and bit 27 code is untested. > > Do you see any problems with this patch? > Could you try to implement CCF support with the idea from the attached > patches (you don't need to re-use them, I just wrote them to make it > clearer in our discussion what we're talking about). > I couldn't agree with you more. I have verified it on a 64-bit S4 platform. Please refer to the attachment for verification output information. I will prepare the second version of patch according to the example ideas you provided. > > Best regards, > Martin # cat /sys/kernel/debug/clk/clk_summary enable prepare protect duty hardware clock count count count rate accuracy phase cycle enable ------------------------------------------------------------------------------------------------------- xtal 7 7 0 24000000 0 0 50000 Y fe07a000.serial#xtal_div2 1 1 0 12000000 0 0 50000 Y fe07a000.serial#xtal2_clk_sel 1 1 0 12000000 0 0 50000 Y fe07a000.serial#use_xtal 1 1 0 12000000 0 0 50000 Y fe07a000.serial#baud_div 1 1 0 923077 0 0 50000 Y fe07a000.serial#xtal_div3 0 0 0 8000000 0 0 50000 Y fe07a000.serial#xtal_clk_sel 0 0 0 8000000 0 0 50000 Y # # stty -F /dev/ttyAML0 115200 # stty -F /dev/ttyAML0 speed 115200 baud; line = 0; intr = ^C; quit = ^\; erase = ^?; kill = ^U; eof = ^D; eol = <undef>; eol2 = <undef>; swtch = <undef>; start = ^Q; stop = ^S; susp = ^Z; rprnt = ^R; werase = ^W; lnext = ^V; flush = ^O; min = 1; time = 0; -brkint ixoff -imaxbel
diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c index 69450a461c48..557c24d954a2 100644 --- a/drivers/tty/serial/meson_uart.c +++ b/drivers/tty/serial/meson_uart.c @@ -19,6 +19,7 @@ #include <linux/serial_core.h> #include <linux/tty.h> #include <linux/tty_flip.h> +#include <linux/of_device.h> /* Register offsets */ #define AML_UART_WFIFO 0x00 @@ -68,6 +69,8 @@ #define AML_UART_BAUD_MASK 0x7fffff #define AML_UART_BAUD_USE BIT(23) #define AML_UART_BAUD_XTAL BIT(24) +#define AML_UART_BAUD_XTAL_TICK BIT(26) +#define AML_UART_BAUD_XTAL_DIV2 BIT(27) #define AML_UART_PORT_NUM 12 #define AML_UART_PORT_OFFSET 6 @@ -80,6 +83,11 @@ static struct uart_driver meson_uart_driver; static struct uart_port *meson_ports[AML_UART_PORT_NUM]; +struct meson_uart_data { + /*A clock source that calculates baud rates*/ + unsigned int xtal_tick_en; +}; + static void meson_uart_set_mctrl(struct uart_port *port, unsigned int mctrl) { } @@ -294,16 +302,29 @@ static int meson_uart_startup(struct uart_port *port) static void meson_uart_change_speed(struct uart_port *port, unsigned long baud) { + struct meson_uart_data *uart_data = port->private_data; u32 val; while (!meson_uart_tx_empty(port)) cpu_relax(); + val = readl_relaxed(port->membase + AML_UART_REG5); + val &= ~AML_UART_BAUD_MASK; + if (port->uartclk == 24000000) { - val = ((port->uartclk / 3) / baud) - 1; - val |= AML_UART_BAUD_XTAL; + if (uart_data->xtal_tick_en) { + val = (port->uartclk / 2 + baud / 2) / baud - 1; + val |= (AML_UART_BAUD_XTAL | AML_UART_BAUD_XTAL_DIV2); + } else { + val = ((port->uartclk / 3) + baud / 2) / baud - 1; + val &= (~(AML_UART_BAUD_XTAL_TICK | + AML_UART_BAUD_XTAL_DIV2)); + val |= AML_UART_BAUD_XTAL; + } } else { val = ((port->uartclk * 10 / (baud * 4) + 5) / 10) - 1; + val &= (~(AML_UART_BAUD_XTAL | AML_UART_BAUD_XTAL_TICK | + AML_UART_BAUD_XTAL_DIV2)); } val |= AML_UART_BAUD_USE; writel(val, port->membase + AML_UART_REG5); @@ -714,6 +735,7 @@ static int meson_uart_probe(struct platform_device *pdev) { struct resource *res_mem, *res_irq; struct uart_port *port; + struct meson_uart_data *uart_data; int ret = 0; int id = -1; @@ -729,6 +751,10 @@ static int meson_uart_probe(struct platform_device *pdev) } } + uart_data = of_device_get_match_data(&pdev->dev); + if (!uart_data) + return -EINVAL; + if (pdev->id < 0 || pdev->id >= AML_UART_PORT_NUM) return -EINVAL; @@ -770,6 +796,7 @@ static int meson_uart_probe(struct platform_device *pdev) port->x_char = 0; port->ops = &meson_uart_ops; port->fifosize = 64; + port->private_data = uart_data; meson_ports[pdev->id] = port; platform_set_drvdata(pdev, port); @@ -798,14 +825,35 @@ static int meson_uart_remove(struct platform_device *pdev) return 0; } +static const struct meson_uart_data meson_uart_data = { + .xtal_tick_en = 0, +}; + +static const struct meson_uart_data s4_meson_uart_data = { + .xtal_tick_en = 1, +}; + static const struct of_device_id meson_uart_dt_match[] = { /* Legacy bindings, should be removed when no more used */ - { .compatible = "amlogic,meson-uart" }, + { .compatible = "amlogic,meson-uart", + .data = &meson_uart_data + }, /* Stable bindings */ - { .compatible = "amlogic,meson6-uart" }, - { .compatible = "amlogic,meson8-uart" }, - { .compatible = "amlogic,meson8b-uart" }, - { .compatible = "amlogic,meson-gx-uart" }, + { .compatible = "amlogic,meson6-uart", + .data = &meson_uart_data + }, + { .compatible = "amlogic,meson8-uart", + .data = &meson_uart_data + }, + { .compatible = "amlogic,meson8b-uart", + .data = &meson_uart_data + }, + { .compatible = "amlogic,meson-gx-uart", + .data = &meson_uart_data + }, + { .compatible = "amlogic,meson-s4-uart", + .data = &s4_meson_uart_data + }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
The S4 SoC on-chip UART uses a 12M clock as the clock source for calculating the baud rate of the UART. But previously, chips used 24M or other clock sources. So add this change. The specific clock source is determined by chip design. Signed-off-by: Yu Tu <yu.tu@amlogic.com> --- drivers/tty/serial/meson_uart.c | 62 +++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 7 deletions(-)