2022-01-10 08:27:19

by Yu Tu

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: serial: meson: Drop compatible = amlogic,meson-gx-uart.

Deprecated, don't use anymore because compatible =
amlogic,meson-gx-uart. Don't differentiate between GXBB, GXL
and G12A which have different revisions of the UART IP.
So it's split into GXBB, GXL and G12A.

Signed-off-by: Yu Tu <[email protected]>
---
.../devicetree/bindings/serial/amlogic,meson-uart.yaml | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
index 72e8868db3e0..ad9f1f4537a0 100644
--- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
@@ -28,7 +28,10 @@ properties:
- amlogic,meson6-uart
- amlogic,meson8-uart
- amlogic,meson8b-uart
- - amlogic,meson-gx-uart
+ - amlogic,meson-gx-uart # deprecated, use revision specific property below
+ - amlogic,meson-gxbb-uart
+ - amlogic,meson-gxl-uart
+ - amlogic,meson-g12a-uart
- amlogic,meson-s4-uart
- const: amlogic,meson-ao-uart
- description: Everything-Else power domain UART controller
@@ -36,7 +39,10 @@ properties:
- amlogic,meson6-uart
- amlogic,meson8-uart
- amlogic,meson8b-uart
- - amlogic,meson-gx-uart
+ - amlogic,meson-gx-uart # deprecated, use revision specific property below
+ - amlogic,meson-gxbb-uart
+ - amlogic,meson-gxl-uart
+ - amlogic,meson-g12a-uart
- amlogic,meson-s4-uart

reg:

base-commit: 99a507a8ea28542ec196e2dd80096708e2482735
--
2.33.1



2022-01-10 08:27:28

by Yu Tu

[permalink] [raw]
Subject: [PATCH] tty: serial: meson: Change request_irq to devm_request_irq and move devm_request_irq to meson_uart_probe()

Because an interrupt error occurs when the user opens /dev/ttyAML* but
don't close it, and then opens the same port again. This problem is
encountered in actual projects.

Signed-off-by: Yu Tu <[email protected]>
---
drivers/tty/serial/meson_uart.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 45e41f20cba3..0dd3f5b35768 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -135,8 +135,6 @@ static void meson_uart_shutdown(struct uart_port *port)
unsigned long flags;
u32 val;

- free_irq(port->irq, port);
-
spin_lock_irqsave(&port->lock, flags);

val = readl(port->membase + AML_UART_CONTROL);
@@ -284,7 +282,6 @@ static void meson_uart_reset(struct uart_port *port)
static int meson_uart_startup(struct uart_port *port)
{
u32 val;
- int ret;

meson_uart_reset(port);

@@ -298,10 +295,7 @@ static int meson_uart_startup(struct uart_port *port)
val = (AML_UART_RECV_IRQ(1) | AML_UART_XMIT_IRQ(port->fifosize / 2));
writel(val, port->membase + AML_UART_MISC);

- ret = request_irq(port->irq, meson_uart_interrupt, 0,
- port->name, port);
-
- return ret;
+ return 0;
}

static void meson_uart_change_speed(struct uart_port *port, unsigned long baud)
@@ -908,6 +902,14 @@ static int meson_uart_probe(struct platform_device *pdev)
meson_ports[pdev->id] = port;
platform_set_drvdata(pdev, port);

+ ret = devm_request_irq(&pdev->dev, port->irq, meson_uart_interrupt,
+ 0, dev_name(&pdev->dev), port);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to request uart irq: %d\n",
+ ret);
+ return ret;
+ }
+
/* reset port before registering (and possibly registering console) */
meson_uart_reset(port);


base-commit: b3a9e3b9622ae10064826dccb4f7a52bd88c7407
prerequisite-patch-id: 97a514f3447511cb204179ce03ae99dc1d5902d9
prerequisite-patch-id: a2e4756ff85f0df0efe111d7e2cb51b8e26e226f
prerequisite-patch-id: af9e3acc8f6ff7602d3a68a57c008e5ec362b353
prerequisite-patch-id: 7af8c81b4c2163240725e96625b1487c280f4f30
prerequisite-patch-id: 8791f6362b14e7c4ff4f85cce550f06abeb1af7a
--
2.33.1


2022-01-10 08:28:01

by Yu Tu

[permalink] [raw]
Subject: [PATCH] tty: serial: meson: Make the bit24 and bit [26,27] of the UART_REG5 register writable

The UART_REG5 register defaults to 0. The console port is set in
ROMCODE. But other UART ports default to 0, so make bit24 and
bit[26,27] writable so that the UART can choose a more
appropriate clock.

Signed-off-by: Yu Tu <[email protected]>
---
drivers/tty/serial/meson_uart.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 7c3f30cea68e..b0551750dff8 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -693,7 +693,7 @@ static int meson_uart_probe_clocks(struct uart_port *port)
CLK_SET_RATE_NO_REPARENT,
port->membase + AML_UART_REG5,
26, 2,
- CLK_DIVIDER_READ_ONLY,
+ CLK_DIVIDER_ROUND_CLOSEST,
xtal_div_table, NULL);
if (IS_ERR(hw))
return PTR_ERR(hw);
@@ -719,7 +719,7 @@ static int meson_uart_probe_clocks(struct uart_port *port)
CLK_SET_RATE_PARENT,
port->membase + AML_UART_REG5,
24, 0x1,
- CLK_MUX_READ_ONLY,
+ CLK_MUX_ROUND_CLOSEST,
NULL, NULL);
if (IS_ERR(hw))
return PTR_ERR(hw);

