LinuxLists.cc - [PATCH v2 00/14] serial: Add a helper to parse device properties and more

2024-02-26 14:26:07

Subject: [PATCH v2 00/14] serial: Add a helper to parse device properties and more

I have noticed that many drivers are using the subset of the common
properties and IRQ retrieval code. With the moving it to one place
we have got a common parser one for many.

Tested on Intel Apollo Lake with DesingWare 8250 UARTs (clang compiled)
and in QEMU for Aspeed configuration (gcc compiled). The rest has been
compile tested on x86_64 with clang.

In v2:
- fixed typo (Hugo)
- renamed UPIO_UNSET --> UPIO_UNKNOWN (Florian)
- fixed 8250_of not working (Andrew)
- dropped unused variable in 8250_bcm7271 (LKP)
- added tag to 8250_aspeed_vuart (Andrew)

Andy Shevchenko (14):
serial: core: Move struct uart_port::quirks closer to possible values
serial: core: Add UPIO_UNKNOWN constant for unknown port type
serial: port: Introduce a common helper to read properties
serial: 8250_aspeed_vuart: Switch to use uart_read_port_properties()
serial: 8250_bcm2835aux: Switch to use uart_read_port_properties()
serial: 8250_bcm7271: Switch to use uart_read_port_properties()
serial: 8250_dw: Switch to use uart_read_port_properties()
serial: 8250_ingenic: Switch to use uart_read_port_properties()
serial: 8250_lpc18xx: Switch to use uart_read_port_properties()
serial: 8250_of: Switch to use uart_read_port_properties()
serial: 8250_omap: Switch to use uart_read_port_properties()
serial: 8250_pxa: Switch to use uart_read_port_properties()
serial: 8250_tegra: Switch to use uart_read_port_properties()
serial: 8250_uniphier: Switch to use uart_read_port_properties()

drivers/tty/serial/8250/8250_aspeed_vuart.c | 50 +++-----
drivers/tty/serial/8250/8250_bcm2835aux.c | 92 ++++++--------
drivers/tty/serial/8250/8250_bcm7271.c | 56 +++-----
drivers/tty/serial/8250/8250_dw.c | 67 ++++------
drivers/tty/serial/8250/8250_ingenic.c | 20 +--
drivers/tty/serial/8250/8250_lpc18xx.c | 20 ++-
drivers/tty/serial/8250/8250_of.c | 105 ++++-----------
drivers/tty/serial/8250/8250_omap.c | 29 ++---
drivers/tty/serial/8250/8250_pxa.c | 22 ++--
drivers/tty/serial/8250/8250_tegra.c | 26 ++--
drivers/tty/serial/8250/8250_uniphier.c | 17 +--
drivers/tty/serial/serial_port.c | 134 ++++++++++++++++++++
include/linux/serial_core.h | 10 +-
13 files changed, 313 insertions(+), 335 deletions(-)

--
2.43.0.rc1.1.gbec44491f096

2024-02-26 14:27:06

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 09/14] serial: 8250_lpc18xx: Switch to use uart_read_port_properties()

2024-02-26 14:27:35

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 07/14] serial: 8250_dw: Switch to use uart_read_port_properties()

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Reviewed-by: Andi Shyti <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_dw.c | 67 +++++++++++++------------------
1 file changed, 27 insertions(+), 40 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 2d1f350a4bea..a1825e83231f 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -17,7 +17,6 @@
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/notifier.h>
-#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
@@ -449,12 +448,7 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)

if (np) {
unsigned int quirks = data->pdata->quirks;
- int id;

- /* get index of serial line, if found in DT aliases */
- id = of_alias_get_id(np, "serial");
- if (id >= 0)
- p->line = id;
#ifdef CONFIG_64BIT
if (quirks & DW_UART_QUIRK_OCTEON) {
p->serial_in = dw8250_serial_inq;
@@ -465,12 +459,6 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
}
#endif

- if (of_device_is_big_endian(np)) {
- p->iotype = UPIO_MEM32BE;
- p->serial_in = dw8250_serial_in32be;
- p->serial_out = dw8250_serial_out32be;
- }
-
if (quirks & DW_UART_QUIRK_ARMADA_38X)
p->serial_out = dw8250_serial_out38x;
if (quirks & DW_UART_QUIRK_SKIP_SET_RATE)
@@ -510,39 +498,21 @@ static int dw8250_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct dw8250_data *data;
struct resource *regs;
- int irq;
int err;
- u32 val;

regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs)
return dev_err_probe(dev, -EINVAL, "no registers defined\n");

