Message ID | 20220225073922.3947-1-yu.tu@amlogic.com (mailing list archive) |
---|---|
Headers | show |
Series | Use CCF to describe the UART baud rate clock | expand |
On Fri 25 Feb 2022 at 15:39, Yu Tu <yu.tu@amlogic.com> wrote: > Using the common Clock code to describe the UART baud rate > clock makes it easier for the UART driver to be compatible > with the baud rate requirements of the UART IP on different > meson chips. Add Meson S4 SoC compatible. > > The test method: > Start the console and run the following commands in turn: > stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600. > > Since most SoCs are too old, I was able to find all the platforms myself > such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with > G12A and S4. GXL based board are still very common an easy to come by. I'm quite surprised that you are unable to test on this SoC family > > Yu Tu (6): > tty: serial: meson: Move request the register region to probe > tty: serial: meson: Use devm_ioremap_resource to get register mapped > memory > tty: serial: meson: Describes the calculation of the UART baud rate > clock using a clock frame > tty: serial: meson: Make some bit of the REG5 register writable > tty: serial: meson: The system stuck when you run the stty command on > the console to change the baud rate > tty: serial: meson: Added S4 SOC compatibility > > V6 -> V7: To solve the system stuck when you run the stty command on > the console to change the baud rate. > V5 -> V6: Change error format as discussed in the email. > V4 -> V5: Change error format. > V3 -> V4: Change CCF to describe the UART baud rate clock as discussed > in the email. > V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it must change > the DTS before it can be deleted > V1 -> V2: Use CCF to describe the UART baud rate clock.Make some changes as > discussed in the email > > Link:https://lore.kernel.org/linux-amlogic/20220118030911.12815-4-yu.tu@amlogic.com/ > > drivers/tty/serial/meson_uart.c | 221 ++++++++++++++++++++++---------- > 1 file changed, 154 insertions(+), 67 deletions(-) > > > base-commit: a603ca60cebff8589882427a67f870ed946b3fc8
Hi Jerome, On 2022/2/28 18:59, Jerome Brunet wrote: > [ EXTERNAL EMAIL ] > > > On Fri 25 Feb 2022 at 15:39, Yu Tu <yu.tu@amlogic.com> wrote: > >> Using the common Clock code to describe the UART baud rate >> clock makes it easier for the UART driver to be compatible >> with the baud rate requirements of the UART IP on different >> meson chips. Add Meson S4 SoC compatible. >> >> The test method: >> Start the console and run the following commands in turn: >> stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600. >> >> Since most SoCs are too old, I was able to find all the platforms myself >> such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with >> G12A and S4. > > GXL based board are still very common an easy to come by. > I'm quite surprised that you are unable to test on this SoC family The fact of the matter is that the S4 is our end-2020 chip, the G12A is five years old, and the GXL is seven years old. If you must ask for a test, I will report this problem to the leadership to coordinate resources. > >> >> Yu Tu (6): >> tty: serial: meson: Move request the register region to probe >> tty: serial: meson: Use devm_ioremap_resource to get register mapped >> memory >> tty: serial: meson: Describes the calculation of the UART baud rate >> clock using a clock frame >> tty: serial: meson: Make some bit of the REG5 register writable >> tty: serial: meson: The system stuck when you run the stty command on >> the console to change the baud rate >> tty: serial: meson: Added S4 SOC compatibility >> >> V6 -> V7: To solve the system stuck when you run the stty command on >> the console to change the baud rate. >> V5 -> V6: Change error format as discussed in the email. >> V4 -> V5: Change error format. >> V3 -> V4: Change CCF to describe the UART baud rate clock as discussed >> in the email. >> V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it must change >> the DTS before it can be deleted >> V1 -> V2: Use CCF to describe the UART baud rate clock.Make some changes as >> discussed in the email >> >> Link:https://lore.kernel.org/linux-amlogic/20220118030911.12815-4-yu.tu@amlogic.com/ >> >> drivers/tty/serial/meson_uart.c | 221 ++++++++++++++++++++++---------- >> 1 file changed, 154 insertions(+), 67 deletions(-) >> >> >> base-commit: a603ca60cebff8589882427a67f870ed946b3fc8 >
Hi, Le 25/02/2022 à 08:39, Yu Tu a écrit : > Using the common Clock code to describe the UART baud rate > clock makes it easier for the UART driver to be compatible > with the baud rate requirements of the UART IP on different > meson chips. Add Meson S4 SoC compatible. > > The test method: > Start the console and run the following commands in turn: > stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600. > > Since most SoCs are too old, I was able to find all the platforms myself > such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with > G12A and S4. > > Yu Tu (6): > tty: serial: meson: Move request the register region to probe > tty: serial: meson: Use devm_ioremap_resource to get register mapped > memory > tty: serial: meson: Describes the calculation of the UART baud rate > clock using a clock frame > tty: serial: meson: Make some bit of the REG5 register writable > tty: serial: meson: The system stuck when you run the stty command on > the console to change the baud rate > tty: serial: meson: Added S4 SOC compatibility > > V6 -> V7: To solve the system stuck when you run the stty command on > the console to change the baud rate. > V5 -> V6: Change error format as discussed in the email. > V4 -> V5: Change error format. > V3 -> V4: Change CCF to describe the UART baud rate clock as discussed > in the email. > V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it must change > the DTS before it can be deleted > V1 -> V2: Use CCF to describe the UART baud rate clock.Make some changes as > discussed in the email > > Link:https://lore.kernel.org/linux-amlogic/20220118030911.12815-4-yu.tu@amlogic.com/ > > drivers/tty/serial/meson_uart.c | 221 ++++++++++++++++++++++---------- > 1 file changed, 154 insertions(+), 67 deletions(-) > > > base-commit: a603ca60cebff8589882427a67f870ed946b3fc8 Could you send the emails To Kevin, Jerome, Martin & me, and put the various lists in CC instead ? otherwise we are not notified when the patch is accepted by the tty maintainer. Thanks, Neil
Hi Neil, On 2022/3/1 15:25, Neil Armstrong wrote: > [ EXTERNAL EMAIL ] > > Hi, > > Le 25/02/2022 à 08:39, Yu Tu a écrit : >> Using the common Clock code to describe the UART baud rate >> clock makes it easier for the UART driver to be compatible >> with the baud rate requirements of the UART IP on different >> meson chips. Add Meson S4 SoC compatible. >> >> The test method: >> Start the console and run the following commands in turn: >> stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600. >> >> Since most SoCs are too old, I was able to find all the platforms myself >> such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with >> G12A and S4. >> >> Yu Tu (6): >> tty: serial: meson: Move request the register region to probe >> tty: serial: meson: Use devm_ioremap_resource to get register mapped >> memory >> tty: serial: meson: Describes the calculation of the UART baud rate >> clock using a clock frame >> tty: serial: meson: Make some bit of the REG5 register writable >> tty: serial: meson: The system stuck when you run the stty command on >> the console to change the baud rate >> tty: serial: meson: Added S4 SOC compatibility >> >> V6 -> V7: To solve the system stuck when you run the stty command on >> the console to change the baud rate. >> V5 -> V6: Change error format as discussed in the email. >> V4 -> V5: Change error format. >> V3 -> V4: Change CCF to describe the UART baud rate clock as discussed >> in the email. >> V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it must >> change >> the DTS before it can be deleted >> V1 -> V2: Use CCF to describe the UART baud rate clock.Make some >> changes as >> discussed in the email >> >> Link:https://lore.kernel.org/linux-amlogic/20220118030911.12815-4-yu.tu@amlogic.com/ >> >> >> drivers/tty/serial/meson_uart.c | 221 ++++++++++++++++++++++---------- >> 1 file changed, 154 insertions(+), 67 deletions(-) >> >> >> base-commit: a603ca60cebff8589882427a67f870ed946b3fc8 > > Could you send the emails To Kevin, Jerome, Martin & me, and put the > various lists in CC instead ? otherwise we are not notified when the > patch is accepted by the tty maintainer. The fact is that sending is adding you up, you see Link: https://lore.kernel.org/linux-amlogic/20220225073922.3947-1-yu.tu@amlogic.com/ > > Thanks, > Neil >
On Tue 01 Mar 2022 at 13:54, Yu Tu <yu.tu@amlogic.com> wrote: > Hi Jerome, > > On 2022/2/28 18:59, Jerome Brunet wrote: >> [ EXTERNAL EMAIL ] >> >> On Fri 25 Feb 2022 at 15:39, Yu Tu <yu.tu@amlogic.com> wrote: >> >>> Using the common Clock code to describe the UART baud rate >>> clock makes it easier for the UART driver to be compatible >>> with the baud rate requirements of the UART IP on different >>> meson chips. Add Meson S4 SoC compatible. >>> >>> The test method: >>> Start the console and run the following commands in turn: >>> stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600. >>> >>> Since most SoCs are too old, I was able to find all the platforms myself >>> such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with >>> G12A and S4. >> GXL based board are still very common an easy to come by. >> I'm quite surprised that you are unable to test on this SoC family > The fact of the matter is that the S4 is our end-2020 chip, the G12A is > five years old, and the GXL is seven years old. If you must ask for a > test, I will report this problem to the leadership to coordinate resources. The age of the SoC is irrelevant. SoCs don't get deprecated based on age in mainline. It is not just GXL, same goes for meson8. These SoCs are actively used. Boards with these SoCs are still sold and easily available. See the VIM1 or the Libretech boards. Breaking things for the the users of these SoCs is not acceptable. So yes, looking at your series, I strongly recommend you do more tests. >> >>> >>> Yu Tu (6): >>> tty: serial: meson: Move request the register region to probe >>> tty: serial: meson: Use devm_ioremap_resource to get register mapped >>> memory >>> tty: serial: meson: Describes the calculation of the UART baud rate >>> clock using a clock frame >>> tty: serial: meson: Make some bit of the REG5 register writable >>> tty: serial: meson: The system stuck when you run the stty command on >>> the console to change the baud rate >>> tty: serial: meson: Added S4 SOC compatibility >>> >>> V6 -> V7: To solve the system stuck when you run the stty command on >>> the console to change the baud rate. >>> V5 -> V6: Change error format as discussed in the email. >>> V4 -> V5: Change error format. >>> V3 -> V4: Change CCF to describe the UART baud rate clock as discussed >>> in the email. >>> V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it must change >>> the DTS before it can be deleted >>> V1 -> V2: Use CCF to describe the UART baud rate clock.Make some changes as >>> discussed in the email >>> >>> Link:https://lore.kernel.org/linux-amlogic/20220118030911.12815-4-yu.tu@amlogic.com/ >>> >>> drivers/tty/serial/meson_uart.c | 221 ++++++++++++++++++++++---------- >>> 1 file changed, 154 insertions(+), 67 deletions(-) >>> >>> >>> base-commit: a603ca60cebff8589882427a67f870ed946b3fc8 >>
Hi, On 01/03/2022 08:57, Yu Tu wrote: > Hi Neil, > > On 2022/3/1 15:25, Neil Armstrong wrote: >> [ EXTERNAL EMAIL ] >> >> Hi, >> >> Le 25/02/2022 à 08:39, Yu Tu a écrit : >>> Using the common Clock code to describe the UART baud rate >>> clock makes it easier for the UART driver to be compatible >>> with the baud rate requirements of the UART IP on different >>> meson chips. Add Meson S4 SoC compatible. >>> >>> The test method: >>> Start the console and run the following commands in turn: >>> stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600. >>> >>> Since most SoCs are too old, I was able to find all the platforms myself >>> such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with >>> G12A and S4. >>> >>> Yu Tu (6): >>> tty: serial: meson: Move request the register region to probe >>> tty: serial: meson: Use devm_ioremap_resource to get register mapped >>> memory >>> tty: serial: meson: Describes the calculation of the UART baud rate >>> clock using a clock frame >>> tty: serial: meson: Make some bit of the REG5 register writable >>> tty: serial: meson: The system stuck when you run the stty command on >>> the console to change the baud rate >>> tty: serial: meson: Added S4 SOC compatibility >>> >>> V6 -> V7: To solve the system stuck when you run the stty command on >>> the console to change the baud rate. >>> V5 -> V6: Change error format as discussed in the email. >>> V4 -> V5: Change error format. >>> V3 -> V4: Change CCF to describe the UART baud rate clock as discussed >>> in the email. >>> V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it must change >>> the DTS before it can be deleted >>> V1 -> V2: Use CCF to describe the UART baud rate clock.Make some changes as >>> discussed in the email >>> >>> Link:https://lore.kernel.org/linux-amlogic/20220118030911.12815-4-yu.tu@amlogic.com/ >>> >>> drivers/tty/serial/meson_uart.c | 221 ++++++++++++++++++++++---------- >>> 1 file changed, 154 insertions(+), 67 deletions(-) >>> >>> >>> base-commit: a603ca60cebff8589882427a67f870ed946b3fc8 >> >> Could you send the emails To Kevin, Jerome, Martin & me, and put the various lists in CC instead ? otherwise we are not notified when the patch is accepted by the tty maintainer. > The fact is that sending is adding you up, you see > Link: https://lore.kernel.org/linux-amlogic/20220225073922.3947-1-yu.tu@amlogic.com/ It's not the point, the order of recipient in Cc and To is important since they are used by maintainers to track patch for review and to notify when patches are taken in their tree. So please make sure the recipients are correctly set before sending the patches. Neil >> >> Thanks, >> Neil >>
Hi Neil, On 2022/3/1 16:37, Neil Armstrong wrote: > [ EXTERNAL EMAIL ] > > Hi, > > On 01/03/2022 08:57, Yu Tu wrote: >> Hi Neil, >> >> On 2022/3/1 15:25, Neil Armstrong wrote: >>> [ EXTERNAL EMAIL ] >>> >>> Hi, >>> >>> Le 25/02/2022 à 08:39, Yu Tu a écrit : >>>> Using the common Clock code to describe the UART baud rate >>>> clock makes it easier for the UART driver to be compatible >>>> with the baud rate requirements of the UART IP on different >>>> meson chips. Add Meson S4 SoC compatible. >>>> >>>> The test method: >>>> Start the console and run the following commands in turn: >>>> stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600. >>>> >>>> Since most SoCs are too old, I was able to find all the platforms >>>> myself >>>> such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with >>>> G12A and S4. >>>> >>>> Yu Tu (6): >>>> tty: serial: meson: Move request the register region to probe >>>> tty: serial: meson: Use devm_ioremap_resource to get register mapped >>>> memory >>>> tty: serial: meson: Describes the calculation of the UART baud rate >>>> clock using a clock frame >>>> tty: serial: meson: Make some bit of the REG5 register writable >>>> tty: serial: meson: The system stuck when you run the stty >>>> command on >>>> the console to change the baud rate >>>> tty: serial: meson: Added S4 SOC compatibility >>>> >>>> V6 -> V7: To solve the system stuck when you run the stty command on >>>> the console to change the baud rate. >>>> V5 -> V6: Change error format as discussed in the email. >>>> V4 -> V5: Change error format. >>>> V3 -> V4: Change CCF to describe the UART baud rate clock as discussed >>>> in the email. >>>> V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it must >>>> change >>>> the DTS before it can be deleted >>>> V1 -> V2: Use CCF to describe the UART baud rate clock.Make some >>>> changes as >>>> discussed in the email >>>> >>>> Link:https://lore.kernel.org/linux-amlogic/20220118030911.12815-4-yu.tu@amlogic.com/ >>>> >>>> >>>> drivers/tty/serial/meson_uart.c | 221 >>>> ++++++++++++++++++++++---------- >>>> 1 file changed, 154 insertions(+), 67 deletions(-) >>>> >>>> >>>> base-commit: a603ca60cebff8589882427a67f870ed946b3fc8 >>> >>> Could you send the emails To Kevin, Jerome, Martin & me, and put the >>> various lists in CC instead ? otherwise we are not notified when the >>> patch is accepted by the tty maintainer. >> The fact is that sending is adding you up, you see >> Link: >> https://lore.kernel.org/linux-amlogic/20220225073922.3947-1-yu.tu@amlogic.com/ >> > > It's not the point, the order of recipient in Cc and To is important > since they are used > by maintainers to track patch for review and to notify when patches are > taken in their tree. > > So please make sure the recipients are correctly set before sending the > patches. > I see what you mean. Mailing lists should all use To. > Neil > >>> >>> Thanks, >>> Neil >>> >
Hi Jerome, On 2022/3/1 16:36, Jerome Brunet wrote: > [ EXTERNAL EMAIL ] > > > On Tue 01 Mar 2022 at 13:54, Yu Tu <yu.tu@amlogic.com> wrote: > >> Hi Jerome, >> >> On 2022/2/28 18:59, Jerome Brunet wrote: >>> [ EXTERNAL EMAIL ] >>> >>> On Fri 25 Feb 2022 at 15:39, Yu Tu <yu.tu@amlogic.com> wrote: >>> >>>> Using the common Clock code to describe the UART baud rate >>>> clock makes it easier for the UART driver to be compatible >>>> with the baud rate requirements of the UART IP on different >>>> meson chips. Add Meson S4 SoC compatible. >>>> >>>> The test method: >>>> Start the console and run the following commands in turn: >>>> stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600. >>>> >>>> Since most SoCs are too old, I was able to find all the platforms myself >>>> such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with >>>> G12A and S4. >>> GXL based board are still very common an easy to come by. >>> I'm quite surprised that you are unable to test on this SoC family >> The fact of the matter is that the S4 is our end-2020 chip, the G12A is >> five years old, and the GXL is seven years old. If you must ask for a >> test, I will report this problem to the leadership to coordinate resources. > > The age of the SoC is irrelevant. SoCs don't get deprecated based on age > in mainline. It is not just GXL, same goes for meson8. > > These SoCs are actively used. Boards with these SoCs are still sold and > easily available. See the VIM1 or the Libretech boards. > > Breaking things for the the users of these SoCs is not acceptable. > So yes, looking at your series, I strongly recommend you do more tests. > You have a point there. Let's go back to the root of the problem. I aim to increase S4. The S4 uses 12MHZ to calculate baud. That's all. Change it to CCF as you suggested. The changes are so large that you ask to test all the chips. I also mentioned last time that using CCF would lead to a longer drive probe time and affect the board startup time. If this problem is not solved, can we reject the way you suggest using CCF? >>> >>>> >>>> Yu Tu (6): >>>> tty: serial: meson: Move request the register region to probe >>>> tty: serial: meson: Use devm_ioremap_resource to get register mapped >>>> memory >>>> tty: serial: meson: Describes the calculation of the UART baud rate >>>> clock using a clock frame >>>> tty: serial: meson: Make some bit of the REG5 register writable >>>> tty: serial: meson: The system stuck when you run the stty command on >>>> the console to change the baud rate >>>> tty: serial: meson: Added S4 SOC compatibility >>>> >>>> V6 -> V7: To solve the system stuck when you run the stty command on >>>> the console to change the baud rate. >>>> V5 -> V6: Change error format as discussed in the email. >>>> V4 -> V5: Change error format. >>>> V3 -> V4: Change CCF to describe the UART baud rate clock as discussed >>>> in the email. >>>> V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it must change >>>> the DTS before it can be deleted >>>> V1 -> V2: Use CCF to describe the UART baud rate clock.Make some changes as >>>> discussed in the email >>>> >>>> Link:https://lore.kernel.org/linux-amlogic/20220118030911.12815-4-yu.tu@amlogic.com/ >>>> >>>> drivers/tty/serial/meson_uart.c | 221 ++++++++++++++++++++++---------- >>>> 1 file changed, 154 insertions(+), 67 deletions(-) >>>> >>>> >>>> base-commit: a603ca60cebff8589882427a67f870ed946b3fc8 >>> >
Hi, On 01/03/2022 10:01, Yu Tu wrote: > Hi Jerome, > > On 2022/3/1 16:36, Jerome Brunet wrote: >> [ EXTERNAL EMAIL ] >> >> >> On Tue 01 Mar 2022 at 13:54, Yu Tu <yu.tu@amlogic.com> wrote: >> >>> Hi Jerome, >>> >>> On 2022/2/28 18:59, Jerome Brunet wrote: >>>> [ EXTERNAL EMAIL ] >>>> >>>> On Fri 25 Feb 2022 at 15:39, Yu Tu <yu.tu@amlogic.com> wrote: >>>> >>>>> Using the common Clock code to describe the UART baud rate >>>>> clock makes it easier for the UART driver to be compatible >>>>> with the baud rate requirements of the UART IP on different >>>>> meson chips. Add Meson S4 SoC compatible. >>>>> >>>>> The test method: >>>>> Start the console and run the following commands in turn: >>>>> stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600. >>>>> >>>>> Since most SoCs are too old, I was able to find all the platforms myself >>>>> such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with >>>>> G12A and S4. >>>> GXL based board are still very common an easy to come by. >>>> I'm quite surprised that you are unable to test on this SoC family >>> The fact of the matter is that the S4 is our end-2020 chip, the G12A is >>> five years old, and the GXL is seven years old. If you must ask for a >>> test, I will report this problem to the leadership to coordinate resources. >> >> The age of the SoC is irrelevant. SoCs don't get deprecated based on age >> in mainline. It is not just GXL, same goes for meson8. >> >> These SoCs are actively used. Boards with these SoCs are still sold and >> easily available. See the VIM1 or the Libretech boards. >> >> Breaking things for the the users of these SoCs is not acceptable. >> So yes, looking at your series, I strongly recommend you do more tests. >> > You have a point there. Let's go back to the root of the problem. I aim to increase S4. The S4 uses 12MHZ to calculate baud. That's all. > Change it to CCF as you suggested. The changes are so large that you ask to test all the chips. > I also mentioned last time that using CCF would lead to a longer drive probe time and affect the board startup time. If this problem is not solved, can we reject the way you suggest using CCF? I have a much simpler proposal (non-tested): ============><=================================================== diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c index 45e00d928253..eda3fdad60d1 100644 --- a/drivers/tty/serial/meson_uart.c +++ b/drivers/tty/serial/meson_uart.c @@ -76,6 +76,12 @@ #define AML_UART_POLL_USEC 5 #define AML_UART_TIMEOUT_USEC 10000 +struct meson_uart_data { + struct clk *clk_pclk; + struct clk *clk_xtal; + struct clk *clk_baud; +}; + static struct uart_driver meson_uart_driver; static struct uart_port *meson_ports[AML_UART_PORT_NUM]; @@ -293,16 +299,17 @@ 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 *private_data = port->private_data; u32 val; while (!meson_uart_tx_empty(port)) cpu_relax(); - if (port->uartclk == 24000000) { - val = ((port->uartclk / 3) / baud) - 1; + if (clk_is_match(private_data->clk_baud, private_data->clk_xtal)) { + val = ((clk_get_rate(private_data->clk_baud) / 3) / baud) - 1; val |= AML_UART_BAUD_XTAL; } else { - val = ((port->uartclk * 10 / (baud * 4) + 5) / 10) - 1; + val = ((clk_get_rate(private_data->clk_baud) * 10 / (baud * 4) + 5) / 10) - 1; } val |= AML_UART_BAUD_USE; writel(val, port->membase + AML_UART_REG5); @@ -666,31 +673,26 @@ static inline struct clk *meson_uart_probe_clock(struct device *dev, } static int meson_uart_probe_clocks(struct platform_device *pdev, - struct uart_port *port) + struct meson_uart_data *private_data) { - struct clk *clk_xtal = NULL; - struct clk *clk_pclk = NULL; - struct clk *clk_baud = NULL; - - clk_pclk = meson_uart_probe_clock(&pdev->dev, "pclk"); - if (IS_ERR(clk_pclk)) - return PTR_ERR(clk_pclk); + private_data->clk_pclk = meson_uart_probe_clock(&pdev->dev, "pclk"); + if (IS_ERR(private_data->clk_pclk)) + return PTR_ERR(private_data->clk_pclk); - clk_xtal = meson_uart_probe_clock(&pdev->dev, "xtal"); - if (IS_ERR(clk_xtal)) - return PTR_ERR(clk_xtal); + private_data->clk_xtal = meson_uart_probe_clock(&pdev->dev, "xtal"); + if (IS_ERR(private_data->clk_xtal)) + return PTR_ERR(private_data->clk_xtal); - clk_baud = meson_uart_probe_clock(&pdev->dev, "baud"); - if (IS_ERR(clk_baud)) - return PTR_ERR(clk_baud); - - port->uartclk = clk_get_rate(clk_baud); + private_data->clk_baud = meson_uart_probe_clock(&pdev->dev, "baud"); + if (IS_ERR(private_data->clk_baud)) + return PTR_ERR(private_data->clk_baud); return 0; } static int meson_uart_probe(struct platform_device *pdev) { + struct meson_uart_data *private_data; struct resource *res_mem; struct uart_port *port; u32 fifosize = 64; /* Default is 64, 128 for EE UART_0 */ @@ -714,6 +716,11 @@ static int meson_uart_probe(struct platform_device *pdev) if (pdev->id < 0 || pdev->id >= AML_UART_PORT_NUM) return -EINVAL; + private_data = devm_kzalloc(&pdev->dev, sizeof(*private_data), + GFP_KERNEL); + if (!private_data) + return -ENOMEM; + res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res_mem) return -ENODEV; @@ -733,7 +740,7 @@ static int meson_uart_probe(struct platform_device *pdev) if (!port) return -ENOMEM; - ret = meson_uart_probe_clocks(pdev, port); + ret = meson_uart_probe_clocks(pdev, private_data); if (ret) return ret; @@ -749,6 +756,7 @@ static int meson_uart_probe(struct platform_device *pdev) port->x_char = 0; port->ops = &meson_uart_ops; port->fifosize = fifosize; + port->private_data = private_data; meson_ports[pdev->id] = port; platform_set_drvdata(pdev, port); ============><=================================================== Neil >>>> >>>>> >>>>> Yu Tu (6): >>>>> tty: serial: meson: Move request the register region to probe >>>>> tty: serial: meson: Use devm_ioremap_resource to get register mapped >>>>> memory >>>>> tty: serial: meson: Describes the calculation of the UART baud rate >>>>> clock using a clock frame >>>>> tty: serial: meson: Make some bit of the REG5 register writable >>>>> tty: serial: meson: The system stuck when you run the stty command on >>>>> the console to change the baud rate >>>>> tty: serial: meson: Added S4 SOC compatibility >>>>> >>>>> V6 -> V7: To solve the system stuck when you run the stty command on >>>>> the console to change the baud rate. >>>>> V5 -> V6: Change error format as discussed in the email. >>>>> V4 -> V5: Change error format. >>>>> V3 -> V4: Change CCF to describe the UART baud rate clock as discussed >>>>> in the email. >>>>> V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it must change >>>>> the DTS before it can be deleted >>>>> V1 -> V2: Use CCF to describe the UART baud rate clock.Make some changes as >>>>> discussed in the email >>>>> >>>>> Link:https://lore.kernel.org/linux-amlogic/20220118030911.12815-4-yu.tu@amlogic.com/ >>>>> >>>>> drivers/tty/serial/meson_uart.c | 221 ++++++++++++++++++++++---------- >>>>> 1 file changed, 154 insertions(+), 67 deletions(-) >>>>> >>>>> >>>>> base-commit: a603ca60cebff8589882427a67f870ed946b3fc8 >>>> >>
Hi Neil, Thank you very much for your advice. On 2022/3/1 17:27, Neil Armstrong wrote: > [ EXTERNAL EMAIL ] > > Hi, > > On 01/03/2022 10:01, Yu Tu wrote: >> Hi Jerome, >> >> On 2022/3/1 16:36, Jerome Brunet wrote: >>> [ EXTERNAL EMAIL ] >>> >>> >>> On Tue 01 Mar 2022 at 13:54, Yu Tu <yu.tu@amlogic.com> wrote: >>> >>>> Hi Jerome, >>>> >>>> On 2022/2/28 18:59, Jerome Brunet wrote: >>>>> [ EXTERNAL EMAIL ] >>>>> >>>>> On Fri 25 Feb 2022 at 15:39, Yu Tu <yu.tu@amlogic.com> wrote: >>>>> >>>>>> Using the common Clock code to describe the UART baud rate >>>>>> clock makes it easier for the UART driver to be compatible >>>>>> with the baud rate requirements of the UART IP on different >>>>>> meson chips. Add Meson S4 SoC compatible. >>>>>> >>>>>> The test method: >>>>>> Start the console and run the following commands in turn: >>>>>> stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600. >>>>>> >>>>>> Since most SoCs are too old, I was able to find all the platforms >>>>>> myself >>>>>> such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it with >>>>>> G12A and S4. >>>>> GXL based board are still very common an easy to come by. >>>>> I'm quite surprised that you are unable to test on this SoC family >>>> The fact of the matter is that the S4 is our end-2020 chip, the G12A is >>>> five years old, and the GXL is seven years old. If you must ask for a >>>> test, I will report this problem to the leadership to coordinate >>>> resources. >>> >>> The age of the SoC is irrelevant. SoCs don't get deprecated based on age >>> in mainline. It is not just GXL, same goes for meson8. >>> >>> These SoCs are actively used. Boards with these SoCs are still sold and >>> easily available. See the VIM1 or the Libretech boards. >>> >>> Breaking things for the the users of these SoCs is not acceptable. >>> So yes, looking at your series, I strongly recommend you do more tests. >>> >> You have a point there. Let's go back to the root of the problem. I >> aim to increase S4. The S4 uses 12MHZ to calculate baud. That's all. >> Change it to CCF as you suggested. The changes are so large that you >> ask to test all the chips. >> I also mentioned last time that using CCF would lead to a longer drive >> probe time and affect the board startup time. If this problem is not >> solved, can we reject the way you suggest using CCF? > > I have a much simpler proposal (non-tested): > I can test it for you. But if you need to add S4 chips. Again, we need to add a judgment. Is that acceptable to you? What we've been talking about for a long time is how to make the least changes and the least impact for the S4. The fact is that Amlogic has recommended 12MHz for baud rates for all chips starting with the G12A. I really doubt whether the G12A/B and SM1 were tested before. So if we want to upload subsequent Amlogic chips, not only the S4 needs to solve the 12MHz problem, but other chips can't get around this area. > ============><=================================================== > diff --git a/drivers/tty/serial/meson_uart.c > b/drivers/tty/serial/meson_uart.c > index 45e00d928253..eda3fdad60d1 100644 > --- a/drivers/tty/serial/meson_uart.c > +++ b/drivers/tty/serial/meson_uart.c > @@ -76,6 +76,12 @@ > #define AML_UART_POLL_USEC 5 > #define AML_UART_TIMEOUT_USEC 10000 > > +struct meson_uart_data { > + struct clk *clk_pclk; > + struct clk *clk_xtal; > + struct clk *clk_baud; > +}; > + > static struct uart_driver meson_uart_driver; > > static struct uart_port *meson_ports[AML_UART_PORT_NUM]; > @@ -293,16 +299,17 @@ 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 *private_data = port->private_data; > u32 val; > > while (!meson_uart_tx_empty(port)) > cpu_relax(); > > - if (port->uartclk == 24000000) { > - val = ((port->uartclk / 3) / baud) - 1; > + if (clk_is_match(private_data->clk_baud, private_data->clk_xtal)) { > + val = ((clk_get_rate(private_data->clk_baud) / 3) / baud) - 1; > val |= AML_UART_BAUD_XTAL; > } else { > - val = ((port->uartclk * 10 / (baud * 4) + 5) / 10) - 1; > + val = ((clk_get_rate(private_data->clk_baud) * 10 / (baud * 4) > + 5) / 10) - 1; > } > val |= AML_UART_BAUD_USE; > writel(val, port->membase + AML_UART_REG5); > @@ -666,31 +673,26 @@ static inline struct clk > *meson_uart_probe_clock(struct device *dev, > } > > static int meson_uart_probe_clocks(struct platform_device *pdev, > - struct uart_port *port) > + struct meson_uart_data *private_data) > { > - struct clk *clk_xtal = NULL; > - struct clk *clk_pclk = NULL; > - struct clk *clk_baud = NULL; > - > - clk_pclk = meson_uart_probe_clock(&pdev->dev, "pclk"); > - if (IS_ERR(clk_pclk)) > - return PTR_ERR(clk_pclk); > + private_data->clk_pclk = meson_uart_probe_clock(&pdev->dev, "pclk"); > + if (IS_ERR(private_data->clk_pclk)) > + return PTR_ERR(private_data->clk_pclk); > > - clk_xtal = meson_uart_probe_clock(&pdev->dev, "xtal"); > - if (IS_ERR(clk_xtal)) > - return PTR_ERR(clk_xtal); > + private_data->clk_xtal = meson_uart_probe_clock(&pdev->dev, "xtal"); > + if (IS_ERR(private_data->clk_xtal)) > + return PTR_ERR(private_data->clk_xtal); > > - clk_baud = meson_uart_probe_clock(&pdev->dev, "baud"); > - if (IS_ERR(clk_baud)) > - return PTR_ERR(clk_baud); > - > - port->uartclk = clk_get_rate(clk_baud); > + private_data->clk_baud = meson_uart_probe_clock(&pdev->dev, "baud"); > + if (IS_ERR(private_data->clk_baud)) > + return PTR_ERR(private_data->clk_baud); > > return 0; > } > > static int meson_uart_probe(struct platform_device *pdev) > { > + struct meson_uart_data *private_data; > struct resource *res_mem; > struct uart_port *port; > u32 fifosize = 64; /* Default is 64, 128 for EE UART_0 */ > @@ -714,6 +716,11 @@ static int meson_uart_probe(struct platform_device > *pdev) > if (pdev->id < 0 || pdev->id >= AML_UART_PORT_NUM) > return -EINVAL; > > + private_data = devm_kzalloc(&pdev->dev, sizeof(*private_data), > + GFP_KERNEL); > + if (!private_data) > + return -ENOMEM; > + > res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!res_mem) > return -ENODEV; > @@ -733,7 +740,7 @@ static int meson_uart_probe(struct platform_device > *pdev) > if (!port) > return -ENOMEM; > > - ret = meson_uart_probe_clocks(pdev, port); > + ret = meson_uart_probe_clocks(pdev, private_data); > if (ret) > return ret; > > @@ -749,6 +756,7 @@ static int meson_uart_probe(struct platform_device > *pdev) > port->x_char = 0; > port->ops = &meson_uart_ops; > port->fifosize = fifosize; > + port->private_data = private_data; > > meson_ports[pdev->id] = port; > platform_set_drvdata(pdev, port); > ============><=================================================== > > Neil > >>>>> >>>>>> >>>>>> Yu Tu (6): >>>>>> tty: serial: meson: Move request the register region to probe >>>>>> tty: serial: meson: Use devm_ioremap_resource to get register >>>>>> mapped >>>>>> memory >>>>>> tty: serial: meson: Describes the calculation of the UART baud >>>>>> rate >>>>>> clock using a clock frame >>>>>> tty: serial: meson: Make some bit of the REG5 register writable >>>>>> tty: serial: meson: The system stuck when you run the stty >>>>>> command on >>>>>> the console to change the baud rate >>>>>> tty: serial: meson: Added S4 SOC compatibility >>>>>> >>>>>> V6 -> V7: To solve the system stuck when you run the stty command on >>>>>> the console to change the baud rate. >>>>>> V5 -> V6: Change error format as discussed in the email. >>>>>> V4 -> V5: Change error format. >>>>>> V3 -> V4: Change CCF to describe the UART baud rate clock as >>>>>> discussed >>>>>> in the email. >>>>>> V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it >>>>>> must change >>>>>> the DTS before it can be deleted >>>>>> V1 -> V2: Use CCF to describe the UART baud rate clock.Make some >>>>>> changes as >>>>>> discussed in the email >>>>>> >>>>>> Link:https://lore.kernel.org/linux-amlogic/20220118030911.12815-4-yu.tu@amlogic.com/ >>>>>> >>>>>> >>>>>> drivers/tty/serial/meson_uart.c | 221 >>>>>> ++++++++++++++++++++++---------- >>>>>> 1 file changed, 154 insertions(+), 67 deletions(-) >>>>>> >>>>>> >>>>>> base-commit: a603ca60cebff8589882427a67f870ed946b3fc8 >>>>> >>> >
Hi Greg, I have no problem with trying to revert this list of submissions. I'm sorry about that. On 2022/3/1 18:12, Yu Tu wrote: > Hi Neil, > Thank you very much for your advice. > > On 2022/3/1 17:27, Neil Armstrong wrote: >> [ EXTERNAL EMAIL ] >> >> Hi, >> >> On 01/03/2022 10:01, Yu Tu wrote: >>> Hi Jerome, >>> >>> On 2022/3/1 16:36, Jerome Brunet wrote: >>>> [ EXTERNAL EMAIL ] >>>> >>>> >>>> On Tue 01 Mar 2022 at 13:54, Yu Tu <yu.tu@amlogic.com> wrote: >>>> >>>>> Hi Jerome, >>>>> >>>>> On 2022/2/28 18:59, Jerome Brunet wrote: >>>>>> [ EXTERNAL EMAIL ] >>>>>> >>>>>> On Fri 25 Feb 2022 at 15:39, Yu Tu <yu.tu@amlogic.com> wrote: >>>>>> >>>>>>> Using the common Clock code to describe the UART baud rate >>>>>>> clock makes it easier for the UART driver to be compatible >>>>>>> with the baud rate requirements of the UART IP on different >>>>>>> meson chips. Add Meson S4 SoC compatible. >>>>>>> >>>>>>> The test method: >>>>>>> Start the console and run the following commands in turn: >>>>>>> stty -F /dev/ttyAML0 115200 and stty -F /dev/ttyAML0 921600. >>>>>>> >>>>>>> Since most SoCs are too old, I was able to find all the platforms >>>>>>> myself >>>>>>> such as Meson6, Meson8, Meson8b, GXL and so on. I only tested it >>>>>>> with >>>>>>> G12A and S4. >>>>>> GXL based board are still very common an easy to come by. >>>>>> I'm quite surprised that you are unable to test on this SoC family >>>>> The fact of the matter is that the S4 is our end-2020 chip, the >>>>> G12A is >>>>> five years old, and the GXL is seven years old. If you must ask for a >>>>> test, I will report this problem to the leadership to coordinate >>>>> resources. >>>> >>>> The age of the SoC is irrelevant. SoCs don't get deprecated based on >>>> age >>>> in mainline. It is not just GXL, same goes for meson8. >>>> >>>> These SoCs are actively used. Boards with these SoCs are still sold and >>>> easily available. See the VIM1 or the Libretech boards. >>>> >>>> Breaking things for the the users of these SoCs is not acceptable. >>>> So yes, looking at your series, I strongly recommend you do more tests. >>>> >>> You have a point there. Let's go back to the root of the problem. I >>> aim to increase S4. The S4 uses 12MHZ to calculate baud. That's all. >>> Change it to CCF as you suggested. The changes are so large that you >>> ask to test all the chips. >>> I also mentioned last time that using CCF would lead to a longer >>> drive probe time and affect the board startup time. If this problem >>> is not solved, can we reject the way you suggest using CCF? >> >> I have a much simpler proposal (non-tested): >> > I can test it for you. But if you need to add S4 chips. Again, we need > to add a judgment. Is that acceptable to you? > > What we've been talking about for a long time is how to make the least > changes and the least impact for the S4. > > The fact is that Amlogic has recommended 12MHz for baud rates for all > chips starting with the G12A. I really doubt whether the G12A/B and SM1 > were tested before. So if we want to upload subsequent Amlogic chips, > not only the S4 needs to solve the 12MHz problem, but other chips can't > get around this area. >> ============><=================================================== >> diff --git a/drivers/tty/serial/meson_uart.c >> b/drivers/tty/serial/meson_uart.c >> index 45e00d928253..eda3fdad60d1 100644 >> --- a/drivers/tty/serial/meson_uart.c >> +++ b/drivers/tty/serial/meson_uart.c >> @@ -76,6 +76,12 @@ >> #define AML_UART_POLL_USEC 5 >> #define AML_UART_TIMEOUT_USEC 10000 >> >> +struct meson_uart_data { >> + struct clk *clk_pclk; >> + struct clk *clk_xtal; >> + struct clk *clk_baud; >> +}; >> + >> static struct uart_driver meson_uart_driver; >> >> static struct uart_port *meson_ports[AML_UART_PORT_NUM]; >> @@ -293,16 +299,17 @@ 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 *private_data = port->private_data; >> u32 val; >> >> while (!meson_uart_tx_empty(port)) >> cpu_relax(); >> >> - if (port->uartclk == 24000000) { >> - val = ((port->uartclk / 3) / baud) - 1; >> + if (clk_is_match(private_data->clk_baud, private_data->clk_xtal)) { >> + val = ((clk_get_rate(private_data->clk_baud) / 3) / baud) - 1; >> val |= AML_UART_BAUD_XTAL; >> } else { >> - val = ((port->uartclk * 10 / (baud * 4) + 5) / 10) - 1; >> + val = ((clk_get_rate(private_data->clk_baud) * 10 / (baud * >> 4) + 5) / 10) - 1; >> } >> val |= AML_UART_BAUD_USE; >> writel(val, port->membase + AML_UART_REG5); >> @@ -666,31 +673,26 @@ static inline struct clk >> *meson_uart_probe_clock(struct device *dev, >> } >> >> static int meson_uart_probe_clocks(struct platform_device *pdev, >> - struct uart_port *port) >> + struct meson_uart_data *private_data) >> { >> - struct clk *clk_xtal = NULL; >> - struct clk *clk_pclk = NULL; >> - struct clk *clk_baud = NULL; >> - >> - clk_pclk = meson_uart_probe_clock(&pdev->dev, "pclk"); >> - if (IS_ERR(clk_pclk)) >> - return PTR_ERR(clk_pclk); >> + private_data->clk_pclk = meson_uart_probe_clock(&pdev->dev, "pclk"); >> + if (IS_ERR(private_data->clk_pclk)) >> + return PTR_ERR(private_data->clk_pclk); >> >> - clk_xtal = meson_uart_probe_clock(&pdev->dev, "xtal"); >> - if (IS_ERR(clk_xtal)) >> - return PTR_ERR(clk_xtal); >> + private_data->clk_xtal = meson_uart_probe_clock(&pdev->dev, "xtal"); >> + if (IS_ERR(private_data->clk_xtal)) >> + return PTR_ERR(private_data->clk_xtal); >> >> - clk_baud = meson_uart_probe_clock(&pdev->dev, "baud"); >> - if (IS_ERR(clk_baud)) >> - return PTR_ERR(clk_baud); >> - >> - port->uartclk = clk_get_rate(clk_baud); >> + private_data->clk_baud = meson_uart_probe_clock(&pdev->dev, "baud"); >> + if (IS_ERR(private_data->clk_baud)) >> + return PTR_ERR(private_data->clk_baud); >> >> return 0; >> } >> >> static int meson_uart_probe(struct platform_device *pdev) >> { >> + struct meson_uart_data *private_data; >> struct resource *res_mem; >> struct uart_port *port; >> u32 fifosize = 64; /* Default is 64, 128 for EE UART_0 */ >> @@ -714,6 +716,11 @@ static int meson_uart_probe(struct >> platform_device *pdev) >> if (pdev->id < 0 || pdev->id >= AML_UART_PORT_NUM) >> return -EINVAL; >> >> + private_data = devm_kzalloc(&pdev->dev, sizeof(*private_data), >> + GFP_KERNEL); >> + if (!private_data) >> + return -ENOMEM; >> + >> res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (!res_mem) >> return -ENODEV; >> @@ -733,7 +740,7 @@ static int meson_uart_probe(struct platform_device >> *pdev) >> if (!port) >> return -ENOMEM; >> >> - ret = meson_uart_probe_clocks(pdev, port); >> + ret = meson_uart_probe_clocks(pdev, private_data); >> if (ret) >> return ret; >> >> @@ -749,6 +756,7 @@ static int meson_uart_probe(struct platform_device >> *pdev) >> port->x_char = 0; >> port->ops = &meson_uart_ops; >> port->fifosize = fifosize; >> + port->private_data = private_data; >> >> meson_ports[pdev->id] = port; >> platform_set_drvdata(pdev, port); >> ============><=================================================== >> >> Neil >> >>>>>> >>>>>>> >>>>>>> Yu Tu (6): >>>>>>> tty: serial: meson: Move request the register region to probe >>>>>>> tty: serial: meson: Use devm_ioremap_resource to get register >>>>>>> mapped >>>>>>> memory >>>>>>> tty: serial: meson: Describes the calculation of the UART >>>>>>> baud rate >>>>>>> clock using a clock frame >>>>>>> tty: serial: meson: Make some bit of the REG5 register writable >>>>>>> tty: serial: meson: The system stuck when you run the stty >>>>>>> command on >>>>>>> the console to change the baud rate >>>>>>> tty: serial: meson: Added S4 SOC compatibility >>>>>>> >>>>>>> V6 -> V7: To solve the system stuck when you run the stty command on >>>>>>> the console to change the baud rate. >>>>>>> V5 -> V6: Change error format as discussed in the email. >>>>>>> V4 -> V5: Change error format. >>>>>>> V3 -> V4: Change CCF to describe the UART baud rate clock as >>>>>>> discussed >>>>>>> in the email. >>>>>>> V2 -> V3: add compatible = "amlogic,meson-gx-uart". Because it >>>>>>> must change >>>>>>> the DTS before it can be deleted >>>>>>> V1 -> V2: Use CCF to describe the UART baud rate clock.Make some >>>>>>> changes as >>>>>>> discussed in the email >>>>>>> >>>>>>> Link:https://lore.kernel.org/linux-amlogic/20220118030911.12815-4-yu.tu@amlogic.com/ >>>>>>> >>>>>>> >>>>>>> drivers/tty/serial/meson_uart.c | 221 >>>>>>> ++++++++++++++++++++++---------- >>>>>>> 1 file changed, 154 insertions(+), 67 deletions(-) >>>>>>> >>>>>>> >>>>>>> base-commit: a603ca60cebff8589882427a67f870ed946b3fc8 >>>>>> >>>> >>