base-commit: 93a770b7e16772530196674ffc79bb13fa927dc6
prerequisite-patch-id: 95191c926509964c8e9bf4128b8bbad8a277b84a
prerequisite-patch-id: a2e4756ff85f0df0efe111d7e2cb51b8e26e226f
prerequisite-patch-id: 4e4d909acabcb7533da20e2207207be73454a88c
--
2.33.1


2022-01-10 08:28:35

by Yu Tu

[permalink] [raw]
Subject: [PATCH] tty: serial: meson: meson_uart_shutdown omit clear AML_UART_TX_EN bit

This change is an improvement. The meson_uart_shutdown function
should have the opposite operation to the meson_uart_startup
function, but the meson_uart_shutdown of AML_UART_TX_EN is logically
missing.

Signed-off-by: Yu Tu <[email protected]>
---
drivers/tty/serial/meson_uart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 9b07e3534969..c17109d6d441 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -140,7 +140,7 @@ static void meson_uart_shutdown(struct uart_port *port)
spin_lock_irqsave(&port->lock, flags);

val = readl(port->membase + AML_UART_CONTROL);
- val &= ~AML_UART_RX_EN;
+ val &= ~(AML_UART_RX_EN | AML_UART_TX_EN);
val &= ~(AML_UART_RX_INT_EN | AML_UART_TX_INT_EN);
writel(val, port->membase + AML_UART_CONTROL);


base-commit: b3a9e3b9622ae10064826dccb4f7a52bd88c7407
prerequisite-patch-id: 97a514f3447511cb204179ce03ae99dc1d5902d9
prerequisite-patch-id: a2e4756ff85f0df0efe111d7e2cb51b8e26e226f
prerequisite-patch-id: af9e3acc8f6ff7602d3a68a57c008e5ec362b353
prerequisite-patch-id: a44e5da7009d9b815eb87395cbd7d14e7d953a09
--
2.33.1


2022-01-10 08:28:47

by Yu Tu

[permalink] [raw]
Subject: [PATCH] tty: serial: meson: Request the register region in meson_uart_probe()

This simplifies resetting the UART controller during probe and will make
it easier to integrate the common clock code which will require the
registers at probe time as well.

Signed-off-by: Yu Tu <[email protected]>
---
drivers/tty/serial/meson_uart.c | 24 ++++++------------------
1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index c9a37602ffd0..99efe62a1507 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -397,24 +397,11 @@ static int meson_uart_verify_port(struct uart_port *port,

static void meson_uart_release_port(struct uart_port *port)
{
- devm_iounmap(port->dev, port->membase);
- port->membase = NULL;
- devm_release_mem_region(port->dev, port->mapbase, port->mapsize);
+ /* nothing to do */
}

static int meson_uart_request_port(struct uart_port *port)
{
- if (!devm_request_mem_region(port->dev, port->mapbase, port->mapsize,
- dev_name(port->dev))) {
- dev_err(port->dev, "Memory region busy\n");
- return -EBUSY;
- }
-
- port->membase = devm_ioremap(port->dev, port->mapbase,
- port->mapsize);
- if (!port->membase)
- return -ENOMEM;
-
return 0;
}

@@ -728,6 +715,10 @@ static int meson_uart_probe(struct platform_device *pdev)
if (!port)
return -ENOMEM;

+ port->membase = devm_ioremap_resource(&pdev->dev, res_mem);
+ if (IS_ERR(port->membase))
+ return PTR_ERR(port->membase);
+
ret = meson_uart_probe_clocks(pdev, port);
if (ret)
return ret;
@@ -749,10 +740,7 @@ static int meson_uart_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, port);

/* reset port before registering (and possibly registering console) */
- if (meson_uart_request_port(port) >= 0) {
- meson_uart_reset(port);
- meson_uart_release_port(port);
- }
+ meson_uart_reset(port);

ret = uart_add_one_port(&meson_uart_driver, port);
if (ret)

base-commit: b3a9e3b9622ae10064826dccb4f7a52bd88c7407
prerequisite-patch-id: 97a514f3447511cb204179ce03ae99dc1d5902d9
--
2.33.1


2022-01-10 08:29:00

by Yu Tu

[permalink] [raw]
Subject: [PATCH 1/2] tty: serial: meson: The UART baud rate calculation is described using the common clock code.

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.

Signed-off-by: Yu Tu <[email protected]>
---
drivers/tty/serial/meson_uart.c | 224 +++++++++++++++++++++++---------
1 file changed, 163 insertions(+), 61 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 7570958d010c..1004fd0b0c9e 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -6,6 +6,7 @@
*/

#include <linux/clk.h>
+#include <linux/clk-provider.h>
#include <linux/console.h>
#include <linux/delay.h>
#include <linux/init.h>
@@ -65,9 +66,7 @@
#define AML_UART_RECV_IRQ(c) ((c) & 0xff)

/* AML_UART_REG5 bits */
-#define AML_UART_BAUD_MASK 0x7fffff
#define AML_UART_BAUD_USE BIT(23)
-#define AML_UART_BAUD_XTAL BIT(24)