- irq = platform_get_irq_optional(pdev, 0);
- /* no interrupt -> fall back to polling */
- if (irq == -ENXIO)
- irq = 0;
- if (irq < 0)
- return irq;
-
spin_lock_init(&p->lock);
- p->mapbase = regs->start;
- p->irq = irq;
p->handle_irq = dw8250_handle_irq;
p->pm = dw8250_do_pm;
p->type = PORT_8250;
- p->flags = UPF_SHARE_IRQ | UPF_FIXED_PORT;
+ p->flags = UPF_FIXED_PORT;
p->dev = dev;
- p->iotype = UPIO_MEM;
- p->serial_in = dw8250_serial_in;
- p->serial_out = dw8250_serial_out;
p->set_ldisc = dw8250_set_ldisc;
p->set_termios = dw8250_set_termios;

- p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
- if (!p->membase)
- return -ENOMEM;
-
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -554,15 +524,35 @@ static int dw8250_probe(struct platform_device *pdev)
data->uart_16550_compatible = device_property_read_bool(dev,
"snps,uart-16550-compatible");

- err = device_property_read_u32(dev, "reg-shift", &val);
- if (!err)
- p->regshift = val;
+ p->mapbase = regs->start;
+ p->mapsize = resource_size(regs);

- err = device_property_read_u32(dev, "reg-io-width", &val);
- if (!err && val == 4) {
- p->iotype = UPIO_MEM32;
+ p->membase = devm_ioremap(dev, p->mapbase, p->mapsize);
+ if (!p->membase)
+ return -ENOMEM;
+
+ err = uart_read_port_properties(p, true);
+ /* no interrupt -> fall back to polling */
+ if (err == -ENXIO)
+ err = 0;
+ if (err)
+ return err;
+
+ switch (p->iotype) {
+ case UPIO_MEM:
+ p->serial_in = dw8250_serial_in;
+ p->serial_out = dw8250_serial_out;
+ break;
+ case UPIO_MEM32:
p->serial_in = dw8250_serial_in32;
p->serial_out = dw8250_serial_out32;
+ break;
+ case UPIO_MEM32BE:
+ p->serial_in = dw8250_serial_in32be;
+ p->serial_out = dw8250_serial_out32be;
+ break;
+ default:
+ return -ENODEV;
}

if (device_property_read_bool(dev, "dcd-override")) {
@@ -589,9 +579,6 @@ static int dw8250_probe(struct platform_device *pdev)
data->msr_mask_off |= UART_MSR_TERI;
}

- /* Always ask for fixed clock rate from a property. */
- device_property_read_u32(dev, "clock-frequency", &p->uartclk);
-
/* If there is separate baudclk, get the rate from it. */
data->clk = devm_clk_get_optional_enabled(dev, "baudclk");
if (data->clk == NULL)
--
2.43.0.rc1.1.gbec44491f096

2024-02-26 14:28:48

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 14/14] serial: 8250_uniphier: Switch to use uart_read_port_properties()

2024-02-26 15:46:43

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 11/14] serial: 8250_omap: Switch to use uart_read_port_properties()

2024-02-26 16:56:34

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Reviewed-by: Andi Shyti <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_of.c | 105 +++++++-----------------------
1 file changed, 22 insertions(+), 83 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 9dcc17e33269..1a699ce2e812 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -69,37 +69,22 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
struct device *dev = &ofdev->dev;
struct device_node *np = dev->of_node;
struct uart_port *port = &up->port;
- u32 clk, spd, prop;
- int ret, irq;
+ u32 spd;
+ int ret;

memset(port, 0, sizeof *port);

pm_runtime_enable(&ofdev->dev);
pm_runtime_get_sync(&ofdev->dev);

