[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6f120165-b6ec-ab71-ee42-e3246726f4f1@amlogic.com>
Date: Wed, 2 Mar 2022 10:15:13 +0800
From: Yu Tu <yu.tu@...ogic.com>
To: Neil Armstrong <narmstrong@...libre.com>,
Jerome Brunet <jbrunet@...libre.com>,
<linux-serial@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-amlogic@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jirislaby@...nel.org>,
Kevin Hilman <khilman@...libre.com>,
Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Subject: Re: [PATCH V7 0/6] Use CCF to describe the UART baud rate clock
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@...ogic.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@...ogic.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
>>>>>>
>>>>
>>
Powered by blists - more mailing lists