| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 1 Mar 2022 10:27:50 +0100
From: Neil Armstrong <narmstrong@...libre.com>
To: Yu Tu <yu.tu@...ogic.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,
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):
============><===================================================
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