- if (of_property_read_u32(np, "clock-frequency", &clk)) {
-
- /* Get clk rate through clk driver if present */
- info->clk = devm_clk_get_enabled(dev, NULL);
- if (IS_ERR(info->clk)) {
- ret = dev_err_probe(dev, PTR_ERR(info->clk), "failed to get clock\n");
- goto err_pmruntime;
- }
-
- clk = clk_get_rate(info->clk);
- }
- /* If current-speed was set, then try not to change it. */
- if (of_property_read_u32(np, "current-speed", &spd) == 0)
- port->custom_divisor = clk / (16 * spd);
-
ret = of_address_to_resource(np, 0, &resource);
if (ret) {
dev_err_probe(dev, ret, "invalid address\n");
goto err_pmruntime;
}

- port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_FIXED_PORT |
- UPF_FIXED_TYPE;
+ port->dev = &ofdev->dev;
+ port->flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE;
spin_lock_init(&port->lock);

if (resource_type(&resource) == IORESOURCE_IO) {
@@ -108,70 +93,31 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
} else {
port->mapbase = resource.start;
port->mapsize = resource_size(&resource);
-
- /* Check for shifted address mapping */
- if (of_property_read_u32(np, "reg-offset", &prop) == 0) {
- if (prop >= port->mapsize) {
- ret = dev_err_probe(dev, -EINVAL, "reg-offset %u exceeds region size %pa\n",
- prop, &port->mapsize);
- goto err_pmruntime;
- }
-
- port->mapbase += prop;
- port->mapsize -= prop;
- }
-
- port->iotype = UPIO_MEM;
- if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
- switch (prop) {
- case 1:
- port->iotype = UPIO_MEM;
- break;
- case 2:
- port->iotype = UPIO_MEM16;
- break;
- case 4:
- port->iotype = of_device_is_big_endian(np) ?
- UPIO_MEM32BE : UPIO_MEM32;
- break;
- default:
- ret = dev_err_probe(dev, -EINVAL, "unsupported reg-io-width (%u)\n",
- prop);
- goto err_pmruntime;
- }
- }
port->flags |= UPF_IOREMAP;
}

+ ret = uart_read_port_properties(port, false);
+ if (ret)
+ goto err_pmruntime;
+
+ /* Get clk rate through clk driver if present */
+ if (!port->uartclk) {
+ info->clk = devm_clk_get_enabled(dev, NULL);
+ if (IS_ERR(info->clk)) {
+ ret = dev_err_probe(dev, PTR_ERR(info->clk), "failed to get clock\n");
+ goto err_pmruntime;
+ }
+
+ port->uartclk = clk_get_rate(info->clk);
+ }
+ /* If current-speed was set, then try not to change it. */
+ if (of_property_read_u32(np, "current-speed", &spd) == 0)
+ port->custom_divisor = port->uartclk / (16 * spd);
+
/* Compatibility with the deprecated pxa driver and 8250_pxa drivers. */
if (of_device_is_compatible(np, "mrvl,mmp-uart"))
port->regshift = 2;

- /* Check for registers offset within the devices address range */
- if (of_property_read_u32(np, "reg-shift", &prop) == 0)
- port->regshift = prop;
-
- /* Check for fifo size */
- if (of_property_read_u32(np, "fifo-size", &prop) == 0)
- port->fifosize = prop;
-
- /* Check for a fixed line number */
- ret = of_alias_get_id(np, "serial");
- if (ret >= 0)
- port->line = ret;
-
- irq = of_irq_get(np, 0);
- if (irq < 0) {
- if (irq == -EPROBE_DEFER) {
- ret = -EPROBE_DEFER;
- goto err_pmruntime;
- }
- /* IRQ support not mandatory */
- irq = 0;
- }
-
- port->irq = irq;
-
info->rst = devm_reset_control_get_optional_shared(&ofdev->dev, NULL);
if (IS_ERR(info->rst)) {
ret = PTR_ERR(info->rst);
@@ -183,12 +129,6 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
goto err_pmruntime;

port->type = type;
- port->uartclk = clk;
-
- if (of_property_read_bool(np, "no-loopback-test"))
- port->flags |= UPF_SKIP_TEST;
-
- port->dev = &ofdev->dev;
port->rs485_config = serial8250_em485_config;
port->rs485_supported = serial8250_em485_supported;
up->rs485_start_tx = serial8250_em485_start_tx;
@@ -280,7 +220,6 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
platform_set_drvdata(ofdev, info);
return 0;
err_dispose:
- irq_dispose_mapping(port8250.port.irq);
pm_runtime_put_sync(&ofdev->dev);
pm_runtime_disable(&ofdev->dev);
err_free:
--
2.43.0.rc1.1.gbec44491f096

2024-02-26 17:31:34

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 06/14] serial: 8250_bcm7271: Switch to use uart_read_port_properties()