#define AML_UART_PORT_NUM 12
#define AML_UART_PORT_OFFSET 6
@@ -76,6 +75,13 @@
#define AML_UART_POLL_USEC 5
#define AML_UART_TIMEOUT_USEC 10000

+struct meson_uart_data {
+ struct uart_port port;
+ struct clk *pclk;
+ struct clk *baud_clk;
+ bool use_xtal_clk;
+};
+
static struct uart_driver meson_uart_driver;

static struct uart_port *meson_ports[AML_UART_PORT_NUM];
@@ -268,14 +274,11 @@ static void meson_uart_reset(struct uart_port *port)
static int meson_uart_startup(struct uart_port *port)
{
u32 val;
- int ret = 0;
+ int ret;

- val = readl(port->membase + AML_UART_CONTROL);
- val |= AML_UART_CLEAR_ERR;
- writel(val, port->membase + AML_UART_CONTROL);
- val &= ~AML_UART_CLEAR_ERR;
- writel(val, port->membase + AML_UART_CONTROL);
+ meson_uart_reset(port);

+ val = readl(port->membase + AML_UART_CONTROL);
val |= (AML_UART_RX_EN | AML_UART_TX_EN);
writel(val, port->membase + AML_UART_CONTROL);

@@ -293,19 +296,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;
- val |= AML_UART_BAUD_XTAL;
- } else {
- val = ((port->uartclk * 10 / (baud * 4) + 5) / 10) - 1;
- }
+ val = readl(port->membase + AML_UART_REG5);
val |= AML_UART_BAUD_USE;
writel(val, port->membase + AML_UART_REG5);
+
+ clk_set_rate(private_data->baud_clk, baud);
}

