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.
The rest has been compile tested on x86_64 with clang.
Andy Shevchenko (14):
serial: core: Move struct uart_port::quirks closer to possible values
serial: core: Add UPIO_UNSET constant for unset 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 | 53 +++-----
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 | 135 ++++++++++++++++++++
include/linux/serial_core.h | 10 +-
13 files changed, 313 insertions(+), 333 deletions(-)
--
2.43.0.rc1.1.gbec44491f096
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_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
Currently it's not crystal clear what UPIO_* and UPQ_* definitions
belong two. 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
Several serial drivers want to read the same or similar set of
the port properties. Make a common helper for them.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/tty/serial/serial_port.c | 135 +++++++++++++++++++++++++++++++
include/linux/serial_core.h | 1 +
2 files changed, 136 insertions(+)
diff --git a/drivers/tty/serial/serial_port.c b/drivers/tty/serial/serial_port.c
index 88975a4df306..111e1f25fbc6 100644
--- a/drivers/tty/serial/serial_port.c
+++ b/drivers/tty/serial/serial_port.c
@@ -8,7 +8,10 @@
#include <linux/device.h>
#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/property.h>
#include <linux/serial_core.h>
#include <linux/spinlock.h>
@@ -82,6 +85,138 @@ void uart_remove_one_port(struct uart_driver *drv, struct uart_port *port)
}
EXPORT_SYMBOL(uart_remove_one_port);
+/**
+ * uart_read_port_properties - read firmware properties of the given UART port
+ * @port: corresponding port
+ * @use_defaults: apply defaults or validate the values
+ *
+ * The following device properties are supported:
+ * - clock-frequency (optional)
+ * - fifo-size (optional)
+ * - no-loopback-test (optional)
+ * - reg-shift (defaults may apply)
+ * - reg-offset (value may be validated)
+ * - reg-io-width (defaults may apply or value may be validated)
+ * - interrupts (OF only)
+ * - serial [alias ID] (OF only)
+ *
+ * If the port->dev is of struct platform_device type the interrupt line
+ * will be retrieved via platform_get_irq() call against that device.
+ * Otherwise it will be assigned by fwnode_irq_get() call. In both cases
+ * the index 0 of the resource is used.
+ *
+ * The caller is responsible to initialize the following fields of the @port
+ * ->dev (must be valid)
+ * ->flags
+ * ->iotype (if @use_defaults is false)
+ * ->mapbase
+ * ->mapsize
+ * ->regshift (if @use_defaults is false)
+ * before calling this function. Alternatively the above mentioned fields
+ * may be zeroed, in such case the only ones, that have associated properties
+ * found, will be set to the respective values.
+ *
+ * When @use_defaults is true and the respective property is not found
+ * the following values will be applied:
+ * ->iotype = UPIO_MEM
+ * ->regshift = 0
+ * In this case IRQ must be provided, otherwise an error will be returned.
+ *
+ * The ->irq, ->mapbase, ->mapsize are always altered if no error happened.
+ *
+ * When @use_defaults is false and the respective property is found
+ * the following values will be validated:
+ * - reg-io-width (->iotype)
+ * - reg-offset (->mapsize against ->mapbase)
+ *
+ * Returns: 0 on success or negative errno on failure
+ */
+int uart_read_port_properties(struct uart_port *port, bool use_defaults)
+{
+ struct device *dev = port->dev;
+ u32 value;
+ int ret;
+
+ /* Read optional UART functional clock frequency */
+ device_property_read_u32(dev, "clock-frequency", &port->uartclk);
+
+ /* Read the registers alignment (default: 8-bit) */
+ ret = device_property_read_u32(dev, "reg-shift", &value);
+ if (ret)
+ port->regshift = use_defaults ? 0 : port->regshift;
+ else
+ port->regshift = value;
+
+ /* Read the registers I/O access type (default: MMIO 8-bit) */
+ ret = device_property_read_u32(dev, "reg-io-width", &value);
+ if (ret) {
+ port->iotype = use_defaults ? UPIO_MEM : port->iotype;
+ } else {
+ switch (value) {
+ case 1:
+ port->iotype = UPIO_MEM;
+ break;
+ case 2:
+ port->iotype = UPIO_MEM16;
+ break;
+ case 4:
+ port->iotype = device_is_big_endian(dev) ? UPIO_MEM32BE : UPIO_MEM32;
+ break;
+ default:
+ if (!use_defaults) {
+ dev_err(dev, "Unsupported reg-io-width (%u)\n", value);
+ return -EINVAL;
+ }
+ port->iotype = UPIO_UNSET;
+ break;
+ }
+ }
+
+ /* Read the address mapping base offset (default: no offset) */
+ ret = device_property_read_u32(dev, "reg-offset", &value);
+ if (ret)
+ value = 0;
+
+ /* Check for shifted address mapping overflow */
+ if (!use_defaults && port->mapsize < value) {
+ dev_err(dev, "reg-offset %u exceeds region size %pa\n", value, &port->mapsize);
+ return -EINVAL;
+ }
+
+ port->mapbase += value;
+ port->mapsize -= value;
+
+ /* Read optional FIFO size */
+ device_property_read_u32(dev, "fifo-size", &port->fifosize);
+
+ if (device_property_read_bool(dev, "no-loopback-test"))
+ port->flags |= UPF_SKIP_TEST;
+
+ /* Get index of serial line, if found in DT aliases */
+ ret = of_alias_get_id(dev_of_node(dev), "serial");
+ if (ret >= 0)
+ port->line = ret;
+
+ if (dev_is_platform(dev))
+ ret = platform_get_irq(to_platform_device(dev), 0);
+ else
+ ret = fwnode_irq_get(dev_fwnode(dev), 0);
+ if (ret == -EPROBE_DEFER)
+ return ret;
+ if (ret > 0)
+ port->irq = ret;
+ else if (use_defaults)
+ /* By default IRQ support is mandatory */
+ return ret;
+ else
+ port->irq = 0;
+
+ port->flags |= UPF_SHARE_IRQ;
+
+ return 0;
+}
+EXPORT_SYMBOL(uart_read_port_properties);
+
static struct device_driver serial_port_driver = {
.name = "port",
.suppress_bind_attrs = true,
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 2b0526ae1fac..c93d775fc8a8 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -962,6 +962,7 @@ int uart_register_driver(struct uart_driver *uart);
void uart_unregister_driver(struct uart_driver *uart);
int uart_add_one_port(struct uart_driver *reg, struct uart_port *port);
void uart_remove_one_port(struct uart_driver *reg, struct uart_port *port);
+int uart_read_port_properties(struct uart_port *port, bool use_defaults);
bool uart_match_port(const struct uart_port *port1,
const struct uart_port *port2);
--
2.43.0.rc1.1.gbec44491f096
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_bcm7271.c | 53 +++++++++-----------------
1 file changed, 18 insertions(+), 35 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c
index 1532fa2e8ec4..5a25e78857f7 100644
--- a/drivers/tty/serial/8250/8250_bcm7271.c
+++ b/drivers/tty/serial/8250/8250_bcm7271.c
@@ -942,10 +942,8 @@ static int brcmuart_probe(struct platform_device *pdev)
struct brcmuart_priv *priv;
struct clk *baud_mux_clk;
struct uart_8250_port up;
- int irq;
void __iomem *membase = NULL;
resource_size_t mapbase = 0;
- u32 clk_rate = 0;
int ret;
int x;
int dma_irq;
@@ -953,9 +951,6 @@ static int brcmuart_probe(struct platform_device *pdev)
"uart", "dma_rx", "dma_tx", "dma_intr2", "dma_arb"
};
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return irq;
priv = devm_kzalloc(dev, sizeof(struct brcmuart_priv),
GFP_KERNEL);
if (!priv)
@@ -1011,7 +1006,23 @@ static int brcmuart_probe(struct platform_device *pdev)
}
}
- of_property_read_u32(np, "clock-frequency", &clk_rate);
+ dev_dbg(dev, "DMA is %senabled\n", priv->dma_enabled ? "" : "not ");
+
+ memset(&up, 0, sizeof(up));
+ up.port.type = PORT_BCM7271;
+ up.port.dev = dev;
+ up.port.mapbase = mapbase;
+ up.port.membase = membase;
+ up.port.handle_irq = brcmuart_handle_irq;
+ up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE;
+ up.port.private_data = priv;
+
+ ret = uart_read_port_properties(&up.port, true);
+ if (ret)
+ goto release_dma;
+
+ up.port.regshift = 2;
+ up.port.iotype = device_is_big_endian(dev) ? UPIO_MEM32BE : UPIO_MEM32;
/* See if a Baud clock has been specified */
baud_mux_clk = devm_clk_get_optional_enabled(dev, "sw_baud");
@@ -1023,39 +1034,11 @@ static int brcmuart_probe(struct platform_device *pdev)
priv->baud_mux_clk = baud_mux_clk;
init_real_clk_rates(dev, priv);
- clk_rate = priv->default_mux_rate;
+ up.port.uartclk = priv->default_mux_rate;
} else {
dev_dbg(dev, "BAUD MUX clock not specified\n");
}
- if (clk_rate == 0) {
- ret = dev_err_probe(dev, -EINVAL, "clock-frequency or clk not defined\n");
- goto release_dma;
- }
-
- dev_dbg(dev, "DMA is %senabled\n", priv->dma_enabled ? "" : "not ");
-
- memset(&up, 0, sizeof(up));
- up.port.type = PORT_BCM7271;
- up.port.uartclk = clk_rate;
- up.port.dev = dev;
- up.port.mapbase = mapbase;
- up.port.membase = membase;
- up.port.irq = irq;
- up.port.handle_irq = brcmuart_handle_irq;
- up.port.regshift = 2;
- up.port.iotype = of_device_is_big_endian(np) ?
- UPIO_MEM32BE : UPIO_MEM32;
- up.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
- | UPF_FIXED_PORT | UPF_FIXED_TYPE;
- up.port.dev = dev;
- up.port.private_data = priv;
-
- /* Check for a fixed line number */
- ret = of_alias_get_id(np, "serial");
- if (ret >= 0)
- up.port.line = ret;
-
/* setup HR timer */
hrtimer_init(&priv->hrt, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
priv->hrt.function = brcmuart_hrtimer_func;
--
2.43.0.rc1.1.gbec44491f096
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
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
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_ingenic.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
index a12f737924c0..7603129f1c07 100644
--- a/drivers/tty/serial/8250/8250_ingenic.c
+++ b/drivers/tty/serial/8250/8250_ingenic.c
@@ -234,7 +234,7 @@ static int ingenic_uart_probe(struct platform_device *pdev)
struct ingenic_uart_data *data;
const struct ingenic_uart_config *cdata;
struct resource *regs;
- int irq, err, line;
+ int err;
cdata = of_device_get_match_data(&pdev->dev);
if (!cdata) {
@@ -242,10 +242,6 @@ static int ingenic_uart_probe(struct platform_device *pdev)
return -ENODEV;
}
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return irq;
-
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs) {
dev_err(&pdev->dev, "no registers defined\n");
@@ -259,21 +255,19 @@ static int ingenic_uart_probe(struct platform_device *pdev)
spin_lock_init(&uart.port.lock);
uart.port.type = PORT_16550A;
uart.port.flags = UPF_SKIP_TEST | UPF_IOREMAP | UPF_FIXED_TYPE;
- uart.port.iotype = UPIO_MEM;
uart.port.mapbase = regs->start;
- uart.port.regshift = 2;
uart.port.serial_out = ingenic_uart_serial_out;
uart.port.serial_in = ingenic_uart_serial_in;
- uart.port.irq = irq;
uart.port.dev = &pdev->dev;
- uart.port.fifosize = cdata->fifosize;
uart.tx_loadsz = cdata->tx_loadsz;
uart.capabilities = UART_CAP_FIFO | UART_CAP_RTOIE;
- /* Check for a fixed line number */
- line = of_alias_get_id(pdev->dev.of_node, "serial");
- if (line >= 0)
- uart.port.line = line;
+ err = uart_read_port_properties(&uart.port, true);
+ if (err)
+ return err;
+
+ uart.port.regshift = 2;
+ uart.port.fifosize = cdata->fifosize;
uart.port.membase = devm_ioremap(&pdev->dev, regs->start,
resource_size(regs));
--
2.43.0.rc1.1.gbec44491f096
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
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_omap.c | 29 ++++++++++-------------------
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 6942990a333c..173af575a43e 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1394,11 +1394,7 @@ static int omap8250_probe(struct platform_device *pdev)
struct uart_8250_port up;
struct resource *regs;
void __iomem *membase;
- int irq, ret;
-
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return irq;
+ int ret;
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs) {
@@ -1419,7 +1415,6 @@ static int omap8250_probe(struct platform_device *pdev)
up.port.dev = &pdev->dev;
up.port.mapbase = regs->start;
up.port.membase = membase;
- up.port.irq = irq;
/*
* It claims to be 16C750 compatible however it is a little different.
* It has EFR and has no FCR7_64byte bit. The AFE (which it claims to
@@ -1429,13 +1424,9 @@ static int omap8250_probe(struct platform_device *pdev)
* or pm callback.
*/
up.port.type = PORT_8250;
- up.port.iotype = UPIO_MEM;
- up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SOFT_FLOW |
- UPF_HARD_FLOW;
+ up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SOFT_FLOW | UPF_HARD_FLOW;
up.port.private_data = priv;
- up.port.regshift = OMAP_UART_REGSHIFT;
- up.port.fifosize = 64;
up.tx_loadsz = 64;
up.capabilities = UART_CAP_FIFO;
#ifdef CONFIG_PM
@@ -1461,14 +1452,14 @@ static int omap8250_probe(struct platform_device *pdev)
up.rs485_stop_tx = serial8250_em485_stop_tx;
up.port.has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
- ret = of_alias_get_id(np, "serial");
- if (ret < 0) {
- dev_err(&pdev->dev, "failed to get alias\n");
+ ret = uart_read_port_properties(&up.port, true);
+ if (ret)
return ret;
- }
- up.port.line = ret;
- if (of_property_read_u32(np, "clock-frequency", &up.port.uartclk)) {
+ up.port.regshift = OMAP_UART_REGSHIFT;
+ up.port.fifosize = 64;
+
+ if (!up.port.uartclk) {
struct clk *clk;
clk = devm_clk_get(&pdev->dev, NULL);
@@ -1560,8 +1551,8 @@ static int omap8250_probe(struct platform_device *pdev)
}
#endif
- irq_set_status_flags(irq, IRQ_NOAUTOEN);
- ret = devm_request_irq(&pdev->dev, irq, omap8250_irq, 0,
+ irq_set_status_flags(up.port.irq, IRQ_NOAUTOEN);
+ ret = devm_request_irq(&pdev->dev, up.port.irq, omap8250_irq, 0,
dev_name(&pdev->dev), priv);
if (ret < 0)
return ret;
--
2.43.0.rc1.1.gbec44491f096
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_lpc18xx.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index 8d728a6a5991..e4a6b7b4caf2 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -92,11 +92,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
struct lpc18xx_uart_data *data;
struct uart_8250_port uart;
struct resource *res;
- int irq, ret;
-
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return irq;
+ int ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -139,19 +135,12 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
goto dis_clk_reg;
}
- ret = of_alias_get_id(pdev->dev.of_node, "serial");
- if (ret >= 0)
- uart.port.line = ret;
-
data->dma.rx_param = data;
data->dma.tx_param = data;
spin_lock_init(&uart.port.lock);
uart.port.dev = &pdev->dev;
- uart.port.irq = irq;
- uart.port.iotype = UPIO_MEM32;
uart.port.mapbase = res->start;
- uart.port.regshift = 2;
uart.port.type = PORT_16550A;
uart.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SKIP_TEST;
uart.port.uartclk = clk_get_rate(data->clk_uart);
@@ -160,6 +149,13 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
uart.port.rs485_supported = lpc18xx_rs485_supported;
uart.port.serial_out = lpc18xx_uart_serial_out;
+ ret = uart_read_port_properties(&uart.port, true);
+ if (ret)
+ return ret;
+
+ uart.port.iotype = UPIO_MEM32;
+ uart.port.regshift = 2;
+
uart.dma = &data->dma;
uart.dma->rxconf.src_maxburst = 1;
uart.dma->txconf.dst_maxburst = 1;
--
2.43.0.rc1.1.gbec44491f096
On Wed, 21 Feb 2024 20:31:17 +0200
Andy Shevchenko <[email protected]> wrote:
> Currently it's not crystal clear what UPIO_* and UPQ_* definitions
> belong two. Reindent the code, so it will be easy to read and understand.
two -> to.
Hugo V.
> 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
>
>
--
Hugo Villeneuve
On Wed, Feb 21, 2024 at 01:54:52PM -0500, Hugo Villeneuve wrote:
> On Wed, 21 Feb 2024 20:31:17 +0200
> Andy Shevchenko <[email protected]> wrote:
>
> > Currently it's not crystal clear what UPIO_* and UPQ_* definitions
> > belong two. Reindent the code, so it will be easy to read and understand.
>
> two -> to.
Ah, thanks, will fix!
--
With Best Regards,
Andy Shevchenko
In some APIs we would like to assign the special value to iotype
and compare against it in another places. Introduce UPIO_UNSET
for this purpose.
Note, we can't use 0, because it's a valid value for IO port access.
Signed-off-by: Andy Shevchenko <[email protected]>
---
include/linux/serial_core.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 2d2ec99eca93..2b0526ae1fac 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -470,6 +470,7 @@ struct uart_port {
unsigned char iotype; /* io access style */
+#define UPIO_UNSET ((unsigned char)~0U) /* UCHAR_MAX */
#define UPIO_PORT (SERIAL_IO_PORT) /* 8b I/O port access */
#define UPIO_HUB6 (SERIAL_IO_HUB6) /* Hub6 ISA card */
#define UPIO_MEM (SERIAL_IO_MEM) /* driver-specific */
--
2.43.0.rc1.1.gbec44491f096
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_tegra.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_tegra.c b/drivers/tty/serial/8250/8250_tegra.c
index ba352262df75..ce48d02dfa0d 100644
--- a/drivers/tty/serial/8250/8250_tegra.c
+++ b/drivers/tty/serial/8250/8250_tegra.c
@@ -57,25 +57,11 @@ static int tegra_uart_probe(struct platform_device *pdev)
port = &port8250.port;
spin_lock_init(&port->lock);
- port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_FIXED_PORT |
- UPF_FIXED_TYPE;
- port->iotype = UPIO_MEM32;
- port->regshift = 2;
+ port->flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE;
port->type = PORT_TEGRA;
- port->irqflags |= IRQF_SHARED;
port->dev = &pdev->dev;
port->handle_break = tegra_uart_handle_break;
- ret = of_alias_get_id(pdev->dev.of_node, "serial");
- if (ret >= 0)
- port->line = ret;
-
- ret = platform_get_irq(pdev, 0);
- if (ret < 0)
- return ret;
-
- port->irq = ret;
-
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -ENODEV;
@@ -88,12 +74,18 @@ static int tegra_uart_probe(struct platform_device *pdev)
port->mapbase = res->start;
port->mapsize = resource_size(res);
+ ret = uart_read_port_properties(port, true);
+ if (ret)
+ return ret;
+
+ port->iotype = UPIO_MEM32;
+ port->regshift = 2;
+
uart->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
if (IS_ERR(uart->rst))
return PTR_ERR(uart->rst);
- if (device_property_read_u32(&pdev->dev, "clock-frequency",
- &port->uartclk)) {
+ if (!port->uartclk) {
uart->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(uart->clk)) {
dev_err(&pdev->dev, "failed to get clock!\n");
--
2.43.0.rc1.1.gbec44491f096
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_uniphier.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
index 6399a38ecce2..d3f270a191ee 100644
--- a/drivers/tty/serial/8250/8250_uniphier.c
+++ b/drivers/tty/serial/8250/8250_uniphier.c
@@ -162,7 +162,6 @@ static int uniphier_uart_probe(struct platform_device *pdev)
struct uniphier8250_priv *priv;
struct resource *regs;
void __iomem *membase;
- int irq;
int ret;
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -175,23 +174,12 @@ static int uniphier_uart_probe(struct platform_device *pdev)
if (!membase)
return -ENOMEM;
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return irq;
-
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
memset(&up, 0, sizeof(up));
- ret = of_alias_get_id(dev->of_node, "serial");
- if (ret < 0) {
- dev_err(dev, "failed to get alias id\n");
- return ret;
- }
- up.port.line = ret;
-
priv->clk = devm_clk_get(dev, NULL);
if (IS_ERR(priv->clk)) {
dev_err(dev, "failed to get clock\n");
@@ -211,7 +199,10 @@ static int uniphier_uart_probe(struct platform_device *pdev)
up.port.mapbase = regs->start;
up.port.mapsize = resource_size(regs);
up.port.membase = membase;
- up.port.irq = irq;
+
+ ret = uart_read_port_properties(&up.port, true);
+ if (ret)
+ return ret;
up.port.type = PORT_16550A;
up.port.iotype = UPIO_MEM32;
--
2.43.0.rc1.1.gbec44491f096
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_pxa.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_pxa.c b/drivers/tty/serial/8250/8250_pxa.c
index 77686da42ce8..0c9140b93414 100644
--- a/drivers/tty/serial/8250/8250_pxa.c
+++ b/drivers/tty/serial/8250/8250_pxa.c
@@ -92,11 +92,7 @@ static int serial_pxa_probe(struct platform_device *pdev)
struct uart_8250_port uart = {};
struct pxa8250_data *data;
struct resource *mmres;
- int irq, ret;
-
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return irq;
+ int ret;
mmres = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!mmres)
@@ -114,21 +110,21 @@ static int serial_pxa_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = of_alias_get_id(pdev->dev.of_node, "serial");
- if (ret >= 0)
- uart.port.line = ret;
-
uart.port.type = PORT_XSCALE;
- uart.port.iotype = UPIO_MEM32;
uart.port.mapbase = mmres->start;
- uart.port.regshift = 2;
- uart.port.irq = irq;
- uart.port.fifosize = 64;
uart.port.flags = UPF_IOREMAP | UPF_SKIP_TEST | UPF_FIXED_TYPE;
uart.port.dev = &pdev->dev;
uart.port.uartclk = clk_get_rate(data->clk);
uart.port.pm = serial_pxa_pm;
uart.port.private_data = data;
+
+ ret = uart_read_port_properties(&uart.port, true);
+ if (ret)
+ return ret;
+
+ uart.port.iotype = UPIO_MEM32;
+ uart.port.regshift = 2;
+ uart.port.fifosize = 64;
uart.dl_write = serial_pxa_dl_write;
ret = serial8250_register_8250_port(&uart);
--
2.43.0.rc1.1.gbec44491f096
On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
> Since we have now a common helper to read port properties
> use it instead of sparse home grown solution.
I did some brief testing of the series for the Aspeed machines under
qemu, building them on top of v6.8-rc5:
export ARCH=arm
export CROSS_COMPILE=arm-linux-gnueabihf-
make aspeed_g5_defconfig
make -j$(nproc)
qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
I got an oops during boot, which bisected to this change:
[ 0.314946] 8<--- cut here ---
[ 0.315051] Unable to handle kernel paging request at virtual address fee00000 when write
[ 0.315201] [fee00000] *pgd=00000000
[ 0.315593] Internal error: Oops: 805 [#1] SMP ARM
[ 0.315847] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc5-00010-g8a2c8fe174cf #13
[ 0.316071] Hardware name: Generic DT based system
[ 0.316216] PC is at io_serial_out+0x18/0x20
[ 0.316677] LR is at serial8250_do_set_mctrl+0x54/0x90
[ 0.316781] pc : [<8060eea8>] lr : [<806108b0>] psr: 40000093
[ 0.316891] sp : bf815b08 ip : 00000000 fp : 00000026
[ 0.316987] r10: 81698240 r9 : 40000013 r8 : 81cae600
[ 0.317087] r7 : 81d7d1a8 r6 : 81d7d110 r5 : 81008158 r4 : 00000000
[ 0.317197] r3 : fee00000 r2 : 00000000 r1 : 00000004 r0 : 81008158
[ 0.317350] Flags: nZcv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
[ 0.317471] Control: 10c5387d Table: 8000406a DAC: 00000051
[ 0.317593] Register r0 information: non-slab/vmalloc memory
[ 0.317892] Register r1 information: non-paged memory
[ 0.317996] Register r2 information: NULL pointer
[ 0.318080] Register r3 information: vmalloc memory
[ 0.318176] Register r4 information: NULL pointer
[ 0.318264] Register r5 information: non-slab/vmalloc memory
[ 0.318362] Register r6 information: slab kmalloc-2k start 81d7d000 pointer offset 272 size 2048
[ 0.318701] Register r7 information: slab kmalloc-2k start 81d7d000 pointer offset 424 size 2048
[ 0.318860] Register r8 information: slab kmalloc-512 start 81cae600 pointer offset 0 size 512
[ 0.319095] Register r9 information: non-paged memory
[ 0.319194] Register r10 information: slab kmalloc-64 start 81698240 pointer offset 0 size 64
[ 0.319266] Freeing initrd memory: 13684K
[ 0.319384] Register r11 information: non-paged memory
[ 0.319593] Register r12 information: NULL pointer
[ 0.319703] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[ 0.320006] Stack: (0xbf815b08 to 0xbf816000)
[ 0.320157] 5b00: 81008158 80f85a88 81d7d110 8060cb78 bf815b34 00000026
[ 0.320313] 5b20: 0016e360 80cba110 81e65e80 80cfcdf4 00000003 204f2f49 00307830 00000000
[ 0.320457] 5b40: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.320600] 5b60: 00000000 00000000 00000000 00000000 00000000 ed1677db 81008158 81008158
[ 0.320744] 5b80: bf815c00 81e5c5c0 81008304 81007f58 bf815d2c bf815dac 00000000 8060e1f4
[ 0.320890] 5ba0: 80cba4ec 8081e2c4 bf815dfc 00000001 00000000 81cf5400 81cf5410 81e65e00
[ 0.321030] 5bc0: 00000004 00000000 00000001 80616538 00000000 00000000 00000000 00000000
[ 0.321176] 5be0: 1e784000 1e784fff bd7c1a94 00000200 00000000 00000000 00000000 00000000
[ 0.321325] 5c00: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.321469] 5c20: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.321624] 5c40: 00000000 00000000 8060fb34 00000000 00000000 00000000 00000026 00000000
[ 0.321777] 5c60: 016e3600 00000000 00000200 00000000 00000000 00000000 00000000 00000000
[ 0.321920] 5c80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.322063] 5ca0: 00000000 00000000 b9000040 00000000 00000000 00000000 00000000 00000000
[ 0.322204] 5cc0: 00000004 00000000 00000000 00000004 00000000 1e784000 00001000 81cf5410
[ 0.322347] 5ce0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.322492] 5d00: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000037
[ 0.322640] 5d20: 00000001 00000001 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.322800] 5d40: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.322957] 5d60: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.323114] 5d80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.323271] 5da0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.323422] 5dc0: 00000000 00000000 806111c8 80610eb4 00000000 00000000 00000000 00000000
[ 0.323573] 5de0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.323723] 5e00: 00000000 ed1677db 00000000 81cf5410 80f85cf8 00000000 00000000 81e5c638
[ 0.323878] 5e20: 80e66f48 8067f888 81cf5410 00000000 80f85cf8 00000000 00000000 8067ca08
[ 0.324029] 5e40: 81cf5410 00000000 81cf5410 81cf5410 80f85cf8 81cf5454 81cf5410 8067cda8
[ 0.324181] 5e60: 60000013 81e5c638 81008d4c 81008d54 81cf5454 81cf5410 00000000 8067cf3c
[ 0.324337] 5e80: 81cf5410 80f85cf8 81cf5454 814cec00 00000000 8067d21c 00000000 80f85cf8
[ 0.324494] 5ea0: 8067d11c 8067aa04 814cec00 814cec58 816a4bb4 ed1677db 814cec00 81e5c600
[ 0.324646] 5ec0: 00000000 80f85cf8 814cec00 8067bc6c 80cba524 00000000 00000006 80f85cf8
[ 0.324795] 5ee0: 8158b480 00000006 bf815f14 00000000 80d19438 8067e284 80e221c4 8158b480
[ 0.324945] 5f00: 00000006 80e01414 80d2d3b0 000000db 8173ad17 00000000 00000000 00000000
[ 0.325096] 5f20: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.325247] 5f40: 00000000 00000000 00000000 00000000 00000000 ed1677db 8173ad00 00000020
[ 0.325403] 5f60: 00000006 80e3b83c 80e3b860 80e01750 00000006 00000006 00000000 80e004f8
[ 0.325553] 5f80: 80f05cc0 80a50e18 00000000 00000000 00000000 00000000 00000000 80a50e34
[ 0.325699] 5fa0: 00000000 80a50e18 00000000 8010016c 00000000 00000000 00000000 00000000
[ 0.325848] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.325995] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 0.326531] io_serial_out from serial8250_do_set_mctrl+0x54/0x90
[ 0.326761] serial8250_do_set_mctrl from serial_core_register_port+0x4c4/0x694
[ 0.326917] serial_core_register_port from serial8250_register_8250_port+0x310/0x4bc
[ 0.327063] serial8250_register_8250_port from of_platform_serial_probe+0x300/0x45c
[ 0.327242] of_platform_serial_probe from platform_probe+0x60/0xb8
[ 0.327367] platform_probe from really_probe+0xd4/0x3e4
[ 0.327471] really_probe from __driver_probe_device+0x90/0x1ec
[ 0.327568] __driver_probe_device from driver_probe_device+0x38/0xd0
[ 0.327674] driver_probe_device from __driver_attach+0x100/0x1dc
[ 0.327793] __driver_attach from bus_for_each_dev+0x84/0xd4
[ 0.327906] bus_for_each_dev from bus_add_driver+0xec/0x1f0
[ 0.328015] bus_add_driver from driver_register+0x84/0x11c
[ 0.328126] driver_register from do_one_initcall+0x84/0x1c8
[ 0.328297] do_one_initcall from kernel_init_freeable+0x19c/0x22c
[ 0.328419] kernel_init_freeable from kernel_init+0x1c/0x138
[ 0.328534] kernel_init from ret_from_fork+0x14/0x28
[ 0.328656] Exception stack(0xbf815fb0 to 0xbf815ff8)
[ 0.328755] 5fa0: 00000000 00000000 00000000 00000000
[ 0.328901] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.329112] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 0.329413] Code: e3a03000 ee073f9a e2433612 e6ef2072 (e5c32000)
[ 0.329824] ---[ end trace 0000000000000000 ]---
[ 0.336692] Kernel panic - not syncing: Fatal exception
Andrew
On 21. 02. 24, 19:31, Andy Shevchenko wrote:
> In some APIs we would like to assign the special value to iotype
> and compare against it in another places. Introduce UPIO_UNSET
> for this purpose.
>
> Note, we can't use 0, because it's a valid value for IO port access.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> include/linux/serial_core.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 2d2ec99eca93..2b0526ae1fac 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -470,6 +470,7 @@ struct uart_port {
>
> unsigned char iotype; /* io access style */
>
> +#define UPIO_UNSET ((unsigned char)~0U) /* UCHAR_MAX */
Perhaps making the var u8 and this U8_MAX then? It would make more sense
to me.
> #define UPIO_PORT (SERIAL_IO_PORT) /* 8b I/O port access */
> #define UPIO_HUB6 (SERIAL_IO_HUB6) /* Hub6 ISA card */
> #define UPIO_MEM (SERIAL_IO_MEM) /* driver-specific */
thanks,
--
js
suse labs
On Thu, Feb 22, 2024 at 07:58:32AM +0100, Jiri Slaby wrote:
> On 21. 02. 24, 19:31, Andy Shevchenko wrote:
..
> > unsigned char iotype; /* io access style */
> > +#define UPIO_UNSET ((unsigned char)~0U) /* UCHAR_MAX */
>
> Perhaps making the var u8 and this U8_MAX then? It would make more sense to
> me.
WFM, should it be a separate change? Btw, how can I justify it?
--
With Best Regards,
Andy Shevchenko
On Thu, Feb 22, 2024 at 03:23:24PM +0200, Andy Shevchenko wrote:
> On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
> > On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
> > > Since we have now a common helper to read port properties
> > > use it instead of sparse home grown solution.
> >
> > I did some brief testing of the series for the Aspeed machines under
> > qemu, building them on top of v6.8-rc5:
> >
> > export ARCH=arm
> > export CROSS_COMPILE=arm-linux-gnueabihf-
> > make aspeed_g5_defconfig
> > make -j$(nproc)
> > qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
> >
> > I got an oops during boot, which bisected to this change:
>
> Thank you for prompt testing! I will look at it.
I found the issue, will be fixed in next version.
--
With Best Regards,
Andy Shevchenko
On Thu, Feb 22, 2024 at 06:43:08PM +0200, Andy Shevchenko wrote:
> On Thu, Feb 22, 2024 at 03:23:24PM +0200, Andy Shevchenko wrote:
> > On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
> > > On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
> > > > Since we have now a common helper to read port properties
> > > > use it instead of sparse home grown solution.
> > >
> > > I did some brief testing of the series for the Aspeed machines under
> > > qemu, building them on top of v6.8-rc5:
> > >
> > > export ARCH=arm
> > > export CROSS_COMPILE=arm-linux-gnueabihf-
> > > make aspeed_g5_defconfig
> > > make -j$(nproc)
> > > qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
> > >
> > > I got an oops during boot, which bisected to this change:
> >
> > Thank you for prompt testing! I will look at it.
>
> I found the issue, will be fixed in next version.
Whoever is going to test this series, the
- port->iotype = use_defaults ? UPIO_MEM : port->iotype;
+ port->iotype = UPIO_MEM;
should be applied to uart_read_port_properties() implementation.
--
With Best Regards,
Andy Shevchenko
On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
> On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
> > Since we have now a common helper to read port properties
> > use it instead of sparse home grown solution.
>
> I did some brief testing of the series for the Aspeed machines under
> qemu, building them on top of v6.8-rc5:
>
> export ARCH=arm
> export CROSS_COMPILE=arm-linux-gnueabihf-
> make aspeed_g5_defconfig
> make -j$(nproc)
> qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
>
> I got an oops during boot, which bisected to this change:
Thank you for prompt testing! I will look at it.
--
With Best Regards,
Andy Shevchenko
On 2/22/24 08:47, Andy Shevchenko wrote:
> On Thu, Feb 22, 2024 at 06:43:08PM +0200, Andy Shevchenko wrote:
>> On Thu, Feb 22, 2024 at 03:23:24PM +0200, Andy Shevchenko wrote:
>>> On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
>>>> On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
>>>>> Since we have now a common helper to read port properties
>>>>> use it instead of sparse home grown solution.
>>>>
>>>> I did some brief testing of the series for the Aspeed machines under
>>>> qemu, building them on top of v6.8-rc5:
>>>>
>>>> export ARCH=arm
>>>> export CROSS_COMPILE=arm-linux-gnueabihf-
>>>> make aspeed_g5_defconfig
>>>> make -j$(nproc)
>>>> qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
>>>>
>>>> I got an oops during boot, which bisected to this change:
>>>
>>> Thank you for prompt testing! I will look at it.
>>
>> I found the issue, will be fixed in next version.
>
> Whoever is going to test this series, the
>
> - port->iotype = use_defaults ? UPIO_MEM : port->iotype;
> + port->iotype = UPIO_MEM;
>
> should be applied to uart_read_port_properties() implementation.
>
Thanks, on 8250_bcm7271.c with the above hunk applied, I did not spot
any differences between the values returned by stty or a cat
/sys/class/tty/ttyS0/* before or after, the console remained fully
functional. I will see if I can run an additional test where I removed
the DT's "clocks" property and confirm that the fall back to
"clock-frequency" works.
Thanks Andy!
--
Florian
On 2/22/24 09:39, Florian Fainelli wrote:
> On 2/22/24 08:47, Andy Shevchenko wrote:
>> On Thu, Feb 22, 2024 at 06:43:08PM +0200, Andy Shevchenko wrote:
>>> On Thu, Feb 22, 2024 at 03:23:24PM +0200, Andy Shevchenko wrote:
>>>> On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
>>>>> On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
>>>>>> Since we have now a common helper to read port properties
>>>>>> use it instead of sparse home grown solution.
>>>>>
>>>>> I did some brief testing of the series for the Aspeed machines under
>>>>> qemu, building them on top of v6.8-rc5:
>>>>>
>>>>> export ARCH=arm
>>>>> export CROSS_COMPILE=arm-linux-gnueabihf-
>>>>> make aspeed_g5_defconfig
>>>>> make -j$(nproc)
>>>>> qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel
>>>>> arch/arm/boot/zImage -dtb
>>>>> arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
>>>>>
>>>>> I got an oops during boot, which bisected to this change:
>>>>
>>>> Thank you for prompt testing! I will look at it.
>>>
>>> I found the issue, will be fixed in next version.
>>
>> Whoever is going to test this series, the
>>
>> - port->iotype = use_defaults ? UPIO_MEM : port->iotype;
>> + port->iotype = UPIO_MEM;
>>
>> should be applied to uart_read_port_properties() implementation.
>>
>
> Thanks, on 8250_bcm7271.c with the above hunk applied, I did not spot
> any differences between the values returned by stty or a cat
> /sys/class/tty/ttyS0/* before or after, the console remained fully
> functional. I will see if I can run an additional test where I removed
> the DT's "clocks" property and confirm that the fall back to
> "clock-frequency" works.
>
> Thanks Andy!
Also appears to work properly on a Raspberry Pi 4 with the console using
the bcm2835-aux driver, will provide Tested-by tags on the next
submission, thanks!
--
Florian
Hi Andy,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tty/tty-testing]
[also build test WARNING on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.8-rc5 next-20240222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/serial-core-Move-struct-uart_port-quirks-closer-to-possible-values/20240222-023850
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link: https://lore.kernel.org/r/20240221183442.4124354-7-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 06/14] serial: 8250_bcm7271: Switch to use uart_read_port_properties()
config: hexagon-randconfig-r123-20240222 (https://download.01.org/0day-ci/archive/20240223/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 36adfec155de366d722f2bac8ff9162289dcf06c)
reproduce: (https://download.01.org/0day-ci/archive/20240223/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from drivers/tty/serial/8250/8250_bcm7271.c:15:
In file included from include/linux/tty.h:11:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:42:
In file included from include/linux/scatterlist.h:9:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/tty/serial/8250/8250_bcm7271.c:15:
In file included from include/linux/tty.h:11:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:42:
In file included from include/linux/scatterlist.h:9:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/tty/serial/8250/8250_bcm7271.c:15:
In file included from include/linux/tty.h:11:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:42:
In file included from include/linux/scatterlist.h:9:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
>> drivers/tty/serial/8250/8250_bcm7271.c:938:22: warning: unused variable 'np' [-Wunused-variable]
938 | struct device_node *np = pdev->dev.of_node;
| ^~
7 warnings generated.
vim +/np +938 drivers/tty/serial/8250/8250_bcm7271.c
41a469482de257e Al Cooper 2021-03-25 933
41a469482de257e Al Cooper 2021-03-25 934
41a469482de257e Al Cooper 2021-03-25 935 static int brcmuart_probe(struct platform_device *pdev)
41a469482de257e Al Cooper 2021-03-25 936 {
41a469482de257e Al Cooper 2021-03-25 937 struct resource *regs;
41a469482de257e Al Cooper 2021-03-25 @938 struct device_node *np = pdev->dev.of_node;
41a469482de257e Al Cooper 2021-03-25 939 const struct of_device_id *of_id = NULL;
41a469482de257e Al Cooper 2021-03-25 940 struct uart_8250_port *new_port;
41a469482de257e Al Cooper 2021-03-25 941 struct device *dev = &pdev->dev;
41a469482de257e Al Cooper 2021-03-25 942 struct brcmuart_priv *priv;
41a469482de257e Al Cooper 2021-03-25 943 struct clk *baud_mux_clk;
41a469482de257e Al Cooper 2021-03-25 944 struct uart_8250_port up;
8a66b31a15966ea Colin Ian King 2021-07-19 945 void __iomem *membase = NULL;
41a469482de257e Al Cooper 2021-03-25 946 resource_size_t mapbase = 0;
41a469482de257e Al Cooper 2021-03-25 947 int ret;
41a469482de257e Al Cooper 2021-03-25 948 int x;
41a469482de257e Al Cooper 2021-03-25 949 int dma_irq;
41a469482de257e Al Cooper 2021-03-25 950 static const char * const reg_names[REGS_MAX] = {
41a469482de257e Al Cooper 2021-03-25 951 "uart", "dma_rx", "dma_tx", "dma_intr2", "dma_arb"
41a469482de257e Al Cooper 2021-03-25 952 };
41a469482de257e Al Cooper 2021-03-25 953
41a469482de257e Al Cooper 2021-03-25 954 priv = devm_kzalloc(dev, sizeof(struct brcmuart_priv),
41a469482de257e Al Cooper 2021-03-25 955 GFP_KERNEL);
41a469482de257e Al Cooper 2021-03-25 956 if (!priv)
41a469482de257e Al Cooper 2021-03-25 957 return -ENOMEM;
41a469482de257e Al Cooper 2021-03-25 958
41a469482de257e Al Cooper 2021-03-25 959 of_id = of_match_node(brcmuart_dt_ids, np);
41a469482de257e Al Cooper 2021-03-25 960 if (!of_id || !of_id->data)
41a469482de257e Al Cooper 2021-03-25 961 priv->rate_table = brcmstb_rate_table;
41a469482de257e Al Cooper 2021-03-25 962 else
41a469482de257e Al Cooper 2021-03-25 963 priv->rate_table = of_id->data;
41a469482de257e Al Cooper 2021-03-25 964
41a469482de257e Al Cooper 2021-03-25 965 for (x = 0; x < REGS_MAX; x++) {
41a469482de257e Al Cooper 2021-03-25 966 regs = platform_get_resource_byname(pdev, IORESOURCE_MEM,
41a469482de257e Al Cooper 2021-03-25 967 reg_names[x]);
41a469482de257e Al Cooper 2021-03-25 968 if (!regs)
41a469482de257e Al Cooper 2021-03-25 969 break;
41a469482de257e Al Cooper 2021-03-25 970 priv->regs[x] = devm_ioremap(dev, regs->start,
41a469482de257e Al Cooper 2021-03-25 971 resource_size(regs));
64b1510642f845d Wei Yongjun 2021-03-29 972 if (!priv->regs[x])
64b1510642f845d Wei Yongjun 2021-03-29 973 return -ENOMEM;
41a469482de257e Al Cooper 2021-03-25 974 if (x == REGS_8250) {
41a469482de257e Al Cooper 2021-03-25 975 mapbase = regs->start;
41a469482de257e Al Cooper 2021-03-25 976 membase = priv->regs[x];
41a469482de257e Al Cooper 2021-03-25 977 }
41a469482de257e Al Cooper 2021-03-25 978 }
41a469482de257e Al Cooper 2021-03-25 979
41a469482de257e Al Cooper 2021-03-25 980 /* We should have just the uart base registers or all the registers */
c77247a52be2359 Andy Shevchenko 2023-09-18 981 if (x != 1 && x != REGS_MAX)
c77247a52be2359 Andy Shevchenko 2023-09-18 982 return dev_err_probe(dev, -EINVAL, "%s registers not specified\n",
c77247a52be2359 Andy Shevchenko 2023-09-18 983 reg_names[x]);
41a469482de257e Al Cooper 2021-03-25 984
41a469482de257e Al Cooper 2021-03-25 985 /* if the DMA registers were specified, try to enable DMA */
41a469482de257e Al Cooper 2021-03-25 986 if (x > REGS_DMA_RX) {
41a469482de257e Al Cooper 2021-03-25 987 if (brcmuart_arbitration(priv, 1) == 0) {
41a469482de257e Al Cooper 2021-03-25 988 u32 txrev = 0;
41a469482de257e Al Cooper 2021-03-25 989 u32 rxrev = 0;
41a469482de257e Al Cooper 2021-03-25 990
41a469482de257e Al Cooper 2021-03-25 991 txrev = udma_readl(priv, REGS_DMA_RX, UDMA_RX_REVISION);
41a469482de257e Al Cooper 2021-03-25 992 rxrev = udma_readl(priv, REGS_DMA_TX, UDMA_TX_REVISION);
41a469482de257e Al Cooper 2021-03-25 993 if ((txrev >= UDMA_TX_REVISION_REQUIRED) &&
41a469482de257e Al Cooper 2021-03-25 994 (rxrev >= UDMA_RX_REVISION_REQUIRED)) {
41a469482de257e Al Cooper 2021-03-25 995
41a469482de257e Al Cooper 2021-03-25 996 /* Enable the use of the DMA hardware */
41a469482de257e Al Cooper 2021-03-25 997 priv->dma_enabled = true;
41a469482de257e Al Cooper 2021-03-25 998 } else {
41a469482de257e Al Cooper 2021-03-25 999 brcmuart_arbitration(priv, 0);
41a469482de257e Al Cooper 2021-03-25 1000 dev_err(dev,
41a469482de257e Al Cooper 2021-03-25 1001 "Unsupported DMA Hardware Revision\n");
41a469482de257e Al Cooper 2021-03-25 1002 }
41a469482de257e Al Cooper 2021-03-25 1003 } else {
41a469482de257e Al Cooper 2021-03-25 1004 dev_err(dev,
41a469482de257e Al Cooper 2021-03-25 1005 "Timeout arbitrating for UART DMA hardware\n");
41a469482de257e Al Cooper 2021-03-25 1006 }
41a469482de257e Al Cooper 2021-03-25 1007 }
41a469482de257e Al Cooper 2021-03-25 1008
19010c5b7125670 Andy Shevchenko 2024-02-21 1009 dev_dbg(dev, "DMA is %senabled\n", priv->dma_enabled ? "" : "not ");
19010c5b7125670 Andy Shevchenko 2024-02-21 1010
19010c5b7125670 Andy Shevchenko 2024-02-21 1011 memset(&up, 0, sizeof(up));
19010c5b7125670 Andy Shevchenko 2024-02-21 1012 up.port.type = PORT_BCM7271;
19010c5b7125670 Andy Shevchenko 2024-02-21 1013 up.port.dev = dev;
19010c5b7125670 Andy Shevchenko 2024-02-21 1014 up.port.mapbase = mapbase;
19010c5b7125670 Andy Shevchenko 2024-02-21 1015 up.port.membase = membase;
19010c5b7125670 Andy Shevchenko 2024-02-21 1016 up.port.handle_irq = brcmuart_handle_irq;
19010c5b7125670 Andy Shevchenko 2024-02-21 1017 up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE;
19010c5b7125670 Andy Shevchenko 2024-02-21 1018 up.port.private_data = priv;
19010c5b7125670 Andy Shevchenko 2024-02-21 1019
19010c5b7125670 Andy Shevchenko 2024-02-21 1020 ret = uart_read_port_properties(&up.port, true);
19010c5b7125670 Andy Shevchenko 2024-02-21 1021 if (ret)
19010c5b7125670 Andy Shevchenko 2024-02-21 1022 goto release_dma;
19010c5b7125670 Andy Shevchenko 2024-02-21 1023
19010c5b7125670 Andy Shevchenko 2024-02-21 1024 up.port.regshift = 2;
19010c5b7125670 Andy Shevchenko 2024-02-21 1025 up.port.iotype = device_is_big_endian(dev) ? UPIO_MEM32BE : UPIO_MEM32;
41a469482de257e Al Cooper 2021-03-25 1026
41a469482de257e Al Cooper 2021-03-25 1027 /* See if a Baud clock has been specified */
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05 1028 baud_mux_clk = devm_clk_get_optional_enabled(dev, "sw_baud");
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05 1029 ret = PTR_ERR_OR_ZERO(baud_mux_clk);
41a469482de257e Al Cooper 2021-03-25 1030 if (ret)
15ac1122fd6d4bf Doug Berger 2023-03-09 1031 goto release_dma;
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05 1032 if (baud_mux_clk) {
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05 1033 dev_dbg(dev, "BAUD MUX clock found\n");
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05 1034
41a469482de257e Al Cooper 2021-03-25 1035 priv->baud_mux_clk = baud_mux_clk;
41a469482de257e Al Cooper 2021-03-25 1036 init_real_clk_rates(dev, priv);
19010c5b7125670 Andy Shevchenko 2024-02-21 1037 up.port.uartclk = priv->default_mux_rate;
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05 1038 } else {
1f34e3defb5c0a0 Andy Shevchenko 2023-10-05 1039 dev_dbg(dev, "BAUD MUX clock not specified\n");
41a469482de257e Al Cooper 2021-03-25 1040 }
41a469482de257e Al Cooper 2021-03-25 1041
41a469482de257e Al Cooper 2021-03-25 1042 /* setup HR timer */
41a469482de257e Al Cooper 2021-03-25 1043 hrtimer_init(&priv->hrt, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
41a469482de257e Al Cooper 2021-03-25 1044 priv->hrt.function = brcmuart_hrtimer_func;
41a469482de257e Al Cooper 2021-03-25 1045
41a469482de257e Al Cooper 2021-03-25 1046 up.port.shutdown = brcmuart_shutdown;
41a469482de257e Al Cooper 2021-03-25 1047 up.port.startup = brcmuart_startup;
41a469482de257e Al Cooper 2021-03-25 1048 up.port.throttle = brcmuart_throttle;
41a469482de257e Al Cooper 2021-03-25 1049 up.port.unthrottle = brcmuart_unthrottle;
41a469482de257e Al Cooper 2021-03-25 1050 up.port.set_termios = brcmstb_set_termios;
41a469482de257e Al Cooper 2021-03-25 1051
41a469482de257e Al Cooper 2021-03-25 1052 if (priv->dma_enabled) {
41a469482de257e Al Cooper 2021-03-25 1053 priv->rx_size = RX_BUF_SIZE * RX_BUFS_COUNT;
41a469482de257e Al Cooper 2021-03-25 1054 priv->rx_bufs = dma_alloc_coherent(dev,
41a469482de257e Al Cooper 2021-03-25 1055 priv->rx_size,
41a469482de257e Al Cooper 2021-03-25 1056 &priv->rx_addr, GFP_KERNEL);
c195438f1e84de8 Lad Prabhakar 2021-12-24 1057 if (!priv->rx_bufs) {
0e479b460e342c5 Lad Prabhakar 2022-01-05 1058 ret = -ENOMEM;
41a469482de257e Al Cooper 2021-03-25 1059 goto err;
c195438f1e84de8 Lad Prabhakar 2021-12-24 1060 }
41a469482de257e Al Cooper 2021-03-25 1061 priv->tx_size = UART_XMIT_SIZE;
41a469482de257e Al Cooper 2021-03-25 1062 priv->tx_buf = dma_alloc_coherent(dev,
41a469482de257e Al Cooper 2021-03-25 1063 priv->tx_size,
41a469482de257e Al Cooper 2021-03-25 1064 &priv->tx_addr, GFP_KERNEL);
c195438f1e84de8 Lad Prabhakar 2021-12-24 1065 if (!priv->tx_buf) {
0e479b460e342c5 Lad Prabhakar 2022-01-05 1066 ret = -ENOMEM;
41a469482de257e Al Cooper 2021-03-25 1067 goto err;
41a469482de257e Al Cooper 2021-03-25 1068 }
c195438f1e84de8 Lad Prabhakar 2021-12-24 1069 }
41a469482de257e Al Cooper 2021-03-25 1070
41a469482de257e Al Cooper 2021-03-25 1071 ret = serial8250_register_8250_port(&up);
41a469482de257e Al Cooper 2021-03-25 1072 if (ret < 0) {
c77247a52be2359 Andy Shevchenko 2023-09-18 1073 dev_err_probe(dev, ret, "unable to register 8250 port\n");
41a469482de257e Al Cooper 2021-03-25 1074 goto err;
41a469482de257e Al Cooper 2021-03-25 1075 }
41a469482de257e Al Cooper 2021-03-25 1076 priv->line = ret;
41a469482de257e Al Cooper 2021-03-25 1077 new_port = serial8250_get_port(ret);
41a469482de257e Al Cooper 2021-03-25 1078 priv->up = &new_port->port;
41a469482de257e Al Cooper 2021-03-25 1079 if (priv->dma_enabled) {
41a469482de257e Al Cooper 2021-03-25 1080 dma_irq = platform_get_irq_byname(pdev, "dma");
41a469482de257e Al Cooper 2021-03-25 1081 if (dma_irq < 0) {
c77247a52be2359 Andy Shevchenko 2023-09-18 1082 ret = dev_err_probe(dev, dma_irq, "no IRQ resource info\n");
41a469482de257e Al Cooper 2021-03-25 1083 goto err1;
41a469482de257e Al Cooper 2021-03-25 1084 }
41a469482de257e Al Cooper 2021-03-25 1085 ret = devm_request_irq(dev, dma_irq, brcmuart_isr,
41a469482de257e Al Cooper 2021-03-25 1086 IRQF_SHARED, "uart DMA irq", &new_port->port);
41a469482de257e Al Cooper 2021-03-25 1087 if (ret) {
c77247a52be2359 Andy Shevchenko 2023-09-18 1088 dev_err_probe(dev, ret, "unable to register IRQ handler\n");
41a469482de257e Al Cooper 2021-03-25 1089 goto err1;
41a469482de257e Al Cooper 2021-03-25 1090 }
41a469482de257e Al Cooper 2021-03-25 1091 }
41a469482de257e Al Cooper 2021-03-25 1092 platform_set_drvdata(pdev, priv);
41a469482de257e Al Cooper 2021-03-25 1093 brcmuart_init_debugfs(priv, dev_name(&pdev->dev));
41a469482de257e Al Cooper 2021-03-25 1094 return 0;
41a469482de257e Al Cooper 2021-03-25 1095
41a469482de257e Al Cooper 2021-03-25 1096 err1:
41a469482de257e Al Cooper 2021-03-25 1097 serial8250_unregister_port(priv->line);
41a469482de257e Al Cooper 2021-03-25 1098 err:
41a469482de257e Al Cooper 2021-03-25 1099 brcmuart_free_bufs(dev, priv);
15ac1122fd6d4bf Doug Berger 2023-03-09 1100 release_dma:
15ac1122fd6d4bf Doug Berger 2023-03-09 1101 if (priv->dma_enabled)
41a469482de257e Al Cooper 2021-03-25 1102 brcmuart_arbitration(priv, 0);
c195438f1e84de8 Lad Prabhakar 2021-12-24 1103 return ret;
41a469482de257e Al Cooper 2021-03-25 1104 }
41a469482de257e Al Cooper 2021-03-25 1105
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 22. 02. 24, 14:21, Andy Shevchenko wrote:
> On Thu, Feb 22, 2024 at 07:58:32AM +0100, Jiri Slaby wrote:
>> On 21. 02. 24, 19:31, Andy Shevchenko wrote:
>
> ...
>
>>> unsigned char iotype; /* io access style */
>>> +#define UPIO_UNSET ((unsigned char)~0U) /* UCHAR_MAX */
>>
>> Perhaps making the var u8 and this U8_MAX then? It would make more sense to
>> me.
>
> WFM, should it be a separate change?
Likely.
> Btw, how can I justify it?
Hmm, thinking about it, why is it not an enum?
But it could be also an u8 because you want it be exactly 8 bits as you
want to be sure values up to 255 fit.
thanks,
--
js
suse labs
On Fri, Feb 23, 2024 at 06:42:15AM +0100, Jiri Slaby wrote:
> On 22. 02. 24, 14:21, Andy Shevchenko wrote:
> > On Thu, Feb 22, 2024 at 07:58:32AM +0100, Jiri Slaby wrote:
> > > On 21. 02. 24, 19:31, Andy Shevchenko wrote:
..
> > > > unsigned char iotype; /* io access style */
> > > > +#define UPIO_UNSET ((unsigned char)~0U) /* UCHAR_MAX */
> > >
> > > Perhaps making the var u8 and this U8_MAX then? It would make more sense to
> > > me.
> >
> > WFM, should it be a separate change?
>
> Likely.
Then I need a commit message, because I'm unable to justify this change myself.
> > Btw, how can I justify it?
>
> Hmm, thinking about it, why is it not an enum?
Maybe, but it is a replica of UAPI definitions, do we want to see it as a enum?
To me it will be a bit ugly looking.
> But it could be also an u8 because you want it be exactly 8 bits as you want
> to be sure values up to 255 fit.
Depends on what we assume UAPI does with those flags. It maybe even less than
8 bits, or great than, currently 8 bits is enough...
TL;DR: I would rather take a patch from you and incorporate into the series
than trying hard to invent a justification and proper type.
--
With Best Regards,
Andy Shevchenko
On Fri, Feb 23, 2024 at 12:14:38PM +0800, kernel test robot wrote:
> Hi Andy,
>
> kernel test robot noticed the following build warnings:
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All warnings (new ones prefixed by >>):
>
> In file included from arch/hexagon/include/asm/io.h:328:
> include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> 547 | val = __raw_readb(PCI_IOBASE + addr);
> | ~~~~~~~~~~ ^
Okay, the above is well known issue with hexagon arch, not related to the series.
> >> drivers/tty/serial/8250/8250_bcm7271.c:938:22: warning: unused variable 'np' [-Wunused-variable]
> 938 | struct device_node *np = pdev->dev.of_node;
> | ^~
This one is a good catch! Will be fixed in a new version.
--
With Best Regards,
Andy Shevchenko
On Thu, Feb 22, 2024 at 11:54:46AM -0800, Florian Fainelli wrote:
> On 2/22/24 09:39, Florian Fainelli wrote:
> > On 2/22/24 08:47, Andy Shevchenko wrote:
..
> > Thanks, on 8250_bcm7271.c with the above hunk applied, I did not spot
> > any differences between the values returned by stty or a cat
> > /sys/class/tty/ttyS0/* before or after, the console remained fully
> > functional. I will see if I can run an additional test where I removed
> > the DT's "clocks" property and confirm that the fall back to
> > "clock-frequency" works.
>
> Also appears to work properly on a Raspberry Pi 4 with the console using the
> bcm2835-aux driver, will provide Tested-by tags on the next submission,
> thanks!
Thank you for prompt testing on real HW!
--
With Best Regards,
Andy Shevchenko
On Thu, 2024-02-22 at 18:47 +0200, Andy Shevchenko wrote:
> On Thu, Feb 22, 2024 at 06:43:08PM +0200, Andy Shevchenko wrote:
> > On Thu, Feb 22, 2024 at 03:23:24PM +0200, Andy Shevchenko wrote:
> > > On Thu, Feb 22, 2024 at 11:07:05AM +1030, Andrew Jeffery wrote:
> > > > On Wed, 2024-02-21 at 20:31 +0200, Andy Shevchenko wrote:
> > > > > Since we have now a common helper to read port properties
> > > > > use it instead of sparse home grown solution.
> > > >
> > > > I did some brief testing of the series for the Aspeed machines under
> > > > qemu, building them on top of v6.8-rc5:
> > > >
> > > > export ARCH=arm
> > > > export CROSS_COMPILE=arm-linux-gnueabihf-
> > > > make aspeed_g5_defconfig
> > > > make -j$(nproc)
> > > > qemu-system-arm -M rainier-bmc -nographic -no-reboot -kernel arch/arm/boot/zImage -dtb arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-rainier.dtb -initrd ...
> > > >
> > > > I got an oops during boot, which bisected to this change:
> > >
> > > Thank you for prompt testing! I will look at it.
> >
> > I found the issue, will be fixed in next version.
>
> Whoever is going to test this series, the
>
> - port->iotype = use_defaults ? UPIO_MEM : port->iotype;
> + port->iotype = UPIO_MEM;
>
> should be applied to uart_read_port_properties() implementation.
>
Thanks, with that fix applied it works fine for me also.
Andrew
On Wed, 2024-02-21 at 20:31 +0200, 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: Andrew Jeffery <[email protected]>
On Fri, Feb 23, 2024 at 04:59:25PM +0200, Andy Shevchenko wrote:
> On Fri, Feb 23, 2024 at 06:42:15AM +0100, Jiri Slaby wrote:
> > On 22. 02. 24, 14:21, Andy Shevchenko wrote:
> > > On Thu, Feb 22, 2024 at 07:58:32AM +0100, Jiri Slaby wrote:
> > > > On 21. 02. 24, 19:31, Andy Shevchenko wrote:
..
> > > > > unsigned char iotype; /* io access style */
> > > > > +#define UPIO_UNSET ((unsigned char)~0U) /* UCHAR_MAX */
> > > >
> > > > Perhaps making the var u8 and this U8_MAX then? It would make more sense to
> > > > me.
> > >
> > > WFM, should it be a separate change?
> >
> > Likely.
>
> Then I need a commit message, because I'm unable to justify this change myself.
>
> > > Btw, how can I justify it?
> >
> > Hmm, thinking about it, why is it not an enum?
>
> Maybe, but it is a replica of UAPI definitions, do we want to see it as a enum?
> To me it will be a bit ugly looking.
>
> > But it could be also an u8 because you want it be exactly 8 bits as you want
> > to be sure values up to 255 fit.
>
> Depends on what we assume UAPI does with those flags. It maybe even less than
> 8 bits, or great than, currently 8 bits is enough...
>
> TL;DR: I would rather take a patch from you and incorporate into the series
> than trying hard to invent a justification and proper type.
Okay, I want to send a new version, for now I leave the type change for
the next time. It looks that quirks as well will benefit from type clarifying.
--
With Best Regards,
Andy Shevchenko