2024-02-26 17:31:43

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 05/14] serial: 8250_bcm2835aux: Switch to use uart_read_port_properties()

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/8250/8250_bcm2835aux.c | 92 +++++++++++------------
1 file changed, 42 insertions(+), 50 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c
index beac6b340ace..69c3c5ca77f7 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -45,10 +45,6 @@ struct bcm2835aux_data {
u32 cntl;
};

-struct bcm2835_aux_serial_driver_data {
- resource_size_t offset;
-};
-
static void bcm2835aux_rs485_start_tx(struct uart_8250_port *up)
{
if (!(up->port.rs485.flags & SER_RS485_RX_DURING_TX)) {
@@ -85,10 +81,9 @@ static void bcm2835aux_rs485_stop_tx(struct uart_8250_port *up)

static int bcm2835aux_serial_probe(struct platform_device *pdev)
{
- const struct bcm2835_aux_serial_driver_data *bcm_data;
+ const struct software_node *bcm2835_swnode;
struct uart_8250_port up = { };
struct bcm2835aux_data *data;
- resource_size_t offset = 0;
struct resource *res;
unsigned int uartclk;
int ret;
@@ -101,12 +96,8 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
/* initialize data */
up.capabilities = UART_CAP_FIFO | UART_CAP_MINI;
up.port.dev = &pdev->dev;
- up.port.regshift = 2;
up.port.type = PORT_16550;
- up.port.iotype = UPIO_MEM;
- up.port.fifosize = 8;
- up.port.flags = UPF_SHARE_IRQ | UPF_FIXED_PORT | UPF_FIXED_TYPE |
- UPF_SKIP_TEST | UPF_IOREMAP;
+ up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SKIP_TEST | UPF_IOREMAP;
up.port.rs485_config = serial8250_em485_config;
up.port.rs485_supported = serial8250_em485_supported;
up.rs485_start_tx = bcm2835aux_rs485_start_tx;
@@ -122,12 +113,6 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
if (IS_ERR(data->clk))
return dev_err_probe(&pdev->dev, PTR_ERR(data->clk), "could not get clk\n");

- /* get the interrupt */
- ret = platform_get_irq(pdev, 0);
- if (ret < 0)
- return ret;
- up.port.irq = ret;
-
/* map the main registers */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -135,52 +120,40 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
return -EINVAL;
}

- bcm_data = device_get_match_data(&pdev->dev);
+ up.port.mapbase = res->start;
+ up.port.mapsize = resource_size(res);

- /* Some UEFI implementations (e.g. tianocore/edk2 for the Raspberry Pi)
- * describe the miniuart with a base address that encompasses the auxiliary
- * registers shared between the miniuart and spi.
- *
- * This is due to historical reasons, see discussion here :
- * https://edk2.groups.io/g/devel/topic/87501357#84349
- *
- * We need to add the offset between the miniuart and auxiliary
- * registers to get the real miniuart base address.
- */
- if (bcm_data)
- offset = bcm_data->offset;
+ bcm2835_swnode = device_get_match_data(&pdev->dev);
+ if (bcm2835_swnode) {
+ ret = device_add_software_node(&pdev->dev, bcm2835_swnode);
+ if (ret)
+ return ret;
+ }

- up.port.mapbase = res->start + offset;
- up.port.mapsize = resource_size(res) - offset;
+ ret = uart_read_port_properties(&up.port, true);
+ if (ret)
+ goto rm_swnode;

- /* Check for a fixed line number */
- ret = of_alias_get_id(pdev->dev.of_node, "serial");
- if (ret >= 0)
- up.port.line = ret;
+ up.port.regshift = 2;
+ up.port.fifosize = 8;

/* enable the clock as a last step */
ret = clk_prepare_enable(data->clk);
if (ret) {
- dev_err(&pdev->dev, "unable to enable uart clock - %d\n",
- ret);
- return ret;
+ dev_err_probe(&pdev->dev, ret, "unable to enable uart clock\n");
+ goto rm_swnode;
}

uartclk = clk_get_rate(data->clk);
- if (!uartclk) {
- ret = device_property_read_u32(&pdev->dev, "clock-frequency", &uartclk);
- if (ret) {
- dev_err_probe(&pdev->dev, ret, "could not get clk rate\n");
- goto dis_clk;
- }
- }
+ if (uartclk)
+ up.port.uartclk = uartclk;

/* the HW-clock divider for bcm2835aux is 8,
* but 8250 expects a divider of 16,
* so we have to multiply the actual clock by 2
* to get identical baudrates.
*/
- up.port.uartclk = uartclk * 2;
+ up.port.uartclk *= 2;

/* register the port */
ret = serial8250_register_8250_port(&up);
@@ -194,6 +167,8 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)

dis_clk:
clk_disable_unprepare(data->clk);
+rm_swnode:
+ device_remove_software_node(&pdev->dev);
return ret;
}