static void meson_uart_set_termios(struct uart_port *port,
@@ -395,11 +396,27 @@ static int meson_uart_verify_port(struct uart_port *port,

static void meson_uart_release_port(struct uart_port *port)
{
- /* nothing to do */
+ struct meson_uart_data *private_data = port->private_data;
+
+ clk_disable_unprepare(private_data->baud_clk);
+ clk_disable_unprepare(private_data->pclk);
}

static int meson_uart_request_port(struct uart_port *port)
{
+ struct meson_uart_data *private_data = port->private_data;
+ int ret;
+
+ ret = clk_prepare_enable(private_data->pclk);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(private_data->baud_clk);
+ if (ret) {
+ clk_disable_unprepare(private_data->pclk);
+ return ret;
+ }
+
return 0;
}

@@ -629,55 +646,105 @@ static struct uart_driver meson_uart_driver = {
.cons = MESON_SERIAL_CONSOLE,
};

-static inline struct clk *meson_uart_probe_clock(struct device *dev,
- const char *id)
+#define CLK_NAME(name) \
+({\
+ char clk_name[32];\
+ snprintf(clk_name, sizeof(clk_name), "%s#%s", dev_name(port->dev), #name);\
+ clk_name;\
+})
+
+static struct clk_div_table xtal_div_table[] = {
+ { 0, 3 },
+ { 1, 1 },
+ { 2, 2 },
+ { 3, 2 },
+};
+
+static int meson_uart_probe_clocks(struct uart_port *port)
{
- struct clk *clk = NULL;
- int ret;
+ struct meson_uart_data *private_data = port->private_data;
+ struct clk *clk_baud, *clk_xtal;
+ struct clk_hw *hw;
+ struct clk_parent_data use_xtal_mux_parents[2] = {
+ { .index = -1, },
+ { .index = -1, },
+ };

- clk = devm_clk_get(dev, id);
- if (IS_ERR(clk))
- return clk;
+ private_data->pclk = devm_clk_get(port->dev, "pclk");
+ if (IS_ERR(private_data->pclk))
+ return dev_err_probe(port->dev, PTR_ERR(private_data->pclk),
+ "Failed to get the 'pclk' clock\n");

- ret = clk_prepare_enable(clk);
- if (ret) {
- dev_err(dev, "couldn't enable clk\n");
- return ERR_PTR(ret);
+ clk_baud = devm_clk_get(port->dev, "baud");
+ if (IS_ERR(clk_baud)) {
+ dev_err(port->dev, "Failed to get the 'baud' clock\n");
+ return PTR_ERR(clk_baud);
}

- devm_add_action_or_reset(dev,
- (void(*)(void *))clk_disable_unprepare,
- clk);
-
- return clk;
-}
-
-static int meson_uart_probe_clocks(struct platform_device *pdev,
- struct uart_port *port)
-{
- 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);
-
- clk_xtal = meson_uart_probe_clock(&pdev->dev, "xtal");
+ clk_xtal = devm_clk_get(port->dev, "xtal");
if (IS_ERR(clk_xtal))
- return PTR_ERR(clk_xtal);
-
- clk_baud = meson_uart_probe_clock(&pdev->dev, "baud");
- if (IS_ERR(clk_baud))
- return PTR_ERR(clk_baud);
+ return dev_err_probe(port->dev, PTR_ERR(clk_xtal),
+ "Failed to get the 'xtal' clock\n");
+
+ if (private_data->use_xtal_clk) {
+ hw = devm_clk_hw_register_divider_table(port->dev,
+ CLK_NAME(xtal_div),
+ __clk_get_name(clk_baud),
+ CLK_SET_RATE_NO_REPARENT,
+ port->membase + AML_UART_REG5,
+ 26, 2,
+ CLK_DIVIDER_READ_ONLY,
+ xtal_div_table, NULL);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+
+ use_xtal_mux_parents[1].hw = hw;
+ } else {
+ hw = devm_clk_hw_register_fixed_factor(port->dev,
+ CLK_NAME(clk81_div4),
+ __clk_get_name(clk_baud),
+ CLK_SET_RATE_NO_REPARENT,
+ 1, 4);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+
+ use_xtal_mux_parents[0].hw = hw;
+ }

- port->uartclk = clk_get_rate(clk_baud);
+ hw = __devm_clk_hw_register_mux(port->dev, NULL,
+ CLK_NAME(use_xtal),
+ ARRAY_SIZE(use_xtal_mux_parents),
+ NULL, NULL,
+ use_xtal_mux_parents,
+ CLK_SET_RATE_PARENT,
+ port->membase + AML_UART_REG5,
+ 24, 0x1,
+ CLK_MUX_READ_ONLY,
+ NULL, NULL);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+
+ port->uartclk = clk_hw_get_rate(hw);
+
+ hw = devm_clk_hw_register_divider(port->dev,
+ CLK_NAME(baud_div),
+ clk_hw_get_name(hw),
+ CLK_SET_RATE_PARENT,
+ port->membase + AML_UART_REG5,
+ 0, 23,
+ CLK_DIVIDER_ROUND_CLOSEST,
+ NULL);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+
+ private_data->baud_clk = clk_hw_get_clk(hw, "baud_rate");

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 */
@@ -716,18 +783,20 @@ static int meson_uart_probe(struct platform_device *pdev)
return -EBUSY;
}

- port = devm_kzalloc(&pdev->dev, sizeof(struct uart_port), GFP_KERNEL);
- if (!port)
+ private_data = devm_kzalloc(&pdev->dev, sizeof(*private_data),
+ GFP_KERNEL);
+ if (!private_data)
return -ENOMEM;

+ if (device_get_match_data(&pdev->dev))
+ private_data->use_xtal_clk = true;
+
+ port = &private_data->port;
+
port->membase = devm_ioremap_resource(&pdev->dev, res_mem);
if (IS_ERR(port->membase))
return PTR_ERR(port->membase);

- ret = meson_uart_probe_clocks(pdev, port);
- if (ret)
- return ret;
-
port->iotype = UPIO_MEM;
port->mapbase = res_mem->start;
port->mapsize = resource_size(res_mem);
@@ -740,7 +809,11 @@ 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;

+ ret = meson_uart_probe_clocks(port);
+ if (ret)
+ return ret;
meson_ports[pdev->id] = port;
platform_set_drvdata(pdev, port);

@@ -766,10 +839,39 @@ static int meson_uart_remove(struct platform_device *pdev)
}

static const struct of_device_id meson_uart_dt_match[] = {
- { .compatible = "amlogic,meson6-uart" },
- { .compatible = "amlogic,meson8-uart" },
- { .compatible = "amlogic,meson8b-uart" },
- { .compatible = "amlogic,meson-gx-uart" },
+ {
+ .compatible = "amlogic,meson6-uart",
+ .data = (void *)false,
+ },
+ {
+ .compatible = "amlogic,meson8-uart",
+ .data = (void *)false,
+ },
+ {
+ .compatible = "amlogic,meson8b-uart",
+ .data = (void *)false,
+ },
+ {
+ .compatible = "amlogic,meson-gxbb-uart",
+ .data = (void *)false,
+ },
+ {
+ .compatible = "amlogic,meson-gxl-uart",
+ .data = (void *)true,
+ },
+ {
+ .compatible = "amlogic,meson-g12a-uart",
+ .data = (void *)true,
+ },
+ /*
+ * deprecated, don't use anymore because it doesn't differentiate
+ * between GXBB, GXL and G12A which have different revisions
+ * of the UART IP.
+ */
+ {
+ .compatible = "amlogic,meson-gx-uart",
+ .data = (void *)false,
+ },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, meson_uart_dt_match);

base-commit: 93a770b7e16772530196674ffc79bb13fa927dc6
prerequisite-patch-id: 95191c926509964c8e9bf4128b8bbad8a277b84a
prerequisite-patch-id: a2e4756ff85f0df0efe111d7e2cb51b8e26e226f
--
2.33.1


2022-01-10 08:55:46

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: serial: meson: Drop compatible = amlogic,meson-gx-uart.

Hi All,
I'm really sorry for sending the wrong specified file. Please ignore
the email. I will resend V4.

On 2022/1/10 16:26, Yu Tu wrote:
> Deprecated, don't use anymore because compatible =
> amlogic,meson-gx-uart. Don't differentiate between GXBB, GXL
> and G12A which have different revisions of the UART IP.
> So it's split into GXBB, GXL and G12A.
>
> Signed-off-by: Yu Tu <[email protected]>
> ---
> .../devicetree/bindings/serial/amlogic,meson-uart.yaml | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> index 72e8868db3e0..ad9f1f4537a0 100644
> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml
> @@ -28,7 +28,10 @@ properties:
> - amlogic,meson6-uart
> - amlogic,meson8-uart
> - amlogic,meson8b-uart
> - - amlogic,meson-gx-uart
> + - amlogic,meson-gx-uart # deprecated, use revision specific property below
> + - amlogic,meson-gxbb-uart
> + - amlogic,meson-gxl-uart
> + - amlogic,meson-g12a-uart
> - amlogic,meson-s4-uart
> - const: amlogic,meson-ao-uart
> - description: Everything-Else power domain UART controller
> @@ -36,7 +39,10 @@ properties:
> - amlogic,meson6-uart
> - amlogic,meson8-uart
> - amlogic,meson8b-uart
> - - amlogic,meson-gx-uart
> + - amlogic,meson-gx-uart # deprecated, use revision specific property below
> + - amlogic,meson-gxbb-uart
> + - amlogic,meson-gxl-uart
> + - amlogic,meson-g12a-uart
> - amlogic,meson-s4-uart
>
> reg:
>
> base-commit: 99a507a8ea28542ec196e2dd80096708e2482735

2022-01-10 08:55:54

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH] tty: serial: meson: Change request_irq to devm_request_irq and move devm_request_irq to meson_uart_probe()

Hi All,
I'm really sorry for sending the wrong specified file. Please ignore
the email. I will resend V4.

On 2022/1/10 16:26, Yu Tu wrote:
> Because an interrupt error occurs when the user opens /dev/ttyAML* but
> don't close it, and then opens the same port again. This problem is
> encountered in actual projects.
>
> Signed-off-by: Yu Tu <[email protected]>
> ---
> drivers/tty/serial/meson_uart.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 45e41f20cba3..0dd3f5b35768 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -135,8 +135,6 @@ static void meson_uart_shutdown(struct uart_port *port)
> unsigned long flags;
> u32 val;
>
> - free_irq(port->irq, port);
> -
> spin_lock_irqsave(&port->lock, flags);
>
> val = readl(port->membase + AML_UART_CONTROL);
> @@ -284,7 +282,6 @@ static void meson_uart_reset(struct uart_port *port)
> static int meson_uart_startup(struct uart_port *port)
> {
> u32 val;
> - int ret;
>
> meson_uart_reset(port);
>
> @@ -298,10 +295,7 @@ static int meson_uart_startup(struct uart_port *port)
> val = (AML_UART_RECV_IRQ(1) | AML_UART_XMIT_IRQ(port->fifosize / 2));
> writel(val, port->membase + AML_UART_MISC);
>
> - ret = request_irq(port->irq, meson_uart_interrupt, 0,
> - port->name, port);
> -
> - return ret;
> + return 0;
> }
>
> static void meson_uart_change_speed(struct uart_port *port, unsigned long baud)
> @@ -908,6 +902,14 @@ static int meson_uart_probe(struct platform_device *pdev)
> meson_ports[pdev->id] = port;
> platform_set_drvdata(pdev, port);
>
> + ret = devm_request_irq(&pdev->dev, port->irq, meson_uart_interrupt,
> + 0, dev_name(&pdev->dev), port);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to request uart irq: %d\n",
> + ret);
> + return ret;
> + }
> +
> /* reset port before registering (and possibly registering console) */
> meson_uart_reset(port);
>
>
> base-commit: b3a9e3b9622ae10064826dccb4f7a52bd88c7407
> prerequisite-patch-id: 97a514f3447511cb204179ce03ae99dc1d5902d9
> prerequisite-patch-id: a2e4756ff85f0df0efe111d7e2cb51b8e26e226f
> prerequisite-patch-id: af9e3acc8f6ff7602d3a68a57c008e5ec362b353
> prerequisite-patch-id: 7af8c81b4c2163240725e96625b1487c280f4f30
> prerequisite-patch-id: 8791f6362b14e7c4ff4f85cce550f06abeb1af7a