@@ -203,10 +178,27 @@ static void bcm2835aux_serial_remove(struct platform_device *pdev)

serial8250_unregister_port(data->line);
clk_disable_unprepare(data->clk);
+ device_remove_software_node(&pdev->dev);
}

-static const struct bcm2835_aux_serial_driver_data bcm2835_acpi_data = {
- .offset = 0x40,
+/*
+ * Some UEFI implementations (e.g. tianocore/edk2 for the Raspberry Pi)
+ * describe the miniuart with a base address that encompasses the auxiliary
+ * registers shared between the miniuart and spi.
+ *
+ * This is due to historical reasons, see discussion here:
+ * https://edk2.groups.io/g/devel/topic/87501357#84349
+ *
+ * We need to add the offset between the miniuart and auxiliary registers
+ * to get the real miniuart base address.
+ */
+static const struct property_entry bcm2835_acpi_properties[] = {
+ PROPERTY_ENTRY_U32("reg-offset", 0x40),
+ { }
+};
+
+static const struct software_node bcm2835_acpi_node = {
+ .properties = bcm2835_acpi_properties,
};

static const struct of_device_id bcm2835aux_serial_match[] = {
@@ -216,7 +208,7 @@ static const struct of_device_id bcm2835aux_serial_match[] = {
MODULE_DEVICE_TABLE(of, bcm2835aux_serial_match);

static const struct acpi_device_id bcm2835aux_serial_acpi_match[] = {
- { "BCM2836", (kernel_ulong_t)&bcm2835_acpi_data },
+ { "BCM2836", (kernel_ulong_t)&bcm2835_acpi_node },
{ }
};
MODULE_DEVICE_TABLE(acpi, bcm2835aux_serial_acpi_match);
--
2.43.0.rc1.1.gbec44491f096

2024-02-26 17:31:52

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 13/14] serial: 8250_tegra: Switch to use uart_read_port_properties()

2024-02-26 17:31:53

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 08/14] serial: 8250_ingenic: Switch to use uart_read_port_properties()

2024-02-26 18:24:06

by Florian Fainelli

[permalink] [raw]

Subject: Re: [PATCH v2 05/14] serial: 8250_bcm2835aux: Switch to use uart_read_port_properties()

On 2/26/24 06:19, Andy Shevchenko wrote:
> Since we have now a common helper to read port properties
> use it instead of sparse home grown solution.
>
> Signed-off-by: Andy Shevchenko <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
Tested-by: Florian Fainelli <[email protected]>
--
Florian

Attachments:

smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2024-02-26 18:30:13

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 04/14] serial: 8250_aspeed_vuart: Switch to use uart_read_port_properties()

Since we have now a common helper to read port properties
use it instead of sparse home grown solution.

Reviewed-by: Andi Shyti <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
Reviewed-by: Andrew Jeffery <[email protected]>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 50 +++++++--------------
1 file changed, 15 insertions(+), 35 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 8c2aaf7af7b7..2a4bc39b11af 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -419,8 +419,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
struct aspeed_vuart *vuart;
struct device_node *np;
struct resource *res;
- u32 clk, prop, sirq[2];
int rc, sirq_polarity;
+ u32 prop, sirq[2];
struct clk *vclk;

np = pdev->dev.of_node;
@@ -447,53 +447,35 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
port.port.status = UPSTAT_SYNC_FIFO;
port.port.dev = &pdev->dev;
port.port.has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
+ port.port.flags = UPF_BOOT_AUTOCONF | UPF_IOREMAP | UPF_FIXED_PORT | UPF_FIXED_TYPE |
+ UPF_NO_THRE_TEST;
port.bugs |= UART_BUG_TXRACE;

rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
if (rc < 0)
return rc;

- if (of_property_read_u32(np, "clock-frequency", &clk)) {
+ rc = uart_read_port_properties(&port.port, true);
+ if (rc)
+ goto err_sysfs_remove;
+
+ /* Get clk rate through clk driver if present */
+ if (!port.port.uartclk) {
vclk = devm_clk_get_enabled(dev, NULL);
if (IS_ERR(vclk)) {
rc = dev_err_probe(dev, PTR_ERR(vclk), "clk or clock-frequency not defined\n");
goto err_sysfs_remove;
}

- clk = clk_get_rate(vclk);
+ port.port.uartclk = clk_get_rate(vclk);
}

/* If current-speed was set, then try not to change it. */
if (of_property_read_u32(np, "current-speed", &prop) == 0)
- port.port.custom_divisor = clk / (16 * prop);
+ port.port.custom_divisor = port.port.uartclk / (16 * prop);

- /* Check for shifted address mapping */
- if (of_property_read_u32(np, "reg-offset", &prop) == 0)
- port.port.mapbase += prop;
-
- /* Check for registers offset within the devices address range */
- if (of_property_read_u32(np, "reg-shift", &prop) == 0)
- port.port.regshift = prop;
-
- /* Check for fifo size */
- if (of_property_read_u32(np, "fifo-size", &prop) == 0)
- port.port.fifosize = prop;
-
- /* Check for a fixed line number */
- rc = of_alias_get_id(np, "serial");
- if (rc >= 0)
- port.port.line = rc;
-
- port.port.irq = irq_of_parse_and_map(np, 0);
port.port.handle_irq = aspeed_vuart_handle_irq;
- port.port.iotype = UPIO_MEM;
port.port.type = PORT_ASPEED_VUART;
- port.port.uartclk = clk;
- port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP
- | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
-
- if (of_property_read_bool(np, "no-loopback-test"))
- port.port.flags |= UPF_SKIP_TEST;

if (port.port.fifosize)
port.capabilities = UART_CAP_FIFO;
@@ -503,7 +485,7 @@ static int aspeed_vuart_probe(struct platform_device *pdev)

rc = serial8250_register_8250_port(&port);
if (rc < 0)
- goto err_clk_disable;
+ goto err_sysfs_remove;

vuart->line = rc;
vuart->port = serial8250_get_port(vuart->line);
@@ -529,7 +511,7 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
rc = aspeed_vuart_set_lpc_address(vuart, prop);
if (rc < 0) {
dev_err_probe(dev, rc, "invalid value in aspeed,lpc-io-reg property\n");
- goto err_clk_disable;
+ goto err_sysfs_remove;
}

rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
@@ -541,14 +523,14 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
if (rc < 0) {
dev_err_probe(dev, rc, "invalid sirq number in aspeed,lpc-interrupts property\n");
- goto err_clk_disable;
+ goto err_sysfs_remove;
}

sirq_polarity = aspeed_vuart_map_irq_polarity(sirq[1]);
if (sirq_polarity < 0) {
rc = dev_err_probe(dev, sirq_polarity,
"invalid sirq polarity in aspeed,lpc-interrupts property\n");
- goto err_clk_disable;
+ goto err_sysfs_remove;
}

aspeed_vuart_set_sirq_polarity(vuart, sirq_polarity);
@@ -559,8 +541,6 @@ static int aspeed_vuart_probe(struct platform_device *pdev)

return 0;

-err_clk_disable:
- irq_dispose_mapping(port.port.irq);
err_sysfs_remove:
sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
return rc;
--
2.43.0.rc1.1.gbec44491f096

2024-02-26 18:47:16

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 01/14] serial: core: Move struct uart_port::quirks closer to possible values

Currently it's not crystal clear what UPIO_* and UPQ_* definitions
belong to. Reindent the code, so it will be easy to read and understand.
No functional changes intended.