2022-01-10 08:56:28

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH] tty: serial: meson: Make the bit24 and bit [26,27] of the UART_REG5 register writable

Hi All,
I'm really sorry for sending the wrong specified file. Please ignore
the email. I will resend V4.

On 2022/1/10 16:26, Yu Tu wrote:
> The UART_REG5 register defaults to 0. The console port is set in
> ROMCODE. But other UART ports default to 0, so make bit24 and
> bit[26,27] writable so that the UART can choose a more
> appropriate clock.
>
> Signed-off-by: Yu Tu <[email protected]>
> ---
> drivers/tty/serial/meson_uart.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 7c3f30cea68e..b0551750dff8 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -693,7 +693,7 @@ static int meson_uart_probe_clocks(struct uart_port *port)
> CLK_SET_RATE_NO_REPARENT,
> port->membase + AML_UART_REG5,
> 26, 2,
> - CLK_DIVIDER_READ_ONLY,
> + CLK_DIVIDER_ROUND_CLOSEST,
> xtal_div_table, NULL);
> if (IS_ERR(hw))
> return PTR_ERR(hw);
> @@ -719,7 +719,7 @@ static int meson_uart_probe_clocks(struct uart_port *port)
> CLK_SET_RATE_PARENT,
> port->membase + AML_UART_REG5,
> 24, 0x1,
> - CLK_MUX_READ_ONLY,
> + CLK_MUX_ROUND_CLOSEST,
> NULL, NULL);
> if (IS_ERR(hw))
> return PTR_ERR(hw);
>
> base-commit: 93a770b7e16772530196674ffc79bb13fa927dc6
> prerequisite-patch-id: 95191c926509964c8e9bf4128b8bbad8a277b84a
> prerequisite-patch-id: a2e4756ff85f0df0efe111d7e2cb51b8e26e226f
> prerequisite-patch-id: 4e4d909acabcb7533da20e2207207be73454a88c

2022-01-10 08:56:29

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH] tty: serial: meson: meson_uart_shutdown omit clear AML_UART_TX_EN bit

Hi All,
I'm really sorry for sending the wrong specified file. Please ignore
the email. I will resend V4.

On 2022/1/10 16:26, Yu Tu wrote:
> This change is an improvement. The meson_uart_shutdown function
> should have the opposite operation to the meson_uart_startup
> function, but the meson_uart_shutdown of AML_UART_TX_EN is logically
> missing.
>
> Signed-off-by: Yu Tu <[email protected]>
> ---
> drivers/tty/serial/meson_uart.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 9b07e3534969..c17109d6d441 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -140,7 +140,7 @@ static void meson_uart_shutdown(struct uart_port *port)
> spin_lock_irqsave(&port->lock, flags);
>
> val = readl(port->membase + AML_UART_CONTROL);
> - val &= ~AML_UART_RX_EN;
> + val &= ~(AML_UART_RX_EN | AML_UART_TX_EN);
> val &= ~(AML_UART_RX_INT_EN | AML_UART_TX_INT_EN);
> writel(val, port->membase + AML_UART_CONTROL);
>
>
> base-commit: b3a9e3b9622ae10064826dccb4f7a52bd88c7407
> prerequisite-patch-id: 97a514f3447511cb204179ce03ae99dc1d5902d9
> prerequisite-patch-id: a2e4756ff85f0df0efe111d7e2cb51b8e26e226f
> prerequisite-patch-id: af9e3acc8f6ff7602d3a68a57c008e5ec362b353
> prerequisite-patch-id: a44e5da7009d9b815eb87395cbd7d14e7d953a09

2022-01-10 08:57:14

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH] tty: serial: meson: Request the register region in meson_uart_probe()

Hi All,
I'm really sorry for sending the wrong specified file. Please ignore
the email. I will resend V4.

On 2022/1/10 16:26, Yu Tu wrote:
> This simplifies resetting the UART controller during probe and will make
> it easier to integrate the common clock code which will require the
> registers at probe time as well.
>
> Signed-off-by: Yu Tu <[email protected]>
> ---
> drivers/tty/serial/meson_uart.c | 24 ++++++------------------
> 1 file changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index c9a37602ffd0..99efe62a1507 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -397,24 +397,11 @@ static int meson_uart_verify_port(struct uart_port *port,
>
> static void meson_uart_release_port(struct uart_port *port)
> {
> - devm_iounmap(port->dev, port->membase);
> - port->membase = NULL;
> - devm_release_mem_region(port->dev, port->mapbase, port->mapsize);
> + /* nothing to do */
> }
>
> static int meson_uart_request_port(struct uart_port *port)
> {
> - if (!devm_request_mem_region(port->dev, port->mapbase, port->mapsize,
> - dev_name(port->dev))) {
> - dev_err(port->dev, "Memory region busy\n");
> - return -EBUSY;
> - }
> -
> - port->membase = devm_ioremap(port->dev, port->mapbase,
> - port->mapsize);
> - if (!port->membase)
> - return -ENOMEM;
> -
> return 0;
> }
>
> @@ -728,6 +715,10 @@ static int meson_uart_probe(struct platform_device *pdev)
> if (!port)
> return -ENOMEM;
>
> + port->membase = devm_ioremap_resource(&pdev->dev, res_mem);
> + if (IS_ERR(port->membase))
> + return PTR_ERR(port->membase);
> +
> ret = meson_uart_probe_clocks(pdev, port);
> if (ret)
> return ret;
> @@ -749,10 +740,7 @@ static int meson_uart_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, port);
>
> /* reset port before registering (and possibly registering console) */
> - if (meson_uart_request_port(port) >= 0) {
> - meson_uart_reset(port);
> - meson_uart_release_port(port);
> - }
> + meson_uart_reset(port);
>
> ret = uart_add_one_port(&meson_uart_driver, port);
> if (ret)
>
> base-commit: b3a9e3b9622ae10064826dccb4f7a52bd88c7407
> prerequisite-patch-id: 97a514f3447511cb204179ce03ae99dc1d5902d9

2022-01-10 08:57:29

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH 1/2] tty: serial: meson: The UART baud rate calculation is described using the common clock code.

Hi All,
I'm really sorry for sending the wrong specified file. Please ignore
the email. I will resend V4.