Reviewed-by: Andi Shyti <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
include/linux/serial_core.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 55b1f3ba48ac..2d2ec99eca93 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -467,8 +467,8 @@ struct uart_port {
unsigned int fifosize; /* tx fifo size */
unsigned char x_char; /* xon/xoff char */
unsigned char regshift; /* reg offset shift */
+
unsigned char iotype; /* io access style */
- unsigned char quirks; /* internal quirks */

#define UPIO_PORT (SERIAL_IO_PORT) /* 8b I/O port access */
#define UPIO_HUB6 (SERIAL_IO_HUB6) /* Hub6 ISA card */
@@ -479,7 +479,9 @@ struct uart_port {
#define UPIO_MEM32BE (SERIAL_IO_MEM32BE) /* 32b big endian */
#define UPIO_MEM16 (SERIAL_IO_MEM16) /* 16b little endian */

- /* quirks must be updated while holding port mutex */
+ unsigned char quirks; /* internal quirks */
+
+ /* internal quirks must be updated while holding port mutex */
#define UPQ_NO_TXEN_TEST BIT(0)

unsigned int read_status_mask; /* driver specific */
--
2.43.0.rc1.1.gbec44491f096

2024-02-26 18:57:32

by Andy Shevchenko

[permalink] [raw]

Subject: [PATCH v2 12/14] serial: 8250_pxa: Switch to use uart_read_port_properties()

2024-02-26 21:25:33

by Florian Fainelli

[permalink] [raw]

Subject: Re: [PATCH v2 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

On 2/26/24 06:19, Andy Shevchenko wrote:
> Since we have now a common helper to read port properties
> use it instead of sparse home grown solution.
>
> Reviewed-by: Andi Shyti <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
Tested-by: Florian Fainelli <[email protected]>
--
Florian

Attachments:

smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2024-02-26 22:20:20

by Florian Fainelli

[permalink] [raw]

Subject: Re: [PATCH v2 06/14] serial: 8250_bcm7271: Switch to use uart_read_port_properties()

Attachments:

smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2024-02-27 09:45:14

by Kunihiko Hayashi

[permalink] [raw]

Subject: Re: [PATCH v2 14/14] serial: 8250_uniphier: Switch to use uart_read_port_properties()

Hi,

On 2024/02/26 23:19, Andy Shevchenko wrote:
> Since we have now a common helper to read port properties
> use it instead of sparse home grown solution.
>
> Signed-off-by: Andy Shevchenko <[email protected]>

I confirmed that it works properly.

Reviewed-by: Kunihiko Hayashi <[email protected]>
Tested-by: Kunihiko Hayashi <[email protected]>

Thank you,

---
Best Regards
Kunihiko Hayashi

2024-02-27 16:16:46

by Andy Shevchenko

[permalink] [raw]

Subject: Re: [PATCH v2 14/14] serial: 8250_uniphier: Switch to use uart_read_port_properties()

On Tue, Feb 27, 2024 at 06:43:51PM +0900, Kunihiko Hayashi wrote:
> Hi,
>
> On 2024/02/26 23:19, Andy Shevchenko wrote:
> > Since we have now a common helper to read port properties
> > use it instead of sparse home grown solution.
> >
> > Signed-off-by: Andy Shevchenko <[email protected]>
>
> I confirmed that it works properly.
>
> Reviewed-by: Kunihiko Hayashi <[email protected]>
> Tested-by: Kunihiko Hayashi <[email protected]>

Thank you for testing!

--
With Best Regards,
Andy Shevchenko

2024-02-27 20:39:14

by Andy Shevchenko

[permalink] [raw]

Subject: Re: [PATCH v2 10/14] serial: 8250_of: Switch to use uart_read_port_properties()

On Mon, Feb 26, 2024 at 10:24:30AM -0800, Florian Fainelli wrote:
> On 2/26/24 06:19, Andy Shevchenko wrote:
> > Since we have now a common helper to read port properties
> > use it instead of sparse home grown solution.
> >
> > Reviewed-by: Andi Shyti <[email protected]>
> > Signed-off-by: Andy Shevchenko <[email protected]>
>
> Reviewed-by: Florian Fainelli <[email protected]>
> Tested-by: Florian Fainelli <[email protected]>

Thank you for testing!

--
With Best Regards,
Andy Shevchenko