On 2022/1/10 16:26, Yu Tu 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.
>
> Signed-off-by: Yu Tu <[email protected]>
> ---
> drivers/tty/serial/meson_uart.c | 224 +++++++++++++++++++++++---------
> 1 file changed, 163 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 7570958d010c..1004fd0b0c9e 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -6,6 +6,7 @@
> */
>
> #include <linux/clk.h>
> +#include <linux/clk-provider.h>
> #include <linux/console.h>
> #include <linux/delay.h>
> #include <linux/init.h>
> @@ -65,9 +66,7 @@
> #define AML_UART_RECV_IRQ(c) ((c) & 0xff)
>
> /* AML_UART_REG5 bits */
> -#define AML_UART_BAUD_MASK 0x7fffff
> #define AML_UART_BAUD_USE BIT(23)
> -#define AML_UART_BAUD_XTAL BIT(24)
>
> #define AML_UART_PORT_NUM 12
> #define AML_UART_PORT_OFFSET 6
> @@ -76,6 +75,13 @@
> #define AML_UART_POLL_USEC 5
> #define AML_UART_TIMEOUT_USEC 10000
>
> +struct meson_uart_data {
> + struct uart_port port;
> + struct clk *pclk;
> + struct clk *baud_clk;
> + bool use_xtal_clk;
> +};
> +
> static struct uart_driver meson_uart_driver;
>
> static struct uart_port *meson_ports[AML_UART_PORT_NUM];
> @@ -268,14 +274,11 @@ static void meson_uart_reset(struct uart_port *port)
> static int meson_uart_startup(struct uart_port *port)
> {
> u32 val;
> - int ret = 0;
> + int ret;
>
> - val = readl(port->membase + AML_UART_CONTROL);
> - val |= AML_UART_CLEAR_ERR;
> - writel(val, port->membase + AML_UART_CONTROL);
> - val &= ~AML_UART_CLEAR_ERR;
> - writel(val, port->membase + AML_UART_CONTROL);
> + meson_uart_reset(port);
>
> + val = readl(port->membase + AML_UART_CONTROL);
> val |= (AML_UART_RX_EN | AML_UART_TX_EN);
> writel(val, port->membase + AML_UART_CONTROL);
>
> @@ -293,19 +296,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;
> - val |= AML_UART_BAUD_XTAL;
> - } else {
> - val = ((port->uartclk * 10 / (baud * 4) + 5) / 10) - 1;
> - }
> + val = readl(port->membase + AML_UART_REG5);
> val |= AML_UART_BAUD_USE;
> writel(val, port->membase + AML_UART_REG5);
> +
> + clk_set_rate(private_data->baud_clk, baud);
> }
>
> static void meson_uart_set_termios(struct uart_port *port,
> @@ -395,11 +396,27 @@ static int meson_uart_verify_port(struct uart_port *port,
>
> static void meson_uart_release_port(struct uart_port *port)
> {
> - /* nothing to do */
> + struct meson_uart_data *private_data = port->private_data;
> +
> + clk_disable_unprepare(private_data->baud_clk);
> + clk_disable_unprepare(private_data->pclk);
> }
>
> static int meson_uart_request_port(struct uart_port *port)
> {
> + struct meson_uart_data *private_data = port->private_data;
> + int ret;
> +
> + ret = clk_prepare_enable(private_data->pclk);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(private_data->baud_clk);
> + if (ret) {
> + clk_disable_unprepare(private_data->pclk);
> + return ret;
> + }
> +
> return 0;
> }
>
> @@ -629,55 +646,105 @@ static struct uart_driver meson_uart_driver = {
> .cons = MESON_SERIAL_CONSOLE,
> };
>
> -static inline struct clk *meson_uart_probe_clock(struct device *dev,
> - const char *id)
> +#define CLK_NAME(name) \
> +({\
> + char clk_name[32];\
> + snprintf(clk_name, sizeof(clk_name), "%s#%s", dev_name(port->dev), #name);\
> + clk_name;\
> +})
> +
> +static struct clk_div_table xtal_div_table[] = {
> + { 0, 3 },
> + { 1, 1 },
> + { 2, 2 },
> + { 3, 2 },
> +};
> +
> +static int meson_uart_probe_clocks(struct uart_port *port)
> {
> - struct clk *clk = NULL;
> - int ret;
> + struct meson_uart_data *private_data = port->private_data;
> + struct clk *clk_baud, *clk_xtal;
> + struct clk_hw *hw;
> + struct clk_parent_data use_xtal_mux_parents[2] = {
> + { .index = -1, },
> + { .index = -1, },
> + };
>
> - clk = devm_clk_get(dev, id);
> - if (IS_ERR(clk))
> - return clk;
> + private_data->pclk = devm_clk_get(port->dev, "pclk");
> + if (IS_ERR(private_data->pclk))
> + return dev_err_probe(port->dev, PTR_ERR(private_data->pclk),
> + "Failed to get the 'pclk' clock\n");
>
> - ret = clk_prepare_enable(clk);
> - if (ret) {
> - dev_err(dev, "couldn't enable clk\n");
> - return ERR_PTR(ret);
> + clk_baud = devm_clk_get(port->dev, "baud");
> + if (IS_ERR(clk_baud)) {
> + dev_err(port->dev, "Failed to get the 'baud' clock\n");
> + return PTR_ERR(clk_baud);
> }
>
> - devm_add_action_or_reset(dev,
> - (void(*)(void *))clk_disable_unprepare,
> - clk);
> -
> - return clk;
> -}
> -
> -static int meson_uart_probe_clocks(struct platform_device *pdev,
> - struct uart_port *port)
> -{
> - 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);
> -
> - clk_xtal = meson_uart_probe_clock(&pdev->dev, "xtal");
> + clk_xtal = devm_clk_get(port->dev, "xtal");
> if (IS_ERR(clk_xtal))
> - return PTR_ERR(clk_xtal);
> -
> - clk_baud = meson_uart_probe_clock(&pdev->dev, "baud");
> - if (IS_ERR(clk_baud))
> - return PTR_ERR(clk_baud);
> + return dev_err_probe(port->dev, PTR_ERR(clk_xtal),
> + "Failed to get the 'xtal' clock\n");
> +
> + if (private_data->use_xtal_clk) {
> + hw = devm_clk_hw_register_divider_table(port->dev,
> + CLK_NAME(xtal_div),
> + __clk_get_name(clk_baud),
> + CLK_SET_RATE_NO_REPARENT,
> + port->membase + AML_UART_REG5,
> + 26, 2,
> + CLK_DIVIDER_READ_ONLY,
> + xtal_div_table, NULL);
> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> +
> + use_xtal_mux_parents[1].hw = hw;
> + } else {
> + hw = devm_clk_hw_register_fixed_factor(port->dev,
> + CLK_NAME(clk81_div4),
> + __clk_get_name(clk_baud),
> + CLK_SET_RATE_NO_REPARENT,
> + 1, 4);
> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> +
> + use_xtal_mux_parents[0].hw = hw;
> + }
>
> - port->uartclk = clk_get_rate(clk_baud);
> + hw = __devm_clk_hw_register_mux(port->dev, NULL,
> + CLK_NAME(use_xtal),
> + ARRAY_SIZE(use_xtal_mux_parents),
> + NULL, NULL,
> + use_xtal_mux_parents,
> + CLK_SET_RATE_PARENT,
> + port->membase + AML_UART_REG5,
> + 24, 0x1,
> + CLK_MUX_READ_ONLY,
> + NULL, NULL);
> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> +
> + port->uartclk = clk_hw_get_rate(hw);
> +
> + hw = devm_clk_hw_register_divider(port->dev,
> + CLK_NAME(baud_div),
> + clk_hw_get_name(hw),
> + CLK_SET_RATE_PARENT,
> + port->membase + AML_UART_REG5,
> + 0, 23,
> + CLK_DIVIDER_ROUND_CLOSEST,
> + NULL);
> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> +
> + private_data->baud_clk = clk_hw_get_clk(hw, "baud_rate");
>
> 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 */
> @@ -716,18 +783,20 @@ static int meson_uart_probe(struct platform_device *pdev)
> return -EBUSY;
> }
>
> - port = devm_kzalloc(&pdev->dev, sizeof(struct uart_port), GFP_KERNEL);
> - if (!port)
> + private_data = devm_kzalloc(&pdev->dev, sizeof(*private_data),
> + GFP_KERNEL);
> + if (!private_data)
> return -ENOMEM;
>
> + if (device_get_match_data(&pdev->dev))
> + private_data->use_xtal_clk = true;
> +
> + port = &private_data->port;
> +
> port->membase = devm_ioremap_resource(&pdev->dev, res_mem);
> if (IS_ERR(port->membase))
> return PTR_ERR(port->membase);
>
> - ret = meson_uart_probe_clocks(pdev, port);
> - if (ret)
> - return ret;
> -
> port->iotype = UPIO_MEM;
> port->mapbase = res_mem->start;
> port->mapsize = resource_size(res_mem);
> @@ -740,7 +809,11 @@ 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;
>
> + ret = meson_uart_probe_clocks(port);
> + if (ret)
> + return ret;
> meson_ports[pdev->id] = port;
> platform_set_drvdata(pdev, port);
>
> @@ -766,10 +839,39 @@ static int meson_uart_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id meson_uart_dt_match[] = {
> - { .compatible = "amlogic,meson6-uart" },
> - { .compatible = "amlogic,meson8-uart" },
> - { .compatible = "amlogic,meson8b-uart" },
> - { .compatible = "amlogic,meson-gx-uart" },
> + {
> + .compatible = "amlogic,meson6-uart",
> + .data = (void *)false,
> + },
> + {
> + .compatible = "amlogic,meson8-uart",
> + .data = (void *)false,
> + },
> + {
> + .compatible = "amlogic,meson8b-uart",
> + .data = (void *)false,
> + },
> + {
> + .compatible = "amlogic,meson-gxbb-uart",
> + .data = (void *)false,
> + },
> + {
> + .compatible = "amlogic,meson-gxl-uart",
> + .data = (void *)true,
> + },
> + {
> + .compatible = "amlogic,meson-g12a-uart",
> + .data = (void *)true,
> + },
> + /*
> + * deprecated, don't use anymore because it doesn't differentiate
> + * between GXBB, GXL and G12A which have different revisions
> + * of the UART IP.
> + */
> + {
> + .compatible = "amlogic,meson-gx-uart",
> + .data = (void *)false,
> + },
> { /* sentinel */ },
> };
> MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
>
> base-commit: 93a770b7e16772530196674ffc79bb13fa927dc6
> prerequisite-patch-id: 95191c926509964c8e9bf4128b8bbad8a277b84a
> prerequisite-patch-id: a2e4756ff85f0df0efe111d7e2cb51b8e26e226f