Hello folks,
here's another attempt of polishing the serial drivers:
* lots of minor cleanups to make checkpatch happier
(eg. formatting, includes, inttypes, ...)
* use appropriate logging helpers instead of printk()
* consequent use of mapsize/mapbase fields:
the basic idea is, all drivers should fill mapbase/mapbase fields at
init time and later only use those fields, instead of hardcoded values
(later on, we can add generic helpers for the map/unmap stuff, etc)
* untwisting serial8250_port_size() at all:
move the iomem size probing to initialization time, move out some
platform specific magic to corresponding platform code, etc.
Unfortunately, I don't have the actual hardware to really test all
the code, so please let me know if there's something broken in here.
have fun,
--mtx
Using dev_err() instead of printk() for more consistent output.
(prints device name, etc).
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/dz.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index 7b57e84..96e35af 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -416,7 +416,7 @@ static int dz_startup(struct uart_port *uport)
IRQF_SHARED, "dz", mux);
if (ret) {
atomic_add(-1, &mux->irq_guard);
- printk(KERN_ERR "dz: Cannot get IRQ %d!\n", dport->port.irq);
+ dev_err(uport->dev, "Cannot get IRQ %d!\n", dport->port.irq);
return ret;
}
@@ -680,7 +680,7 @@ static int dz_map_port(struct uart_port *uport)
uport->membase = ioremap_nocache(uport->mapbase,
dec_kn_slot_size);
if (!uport->membase) {
- printk(KERN_ERR "dz: Cannot map MMIO\n");
+ dev_err(uport->dev, "Cannot map MMIO\n");
return -ENOMEM;
}
return 0;
@@ -697,8 +697,8 @@ static int dz_request_port(struct uart_port *uport)
if (!request_mem_region(uport->mapbase, dec_kn_slot_size,
"dz")) {
atomic_add(-1, &mux->map_guard);
- printk(KERN_ERR
- "dz: Unable to reserve MMIO resource\n");
+ dev_err(uport->dev,
+ "Unable to reserve MMIO resource\n");
return -EBUSY;
}
}
--
1.9.1
Using dev_info() instead of printk() for more consistent output.
(prints device name, etc).
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/sunzilog.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/sunzilog.c b/drivers/tty/serial/sunzilog.c
index bc7af8b..6285bba 100644
--- a/drivers/tty/serial/sunzilog.c
+++ b/drivers/tty/serial/sunzilog.c
@@ -1489,14 +1489,12 @@ static int zs_probe(struct platform_device *op)
}
uart_inst++;
} else {
- printk(KERN_INFO "%s: Keyboard at MMIO 0x%llx (irq = %d) "
+ dev_info(&op->dev, "Keyboard at MMIO 0x%llx (irq = %d) "
"is a %s\n",
- dev_name(&op->dev),
(unsigned long long) up[0].port.mapbase,
op->archdata.irqs[0], sunzilog_type(&up[0].port));
- printk(KERN_INFO "%s: Mouse at MMIO 0x%llx (irq = %d) "
+ dev_info(&op->dev, "Mouse at MMIO 0x%llx (irq = %d) "
"is a %s\n",
- dev_name(&op->dev),
(unsigned long long) up[1].port.mapbase,
op->archdata.irqs[0], sunzilog_type(&up[1].port));
kbm_inst++;
--
1.9.1
Fix checkpatch warning:
ERROR: Macros with complex values should be enclosed in parentheses
#912: FILE: dz.c:912:
+#define SERIAL_DZ_CONSOLE &dz_console
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/dz.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index fd4f0cc..b3e9313 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -909,7 +909,7 @@ static int __init dz_serial_console_init(void)
console_initcall(dz_serial_console_init);
-#define SERIAL_DZ_CONSOLE &dz_console
+#define SERIAL_DZ_CONSOLE (&dz_console)
#else
#define SERIAL_DZ_CONSOLE NULL
#endif /* CONFIG_SERIAL_DZ_CONSOLE */
--
1.9.1
This fixes a bunch of checkpatch warnings:
WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
#930: FILE: drivers/tty/serial/ioc4_serial.c:930:
+ printk(KERN_ERR
WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
#945: FILE: drivers/tty/serial/ioc4_serial.c:945:
+ printk(KERN_ERR
WARNING: printk() should include KERN_<LEVEL> facility level
#1028: FILE: drivers/tty/serial/ioc4_serial.c:1028:
+ printk ("%s : %d : mem 0x%p sio_ir 0x%x sio_ies 0x%x "
WARNING: space prohibited between function name and open parenthesis '('
#1028: FILE: drivers/tty/serial/ioc4_serial.c:1028:
+ printk ("%s : %d : mem 0x%p sio_ir 0x%x sio_ies 0x%x "
WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1064: FILE: drivers/tty/serial/ioc4_serial.c:1064:
+ printk(KERN_INFO "IOC4 firmware revision %d\n", ioc4_revid);
WARNING: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#1066: FILE: drivers/tty/serial/ioc4_serial.c:1066:
+ printk(KERN_WARNING
WARNING: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#1080: FILE: drivers/tty/serial/ioc4_serial.c:1080:
+ printk(KERN_WARNING
WARNING: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#1870: FILE: drivers/tty/serial/ioc4_serial.c:1870:
+ printk(KERN_WARNING "IOC4 serial: "
WARNING: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#2170: FILE: drivers/tty/serial/ioc4_serial.c:2170:
+ printk(KERN_WARNING "IOC4 serial: "
WARNING: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#2796: FILE: drivers/tty/serial/ioc4_serial.c:2796:
+ printk(KERN_WARNING
WARNING: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#2804: FILE: drivers/tty/serial/ioc4_serial.c:2804:
+ printk(KERN_WARNING
WARNING: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#2818: FILE: drivers/tty/serial/ioc4_serial.c:2818:
+ printk(KERN_WARNING "ioc4_attach_one"
WARNING: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#2828: FILE: drivers/tty/serial/ioc4_serial.c:2828:
+ printk(KERN_WARNING
WARNING: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#2861: FILE: drivers/tty/serial/ioc4_serial.c:2861:
+ printk(KERN_WARNING
WARNING: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#2917: FILE: drivers/tty/serial/ioc4_serial.c:2917:
+ printk(KERN_WARNING
WARNING: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#2923: FILE: drivers/tty/serial/ioc4_serial.c:2923:
+ printk(KERN_WARNING
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/ioc4_serial.c | 39 +++++++++++++++------------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/drivers/tty/serial/ioc4_serial.c b/drivers/tty/serial/ioc4_serial.c
index 21c1b8f..4ab67f1 100644
--- a/drivers/tty/serial/ioc4_serial.c
+++ b/drivers/tty/serial/ioc4_serial.c
@@ -927,8 +927,7 @@ static void handle_dma_error_intr(void *arg, uint32_t other_ir)
writel(hooks->intr_dma_error, &port->ip_mem->other_ir.raw);
if (readl(&port->ip_mem->pci_err_addr_l.raw) & IOC4_PCI_ERR_ADDR_VLD) {
- printk(KERN_ERR
- "PCI error address is 0x%llx, "
+ pr_err("PCI error address is 0x%llx, "
"master is serial port %c %s\n",
(((uint64_t)readl(&port->ip_mem->pci_err_addr_h)
<< 32)
@@ -942,8 +941,7 @@ static void handle_dma_error_intr(void *arg, uint32_t other_ir)
if (readl(&port->ip_mem->pci_err_addr_l.raw)
& IOC4_PCI_ERR_ADDR_MUL_ERR) {
- printk(KERN_ERR
- "Multiple errors occurred\n");
+ pr_err("Multiple errors occurred\n");
}
}
spin_unlock_irqrestore(&port->ip_lock, flags);
@@ -1025,7 +1023,7 @@ static irqreturn_t ioc4_intr(int irq, void *arg)
unsigned long flag;
spin_lock_irqsave(&soft->is_ir_lock, flag);
- printk ("%s : %d : mem 0x%p sio_ir 0x%x sio_ies 0x%x "
+ pr_debug("%s : %d : mem 0x%p sio_ir 0x%x sio_ies 0x%x "
"other_ir 0x%x other_ies 0x%x mask 0x%x\n",
__func__, __LINE__,
(void *)mem, readl(&mem->sio_ir.raw),
@@ -1061,9 +1059,9 @@ static inline int ioc4_attach_local(struct ioc4_driver_data *idd)
/* IOC4 firmware must be at least rev 62 */
pci_read_config_word(pdev, PCI_COMMAND_SPECIAL, &ioc4_revid);
- printk(KERN_INFO "IOC4 firmware revision %d\n", ioc4_revid);
+ pr_info("IOC4 firmware revision %d\n", ioc4_revid);
if (ioc4_revid < ioc4_revid_min) {
- printk(KERN_WARNING
+ pr_warn(
"IOC4 serial not supported on firmware rev %d, "
"please upgrade to rev %d or higher\n",
ioc4_revid, ioc4_revid_min);
@@ -1077,8 +1075,7 @@ static inline int ioc4_attach_local(struct ioc4_driver_data *idd)
port_number++) {
port = kzalloc(sizeof(struct ioc4_port), GFP_KERNEL);
if (!port) {
- printk(KERN_WARNING
- "IOC4 serial memory not available for port\n");
+ pr_warn("IOC4 serial memory not available for port\n");
goto free;
}
spin_lock_init(&port->ip_lock);
@@ -1867,7 +1864,7 @@ static void handle_intr(void *arg, uint32_t sio_ir)
uint32_t shadow;
if ( loop_counter-- <= 0 ) {
- printk(KERN_WARNING "IOC4 serial: "
+ pr_warn("IOC4 serial: "
"possible hang condition/"
"port stuck on interrupt.\n");
break;
@@ -2167,7 +2164,7 @@ static inline int do_read(struct uart_port *the_port, unsigned char *buf,
entry = (struct ring_entry *)((caddr_t)inring + cons_ptr);
if ( loop_counter-- <= 0 ) {
- printk(KERN_WARNING "IOC4 serial: "
+ pr_warn("IOC4 serial: "
"possible hang condition/"
"port stuck on read.\n");
break;
@@ -2793,16 +2790,14 @@ static int ioc4_serial_remove_one(struct ioc4_driver_data *idd)
if (!request_mem_region(tmp_addr1, sizeof(struct ioc4_serial),
"sioc4_uart")) {
- printk(KERN_WARNING
- "ioc4 (%p): unable to get request region for "
+ pr_warn("ioc4 (%p): unable to get request region for "
"uart space\n", (void *)idd->idd_pdev);
ret = -ENODEV;
goto out1;
}
serial = ioremap(tmp_addr1, sizeof(struct ioc4_serial));
if (!serial) {
- printk(KERN_WARNING
- "ioc4 (%p) : unable to remap ioc4 serial register\n",
+ pr_warn("ioc4 (%p) : unable to remap ioc4 serial register\n",
(void *)idd->idd_pdev);
ret = -ENODEV;
goto out2;
@@ -2815,7 +2810,7 @@ static int ioc4_serial_remove_one(struct ioc4_driver_data *idd)
control = kzalloc(sizeof(struct ioc4_control), GFP_KERNEL);
if (!control) {
- printk(KERN_WARNING "ioc4_attach_one"
+ pr_warn("ioc4_attach_one"
": unable to get memory for the IOC4\n");
ret = -ENOMEM;
goto out2;
@@ -2825,8 +2820,7 @@ static int ioc4_serial_remove_one(struct ioc4_driver_data *idd)
/* Allocate the soft structure */
soft = kzalloc(sizeof(struct ioc4_soft), GFP_KERNEL);
if (!soft) {
- printk(KERN_WARNING
- "ioc4 (%p): unable to get memory for the soft struct\n",
+ pr_warn("ioc4 (%p): unable to get memory for the soft struct\n",
(void *)idd->idd_pdev);
ret = -ENOMEM;
goto out3;
@@ -2858,8 +2852,7 @@ static int ioc4_serial_remove_one(struct ioc4_driver_data *idd)
"sgi-ioc4serial", soft)) {
control->ic_irq = idd->idd_pdev->irq;
} else {
- printk(KERN_WARNING
- "%s : request_irq fails for IRQ 0x%x\n ",
+ pr_warn("%s : request_irq fails for IRQ 0x%x\n ",
__func__, idd->idd_pdev->irq);
}
ret = ioc4_attach_local(idd);
@@ -2914,14 +2907,12 @@ static int __init ioc4_serial_init(void)
/* register with serial core */
if ((ret = uart_register_driver(&ioc4_uart_rs232)) < 0) {
- printk(KERN_WARNING
- "%s: Couldn't register rs232 IOC4 serial driver\n",
+ pr_warn("%s: Couldn't register rs232 IOC4 serial driver\n",
__func__);
goto out;
}
if ((ret = uart_register_driver(&ioc4_uart_rs422)) < 0) {
- printk(KERN_WARNING
- "%s: Couldn't register rs422 IOC4 serial driver\n",
+ pr_warn("%s: Couldn't register rs422 IOC4 serial driver\n",
__func__);
goto out_uart_rs232;
}
--
1.9.1
Simpily io resource size computation by setting mapsize field.
Some of the special cases handled by serial8250_port_size() can be
simplified by putting this data to corresponding platform data
or probe function.
Signed-off-by: Enrico Weigelt <[email protected]>
---
arch/mips/alchemy/common/platform.c | 1 +
drivers/tty/serial/8250/8250.h | 1 +
drivers/tty/serial/8250/8250_of.c | 1 +
drivers/tty/serial/8250/8250_port.c | 6 +-----
4 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/mips/alchemy/common/platform.c b/arch/mips/alchemy/common/platform.c
index 1454d9f..226096d 100644
--- a/arch/mips/alchemy/common/platform.c
+++ b/arch/mips/alchemy/common/platform.c
@@ -51,6 +51,7 @@ static void alchemy_8250_pm(struct uart_port *port, unsigned int state,
#define PORT(_base, _irq) \
{ \
.mapbase = _base, \
+ .mapsize = 0x1000, \
.irq = _irq, \
.regshift = 2, \
.iotype = UPIO_AU, \
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 89e3f09..7984aad 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -105,6 +105,7 @@ struct serial8250_config {
#define SERIAL8250_PORT(_base, _irq) SERIAL8250_PORT_FLAGS(_base, _irq, 0)
+#define SERIAL_RT2880_IOSIZE 0x100
static inline int serial_in(struct uart_8250_port *up, int offset)
{
diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 0277479c..08157a1 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -185,6 +185,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
case PORT_RT2880:
port->iotype = UPIO_AU;
+ port->mapsize = SERIAL_RT2880_IOSIZE;
break;
}
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index d09af4c..51d6076 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2833,11 +2833,7 @@ unsigned int serial8250_port_size(struct uart_8250_port *pt)
{
if (pt->port.mapsize)
return pt->port.mapsize;
- if (pt->port.iotype == UPIO_AU) {
- if (pt->port.type == PORT_RT2880)
- return 0x100;
- return 0x1000;
- }
+
if (is_omap1_8250(pt))
return 0x16 << pt->port.regshift;
--
1.9.1
Introduce a little helpers for settings the mmio range from an
struct resource or start/len parameters with less code.
(also setting iotype to UPIO_MEM)
Also converting drivers to use these new helpers as well as
fetching mapsize field instead of using hardcoded values.
(the runtime overhead of that should be negligible)
The idea is moving to a consistent scheme, so later common
calls like request+ioremap combination can be done by generic
helpers.
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/8250/8250_acorn.c | 5 ++--
drivers/tty/serial/8250/8250_aspeed_vuart.c | 4 +--
drivers/tty/serial/8250/8250_dw.c | 3 +--
drivers/tty/serial/8250/8250_em.c | 2 +-
drivers/tty/serial/8250/8250_exar.c | 6 +++--
drivers/tty/serial/8250/8250_hp300.c | 11 +++++---
drivers/tty/serial/8250/8250_of.c | 9 ++-----
drivers/tty/serial/meson_uart.c | 5 ++--
drivers/tty/serial/mps2-uart.c | 5 ++--
drivers/tty/serial/pmac_zilog.c | 8 ++----
drivers/tty/serial/vt8500_serial.c | 4 +--
drivers/tty/serial/xilinx_uartps.c | 3 +--
drivers/tty/serial/zs.c | 10 +++++---
include/linux/serial_core.h | 40 +++++++++++++++++++++++++++++
14 files changed, 76 insertions(+), 39 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_acorn.c b/drivers/tty/serial/8250/8250_acorn.c
index 758c4aa..359171b 100644
--- a/drivers/tty/serial/8250/8250_acorn.c
+++ b/drivers/tty/serial/8250/8250_acorn.c
@@ -63,14 +63,15 @@ struct serial_card_info {
uart.port.irq = ec->irq;
uart.port.flags = UPF_BOOT_AUTOCONF | UPF_SHARE_IRQ;
uart.port.uartclk = type->uartclk;
- uart.port.iotype = UPIO_MEM;
uart.port.regshift = 2;
uart.port.dev = &ec->dev;
for (i = 0; i < info->num_ports; i++) {
uart.port.membase = info->vaddr + type->offset[i];
- uart.port.mapbase = bus_addr + type->offset[i];
+ /* mapsize is computed by serial8250_register_8250_port() */
+ uart_memres_set_start_len(&uart.port,
+ bus_addr + type->offset[i], 0);
info->ports[i] = serial8250_register_8250_port(&uart);
}
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 0438d9a..0e06391 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -328,8 +328,6 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
memset(&port, 0, sizeof(port));
port.port.private_data = vuart;
port.port.membase = vuart->regs;
- port.port.mapbase = res->start;
- port.port.mapsize = resource_size(res);
port.port.startup = aspeed_vuart_startup;
port.port.shutdown = aspeed_vuart_shutdown;
port.port.throttle = aspeed_vuart_throttle;
@@ -337,6 +335,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
port.port.status = UPSTAT_SYNC_FIFO;
port.port.dev = &pdev->dev;
+ uart_memres_set_res(&port.port, res);
+
rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
if (rc < 0)
return rc;
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index d31b975..cb65256 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -513,18 +513,17 @@ static int dw8250_probe(struct platform_device *pdev)
}
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->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;
+ uart_memres_set_res(p, regs);
p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
if (!p->membase)
diff --git a/drivers/tty/serial/8250/8250_em.c b/drivers/tty/serial/8250/8250_em.c
index 2a76e22..7610441 100644
--- a/drivers/tty/serial/8250/8250_em.c
+++ b/drivers/tty/serial/8250/8250_em.c
@@ -100,12 +100,12 @@ static int serial8250_em_probe(struct platform_device *pdev)
}
memset(&up, 0, sizeof(up));
- up.port.mapbase = regs->start;
up.port.irq = irq->start;
up.port.type = PORT_UNKNOWN;
up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_IOREMAP;
up.port.dev = &pdev->dev;
up.port.private_data = priv;
+ uart_memres_set_res(&up.port, res);
clk_prepare_enable(priv->sclk);
up.port.uartclk = clk_get_rate(priv->sclk);
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 0089aa3..f4c1289 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -134,8 +134,10 @@ static int default_setup(struct exar8250 *priv, struct pci_dev *pcidev,
const struct exar8250_board *board = priv->board;
unsigned int bar = 0;
- port->port.iotype = UPIO_MEM;
- port->port.mapbase = pci_resource_start(pcidev, bar) + offset;
+ uart_memres_set_start_len(&port->port,
+ pci_resource_start(pcidev, bar) + offset,
+ pci_resource_len(pcidev, bar));
+
port->port.membase = priv->virt + offset;
port->port.regshift = board->reg_shift;
diff --git a/drivers/tty/serial/8250/8250_hp300.c b/drivers/tty/serial/8250/8250_hp300.c
index 3012ea0..09f9dd3 100644
--- a/drivers/tty/serial/8250/8250_hp300.c
+++ b/drivers/tty/serial/8250/8250_hp300.c
@@ -114,7 +114,9 @@ int __init hp300_setup_serial_console(void)
pr_info("Serial console is HP APCI 1\n");
port.uartclk = HPAPCI_BAUD_BASE * 16;
- port.mapbase = (FRODO_BASE + FRODO_APCI_OFFSET(1));
+ uart_memres_set_start_len(
+ &port,
+ FRODO_BASE + FRODO_APCI_OFFSET(1), 0);
port.membase = (char *)(port.mapbase + DIO_VIRADDRBASE);
port.regshift = 2;
add_preferred_console("ttyS", port.line, "9600n8");
@@ -131,7 +133,8 @@ int __init hp300_setup_serial_console(void)
pr_info("Serial console is HP DCA at select code %d\n", scode);
port.uartclk = HPDCA_BAUD_BASE * 16;
- port.mapbase = (pa + UART_OFFSET);
+
+ uart_memres_set_start_len(&port, (pa + UART_OFFSET));
port.membase = (char *)(port.mapbase + DIO_VIRADDRBASE);
port.regshift = 1;
port.irq = DIO_IPL(pa + DIO_VIRADDRBASE);
@@ -173,7 +176,9 @@ static int hpdca_init_one(struct dio_dev *d,
uart.port.flags = UPF_SKIP_TEST | UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF;
uart.port.irq = d->ipl;
uart.port.uartclk = HPDCA_BAUD_BASE * 16;
- uart.port.mapbase = (d->resource.start + UART_OFFSET);
+ uart_memres_set_start_len(&uart.port,
+ (d->resource.start + UART_OFFSET),
+ resource_size(&d->resource));
uart.port.membase = (char *)(uart.port.mapbase + DIO_VIRADDRBASE);
uart.port.regshift = 1;
uart.port.dev = &d->dev;
diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 08157a1..1c5896c 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -96,18 +96,13 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
UPF_FIXED_TYPE;
spin_lock_init(&port->lock);
- if (resource_type(&resource) == IORESOURCE_IO) {
- port->iotype = UPIO_PORT;
- port->iobase = resource.start;
- } else {
- port->mapbase = resource.start;
- port->mapsize = resource_size(&resource);
+ uart_memres_set_res(port, &resource);
+ if (resource_type(&resource) == IORESOURCE_MEM) {
/* Check for shifted address mapping */
if (of_property_read_u32(np, "reg-offset", &prop) == 0)
port->mapbase += prop;
- port->iotype = UPIO_MEM;
if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
switch (prop) {
case 1:
diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index fbc5bc0..bfcaa2f 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -698,9 +698,8 @@ static int meson_uart_probe(struct platform_device *pdev)
if (ret)
return ret;
- port->iotype = UPIO_MEM;
- port->mapbase = res_mem->start;
- port->mapsize = resource_size(res_mem);
+ uart_memset_set_res(port, res_mem);
+
port->irq = res_irq->start;
port->flags = UPF_BOOT_AUTOCONF | UPF_LOW_LATENCY;
port->dev = &pdev->dev;
diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c
index 587b42f..a4fd1de 100644
--- a/drivers/tty/serial/mps2-uart.c
+++ b/drivers/tty/serial/mps2-uart.c
@@ -562,9 +562,8 @@ static int mps2_init_port(struct platform_device *pdev,
if (IS_ERR(mps_port->port.membase))
return PTR_ERR(mps_port->port.membase);
- mps_port->port.mapbase = res->start;
- mps_port->port.mapsize = resource_size(res);
- mps_port->port.iotype = UPIO_MEM;
+ uart_memres_set_res(&mps_port->port, res);
+
mps_port->port.flags = UPF_BOOT_AUTOCONF;
mps_port->port.fifosize = 1;
mps_port->port.ops = &mps2_uart_pops;
diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index 1fef014..7017dd2 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -1412,8 +1412,7 @@ static int __init pmz_init_port(struct uart_pmac_port *uap)
*/
if (of_address_to_resource(np, 0, &r_ports))
return -ENODEV;
- uap->port.mapbase = r_ports.start;
- uap->port.mapsize = PMZ_MAPSIZE;
+ uart_memres_set_start_len(&uap->port, r_ports.start, PMZ_MAPSIZE);
uap->port.membase = ioremap(uap->port.mapbase, uap->port.mapsize);
uap->control_reg = uap->port.membase;
@@ -1493,7 +1492,6 @@ static int __init pmz_init_port(struct uart_pmac_port *uap)
/*
* Init remaining bits of "port" structure
*/
- uap->port.iotype = UPIO_MEM;
uap->port.irq = irq_of_parse_and_map(np, 0);
uap->port.uartclk = ZS_CLOCK;
uap->port.fifosize = 1;
@@ -1711,10 +1709,8 @@ static int __init pmz_init_port(struct uart_pmac_port *uap)
if (!r_ports || irq <= 0)
return -ENODEV;
- uap->port.mapbase = r_ports->start;
- uap->port.mapsize = PMZ_MAPSIZE;
+ uart_memres_set_start_len(&uap->port, r_ports->start, PMZ_MAPSIZE);
uap->port.membase = (unsigned char __iomem *) r_ports->start;
- uap->port.iotype = UPIO_MEM;
uap->port.irq = irq;
uap->port.uartclk = ZS_CLOCK;
uap->port.fifosize = 1;
diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index 3d58e9b..b303de4 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -695,8 +695,6 @@ static int vt8500_serial_probe(struct platform_device *pdev)
VT8500_RECOMMENDED_CLK
);
vt8500_port->uart.type = PORT_VT8500;
- vt8500_port->uart.iotype = UPIO_MEM;
- vt8500_port->uart.mapbase = mmres->start;
vt8500_port->uart.irq = irqres->start;
vt8500_port->uart.fifosize = 16;
vt8500_port->uart.ops = &vt8500_uart_pops;
@@ -704,6 +702,8 @@ static int vt8500_serial_probe(struct platform_device *pdev)
vt8500_port->uart.dev = &pdev->dev;
vt8500_port->uart.flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF;
+ uart_memres_set_res(&vt8500_port->uart, mmres);
+
/* Serial core uses the magic "16" everywhere - adjust for it */
vt8500_port->uart.uartclk = 16 * clk_get_rate(vt8500_port->clk) /
vt8500_port->clk_predivisor /
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index cf8ca66..895c90c 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1626,8 +1626,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
* This function also registers this device with the tty layer
* and triggers invocation of the config_port() entry point.
*/
- port->mapbase = res->start;
- port->mapsize = CDNS_UART_REGISTER_SPACE;
+ uart_memres_set_start_len(res->start, CDNS_UART_REGISTER_SPACE);
port->irq = irq;
port->dev = &pdev->dev;
port->uartclk = clk_get_rate(cdns_uart_data->uartclk);
diff --git a/drivers/tty/serial/zs.c b/drivers/tty/serial/zs.c
index ab432ba..87990b1 100644
--- a/drivers/tty/serial/zs.c
+++ b/drivers/tty/serial/zs.c
@@ -1113,10 +1113,12 @@ static int __init zs_probe_sccs(void)
uport->flags = UPF_BOOT_AUTOCONF;
uport->ops = &zs_ops;
uport->line = chip * ZS_NUM_CHAN + side;
- uport->mapsize = ZS_CHAN_IO_SIZE;
- uport->mapbase = dec_kn_slot_base +
- zs_parms.scc[chip] +
- (side ^ ZS_CHAN_B) * ZS_CHAN_IO_SIZE;
+
+ uart_memres_set_start_len(dec_kn_slot_base +
+ zs_parms.scc[chip] +
+ (side ^ ZS_CHAN_B) *
+ ZS_CHAN_IO_SIZE,
+ ZS_CHAN_IO_SIZE);
for (i = 0; i < ZS_NUM_REGS; i++)
zport->regs[i] = zs_init_regs[i];
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 5fe2b03..d891c8d 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -427,6 +427,46 @@ void uart_console_write(struct uart_port *port, const char *s,
int uart_match_port(struct uart_port *port1, struct uart_port *port2);
/*
+ * set physical io range from struct resource
+ * if resource is NULL, clear the fields
+ * also set the iotype to UPIO_MEM
+ */
+static inline void uart_memres_set_res(struct uart_port *port,
+ struct resource *res)
+{
+ if (!res) {
+ port->mapsize = 0;
+ port->mapbase = 0;
+ port->iobase = 0;
+ return;
+ }
+
+ if (resource_type(res) == IORESOURCE_IO) {
+ port->iotype = UPIO_PORT;
+ port->iobase = resource->start;
+ return;
+ }
+
+ uart->mapbase = res->start;
+ uart->mapsize = resource_size(res);
+ uart->iotype = UPIO_MEM;
+}
+
+/*
+ * set physical io range by start address and length
+ * if resource is NULL, clear the fields
+ * also set the iotype to UPIO_MEM
+ */
+static inline void uart_memres_set_start_len(struct uart_driver *uart,
+ resource_size_t start,
+ resource_size_t len)
+{
+ uart->mapbase = start;
+ uart->mapsize = len;
+ uart->iotype = UPIO_MEM;
+}
+
+/*
* Power Management
*/
int uart_suspend_port(struct uart_driver *reg, struct uart_port *port);
--
1.9.1
Fill the struct uart_port->mapsize field and use it, insteaf of
hardcoded values in many places. This makes the code layout a bit
more consistent and easily allows using generic helpers for the
io memory handling.
Candidates for such helpers could be eg. the request+ioremap and
iounmap+release combinations.
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/lpc32xx_hs.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/lpc32xx_hs.c b/drivers/tty/serial/lpc32xx_hs.c
index f4e27d0..d1f09aa 100644
--- a/drivers/tty/serial/lpc32xx_hs.c
+++ b/drivers/tty/serial/lpc32xx_hs.c
@@ -579,7 +579,7 @@ static void serial_lpc32xx_release_port(struct uart_port *port)
port->membase = NULL;
}
- release_mem_region(port->mapbase, SZ_4K);
+ release_mem_region(port->mapbase, port->mapsize);
}
}
@@ -590,12 +590,15 @@ static int serial_lpc32xx_request_port(struct uart_port *port)
if ((port->iotype == UPIO_MEM32) && (port->mapbase)) {
ret = 0;
- if (!request_mem_region(port->mapbase, SZ_4K, MODNAME))
+ if (!request_mem_region(port->mapbase,
+ port->mapsize, MODNAME))
ret = -EBUSY;
else if (port->flags & UPF_IOREMAP) {
- port->membase = ioremap(port->mapbase, SZ_4K);
+ port->membase = ioremap(port->mapbase,
+ port->mapsize);
if (!port->membase) {
- release_mem_region(port->mapbase, SZ_4K);
+ release_mem_region(port->mapbase,
+ port->mapsize);
ret = -ENOMEM;
}
}
@@ -684,6 +687,7 @@ static int serial_hs_lpc32xx_probe(struct platform_device *pdev)
return -ENXIO;
}
p->port.mapbase = res->start;
+ p->port.mapsize = SZ_4K;
p->port.membase = NULL;
ret = platform_get_irq(pdev, 0);
--
1.9.1
Using dev_err() instead of printk() for more consistent output.
(prints device name, etc).
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/zs.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/zs.c b/drivers/tty/serial/zs.c
index b03d3e4..adbfe79 100644
--- a/drivers/tty/serial/zs.c
+++ b/drivers/tty/serial/zs.c
@@ -767,7 +767,7 @@ static int zs_startup(struct uart_port *uport)
IRQF_SHARED, "scc", scc);
if (ret) {
atomic_add(-1, &scc->irq_guard);
- printk(KERN_ERR "zs: can't get irq %d\n",
+ dev_err(uport->dev, "zs: can't get irq %d\n",
zport->port.irq);
return ret;
}
@@ -995,7 +995,7 @@ static int zs_map_port(struct uart_port *uport)
uport->membase = ioremap_nocache(uport->mapbase,
ZS_CHAN_IO_SIZE);
if (!uport->membase) {
- printk(KERN_ERR "zs: Cannot map MMIO\n");
+ dev_err(port->dev, "zs: Cannot map MMIO\n");
return -ENOMEM;
}
return 0;
@@ -1006,7 +1006,7 @@ static int zs_request_port(struct uart_port *uport)
int ret;
if (!request_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE, "scc")) {
- printk(KERN_ERR "zs: Unable to reserve MMIO resource\n");
+ dev_err(uport->dev, "zs: Unable to reserve MMIO resource\n");
return -EBUSY;
}
ret = zs_map_port(uport);
--
1.9.1
Fill the struct uart_port->mapsize field and use it, insteaf of
hardcoded values in many places. This makes the code layout a bit
more consistent and easily allows using generic helpers for the
io memory handling.
Candidates for such helpers could be eg. the request+ioremap and
iounmap+release combinations.
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/pmac_zilog.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index bcb5bf7..1fef014 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -88,6 +88,8 @@
#define PMACZILOG_NAME "ttyPZ"
#endif
+#define PMZ_MAPSIZE 0x1000
+
#define pmz_debug(fmt, arg...) pr_debug("ttyPZ%d: " fmt, uap->port.line, ## arg)
#define pmz_error(fmt, arg...) pr_err("ttyPZ%d: " fmt, uap->port.line, ## arg)
#define pmz_info(fmt, arg...) pr_info("ttyPZ%d: " fmt, uap->port.line, ## arg)
@@ -1411,7 +1413,8 @@ static int __init pmz_init_port(struct uart_pmac_port *uap)
if (of_address_to_resource(np, 0, &r_ports))
return -ENODEV;
uap->port.mapbase = r_ports.start;
- uap->port.membase = ioremap(uap->port.mapbase, 0x1000);
+ uap->port.mapsize = PMZ_MAPSIZE;
+ uap->port.membase = ioremap(uap->port.mapbase, uap->port.mapsize);
uap->control_reg = uap->port.membase;
uap->data_reg = uap->control_reg + 0x10;
@@ -1709,6 +1712,7 @@ static int __init pmz_init_port(struct uart_pmac_port *uap)
return -ENODEV;
uap->port.mapbase = r_ports->start;
+ uap->port.mapsize = PMZ_MAPSIZE;
uap->port.membase = (unsigned char __iomem *) r_ports->start;
uap->port.iotype = UPIO_MEM;
uap->port.irq = irq;
--
1.9.1
Fix checkpatch warning:
WARNING: Use #include <linux/io.h> instead of <asm/io.h>
#41: FILE: drivers/tty/serial/sb1250-duart.c:41:
+#include <asm/io.h>
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/sb1250-duart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sb1250-duart.c b/drivers/tty/serial/sb1250-duart.c
index 655961c..b4342c8 100644
--- a/drivers/tty/serial/sb1250-duart.c
+++ b/drivers/tty/serial/sb1250-duart.c
@@ -38,7 +38,7 @@
#include <linux/types.h>
#include <linux/refcount.h>
-#include <asm/io.h>
+#include <linux/io.h>
#include <asm/war.h>
#include <asm/sibyte/sb1250.h>
--
1.9.1
Fill the struct uart_port->mapsize field and use it, insteaf of
hardcoded values in many places. This makes the code layout a bit
more consistent and easily allows using generic helpers for the
io memory handling.
Candidates for such helpers could be eg. the request+ioremap and
iounmap+release combinations.
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/xilinx_uartps.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 74089f5..cf8ca66 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -953,15 +953,15 @@ static int cdns_uart_verify_port(struct uart_port *port,
*/
static int cdns_uart_request_port(struct uart_port *port)
{
- if (!request_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE,
+ if (!request_mem_region(port->mapbase, port->mapsize,
CDNS_UART_NAME)) {
return -ENOMEM;
}
- port->membase = ioremap(port->mapbase, CDNS_UART_REGISTER_SPACE);
+ port->membase = ioremap(port->mapbase, port->mapsize);
if (!port->membase) {
dev_err(port->dev, "Unable to map registers\n");
- release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+ release_mem_region(port->mapbase, port->mapsize);
return -ENOMEM;
}
return 0;
@@ -976,7 +976,7 @@ static int cdns_uart_request_port(struct uart_port *port)
*/
static void cdns_uart_release_port(struct uart_port *port)
{
- release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+ release_mem_region(port->mapbase, port->mapsize);
iounmap(port->membase);
port->membase = NULL;
}
@@ -1627,6 +1627,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
* and triggers invocation of the config_port() entry point.
*/
port->mapbase = res->start;
+ port->mapsize = CDNS_UART_REGISTER_SPACE;
port->irq = irq;
port->dev = &pdev->dev;
port->uartclk = clk_get_rate(cdns_uart_data->uartclk);
--
1.9.1
Adding a mapsize field for the 8250 port platform data struct,
so we can now set the resource size (eg. *1) and don't need
funny runtime detections like serial8250_port_size() anymore.
For now, serial8250_port_size() is called everytime we need
the io resource size. That function checks which chip we
actually have and returns the appropriate size. This approach
is a bit clumpsy and not entirely easy to understand, and
it's a violation of layers :p
Obviously, that information cannot change after the driver init,
so we can safely do that probing once on driver init and just
use the stored value later.
*1) arch/mips/alchemy/common/platform.c
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/8250/8250_core.c | 1 +
include/linux/serial_8250.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e441221..71a398b 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -814,6 +814,7 @@ static int serial8250_probe(struct platform_device *dev)
uart.port.iotype = p->iotype;
uart.port.flags = p->flags;
uart.port.mapbase = p->mapbase;
+ uart.port.mapsize = p->mapsize;
uart.port.hub6 = p->hub6;
uart.port.private_data = p->private_data;
uart.port.type = p->type;
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 5a655ba..8b8183a 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -22,6 +22,7 @@ struct plat_serial8250_port {
unsigned long iobase; /* io base address */
void __iomem *membase; /* ioremap cookie or NULL */
resource_size_t mapbase; /* resource base */
+ resource_size_t mapsize; /* resource size */
unsigned int irq; /* interrupt number */
unsigned long irqflags; /* request_irq flags */
unsigned int uartclk; /* UART clock rate */
--
1.9.1
Fill the struct uart_port->mapsize field and use it, insteaf of
hardcoded values in many places. This makes the code layout a bit
more consistent and easily allows using generic helpers for the
io memory handling.
Candidates for such helpers could be eg. the request+ioremap and
iounmap+release combinations.
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/zs.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/zs.c b/drivers/tty/serial/zs.c
index adbfe79..ab432ba 100644
--- a/drivers/tty/serial/zs.c
+++ b/drivers/tty/serial/zs.c
@@ -986,14 +986,14 @@ static void zs_release_port(struct uart_port *uport)
{
iounmap(uport->membase);
uport->membase = 0;
- release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
+ release_mem_region(uport->mapbase, uport->mapsize);
}
static int zs_map_port(struct uart_port *uport)
{
if (!uport->membase)
uport->membase = ioremap_nocache(uport->mapbase,
- ZS_CHAN_IO_SIZE);
+ uport->mapsize);
if (!uport->membase) {
dev_err(port->dev, "zs: Cannot map MMIO\n");
return -ENOMEM;
@@ -1005,13 +1005,13 @@ static int zs_request_port(struct uart_port *uport)
{
int ret;
- if (!request_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE, "scc")) {
+ if (!request_mem_region(uport->mapbase, uport->mapsize, "scc")) {
dev_err(uport->dev, "zs: Unable to reserve MMIO resource\n");
return -EBUSY;
}
ret = zs_map_port(uport);
if (ret) {
- release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
+ release_mem_region(uport->mapbase, uport->mapsize);
return ret;
}
return 0;
@@ -1113,6 +1113,7 @@ static int __init zs_probe_sccs(void)
uport->flags = UPF_BOOT_AUTOCONF;
uport->ops = &zs_ops;
uport->line = chip * ZS_NUM_CHAN + side;
+ uport->mapsize = ZS_CHAN_IO_SIZE;
uport->mapbase = dec_kn_slot_base +
zs_parms.scc[chip] +
(side ^ ZS_CHAN_B) * ZS_CHAN_IO_SIZE;
--
1.9.1
Fix checkpatch errors:
ERROR: else should follow close brace '}'
#121: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c:121:
+ }
+ else
WARNING: line over 80 characters
#150: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c:150:
+ pinfo->tx_fifosize), (void __force *)pinfo->mem_addr,
WARNING: Block comments should align the * on each line
#66: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:66:
+ * Check, if transmit buffers are processed
+*/
WARNING: braces {} are not necessary for any arm of this statement
#170: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:170:
+ if (IS_SMC(pinfo)) {
[...]
+ } else {
[...]
WARNING: labels should not be indented
#292: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:292:
+ error_return:
ERROR: code indent should use tabs where possible
#299: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:299:
+^I^I BD_SC_OV | BD_SC_ID);$
WARNING: labels should not be indented
#319: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:319:
+ handle_error:
WARNING: line over 80 characters
#423: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:423:
+ setbits32(&pinfo->sccp->scc_gsmrl, (SCC_GSMRL_ENR | SCC_GSMRL_ENT));
ERROR: space required before the open parenthesis '('
#451: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:451:
+ while(!cpm_uart_tx_empty(port)) {
WARNING: Missing a blank line after declarations
#462: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:462:
+ smc_t __iomem *smcp = pinfo->smcp;
+ clrbits16(&smcp->smc_smcmr, SMCMR_REN | SMCMR_TEN);
WARNING: line over 80 characters
#466: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:466:
+ clrbits32(&sccp->scc_gsmrl, SCC_GSMRL_ENR | SCC_GSMRL_ENT);
WARNING: Missing a blank line after declarations
#466: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:466:
+ scc_t __iomem *sccp = pinfo->sccp;
+ clrbits32(&sccp->scc_gsmrl, SCC_GSMRL_ENR | SCC_GSMRL_ENT);
ERROR: code indent should use tabs where possible
#484: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:484:
+ struct ktermios *termios,$
WARNING: please, no spaces at the start of a line
#484: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:484:
+ struct ktermios *termios,$
ERROR: code indent should use tabs where possible
#485: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:485:
+ struct ktermios *old)$
WARNING: please, no spaces at the start of a line
#485: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:485:
+ struct ktermios *old)$
WARNING: line over 80 characters
#624: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:624:
+ /* Output in *one* operation, so we don't interrupt RX/TX if they
WARNING: Block comments use a trailing */ on a separate line
#625: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:625:
+ * were already enabled. */
WARNING: line over 80 characters
#629: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:629:
+ out_be16(&pinfo->sccup->scc_genscc.scc_mrblr, pinfo->rx_fifosize);
WARNING: line over 80 characters
#773: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:773:
+ mem_addr = pinfo->mem_addr + L1_CACHE_ALIGN(pinfo->rx_nrfifos * pinfo->rx_fifosize);
ERROR: code indent should use tabs where possible
#797: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:797:
+^I (u8 __iomem *)pinfo->rx_bd_base - DPRAM_BASE);$
ERROR: code indent should use tabs where possible
#799: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:799:
+^I (u8 __iomem *)pinfo->tx_bd_base - DPRAM_BASE);$
ERROR: code indent should use tabs where possible
#836: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:836:
+^I SCC_GSMRL_MODE_UART | SCC_GSMRL_TDCR_16 | SCC_GSMRL_RDCR_16);$
ERROR: code indent should use tabs where possible
#859: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:859:
+^I (u8 __iomem *)pinfo->rx_bd_base - DPRAM_BASE);$
ERROR: code indent should use tabs where possible
#861: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:861:
+^I (u8 __iomem *)pinfo->tx_bd_base - DPRAM_BASE);$
WARNING: space prohibited between function name and open parenthesis '('
#866: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:866:
+#if defined (CONFIG_I2C_SPI_SMC1_UCODE_PATCH)
WARNING: line over 80 characters
#921: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:921:
+ clrbits32(&pinfo->sccp->scc_gsmrl, SCC_GSMRL_ENR | SCC_GSMRL_ENT);
WARNING: Missing a blank line after declarations
#462: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:462:
+ smc_t __iomem *smcp = pinfo->smcp;
+ clrbits16(&smcp->smc_smcmr,
WARNING: Missing a blank line after declarations
#467: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:467:
+ scc_t __iomem *sccp = pinfo->sccp;
+ clrbits32(&sccp->scc_gsmrl,
ERROR: code indent should use tabs where possible
#1151: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1151:
+ struct uart_cpm_port *pinfo)$
WARNING: please, no spaces at the start of a line
#1151: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1151:
+ struct uart_cpm_port *pinfo)$
ERROR: "(foo*)" should be "(foo *)"
#1161: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1161:
+ struct clk *clk = clk_get(NULL, (const char*)data);
WARNING: Missing a blank line after declarations
#1162: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1162:
+ struct clk *clk = clk_get(NULL, (const char*)data);
+ if (!IS_ERR(clk))
ERROR: code indent should use tabs where possible
#1169: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1169:
+^I^I^I "fsl,cpm-brg property.\n", np);$
ERROR: code indent should use tabs where possible
#1178: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1178:
+^I^I "fsl,cpm-command property.\n", np);$
ERROR: code indent should use tabs where possible
#1192: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1192:
WARNING: braces {} are not necessary for any arm of this statement
#1279: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1279:
+ if (unlikely(nolock)) {
[...]
+ } else {
[...]
WARNING: braces {} are not necessary for any arm of this statement
#1287: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1287:
+ if (unlikely(nolock)) {
[...]
+ } else {
[...]
WARNING: line over 80 characters
#1354: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1354:
+ clrbits32(&pinfo->sccp->scc_gsmrl, SCC_GSMRL_ENR | SCC_GSMRL_ENT);
ERROR: Macros with complex values should be enclosed in parentheses
#1394: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1394:
+#define CPM_UART_CONSOLE &cpm_scc_uart_console
WARNING: Missing a blank line after declarations
#1437: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1437:
+ struct uart_cpm_port *pinfo = platform_get_drvdata(ofdev);
+ return uart_remove_one_port(&cpm_reg, &pinfo->port);
WARNING: please, no spaces at the start of a line
#1464: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1464:
+ };$
WARNING: Missing a blank line after declarations
#1469: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1469:
+ int ret = uart_register_driver(&cpm_reg);
+ if (ret)
WARNING: Missing a blank line after declarations
#1062: FILE: drivers/tty/serial/cpm_uart/cpm_uart_core.c:1062:
+ int i;
+ volatile cbd_t *bdp;
ERROR: "foo * bar" should be "foo *bar"
#19: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm1.h:19:
+static inline void cpm_set_scc_fcr(scc_uart_t __iomem * sup)
ERROR: "foo * bar" should be "foo *bar"
#25: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm1.h:25:
+static inline void cpm_set_smc_fcr(smc_uart_t __iomem * up)
WARNING: Improper SPDX comment style for 'drivers/tty/serial/cpm_uart/cpm_uart.h', please use '/*' instead
#1: FILE: drivers/tty/serial/cpm_uart/cpm_uart.h:1:
+// SPDX-License-Identifier: GPL-2.0
WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#1: FILE: drivers/tty/serial/cpm_uart/cpm_uart.h:1:
+// SPDX-License-Identifier: GPL-2.0
WARNING: Block comments use * on subsequent lines
#106: FILE: drivers/tty/serial/cpm_uart/cpm_uart.h:106:
+/*
+ virtual to phys transtalion
ERROR: code indent should use tabs where possible
#109: FILE: drivers/tty/serial/cpm_uart/cpm_uart.h:109:
+ struct uart_cpm_port *pinfo)$
WARNING: please, no spaces at the start of a line
#109: FILE: drivers/tty/serial/cpm_uart/cpm_uart.h:109:
+ struct uart_cpm_port *pinfo)$
ERROR: code indent should use tabs where possible
#125: FILE: drivers/tty/serial/cpm_uart/cpm_uart.h:125:
+ struct uart_cpm_port *pinfo)$
WARNING: please, no spaces at the start of a line
#125: FILE: drivers/tty/serial/cpm_uart/cpm_uart.h:125:
+ struct uart_cpm_port *pinfo)$
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/cpm_uart/cpm_uart.h | 10 +--
drivers/tty/serial/cpm_uart/cpm_uart_core.c | 95 ++++++++++++++++-------------
drivers/tty/serial/cpm_uart/cpm_uart_cpm1.h | 4 +-
drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c | 6 +-
4 files changed, 64 insertions(+), 51 deletions(-)
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart.h b/drivers/tty/serial/cpm_uart/cpm_uart.h
index 9f175a9..e7e225f 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart.h
+++ b/drivers/tty/serial/cpm_uart/cpm_uart.h
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* Driver for CPM (SCC/SMC) serial ports
*
@@ -103,10 +103,10 @@ void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port,
void scc4_lineif(struct uart_cpm_port *pinfo);
/*
- virtual to phys transtalion
-*/
+ * virtual to phys transtalion
+ */
static inline unsigned long cpu2cpm_addr(void *addr,
- struct uart_cpm_port *pinfo)
+ struct uart_cpm_port *pinfo)
{
int offset;
u32 val = (u32)addr;
@@ -122,7 +122,7 @@ static inline unsigned long cpu2cpm_addr(void *addr,
}
static inline void *cpm2cpu_addr(unsigned long addr,
- struct uart_cpm_port *pinfo)
+ struct uart_cpm_port *pinfo)
{
int offset;
u32 val = addr;
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_core.c b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
index c831d31..4d6cea9 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
@@ -63,7 +63,7 @@
/*
* Check, if transmit buffers are processed
-*/
+ */
static unsigned int cpm_uart_tx_empty(struct uart_port *port)
{
struct uart_cpm_port *pinfo =
@@ -167,11 +167,10 @@ static void cpm_uart_start_tx(struct uart_port *port)
}
if (cpm_uart_tx_pump(port) != 0) {
- if (IS_SMC(pinfo)) {
+ if (IS_SMC(pinfo))
setbits8(&smcp->smc_smcm, SMCM_TX);
- } else {
+ else
setbits16(&sccp->scc_sccm, UART_SCCM_TX);
- }
}
}
@@ -289,14 +288,14 @@ static void cpm_uart_int_rx(struct uart_port *port)
return;
}
#endif
- error_return:
+error_return:
tty_insert_flip_char(tport, ch, flg);
} /* End while (i--) */
/* This BD is ready to be used again. Clear status. get next */
clrbits16(&bdp->cbd_sc, BD_SC_BR | BD_SC_FR | BD_SC_PR |
- BD_SC_OV | BD_SC_ID);
+ BD_SC_OV | BD_SC_ID);
setbits16(&bdp->cbd_sc, BD_SC_EMPTY);
if (in_be16(&bdp->cbd_sc) & BD_SC_WRAP)
@@ -316,7 +315,7 @@ static void cpm_uart_int_rx(struct uart_port *port)
/* Error processing */
- handle_error:
+handle_error:
/* Statistics */
if (status & BD_SC_BR)
port->icount.brk++;
@@ -420,7 +419,8 @@ static int cpm_uart_startup(struct uart_port *port)
setbits16(&pinfo->smcp->smc_smcmr, (SMCMR_REN | SMCMR_TEN));
} else {
setbits16(&pinfo->sccp->scc_sccm, UART_SCCM_RX);
- setbits32(&pinfo->sccp->scc_gsmrl, (SCC_GSMRL_ENR | SCC_GSMRL_ENT));
+ setbits32(&pinfo->sccp->scc_gsmrl,
+ (SCC_GSMRL_ENR | SCC_GSMRL_ENT));
}
return 0;
@@ -448,7 +448,7 @@ static void cpm_uart_shutdown(struct uart_port *port)
/* If the port is not the console, disable Rx and Tx. */
if (!(pinfo->flags & FLAG_CONSOLE)) {
/* Wait for all the BDs marked sent */
- while(!cpm_uart_tx_empty(port)) {
+ while (!cpm_uart_tx_empty(port)) {
set_current_state(TASK_UNINTERRUPTIBLE);
schedule_timeout(2);
}
@@ -459,12 +459,17 @@ static void cpm_uart_shutdown(struct uart_port *port)
/* Stop uarts */
if (IS_SMC(pinfo)) {
smc_t __iomem *smcp = pinfo->smcp;
- clrbits16(&smcp->smc_smcmr, SMCMR_REN | SMCMR_TEN);
+
+ clrbits16(&smcp->smc_smcmr,
+ SMCMR_REN | SMCMR_TEN);
clrbits8(&smcp->smc_smcm, SMCM_RX | SMCM_TX);
} else {
scc_t __iomem *sccp = pinfo->sccp;
- clrbits32(&sccp->scc_gsmrl, SCC_GSMRL_ENR | SCC_GSMRL_ENT);
- clrbits16(&sccp->scc_sccm, UART_SCCM_TX | UART_SCCM_RX);
+
+ clrbits32(&sccp->scc_gsmrl,
+ SCC_GSMRL_ENR | SCC_GSMRL_ENT);
+ clrbits16(&sccp->scc_sccm,
+ UART_SCCM_TX | UART_SCCM_RX);
}
/* Shut them really down and reinit buffer descriptors */
@@ -481,8 +486,8 @@ static void cpm_uart_shutdown(struct uart_port *port)
}
static void cpm_uart_set_termios(struct uart_port *port,
- struct ktermios *termios,
- struct ktermios *old)
+ struct ktermios *termios,
+ struct ktermios *old)
{
int baud;
unsigned long flags;
@@ -621,12 +626,14 @@ static void cpm_uart_set_termios(struct uart_port *port,
* present.
*/
prev_mode = in_be16(&smcp->smc_smcmr) & (SMCMR_REN | SMCMR_TEN);
- /* Output in *one* operation, so we don't interrupt RX/TX if they
- * were already enabled. */
+ /* Output in *one* operation, so we don't interrupt RX/TX if
+ * they were already enabled.
+ */
out_be16(&smcp->smc_smcmr, smcr_mk_clen(bits) | cval |
SMCMR_SM_UART | prev_mode);
} else {
- out_be16(&pinfo->sccup->scc_genscc.scc_mrblr, pinfo->rx_fifosize);
+ out_be16(&pinfo->sccup->scc_genscc.scc_mrblr,
+ pinfo->rx_fifosize);
out_be16(&pinfo->sccup->scc_maxidl, maxidl);
out_be16(&sccp->scc_psmr, (sbits << 12) | scval);
}
@@ -770,7 +777,8 @@ static void cpm_uart_initbd(struct uart_cpm_port *pinfo)
* buffers in the buffer descriptors, and the
* virtual address for us to work with.
*/
- mem_addr = pinfo->mem_addr + L1_CACHE_ALIGN(pinfo->rx_nrfifos * pinfo->rx_fifosize);
+ mem_addr = pinfo->mem_addr +
+ L1_CACHE_ALIGN(pinfo->rx_nrfifos * pinfo->rx_fifosize);
bdp = pinfo->tx_cur = pinfo->tx_bd_base;
for (i = 0; i < (pinfo->tx_nrfifos - 1); i++, bdp++) {
out_be32(&bdp->cbd_bufaddr, cpu2cpm_addr(mem_addr, pinfo));
@@ -794,9 +802,9 @@ static void cpm_uart_init_scc(struct uart_cpm_port *pinfo)
/* Store address */
out_be16(&pinfo->sccup->scc_genscc.scc_rbase,
- (u8 __iomem *)pinfo->rx_bd_base - DPRAM_BASE);
+ (u8 __iomem *)pinfo->rx_bd_base - DPRAM_BASE);
out_be16(&pinfo->sccup->scc_genscc.scc_tbase,
- (u8 __iomem *)pinfo->tx_bd_base - DPRAM_BASE);
+ (u8 __iomem *)pinfo->tx_bd_base - DPRAM_BASE);
/* Set up the uart parameters in the
* parameter ram.
@@ -833,7 +841,7 @@ static void cpm_uart_init_scc(struct uart_cpm_port *pinfo)
*/
out_be32(&scp->scc_gsmrh, 0);
out_be32(&scp->scc_gsmrl,
- SCC_GSMRL_MODE_UART | SCC_GSMRL_TDCR_16 | SCC_GSMRL_RDCR_16);
+ SCC_GSMRL_MODE_UART | SCC_GSMRL_TDCR_16 | SCC_GSMRL_RDCR_16);
/* Enable rx interrupts and clear all pending events. */
out_be16(&scp->scc_sccm, 0);
@@ -856,14 +864,14 @@ static void cpm_uart_init_smc(struct uart_cpm_port *pinfo)
/* Store address */
out_be16(&pinfo->smcup->smc_rbase,
- (u8 __iomem *)pinfo->rx_bd_base - DPRAM_BASE);
+ (u8 __iomem *)pinfo->rx_bd_base - DPRAM_BASE);
out_be16(&pinfo->smcup->smc_tbase,
- (u8 __iomem *)pinfo->tx_bd_base - DPRAM_BASE);
+ (u8 __iomem *)pinfo->tx_bd_base - DPRAM_BASE);
/*
* In case SMC1 is being relocated...
*/
-#if defined (CONFIG_I2C_SPI_SMC1_UCODE_PATCH)
+#if defined(CONFIG_I2C_SPI_SMC1_UCODE_PATCH)
out_be16(&up->smc_rbptr, in_be16(&pinfo->smcup->smc_rbase));
out_be16(&up->smc_tbptr, in_be16(&pinfo->smcup->smc_tbase));
out_be32(&up->smc_rstate, 0);
@@ -917,8 +925,10 @@ static int cpm_uart_request_port(struct uart_port *port)
clrbits8(&pinfo->smcp->smc_smcm, SMCM_RX | SMCM_TX);
clrbits16(&pinfo->smcp->smc_smcmr, SMCMR_REN | SMCMR_TEN);
} else {
- clrbits16(&pinfo->sccp->scc_sccm, UART_SCCM_TX | UART_SCCM_RX);
- clrbits32(&pinfo->sccp->scc_gsmrl, SCC_GSMRL_ENR | SCC_GSMRL_ENT);
+ clrbits16(&pinfo->sccp->scc_sccm,
+ UART_SCCM_TX | UART_SCCM_RX);
+ clrbits32(&pinfo->sccp->scc_gsmrl,
+ SCC_GSMRL_ENR | SCC_GSMRL_ENT);
}
ret = cpm_uart_allocbuf(pinfo, 0);
@@ -1048,9 +1058,10 @@ static void cpm_uart_early_write(struct uart_cpm_port *pinfo,
static int poll_wait_key(char *obuf, struct uart_cpm_port *pinfo)
{
u_char c, *cp;
- volatile cbd_t *bdp;
int i;
+ volatile cbd_t *bdp;
+
/* Get the address of the host memory buffer.
*/
bdp = pinfo->rx_cur;
@@ -1138,7 +1149,7 @@ static void cpm_put_poll_char(struct uart_port *port,
struct uart_cpm_port cpm_uart_ports[UART_NR];
static int cpm_uart_init_port(struct device_node *np,
- struct uart_cpm_port *pinfo)
+ struct uart_cpm_port *pinfo)
{
const u32 *data;
void __iomem *mem, *pram;
@@ -1148,7 +1159,8 @@ static int cpm_uart_init_port(struct device_node *np,
data = of_get_property(np, "clock", NULL);
if (data) {
- struct clk *clk = clk_get(NULL, (const char*)data);
+ struct clk *clk = clk_get(NULL, (const char *)data);
+
if (!IS_ERR(clk))
pinfo->clk = clk;
}
@@ -1156,7 +1168,7 @@ static int cpm_uart_init_port(struct device_node *np,
data = of_get_property(np, "fsl,cpm-brg", &len);
if (!data || len != 4) {
dev_err(port->dev, "CPM UART %pOFn has no/invalid "
- "fsl,cpm-brg property.\n", np);
+ "fsl,cpm-brg property.\n", np);
return -EINVAL;
}
pinfo->brg = *data;
@@ -1165,7 +1177,7 @@ static int cpm_uart_init_port(struct device_node *np,
data = of_get_property(np, "fsl,cpm-command", &len);
if (!data || len != 4) {
dev_err(port->dev, "CPM UART %pOFn has no/invalid "
- "fsl,cpm-command property.\n", np);
+ "fsl,cpm-command property.\n", np);
return -EINVAL;
}
pinfo->command = *data;
@@ -1179,7 +1191,7 @@ static int cpm_uart_init_port(struct device_node *np,
pinfo->sccp = mem;
pinfo->sccup = pram = cpm_uart_map_pram(pinfo, np);
} else if (of_device_is_compatible(np, "fsl,cpm1-smc-uart") ||
- of_device_is_compatible(np, "fsl,cpm2-smc-uart")) {
+ of_device_is_compatible(np, "fsl,cpm2-smc-uart")) {
pinfo->flags |= FLAG_SMC;
pinfo->smcp = mem;
pinfo->smcup = pram = cpm_uart_map_pram(pinfo, np);
@@ -1266,19 +1278,17 @@ static void cpm_uart_console_write(struct console *co, const char *s,
unsigned long flags;
int nolock = oops_in_progress;
- if (unlikely(nolock)) {
+ if (unlikely(nolock))
local_irq_save(flags);
- } else {
+ else
spin_lock_irqsave(&pinfo->port.lock, flags);
- }
cpm_uart_early_write(pinfo, s, count, true);
- if (unlikely(nolock)) {
+ if (unlikely(nolock))
local_irq_restore(flags);
- } else {
+ else
spin_unlock_irqrestore(&pinfo->port.lock, flags);
- }
}
@@ -1341,7 +1351,8 @@ static int __init cpm_uart_console_setup(struct console *co, char *options)
out_be16(&pinfo->sccup->scc_brkcr, 0);
cpm_line_cr_cmd(pinfo, CPM_CR_GRA_STOP_TX);
clrbits16(&pinfo->sccp->scc_sccm, UART_SCCM_TX | UART_SCCM_RX);
- clrbits32(&pinfo->sccp->scc_gsmrl, SCC_GSMRL_ENR | SCC_GSMRL_ENT);
+ clrbits32(&pinfo->sccp->scc_gsmrl,
+ SCC_GSMRL_ENR | SCC_GSMRL_ENT);
}
ret = cpm_uart_allocbuf(pinfo, 1);
@@ -1381,7 +1392,7 @@ static int __init cpm_uart_console_init(void)
console_initcall(cpm_uart_console_init);
-#define CPM_UART_CONSOLE &cpm_scc_uart_console
+#define CPM_UART_CONSOLE (&cpm_scc_uart_console)
#else
#define CPM_UART_CONSOLE NULL
#endif
@@ -1424,6 +1435,7 @@ static int cpm_uart_probe(struct platform_device *ofdev)
static int cpm_uart_remove(struct platform_device *ofdev)
{
struct uart_cpm_port *pinfo = platform_get_drvdata(ofdev);
+
return uart_remove_one_port(&cpm_reg, &pinfo->port);
}
@@ -1451,11 +1463,12 @@ static int cpm_uart_remove(struct platform_device *ofdev)
},
.probe = cpm_uart_probe,
.remove = cpm_uart_remove,
- };
+};
static int __init cpm_uart_init(void)
{
int ret = uart_register_driver(&cpm_reg);
+
if (ret)
return ret;
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.h b/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.h
index 18ec084..eafff96 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.h
+++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.h
@@ -16,13 +16,13 @@ static inline void cpm_set_brg(int brg, int baud)
cpm_setbrg(brg, baud);
}
-static inline void cpm_set_scc_fcr(scc_uart_t __iomem * sup)
+static inline void cpm_set_scc_fcr(scc_uart_t __iomem *sup)
{
out_8(&sup->scc_genscc.scc_rfcr, SMC_EB);
out_8(&sup->scc_genscc.scc_tfcr, SMC_EB);
}
-static inline void cpm_set_smc_fcr(smc_uart_t __iomem * up)
+static inline void cpm_set_smc_fcr(smc_uart_t __iomem *up)
{
out_8(&up->smc_rfcr, SMC_EB);
out_8(&up->smc_tfcr, SMC_EB);
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
index a0fccda..154ac19 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
+++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
@@ -117,8 +117,7 @@ int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con)
if (is_con) {
mem_addr = kzalloc(memsz, GFP_NOWAIT);
dma_addr = virt_to_bus(mem_addr);
- }
- else
+ } else
mem_addr = dma_alloc_coherent(pinfo->port.dev, memsz, &dma_addr,
GFP_KERNEL);
@@ -148,7 +147,8 @@ void cpm_uart_freebuf(struct uart_cpm_port *pinfo)
dma_free_coherent(pinfo->port.dev, L1_CACHE_ALIGN(pinfo->rx_nrfifos *
pinfo->rx_fifosize) +
L1_CACHE_ALIGN(pinfo->tx_nrfifos *
- pinfo->tx_fifosize), (void __force *)pinfo->mem_addr,
+ pinfo->tx_fifosize),
+ (void __force *)pinfo->mem_addr,
pinfo->dma_addr);
cpm_dpfree(pinfo->dp_addr);
--
1.9.1
Using dev_warn() instead of printk() for more consistent output.
(prints device name, etc).
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/ioc4_serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/ioc4_serial.c b/drivers/tty/serial/ioc4_serial.c
index db5b979..21c1b8f 100644
--- a/drivers/tty/serial/ioc4_serial.c
+++ b/drivers/tty/serial/ioc4_serial.c
@@ -2752,7 +2752,7 @@ static int ioc4_serial_remove_one(struct ioc4_driver_data *idd)
the_port->dev = &pdev->dev;
spin_lock_init(&the_port->lock);
if (uart_add_one_port(u_driver, the_port) < 0) {
- printk(KERN_WARNING
+ dev_warn(&pdev->dev,
"%s: unable to add port %d bus %d\n",
__func__, the_port->line, pdev->bus->number);
} else {
--
1.9.1
Instead of hardcoding raw numbers, add define's for the mmio address/size.
Also fill the mapsize field and use it on mem request/release calls, for
more consistency and allowing generic helpers to be used later.
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/21285.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c
index 32b3acf..90684cd 100644
--- a/drivers/tty/serial/21285.c
+++ b/drivers/tty/serial/21285.c
@@ -27,6 +27,9 @@
#define SERIAL_21285_MAJOR 204
#define SERIAL_21285_MINOR 4
+#define SERIAL_21285_ADDRESS 0x42000160
+#define SERIAL_21285_SIZE 32
+
#define RXSTAT_DUMMY_READ 0x80000000
#define RXSTAT_FRAME (1 << 0)
#define RXSTAT_PARITY (1 << 1)
@@ -305,12 +308,14 @@ static const char *serial21285_type(struct uart_port *port)
static void serial21285_release_port(struct uart_port *port)
{
- release_mem_region(port->mapbase, 32);
+ release_mem_region(port->mapbase, port->mapsize);
}
static int serial21285_request_port(struct uart_port *port)
{
- return request_mem_region(port->mapbase, 32, serial21285_name)
+ return request_mem_region(port->mapbase,
+ port->mapsize,
+ serial21285_name)
!= NULL ? 0 : -EBUSY;
}
@@ -354,7 +359,8 @@ static int serial21285_verify_port(struct uart_port *port, struct serial_struct
};
static struct uart_port serial21285_port = {
- .mapbase = 0x42000160,
+ .mapbase = SERIAL_21285_BASE,
+ .mapsize = SERIAL_21285_SIZE,
.iotype = UPIO_MEM,
.irq = 0,
.fifosize = 16,
--
1.9.1
Fix checkpatch warnings:
WARNING: Missing a blank line after declarations
#43: FILE: drivers/tty/serial/timbuart.c:43:
+ u32 ier = ioread32(port->membase + TIMBUART_IER) & ~RXFLAGS;
+ iowrite32(ier, port->membase + TIMBUART_IER);
WARNING: Missing a blank line after declarations
#50: FILE: drivers/tty/serial/timbuart.c:50:
+ u32 ier = ioread32(port->membase + TIMBUART_IER) & ~TXBAE;
+ iowrite32(ier, port->membase + TIMBUART_IER);
WARNING: Missing a blank line after declarations
#86: FILE: drivers/tty/serial/timbuart.c:86:
+ u8 ch = ioread8(port->membase + TIMBUART_RXFIFO);
+ port->icount.rx++;
WARNING: Missing a blank line after declarations
#202: FILE: drivers/tty/serial/timbuart.c:202:
+ u8 cts = ioread8(port->membase + TIMBUART_CTRL);
+ dev_dbg(port->dev, "%s - cts %x\n", __func__, cts);
WARNING: Block comments use * on subsequent lines
#296: FILE: drivers/tty/serial/timbuart.c:296:
+ /* The serial layer calls into this once with old = NULL when setting
+ up initially */
WARNING: Block comments use a trailing */ on a separate line
#296: FILE: drivers/tty/serial/timbuart.c:296:
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/timbuart.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/timbuart.c b/drivers/tty/serial/timbuart.c
index dcce936..d80c332 100644
--- a/drivers/tty/serial/timbuart.c
+++ b/drivers/tty/serial/timbuart.c
@@ -40,6 +40,7 @@ static void timbuart_stop_rx(struct uart_port *port)
{
/* spin lock held by upper layer, disable all RX interrupts */
u32 ier = ioread32(port->membase + TIMBUART_IER) & ~RXFLAGS;
+
iowrite32(ier, port->membase + TIMBUART_IER);
}
@@ -47,6 +48,7 @@ static void timbuart_stop_tx(struct uart_port *port)
{
/* spinlock held by upper layer, disable TX interrupt */
u32 ier = ioread32(port->membase + TIMBUART_IER) & ~TXBAE;
+
iowrite32(ier, port->membase + TIMBUART_IER);
}
@@ -83,6 +85,7 @@ static void timbuart_rx_chars(struct uart_port *port)
while (ioread32(port->membase + TIMBUART_ISR) & RXDP) {
u8 ch = ioread8(port->membase + TIMBUART_RXFIFO);
+
port->icount.rx++;
tty_insert_flip_char(tport, ch, TTY_NORMAL);
}
@@ -199,6 +202,7 @@ static void timbuart_tasklet(unsigned long arg)
static unsigned int timbuart_get_mctrl(struct uart_port *port)
{
u8 cts = ioread8(port->membase + TIMBUART_CTRL);
+
dev_dbg(port->dev, "%s - cts %x\n", __func__, cts);
if (cts & TIMBUART_CTRL_CTS)
@@ -293,7 +297,8 @@ static void timbuart_set_termios(struct uart_port *port,
baud = baudrates[bindex];
/* The serial layer calls into this once with old = NULL when setting
- up initially */
+ * up initially
+ */
if (old)
tty_termios_copy_hw(termios, old);
tty_termios_encode_baud_rate(termios, baud, baud);
@@ -500,4 +505,3 @@ static int timbuart_remove(struct platform_device *dev)
MODULE_DESCRIPTION("Timberdale UART driver");
MODULE_LICENSE("GPL v2");
MODULE_ALIAS("platform:timb-uart");
-
--
1.9.1
Fix checkpatch warnings by using pr_err():
WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
#109: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c:109:
+ printk(KERN_ERR
WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
#128: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c:128:
+ printk(KERN_ERR
WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
+ printk(KERN_ERR
WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
+ printk(KERN_ERR
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/cpm_uart/cpm_uart_cpm1.c | 6 ++----
drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c | 6 ++----
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.c
index 56fc527..aed61e9 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.c
+++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.c
@@ -71,8 +71,7 @@ int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con)
dpmemsz = sizeof(cbd_t) * (pinfo->rx_nrfifos + pinfo->tx_nrfifos);
dp_offset = cpm_dpalloc(dpmemsz, 8);
if (IS_ERR_VALUE(dp_offset)) {
- printk(KERN_ERR
- "cpm_uart_cpm1.c: could not allocate buffer descriptors\n");
+ pr_err("cpm_uart_cpm1.c: could not allocate buffer descriptors\n");
return -ENOMEM;
}
dp_mem = cpm_dpram_addr(dp_offset);
@@ -90,8 +89,7 @@ int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con)
if (mem_addr == NULL) {
cpm_dpfree(dp_offset);
- printk(KERN_ERR
- "cpm_uart_cpm1.c: could not allocate coherent memory\n");
+ pr_err("cpm_uart_cpm1.c: could not allocate coherent memory\n");
return -ENOMEM;
}
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
index 40cfcf4..a0fccda 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
+++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
@@ -106,8 +106,7 @@ int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con)
dpmemsz = sizeof(cbd_t) * (pinfo->rx_nrfifos + pinfo->tx_nrfifos);
dp_offset = cpm_dpalloc(dpmemsz, 8);
if (IS_ERR_VALUE(dp_offset)) {
- printk(KERN_ERR
- "cpm_uart_cpm.c: could not allocate buffer descriptors\n");
+ pr_err("cpm_uart_cpm.c: could not allocate buffer descriptors\n");
return -ENOMEM;
}
@@ -125,8 +124,7 @@ int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con)
if (mem_addr == NULL) {
cpm_dpfree(dp_offset);
- printk(KERN_ERR
- "cpm_uart_cpm.c: could not allocate coherent memory\n");
+ pr_err("cpm_uart_cpm.c: could not allocate coherent memory\n");
return -ENOMEM;
}
--
1.9.1
Fix checkpatch warnings:
WARNING: Prefer using '"%s...", __func__' to using 'apbuart_console_setup', this function's name, in a string
#491: FILE: drivers/tty/serial/apbuart.c:491:
+ pr_debug("apbuart_console_setup co=%p, co->index=%i, options=%s\n",
WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#661: FILE: drivers/tty/serial/apbuart.c:661:
+ printk(KERN_INFO "Serial: GRLIB APBUART driver\n");
WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
#666: FILE: drivers/tty/serial/apbuart.c:666:
+ printk(KERN_ERR "%s: uart_register_driver failed (%i)\n",
WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
#673: FILE: drivers/tty/serial/apbuart.c:673:
+ printk(KERN_ERR
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/apbuart.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/apbuart.c b/drivers/tty/serial/apbuart.c
index 60cd133..d2b86f7 100644
--- a/drivers/tty/serial/apbuart.c
+++ b/drivers/tty/serial/apbuart.c
@@ -482,8 +482,8 @@ static int __init apbuart_console_setup(struct console *co, char *options)
int parity = 'n';
int flow = 'n';
- pr_debug("apbuart_console_setup co=%p, co->index=%i, options=%s\n",
- co, co->index, options);
+ pr_debug("%s() co=%p, co->index=%i, options=%s\n",
+ __func__, co, co->index, options);
/*
* Check whether an invalid uart number has been specified, and
@@ -650,21 +650,20 @@ static int __init grlib_apbuart_init(void)
if (ret)
return ret;
- printk(KERN_INFO "Serial: GRLIB APBUART driver\n");
+ pr_info("Serial: GRLIB APBUART driver\n");
ret = uart_register_driver(&grlib_apbuart_driver);
if (ret) {
- printk(KERN_ERR "%s: uart_register_driver failed (%i)\n",
- __FILE__, ret);
+ pr_err("%s: uart_register_driver failed (%i)\n",
+ __func__, ret);
return ret;
}
ret = platform_driver_register(&grlib_apbuart_of_driver);
if (ret) {
- printk(KERN_ERR
- "%s: platform_driver_register failed (%i)\n",
- __FILE__, ret);
+ pr_err("%s: platform_driver_register failed (%i)\n",
+ __func__, ret);
uart_unregister_driver(&grlib_apbuart_driver);
return ret;
}
--
1.9.1
Using dev_err() instead of printk() for more consistent output.
(prints device name, etc).
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/timbuart.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/serial/timbuart.c b/drivers/tty/serial/timbuart.c
index 19d38b5..dcce936 100644
--- a/drivers/tty/serial/timbuart.c
+++ b/drivers/tty/serial/timbuart.c
@@ -470,8 +470,7 @@ static int timbuart_probe(struct platform_device *dev)
err_register:
kfree(uart);
err_mem:
- printk(KERN_ERR "timberdale: Failed to register Timberdale UART: %d\n",
- err);
+ dev_err(&dev->dev, "Failed to register Timberdale UART: %d\n", err);
return err;
}
--
1.9.1
Fix checkpatch warnings:
WARNING: line over 80 characters
#9: FILE: drivers/tty/serial/apbuart.c:9:
+ * Copyright (C) 2006 Daniel Hellstrom <[email protected]>, Aeroflex Gaisler AB
WARNING: line over 80 characters
#11: FILE: drivers/tty/serial/apbuart.c:11:
+ * Copyright (C) 2009 Kristoffer Glembo <[email protected]>, Aeroflex Gaisler AB
WARNING: line over 80 characters
#16: FILE: drivers/tty/serial/apbuart.c:16:
+#if defined(CONFIG_SERIAL_GRLIB_GAISLER_APBUART_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
WARNING: labels should not be indented
#122: FILE: drivers/tty/serial/apbuart.c:122:
+ ignore_char:
WARNING: Missing a blank line after declarations
#186: FILE: drivers/tty/serial/apbuart.c:186:
+ unsigned int status = UART_GET_STATUS(port);
+ return status & UART_STATUS_THE ? TIOCSER_TEMT : 0;
WARNING: Missing a blank line after declarations
#322: FILE: drivers/tty/serial/apbuart.c:322:
+ int ret = 0;
+ if (ser->type != PORT_UNKNOWN && ser->type != PORT_APBUART)
WARNING: Missing a blank line after declarations
#427: FILE: drivers/tty/serial/apbuart.c:427:
+ unsigned int status;
+ do {
WARNING: Missing a blank line after declarations
#463: FILE: drivers/tty/serial/apbuart.c:463:
+ unsigned int quot, status;
+ status = UART_GET_STATUS(port);
WARNING: line over 80 characters
#627: FILE: drivers/tty/serial/apbuart.c:627:
+ port->membase = ioremap(addr, sizeof(struct grlib_apbuart_regs_map));
WARNING: line over 80 characters
#634: FILE: drivers/tty/serial/apbuart.c:634:
+ port->fifosize = apbuart_scan_fifo_size((struct uart_port *) port, line);
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/apbuart.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/apbuart.c b/drivers/tty/serial/apbuart.c
index 89e19b6..515a562 100644
--- a/drivers/tty/serial/apbuart.c
+++ b/drivers/tty/serial/apbuart.c
@@ -6,12 +6,15 @@
*
* Copyright (C) 2000 Deep Blue Solutions Ltd.
* Copyright (C) 2003 Konrad Eisele <[email protected]>
- * Copyright (C) 2006 Daniel Hellstrom <[email protected]>, Aeroflex Gaisler AB
+ * Copyright (C) 2006 Daniel Hellstrom <[email protected]>,
+ * Aeroflex Gaisler AB
* Copyright (C) 2008 Gilead Kutnick <[email protected]>
- * Copyright (C) 2009 Kristoffer Glembo <[email protected]>, Aeroflex Gaisler AB
+ * Copyright (C) 2009 Kristoffer Glembo <[email protected]>,
+ * Aeroflex Gaisler AB
*/
-#if defined(CONFIG_SERIAL_GRLIB_GAISLER_APBUART_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
+#if defined(CONFIG_SERIAL_GRLIB_GAISLER_APBUART_CONSOLE) \
+ && defined(CONFIG_MAGIC_SYSRQ)
#define SUPPORT_SYSRQ
#endif
@@ -116,8 +119,7 @@ static void apbuart_rx_chars(struct uart_port *port)
uart_insert_char(port, rsr, UART_STATUS_OE, ch, flag);
-
- ignore_char:
+ignore_char:
status = UART_GET_STATUS(port);
}
@@ -181,6 +183,7 @@ static irqreturn_t apbuart_int(int irq, void *dev_id)
static unsigned int apbuart_tx_empty(struct uart_port *port)
{
unsigned int status = UART_GET_STATUS(port);
+
return status & UART_STATUS_THE ? TIOCSER_TEMT : 0;
}
@@ -317,6 +320,7 @@ static int apbuart_verify_port(struct uart_port *port,
struct serial_struct *ser)
{
int ret = 0;
+
if (ser->type != PORT_UNKNOWN && ser->type != PORT_APBUART)
ret = -EINVAL;
if (ser->irq < 0 || ser->irq >= NR_IRQS)
@@ -422,6 +426,7 @@ static void apbuart_flush_fifo(struct uart_port *port)
static void apbuart_console_putchar(struct uart_port *port, int ch)
{
unsigned int status;
+
do {
status = UART_GET_STATUS(port);
} while (!UART_TX_READY(status));
@@ -458,6 +463,7 @@ static void apbuart_console_putchar(struct uart_port *port, int ch)
if (UART_GET_CTRL(port) & (UART_CTRL_RE | UART_CTRL_TE)) {
unsigned int quot, status;
+
status = UART_GET_STATUS(port);
*parity = 'n';
@@ -622,14 +628,16 @@ static int __init grlib_apbuart_configure(void)
port = &grlib_apbuart_ports[line];
port->mapbase = addr;
- port->membase = ioremap(addr, sizeof(struct grlib_apbuart_regs_map));
+ port->membase = ioremap(addr,
+ sizeof(struct grlib_apbuart_regs_map));
port->irq = 0;
port->iotype = UPIO_MEM;
port->ops = &grlib_apbuart_ops;
port->flags = UPF_BOOT_AUTOCONF;
port->line = line;
port->uartclk = *freq_hz;
- port->fifosize = apbuart_scan_fifo_size((struct uart_port *) port, line);
+ port->fifosize = apbuart_scan_fifo_size(
+ (struct uart_port *) port, line);
line++;
/* We support maximum UART_NR uarts ... */
--
1.9.1
Fix checkpatch warnings:
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#562: FILE: drivers/tty/serial/uartlite.c:562:
+ unsigned retries = 1000000;
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#574: FILE: drivers/tty/serial/uartlite.c:574:
+ const char *s, unsigned n)
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/uartlite.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 4c28600..6f79353 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -559,7 +559,7 @@ static void early_uartlite_putc(struct uart_port *port, int c)
* we'll never timeout on a working UART.
*/
- unsigned retries = 1000000;
+ unsigned int retries = 1000000;
/* read status bit - 0x8 offset */
while (--retries && (readl(port->membase + 8) & (1 << 3)))
;
@@ -571,7 +571,7 @@ static void early_uartlite_putc(struct uart_port *port, int c)
}
static void early_uartlite_write(struct console *console,
- const char *s, unsigned n)
+ const char *s, unsigned int n)
{
struct earlycon_device *device = console->data;
uart_console_write(&device->port, s, n, early_uartlite_putc);
--
1.9.1
Fix checkpatch warning:
WARNING: Use #include <linux/io.h> instead of <asm/io.h>
#38: FILE: drivers/tty/serial/sunzilog.c:38:
+#include <asm/io.h>
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/sunzilog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sunzilog.c b/drivers/tty/serial/sunzilog.c
index 17b0520..85edb0d 100644
--- a/drivers/tty/serial/sunzilog.c
+++ b/drivers/tty/serial/sunzilog.c
@@ -34,8 +34,8 @@
#endif
#include <linux/init.h>
#include <linux/of_device.h>
+#include <linux/io.h>
-#include <asm/io.h>
#include <asm/irq.h>
#include <asm/prom.h>
#include <asm/setup.h>
--
1.9.1
Using dev_err() instead of printk() for more consistent output.
(prints device name, etc).
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/apbuart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/apbuart.c b/drivers/tty/serial/apbuart.c
index d2b86f7..89e19b6 100644
--- a/drivers/tty/serial/apbuart.c
+++ b/drivers/tty/serial/apbuart.c
@@ -568,7 +568,7 @@ static int apbuart_probe(struct platform_device *op)
apbuart_flush_fifo((struct uart_port *) port);
- printk(KERN_INFO "grlib-apbuart at 0x%llx, irq %d\n",
+ dev_info(&pdev->pdev, "grlib-apbuart at 0x%llx, irq %d\n",
(unsigned long long) port->mapbase, port->irq);
return 0;
}
--
1.9.1
checkpatch complaints:
WARNING: printk() should include KERN_<LEVEL> facility level
#698: FILE: drivers/tty/serial/sb1250-duart.c:698:
+ printk(err);
WARNING: printk() should include KERN_<LEVEL> facility level
#706: FILE: drivers/tty/serial/sb1250-duart.c:706:
+ printk(err);
Even though it's a false alarm here (the string is already prefixed
w/ KERN_ERR), it's nicer to use pr_err() here, which also makes
checkpatch happy.
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/sb1250-duart.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/sb1250-duart.c b/drivers/tty/serial/sb1250-duart.c
index b4342c8..227af87 100644
--- a/drivers/tty/serial/sb1250-duart.c
+++ b/drivers/tty/serial/sb1250-duart.c
@@ -689,13 +689,13 @@ static int sbd_map_port(struct uart_port *uport)
static int sbd_request_port(struct uart_port *uport)
{
- const char *err = KERN_ERR "sbd: Unable to reserve MMIO resource\n";
+ const char *err = "sbd: Unable to reserve MMIO resource\n";
struct sbd_duart *duart = to_sport(uport)->duart;
int ret = 0;
if (!request_mem_region(uport->mapbase, DUART_CHANREG_SPACING,
"sb1250-duart")) {
- printk(err);
+ pr_err(err);
return -EBUSY;
}
refcount_inc(&duart->map_guard);
@@ -703,7 +703,7 @@ static int sbd_request_port(struct uart_port *uport)
if (!request_mem_region(duart->mapctrl, DUART_CHANREG_SPACING,
"sb1250-duart")) {
refcount_dec(&duart->map_guard);
- printk(err);
+ pr_err(err);
ret = -EBUSY;
}
}
--
1.9.1
Using dev_err()/dev_warn() instead of printk() for more consistent output.
(prints device name, etc).
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/cpm_uart/cpm_uart_core.c | 6 +++---
drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_core.c b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
index b929c7a..374b8bb 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
@@ -265,7 +265,7 @@ static void cpm_uart_int_rx(struct uart_port *port)
* later, which will be the next rx-interrupt or a timeout
*/
if (tty_buffer_request_room(tport, i) < i) {
- printk(KERN_WARNING "No room in flip buffer\n");
+ dev_warn(port->dev, "No room in flip buffer\n");
return;
}
@@ -1155,7 +1155,7 @@ static int cpm_uart_init_port(struct device_node *np,
if (!pinfo->clk) {
data = of_get_property(np, "fsl,cpm-brg", &len);
if (!data || len != 4) {
- printk(KERN_ERR "CPM UART %pOFn has no/invalid "
+ dev_err(port->dev, "CPM UART %pOFn has no/invalid "
"fsl,cpm-brg property.\n", np);
return -EINVAL;
}
@@ -1164,7 +1164,7 @@ static int cpm_uart_init_port(struct device_node *np,
data = of_get_property(np, "fsl,cpm-command", &len);
if (!data || len != 4) {
- printk(KERN_ERR "CPM UART %pOFn has no/invalid "
+ dev_err(port->dev, "CPM UART %pOFn has no/invalid "
"fsl,cpm-command property.\n", np);
return -EINVAL;
}
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
index 6a1cd03..ef1ae08 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
+++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
@@ -67,7 +67,7 @@ void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port,
return pram;
if (len != 2) {
- printk(KERN_WARNING "cpm_uart[%d]: device tree references "
+ dev_warn(port->dev, "cpm_uart[%d]: device tree references "
"SMC pram, using boot loader/wrapper pram mapping. "
"Please fix your device tree to reference the pram "
"base register instead.\n",
--
1.9.1
fixing checkpatch warning:
WARNING: Use #include <linux/io.h> instead of <asm/io.h>
#55: FILE: dz.c:55:
+#include <asm/io.h>
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/dz.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index 96e35af..fd4f0cc 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -52,7 +52,7 @@
#include <linux/atomic.h>
#include <asm/bootinfo.h>
-#include <asm/io.h>
+#include <linux/io.h>
#include <asm/dec/interrupts.h>
#include <asm/dec/kn01.h>
--
1.9.1
Fill the struct uart_port->mapsize field and use it, insteaf of
hardcoded values in many places. This makes the code layout a bit
more consistent and easily allows using generic helpers for the
io memory handling.
Candidates for such helpers could be eg. the request+ioremap and
iounmap+release combinations.
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/uartlite.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 44d65bd..c322ab6 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -342,7 +342,7 @@ static const char *ulite_type(struct uart_port *port)
static void ulite_release_port(struct uart_port *port)
{
- release_mem_region(port->mapbase, ULITE_REGION);
+ release_mem_region(port->mapbase, port->mapsize);
iounmap(port->membase);
port->membase = NULL;
}
@@ -356,15 +356,15 @@ static int ulite_request_port(struct uart_port *port)
"ulite console: port=%p; port->mapbase=%llx\n",
port, (unsigned long long) port->mapbase);
- if (!request_mem_region(port->mapbase, ULITE_REGION, "uartlite")) {
+ if (!request_mem_region(port->mapbase, port->mapsize, "uartlite")) {
dev_err(port->dev, "Memory region busy\n");
return -EBUSY;
}
- port->membase = ioremap(port->mapbase, ULITE_REGION);
+ port->membase = ioremap(port->mapbase, port->mapsize);
if (!port->membase) {
dev_err(port->dev, "Unable to map registers\n");
- release_mem_region(port->mapbase, ULITE_REGION);
+ release_mem_region(port->mapbase, port->mapsize);
return -EBUSY;
}
@@ -649,6 +649,7 @@ static int ulite_assign(struct device *dev, int id, u32 base, int irq,
port->iotype = UPIO_MEM;
port->iobase = 1; /* mark port in use */
port->mapbase = base;
+ port->mapsize = ULITE_REGION;
port->membase = NULL;
port->ops = &ulite_ops;
port->irq = irq;
--
1.9.1
Fixing checkpatch warning:
WARNING: Use #include <linux/io.h> instead of <asm/io.h>
#25: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c:25:
+#include <asm/io.h>
WARNING: Use #include <linux/io.h> instead of <asm/io.h>
+#include <asm/io.h>
WARNING: Use #include <linux/delay.h> instead of <asm/delay.h>
+#include <asm/delay.h>
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/cpm_uart/cpm_uart_core.c | 4 ++--
drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_core.c b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
index 374b8bb..c831d31 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
@@ -33,10 +33,10 @@
#include <linux/gpio.h>
#include <linux/of_gpio.h>
#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/delay.h>
-#include <asm/io.h>
#include <asm/irq.h>
-#include <asm/delay.h>
#include <asm/fs_pd.h>
#include <asm/udbg.h>
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
index ef1ae08..40cfcf4 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
+++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
@@ -21,8 +21,8 @@
#include <linux/device.h>
#include <linux/memblock.h>
#include <linux/dma-mapping.h>
+#include <linux/io.h>
-#include <asm/io.h>
#include <asm/irq.h>
#include <asm/fs_pd.h>
#include <asm/prom.h>
--
1.9.1
Using dev_dbg() instead of pr_debg() for more consistent output.
(prints device name, etc).
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/uartlite.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index b8b912b..44d65bd 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -352,7 +352,8 @@ static int ulite_request_port(struct uart_port *port)
struct uartlite_data *pdata = port->private_data;
int ret;
- pr_debug("ulite console: port=%p; port->mapbase=%llx\n",
+ dev_dbg(port->dev,
+ "ulite console: port=%p; port->mapbase=%llx\n",
port, (unsigned long long) port->mapbase);
if (!request_mem_region(port->mapbase, ULITE_REGION, "uartlite")) {
@@ -519,7 +520,8 @@ static int ulite_console_setup(struct console *co, char *options)
/* Has the device been initialized yet? */
if (!port->mapbase) {
- pr_debug("console on ttyUL%i not present\n", co->index);
+ dev_dbg(port->dev, "console on ttyUL%i not present\n",
+ co->index);
return -ENODEV;
}
--
1.9.1
Fix the checkpatch warning:
WARNING: printk() should include KERN_<LEVEL> facility level
#934: FILE: dz.c:934:
+ printk("%s%s\n", dz_name, dz_version);
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/dz.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index 559d076..e2670c4 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -931,7 +931,7 @@ static int __init dz_init(void)
if (IOASIC)
return -ENXIO;
- printk("%s%s\n", dz_name, dz_version);
+ pr_info("%s%s\n", dz_name, dz_version);
dz_init_ports();
--
1.9.1
Fix checkpatch warning:
ERROR: Macros with complex values should be enclosed in parentheses
#911: FILE: drivers/tty/serial/sb1250-duart.c:911:
+#define SERIAL_SB1250_DUART_CONSOLE &sbd_console
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/sb1250-duart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sb1250-duart.c b/drivers/tty/serial/sb1250-duart.c
index 1184226..ec74f09 100644
--- a/drivers/tty/serial/sb1250-duart.c
+++ b/drivers/tty/serial/sb1250-duart.c
@@ -908,7 +908,7 @@ static int __init sbd_serial_console_init(void)
console_initcall(sbd_serial_console_init);
-#define SERIAL_SB1250_DUART_CONSOLE &sbd_console
+#define SERIAL_SB1250_DUART_CONSOLE (&sbd_console)
#else
#define SERIAL_SB1250_DUART_CONSOLE NULL
#endif /* CONFIG_SERIAL_SB1250_DUART_CONSOLE */
--
1.9.1
Fill the struct uart_port->mapsize field and use it, insteaf of
hardcoded values in many places. This makes the code layout a bit
more consistent and easily allows using generic helpers for the
io memory handling.
Candidates for such helpers could be eg. the request+ioremap and
iounmap+release combinations.
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/sb1250-duart.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/sb1250-duart.c b/drivers/tty/serial/sb1250-duart.c
index 227af87..1184226 100644
--- a/drivers/tty/serial/sb1250-duart.c
+++ b/drivers/tty/serial/sb1250-duart.c
@@ -658,7 +658,7 @@ static void sbd_release_port(struct uart_port *uport)
if(refcount_dec_and_test(&duart->map_guard))
release_mem_region(duart->mapctrl, DUART_CHANREG_SPACING);
- release_mem_region(uport->mapbase, DUART_CHANREG_SPACING);
+ release_mem_region(uport->mapbase, uport->mapsize);
}
static int sbd_map_port(struct uart_port *uport)
@@ -668,7 +668,7 @@ static int sbd_map_port(struct uart_port *uport)
if (!uport->membase)
uport->membase = ioremap_nocache(uport->mapbase,
- DUART_CHANREG_SPACING);
+ uport->mapsize);
if (!uport->membase) {
dev_err(uport->dev, "Cannot map MMIO (base)\n");
return -ENOMEM;
@@ -693,7 +693,7 @@ static int sbd_request_port(struct uart_port *uport)
struct sbd_duart *duart = to_sport(uport)->duart;
int ret = 0;
- if (!request_mem_region(uport->mapbase, DUART_CHANREG_SPACING,
+ if (!request_mem_region(uport->mapbase, uport->mapsize,
"sb1250-duart")) {
pr_err(err);
return -EBUSY;
@@ -716,7 +716,7 @@ static int sbd_request_port(struct uart_port *uport)
}
}
if (ret) {
- release_mem_region(uport->mapbase, DUART_CHANREG_SPACING);
+ release_mem_region(uport->mapbase, uport->mapsize);
return ret;
}
return 0;
@@ -812,6 +812,7 @@ static void __init sbd_probe_duarts(void)
uport->ops = &sbd_ops;
uport->line = line;
uport->mapbase = SBD_CHANREGS(line);
+ uport->mapsize = DUART_CHANREG_SPACING;
}
}
}
--
1.9.1
checkpatch complains:
ERROR: space required before the open parenthesis '('
#659: FILE: drivers/tty/serial/sb1250-duart.c:659:
+ if(refcount_dec_and_test(&duart->map_guard))
Just add this missing space to make checkpatch happy.
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/sb1250-duart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sb1250-duart.c b/drivers/tty/serial/sb1250-duart.c
index ec74f09..0023ed0 100644
--- a/drivers/tty/serial/sb1250-duart.c
+++ b/drivers/tty/serial/sb1250-duart.c
@@ -656,7 +656,7 @@ static void sbd_release_port(struct uart_port *uport)
iounmap(uport->membase);
uport->membase = NULL;
- if(refcount_dec_and_test(&duart->map_guard))
+ if (refcount_dec_and_test(&duart->map_guard))
release_mem_region(duart->mapctrl, DUART_CHANREG_SPACING);
release_mem_region(uport->mapbase, uport->mapsize);
}
--
1.9.1
checkpatch complains:
WARNING: braces {} are not necessary for any arm of this statement
#489: FILE: drivers/tty/serial/uartlite.c:489:
+ if (oops_in_progress) {
[...]
+ } else
[...]
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/uartlite.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index c322ab6..4c28600 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -486,9 +486,9 @@ static void ulite_console_write(struct console *co, const char *s,
unsigned int ier;
int locked = 1;
- if (oops_in_progress) {
+ if (oops_in_progress)
locked = spin_trylock_irqsave(&port->lock, flags);
- } else
+ else
spin_lock_irqsave(&port->lock, flags);
/* save and disable interrupt */
--
1.9.1
Fix checkpatch warnings:
WARNING: line over 80 characters
#283: FILE: drivers/tty/serial/uartlite.c:283:
+ ret = request_irq(port->irq, ulite_isr, IRQF_SHARED | IRQF_TRIGGER_RISING,
WARNING: Missing a blank line after declarations
#577: FILE: drivers/tty/serial/uartlite.c:577:
+ struct earlycon_device *device = console->data;
+ uart_console_write(&device->port, s, n, early_uartlite_putc);
WARNING: line over 80 characters
#590: FILE: drivers/tty/serial/uartlite.c:590:
+OF_EARLYCON_DECLARE(uartlite_b, "xlnx,opb-uartlite-1.00.b", early_uartlite_setup);
WARNING: line over 80 characters
#591: FILE: drivers/tty/serial/uartlite.c:591:
+OF_EARLYCON_DECLARE(uartlite_a, "xlnx,xps-uartlite-1.00.a", early_uartlite_setup);
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/uartlite.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index 6f79353..0140cec 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -280,7 +280,8 @@ static int ulite_startup(struct uart_port *port)
return ret;
}
- ret = request_irq(port->irq, ulite_isr, IRQF_SHARED | IRQF_TRIGGER_RISING,
+ ret = request_irq(port->irq, ulite_isr,
+ IRQF_SHARED | IRQF_TRIGGER_RISING,
"uartlite", port);
if (ret)
return ret;
@@ -574,6 +575,7 @@ static void early_uartlite_write(struct console *console,
const char *s, unsigned int n)
{
struct earlycon_device *device = console->data;
+
uart_console_write(&device->port, s, n, early_uartlite_putc);
}
@@ -587,8 +589,10 @@ static int __init early_uartlite_setup(struct earlycon_device *device,
return 0;
}
EARLYCON_DECLARE(uartlite, early_uartlite_setup);
-OF_EARLYCON_DECLARE(uartlite_b, "xlnx,opb-uartlite-1.00.b", early_uartlite_setup);
-OF_EARLYCON_DECLARE(uartlite_a, "xlnx,xps-uartlite-1.00.a", early_uartlite_setup);
+OF_EARLYCON_DECLARE(uartlite_b, "xlnx,opb-uartlite-1.00.b",
+ early_uartlite_setup);
+OF_EARLYCON_DECLARE(uartlite_a, "xlnx,xps-uartlite-1.00.a",
+ early_uartlite_setup);
#endif /* CONFIG_SERIAL_UARTLITE_CONSOLE */
--
1.9.1
On 4/27/19 2:52 PM, Enrico Weigelt, metux IT consult wrote:
> Simpily io resource size computation by setting mapsize field.
^^^^
Here's a typo
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Using dev_err() instead of printk() for more consistent output.
(prints device name, etc).
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/sb1250-duart.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/sb1250-duart.c b/drivers/tty/serial/sb1250-duart.c
index 329aced..655961c 100644
--- a/drivers/tty/serial/sb1250-duart.c
+++ b/drivers/tty/serial/sb1250-duart.c
@@ -663,7 +663,6 @@ static void sbd_release_port(struct uart_port *uport)
static int sbd_map_port(struct uart_port *uport)
{
- const char *err = KERN_ERR "sbd: Cannot map MMIO\n";
struct sbd_port *sport = to_sport(uport);
struct sbd_duart *duart = sport->duart;
@@ -671,7 +670,7 @@ static int sbd_map_port(struct uart_port *uport)
uport->membase = ioremap_nocache(uport->mapbase,
DUART_CHANREG_SPACING);
if (!uport->membase) {
- printk(err);
+ dev_err(uport->dev, "Cannot map MMIO (base)\n");
return -ENOMEM;
}
@@ -679,7 +678,7 @@ static int sbd_map_port(struct uart_port *uport)
sport->memctrl = ioremap_nocache(duart->mapctrl,
DUART_CHANREG_SPACING);
if (!sport->memctrl) {
- printk(err);
+ dev_err(uport->dev, "Cannot map MMIO (ctrl)\n");
iounmap(uport->membase);
uport->membase = NULL;
return -ENOMEM;
--
1.9.1
Fix checkpatch warning:
WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1238: FILE: drivers/tty/serial/sunzilog.c:1238:
+ printk(KERN_INFO "Console: ttyS%d (SunZilog zs%d)\n",
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/sunzilog.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/sunzilog.c b/drivers/tty/serial/sunzilog.c
index 85edb0d..dba723c 100644
--- a/drivers/tty/serial/sunzilog.c
+++ b/drivers/tty/serial/sunzilog.c
@@ -1235,7 +1235,7 @@ static int __init sunzilog_console_setup(struct console *con, char *options)
if (up->port.type != PORT_SUNZILOG)
return -1;
- printk(KERN_INFO "Console: ttyS%d (SunZilog zs%d)\n",
+ pr_info("Console: ttyS%d (SunZilog zs%d)\n",
(sunzilog_reg.minor - 64) + con->index, con->index);
/* Get firmware console settings. */
@@ -1615,9 +1615,8 @@ static int __init sunzilog_init(void)
while (up) {
struct zilog_channel __iomem *channel;
- /* printk(KERN_INFO
- * "Enable IRQ for ZILOG Hardware %p\n",
- * up);
+ /* pr_info("Enable IRQ for ZILOG Hardware %p\n",
+ * up);
*/
channel = ZILOG_CHANNEL_FROM_PORT(&up->port);
up->flags |= SUNZILOG_FLAG_ISR_HANDLER;
@@ -1655,9 +1654,8 @@ static void __exit sunzilog_exit(void)
while (up) {
struct zilog_channel __iomem *channel;
- /* printk(KERN_INFO
- * "Disable IRQ for ZILOG Hardware %p\n",
- * up);
+ /* pr_info("Disable IRQ for ZILOG Hardware %p\n",
+ * up);
*/
channel = ZILOG_CHANNEL_FROM_PORT(&up->port);
up->flags &= ~SUNZILOG_FLAG_ISR_HANDLER;
--
1.9.1
Fix checkpatch warnings:
WARNING: Use #include <linux/io.h> instead of <asm/io.h>
#38: FILE: drivers/tty/serial/sunzilog.c:38:
+#include <asm/io.h>
WARNING: line over 80 characters
#109: FILE: drivers/tty/serial/sunzilog.c:109:
+#define ZILOG_CHANNEL_FROM_PORT(PORT) ((struct zilog_channel __iomem *)((PORT)->membase))
WARNING: line over 80 characters
#116: FILE: drivers/tty/serial/sunzilog.c:116:
+#define ZS_WANTS_MODEM_STATUS(UP) ((UP)->flags & SUNZILOG_FLAG_MODEM_STATUS)
WARNING: line over 80 characters
#179: FILE: drivers/tty/serial/sunzilog.c:179:
+static int __load_zsregs(struct zilog_channel __iomem *channel, unsigned char *regs)
WARNING: Missing a blank line after declarations
#188: FILE: drivers/tty/serial/sunzilog.c:188:
+ unsigned char stat = read_zsreg(channel, R1);
+ if (stat & ALL_SNT)
ERROR: trailing whitespace
#231: FILE: drivers/tty/serial/sunzilog.c:231:
+^I$
WARNING: braces {} are not necessary for any arm of this statement
#276: FILE: drivers/tty/serial/sunzilog.c:276:
+ if (ZS_TX_ACTIVE(up)) {
[...]
+ } else {
[...]
ERROR: else should follow close brace '}'
#378: FILE: drivers/tty/serial/sunzilog.c:378:
+ }
+ else if (r1 & PAR_ERR)
ERROR: code indent should use tabs where possible
#397: FILE: drivers/tty/serial/sunzilog.c:397:
+^I^I ^Itty_insert_flip_char(port, ch, flag);$
WARNING: please, no space before tabs
#397: FILE: drivers/tty/serial/sunzilog.c:397:
+^I^I ^Itty_insert_flip_char(port, ch, flag);$
WARNING: line over 80 characters
#440: FILE: drivers/tty/serial/sunzilog.c:440:
+ /* The Zilog just gives us an interrupt when DCD/CTS/etc. change.
WARNING: line over 80 characters
#441: FILE: drivers/tty/serial/sunzilog.c:441:
+ * But it does not tell us which bit has changed, we have to keep
WARNING: Missing a blank line after declarations
#464: FILE: drivers/tty/serial/sunzilog.c:464:
+ unsigned char status = readb(&channel->control);
+ ZSDELAY();
WARNING: line over 80 characters
#468: FILE: drivers/tty/serial/sunzilog.c:468:
+ * It can occur because of how we do serial console writes. It would
WARNING: line over 80 characters
#469: FILE: drivers/tty/serial/sunzilog.c:469:
+ * be nice to transmit console writes just like we normally would for
WARNING: line over 80 characters
#470: FILE: drivers/tty/serial/sunzilog.c:470:
+ * a TTY line. (ie. buffered and TX interrupt driven). That is not
WARNING: line over 80 characters
#471: FILE: drivers/tty/serial/sunzilog.c:471:
+ * easy because console writes cannot sleep. One solution might be
WARNING: line over 80 characters
#593: FILE: drivers/tty/serial/sunzilog.c:593:
+static __inline__ unsigned char sunzilog_read_channel_status(struct uart_port *port)
WARNING: plain inline is preferred over __inline__
#593: FILE: drivers/tty/serial/sunzilog.c:593:
+static __inline__ unsigned char sunzilog_read_channel_status(struct uart_port *port)
ERROR: trailing whitespace
#664: FILE: drivers/tty/serial/sunzilog.c:664:
+^I/* NOTE: Not subject to 'transmitter active' rule. */ $
ERROR: trailing whitespace
#752: FILE: drivers/tty/serial/sunzilog.c:752:
+^I^I/* NOTE: Not subject to 'transmitter active' rule. */ $
ERROR: trailing whitespace
#779: FILE: drivers/tty/serial/sunzilog.c:779:
+^I^I/* NOTE: Not subject to 'transmitter active' rule. */ $
WARNING: line over 80 characters
#999: FILE: drivers/tty/serial/sunzilog.c:999:
+static int sunzilog_verify_port(struct uart_port *port, struct serial_struct *ser)
WARNING: Missing a blank line after declarations
#1142: FILE: drivers/tty/serial/sunzilog.c:1142:
+ unsigned char val = readb(&channel->control);
+ if (val & Tx_BUF_EMP) {
WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#1230: FILE: drivers/tty/serial/sunzilog.c:1230:
+ printk(KERN_INFO "Console: ttyS%d (SunZilog zs%d)\n",
WARNING: braces {} are not necessary for single statement blocks
#1383: FILE: drivers/tty/serial/sunzilog.c:1383:
+ if (__load_zsregs(channel, up->curregs)) {
+ up->flags |= SUNZILOG_FLAG_ESCC;
+ }
WARNING: quoted string split across lines
#1493: FILE: drivers/tty/serial/sunzilog.c:1493:
+ dev_info(&op->dev, "Keyboard at MMIO 0x%llx (irq = %d) "
+ "is a %s\n",
WARNING: quoted string split across lines
#1497: FILE: drivers/tty/serial/sunzilog.c:1497:
+ dev_info(&op->dev, "Mouse at MMIO 0x%llx (irq = %d) "
+ "is a %s\n",
WARNING: Missing a blank line after declarations
#1581: FILE: drivers/tty/serial/sunzilog.c:1581:
+ struct uart_sunzilog_port *up = sunzilog_irq_chain;
+ err = request_irq(zilog_irq, sunzilog_interrupt, IRQF_SHARED,
WARNING: line over 80 characters
#1590: FILE: drivers/tty/serial/sunzilog.c:1590:
+ /* printk (KERN_INFO "Enable IRQ for ZILOG Hardware %p\n", up); */
WARNING: line over 80 characters
#1627: FILE: drivers/tty/serial/sunzilog.c:1627:
+ /* printk (KERN_INFO "Disable IRQ for ZILOG Hardware %p\n", up); */
ERROR: trailing statements should be on next line
#1248: FILE: drivers/tty/serial/sunzilog.c:1248:
+ case B150: baud = 150; break;
ERROR: trailing statements should be on next line
#1249: FILE: drivers/tty/serial/sunzilog.c:1249:
+ case B300: baud = 300; break;
ERROR: trailing statements should be on next line
#1250: FILE: drivers/tty/serial/sunzilog.c:1250:
+ case B600: baud = 600; break;
ERROR: trailing statements should be on next line
#1251: FILE: drivers/tty/serial/sunzilog.c:1251:
+ case B1200: baud = 1200; break;
ERROR: trailing statements should be on next line
#1252: FILE: drivers/tty/serial/sunzilog.c:1252:
+ case B2400: baud = 2400; break;
ERROR: trailing statements should be on next line
#1253: FILE: drivers/tty/serial/sunzilog.c:1253:
+ case B4800: baud = 4800; break;
ERROR: trailing statements should be on next line
#1254: FILE: drivers/tty/serial/sunzilog.c:1254:
+ default: case B9600: baud = 9600; break;
ERROR: trailing statements should be on next line
#1255: FILE: drivers/tty/serial/sunzilog.c:1255:
+ case B19200: baud = 19200; break;
ERROR: trailing statements should be on next line
#1256: FILE: drivers/tty/serial/sunzilog.c:1256:
+ case B38400: baud = 38400; break;
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/sunzilog.c | 118 +++++++++++++++++++++++++++---------------
1 file changed, 76 insertions(+), 42 deletions(-)
diff --git a/drivers/tty/serial/sunzilog.c b/drivers/tty/serial/sunzilog.c
index 6285bba..17b0520 100644
--- a/drivers/tty/serial/sunzilog.c
+++ b/drivers/tty/serial/sunzilog.c
@@ -106,14 +106,17 @@ struct uart_sunzilog_port {
static void sunzilog_putchar(struct uart_port *port, int ch);
-#define ZILOG_CHANNEL_FROM_PORT(PORT) ((struct zilog_channel __iomem *)((PORT)->membase))
-#define UART_ZILOG(PORT) ((struct uart_sunzilog_port *)(PORT))
+#define ZILOG_CHANNEL_FROM_PORT(PORT) \
+ ((struct zilog_channel __iomem *)((PORT)->membase))
+#define UART_ZILOG(PORT) \
+ ((struct uart_sunzilog_port *)(PORT))
#define ZS_IS_KEYB(UP) ((UP)->flags & SUNZILOG_FLAG_CONS_KEYB)
#define ZS_IS_MOUSE(UP) ((UP)->flags & SUNZILOG_FLAG_CONS_MOUSE)
#define ZS_IS_CONS(UP) ((UP)->flags & SUNZILOG_FLAG_IS_CONS)
#define ZS_IS_KGDB(UP) ((UP)->flags & SUNZILOG_FLAG_IS_KGDB)
-#define ZS_WANTS_MODEM_STATUS(UP) ((UP)->flags & SUNZILOG_FLAG_MODEM_STATUS)
+#define ZS_WANTS_MODEM_STATUS(UP) \
+ ((UP)->flags & SUNZILOG_FLAG_MODEM_STATUS)
#define ZS_IS_CHANNEL_A(UP) ((UP)->flags & SUNZILOG_FLAG_IS_CHANNEL_A)
#define ZS_REGS_HELD(UP) ((UP)->flags & SUNZILOG_FLAG_REGS_HELD)
#define ZS_TX_STOPPED(UP) ((UP)->flags & SUNZILOG_FLAG_TX_STOPPED)
@@ -176,7 +179,8 @@ static void sunzilog_clear_fifo(struct zilog_channel __iomem *channel)
/* This function must only be called when the TX is not busy. The UART
* port lock must be held and local interrupts disabled.
*/
-static int __load_zsregs(struct zilog_channel __iomem *channel, unsigned char *regs)
+static int __load_zsregs(struct zilog_channel __iomem *channel,
+ unsigned char *regs)
{
int i;
int escc;
@@ -185,6 +189,7 @@ static int __load_zsregs(struct zilog_channel __iomem *channel, unsigned char *r
/* Let pending transmits finish. */
for (i = 0; i < 1000; i++) {
unsigned char stat = read_zsreg(channel, R1);
+
if (stat & ALL_SNT)
break;
udelay(100);
@@ -228,7 +233,7 @@ static int __load_zsregs(struct zilog_channel __iomem *channel, unsigned char *r
/* Lower and upper byte of baud rate generator divisor. */
write_zsreg(channel, R12, regs[R12]);
write_zsreg(channel, R13, regs[R13]);
-
+
/* Now rewrite R14, with BRENAB (if set). */
write_zsreg(channel, R14, regs[R14]);
@@ -273,11 +278,10 @@ static void sunzilog_maybe_update_regs(struct uart_sunzilog_port *up,
struct zilog_channel __iomem *channel)
{
if (!ZS_REGS_HELD(up)) {
- if (ZS_TX_ACTIVE(up)) {
+ if (ZS_TX_ACTIVE(up))
up->flags |= SUNZILOG_FLAG_REGS_HELD;
- } else {
+ else
__load_zsregs(channel, up->curregs);
- }
}
}
@@ -374,8 +378,7 @@ static void sunzilog_kbdms_receive_chars(struct uart_sunzilog_port *up,
up->port.icount.brk++;
if (uart_handle_break(&up->port))
continue;
- }
- else if (r1 & PAR_ERR)
+ } else if (r1 & PAR_ERR)
up->port.icount.parity++;
else if (r1 & CRC_ERR)
up->port.icount.frame++;
@@ -394,7 +397,7 @@ static void sunzilog_kbdms_receive_chars(struct uart_sunzilog_port *up,
if (up->port.ignore_status_mask == 0xff ||
(r1 & up->port.ignore_status_mask) == 0) {
- tty_insert_flip_char(port, ch, flag);
+ tty_insert_flip_char(port, ch, flag);
}
if (r1 & Rx_OVR)
tty_insert_flip_char(port, 0, TTY_OVERRUN);
@@ -437,9 +440,9 @@ static void sunzilog_status_handle(struct uart_sunzilog_port *up,
if (status & SYNC)
up->port.icount.dsr++;
- /* The Zilog just gives us an interrupt when DCD/CTS/etc. change.
- * But it does not tell us which bit has changed, we have to keep
- * track of this ourselves.
+ /* The Zilog just gives us an interrupt when DCD/CTS/etc.
+ * change.But it does not tell us which bit has changed,
+ * we have to keep track of this ourselves.
*/
if ((status ^ up->prev_status) ^ DCD)
uart_handle_dcd_change(&up->port,
@@ -461,15 +464,17 @@ static void sunzilog_transmit_chars(struct uart_sunzilog_port *up,
if (ZS_IS_CONS(up)) {
unsigned char status = readb(&channel->control);
+
ZSDELAY();
/* TX still busy? Just wait for the next TX done interrupt.
*
- * It can occur because of how we do serial console writes. It would
- * be nice to transmit console writes just like we normally would for
- * a TTY line. (ie. buffered and TX interrupt driven). That is not
- * easy because console writes cannot sleep. One solution might be
- * to poll on enough port->xmit space becoming free. -DaveM
+ * It can occur because of how we do serial console writes.
+ * It would be nice to transmit console writes just like we
+ * normally would for a TTY line. (ie. buffered and TX
+ * interrupt driven). That is not easy because console
+ * writes cannot sleep. One solution might be to poll on
+ * enough port->xmit space becoming free. -DaveM
*/
if (!(status & Tx_BUF_EMP))
return;
@@ -590,7 +595,8 @@ static irqreturn_t sunzilog_interrupt(int irq, void *dev_id)
/* A convenient way to quickly get R0 status. The caller must _not_ hold the
* port lock, it is acquired here.
*/
-static __inline__ unsigned char sunzilog_read_channel_status(struct uart_port *port)
+static inline unsigned char
+sunzilog_read_channel_status(struct uart_port *port)
{
struct zilog_channel __iomem *channel;
unsigned char status;
@@ -661,13 +667,13 @@ static void sunzilog_set_mctrl(struct uart_port *port, unsigned int mctrl)
else
clear_bits |= DTR;
- /* NOTE: Not subject to 'transmitter active' rule. */
+ /* NOTE: Not subject to 'transmitter active' rule. */
up->curregs[R5] |= set_bits;
up->curregs[R5] &= ~clear_bits;
write_zsreg(channel, R5, up->curregs[R5]);
}
-/* The port lock is held and interrupts are disabled. */
+/* The port lock is held and interrupts are disabled. */
static void sunzilog_stop_tx(struct uart_port *port)
{
struct uart_sunzilog_port *up =
@@ -749,7 +755,7 @@ static void sunzilog_enable_ms(struct uart_port *port)
if (new_reg != up->curregs[R15]) {
up->curregs[R15] = new_reg;
- /* NOTE: Not subject to 'transmitter active' rule. */
+ /* NOTE: Not subject to 'transmitter active' rule. */
write_zsreg(channel, R15, up->curregs[R15] & ~WR7pEN);
}
}
@@ -776,7 +782,7 @@ static void sunzilog_break_ctl(struct uart_port *port, int break_state)
if (new_reg != up->curregs[R5]) {
up->curregs[R5] = new_reg;
- /* NOTE: Not subject to 'transmitter active' rule. */
+ /* NOTE: Not subject to 'transmitter active' rule. */
write_zsreg(channel, R5, up->curregs[R5]);
}
@@ -996,7 +1002,8 @@ static void sunzilog_config_port(struct uart_port *port, int flags)
}
/* We do not support letting the user mess with the divisor, IRQ, etc. */
-static int sunzilog_verify_port(struct uart_port *port, struct serial_struct *ser)
+static int sunzilog_verify_port(struct uart_port *port,
+ struct serial_struct *ser)
{
return -EINVAL;
}
@@ -1139,6 +1146,7 @@ static void sunzilog_putchar(struct uart_port *port, int ch)
*/
do {
unsigned char val = readb(&channel->control);
+
if (val & Tx_BUF_EMP) {
ZSDELAY();
break;
@@ -1237,15 +1245,34 @@ static int __init sunzilog_console_setup(struct console *con, char *options)
* this hackish cflag thing is OK.
*/
switch (con->cflag & CBAUD) {
- case B150: baud = 150; break;
- case B300: baud = 300; break;
- case B600: baud = 600; break;
- case B1200: baud = 1200; break;
- case B2400: baud = 2400; break;
- case B4800: baud = 4800; break;
- default: case B9600: baud = 9600; break;
- case B19200: baud = 19200; break;
- case B38400: baud = 38400; break;
+ case B150:
+ baud = 150;
+ break;
+ case B300:
+ baud = 300;
+ break;
+ case B600:
+ baud = 600;
+ break;
+ case B1200:
+ baud = 1200;
+ break;
+ case B2400:
+ baud = 2400;
+ break;
+ case B4800:
+ baud = 4800;
+ break;
+ default:
+ case B9600:
+ baud = 9600;
+ break;
+ case B19200:
+ baud = 19200;
+ break;
+ case B38400:
+ baud = 38400;
+ break;
}
brg = BPS_TO_BRG(baud, ZS_CLOCK / ZS_CLOCK_DIVISOR);
@@ -1380,9 +1407,9 @@ static void sunzilog_init_hw(struct uart_sunzilog_port *up)
up->curregs[R14] = BRSRC | BRENAB;
up->curregs[R15] = FIFOEN; /* Use FIFO if on ESCC */
up->curregs[R7p] = TxFIFO_LVL | RxFIFO_LVL;
- if (__load_zsregs(channel, up->curregs)) {
+ if (__load_zsregs(channel, up->curregs))
up->flags |= SUNZILOG_FLAG_ESCC;
- }
+
/* Only enable interrupts if an ISR handler available */
if (up->flags & SUNZILOG_FLAG_ISR_HANDLER)
up->curregs[R9] |= MIE;
@@ -1489,12 +1516,12 @@ static int zs_probe(struct platform_device *op)
}
uart_inst++;
} else {
- dev_info(&op->dev, "Keyboard at MMIO 0x%llx (irq = %d) "
- "is a %s\n",
+ dev_info(&op->dev,
+ "Keyboard at MMIO 0x%llx (irq = %d) is a %s\n",
(unsigned long long) up[0].port.mapbase,
op->archdata.irqs[0], sunzilog_type(&up[0].port));
- dev_info(&op->dev, "Mouse at MMIO 0x%llx (irq = %d) "
- "is a %s\n",
+ dev_info(&op->dev,
+ "Mouse at MMIO 0x%llx (irq = %d) is a %s\n",
(unsigned long long) up[1].port.mapbase,
op->archdata.irqs[0], sunzilog_type(&up[1].port));
kbm_inst++;
@@ -1578,6 +1605,7 @@ static int __init sunzilog_init(void)
if (zilog_irq) {
struct uart_sunzilog_port *up = sunzilog_irq_chain;
+
err = request_irq(zilog_irq, sunzilog_interrupt, IRQF_SHARED,
"zs", sunzilog_irq_chain);
if (err)
@@ -1587,7 +1615,10 @@ static int __init sunzilog_init(void)
while (up) {
struct zilog_channel __iomem *channel;
- /* printk (KERN_INFO "Enable IRQ for ZILOG Hardware %p\n", up); */
+ /* printk(KERN_INFO
+ * "Enable IRQ for ZILOG Hardware %p\n",
+ * up);
+ */
channel = ZILOG_CHANNEL_FROM_PORT(&up->port);
up->flags |= SUNZILOG_FLAG_ISR_HANDLER;
up->curregs[R9] |= MIE;
@@ -1624,7 +1655,10 @@ static void __exit sunzilog_exit(void)
while (up) {
struct zilog_channel __iomem *channel;
- /* printk (KERN_INFO "Disable IRQ for ZILOG Hardware %p\n", up); */
+ /* printk(KERN_INFO
+ * "Disable IRQ for ZILOG Hardware %p\n",
+ * up);
+ */
channel = ZILOG_CHANNEL_FROM_PORT(&up->port);
up->flags &= ~SUNZILOG_FLAG_ISR_HANDLER;
up->curregs[R9] &= ~MIE;
--
1.9.1
The io resource size is currently recomputed when it's needed but this
actually needs to be done once (or drivers could specify fixed values)
Simplify that by doing this computation only once and storing the result
into the mapsize field. serial8250_register_8250_port() is now called
only once on driver init, the previous call sites now just fetch the
value from the mapsize field.
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/8250/8250.h | 2 ++
drivers/tty/serial/8250/8250_core.c | 3 +++
drivers/tty/serial/8250/8250_port.c | 33 +++++++++++++++------------------
3 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index ebfb0bd..89e3f09 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -255,3 +255,5 @@ static inline int serial_index(struct uart_port *port)
{
return port->minor - 64;
}
+
+unsigned int serial8250_port_size(struct uart_8250_port *pt);
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 71a398b..a9d4ba1 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -979,6 +979,9 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
if (up->port.uartclk == 0)
return -EINVAL;
+ /* compute the mapsize in case the driver didn't specify one */
+ up->mapsize = serial8250_port_size(up);
+
mutex_lock(&serial_mutex);
uart = serial8250_find_match_or_unused(&up->port);
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index d2f3310..d09af4c 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2829,7 +2829,7 @@ void serial8250_do_pm(struct uart_port *port, unsigned int state,
serial8250_do_pm(port, state, oldstate);
}
-static unsigned int serial8250_port_size(struct uart_8250_port *pt)
+unsigned int serial8250_port_size(struct uart_8250_port *pt)
{
if (pt->port.mapsize)
return pt->port.mapsize;
@@ -2849,9 +2849,7 @@ static unsigned int serial8250_port_size(struct uart_8250_port *pt)
*/
static int serial8250_request_std_resource(struct uart_8250_port *up)
{
- unsigned int size = serial8250_port_size(up);
struct uart_port *port = &up->port;
- int ret = 0;
switch (port->iotype) {
case UPIO_AU:
@@ -2863,32 +2861,31 @@ static int serial8250_request_std_resource(struct uart_8250_port *up)
if (!port->mapbase)
break;
- if (!request_mem_region(port->mapbase, size, "serial")) {
- ret = -EBUSY;
- break;
- }
+ if (!request_mem_region(port->mapbase,
+ port->mapsize, "serial"))
+ return -EBUSY;
if (port->flags & UPF_IOREMAP) {
- port->membase = ioremap_nocache(port->mapbase, size);
- if (!port->membase) {
- release_mem_region(port->mapbase, size);
- ret = -ENOMEM;
- }
+ port->membase = ioremap_nocache(port->mapbase,
+ port->mapsize);
+ if (!port->membase)
+ release_mem_region(port->mapbase,
+ port->mapsize);
+ return -ENOMEM;
}
break;
case UPIO_HUB6:
case UPIO_PORT:
- if (!request_region(port->iobase, size, "serial"))
- ret = -EBUSY;
+ if (!request_region(port->iobase, port->mapsize, "serial"))
+ return -EBUSY;
break;
}
- return ret;
+ return 0;
}
static void serial8250_release_std_resource(struct uart_8250_port *up)
{
- unsigned int size = serial8250_port_size(up);
struct uart_port *port = &up->port;
switch (port->iotype) {
@@ -2906,12 +2903,12 @@ static void serial8250_release_std_resource(struct uart_8250_port *up)
port->membase = NULL;
}
- release_mem_region(port->mapbase, size);
+ release_mem_region(port->mapbase, port->mapsize);
break;
case UPIO_HUB6:
case UPIO_PORT:
- release_region(port->iobase, size);
+ release_region(port->iobase, port->mapsize);
break;
}
}
--
1.9.1
Fix checkpatch warnings:
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#103: FILE: dz.c:103:
+static u16 dz_in(struct dz_port *dport, unsigned offset)
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#110: FILE: dz.c:110:
+static void dz_out(struct dz_port *dport, unsigned offset, u16 value)
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/tty/serial/dz.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index b3e9313..559d076 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -100,14 +100,14 @@ static inline struct dz_port *to_dport(struct uart_port *uport)
* ------------------------------------------------------------
*/
-static u16 dz_in(struct dz_port *dport, unsigned offset)
+static u16 dz_in(struct dz_port *dport, unsigned int offset)
{
void __iomem *addr = dport->port.membase + offset;
return readw(addr);
}
-static void dz_out(struct dz_port *dport, unsigned offset, u16 value)
+static void dz_out(struct dz_port *dport, unsigned int offset, u16 value)
{
void __iomem *addr = dport->port.membase + offset;
--
1.9.1
On Sat, Apr 27, 2019 at 02:51:42PM +0200, Enrico Weigelt, metux IT consult wrote:
> Using dev_err() instead of printk() for more consistent output.
> (prints device name, etc).
>
> Signed-off-by: Enrico Weigelt <[email protected]>
Your "From:" line does not match the signed-off-by line, so I can't take
any of these if I wanted to :(
Please fix up.
thanks,
greg k-h
On Sat, Apr 27, 2019 at 02:51:51PM +0200, Enrico Weigelt, metux IT consult wrote:
> Fix checkpatch warning:
>
> ERROR: Macros with complex values should be enclosed in parentheses
> #911: FILE: drivers/tty/serial/sb1250-duart.c:911:
> +#define SERIAL_SB1250_DUART_CONSOLE &sbd_console
>
> Signed-off-by: Enrico Weigelt <[email protected]>
> ---
> drivers/tty/serial/sb1250-duart.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/sb1250-duart.c b/drivers/tty/serial/sb1250-duart.c
> index 1184226..ec74f09 100644
> --- a/drivers/tty/serial/sb1250-duart.c
> +++ b/drivers/tty/serial/sb1250-duart.c
> @@ -908,7 +908,7 @@ static int __init sbd_serial_console_init(void)
>
> console_initcall(sbd_serial_console_init);
>
> -#define SERIAL_SB1250_DUART_CONSOLE &sbd_console
> +#define SERIAL_SB1250_DUART_CONSOLE (&sbd_console)
No, that's foolish.
checkpatch is a hint, it's not always right.
Also, checkpatch cleanups for really old drivers is not generally a good
idea, especially if you do not have the hardware for them. Please don't
cause unneeded churn for this type of thing in this subsystem, unless
you have the hardware.
thanks,
greg k-h
On Sat, Apr 27, 2019 at 02:51:42PM +0200, Enrico Weigelt, metux IT consult wrote:
> Using dev_err() instead of printk() for more consistent output.
> (prints device name, etc).
>
> Signed-off-by: Enrico Weigelt <[email protected]>
> ---
> drivers/tty/serial/dz.c | 8 ++++----
Do you have this hardware to test any of these changes with?
thanks,
greg k-h
On Sat, Apr 27, 2019 at 02:51:46PM +0200, Enrico Weigelt, metux IT consult wrote:
> Fix the checkpatch warning:
>
> WARNING: printk() should include KERN_<LEVEL> facility level
> #934: FILE: dz.c:934:
> + printk("%s%s\n", dz_name, dz_version);
>
> Signed-off-by: Enrico Weigelt <[email protected]>
> ---
> drivers/tty/serial/dz.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
> index 559d076..e2670c4 100644
> --- a/drivers/tty/serial/dz.c
> +++ b/drivers/tty/serial/dz.c
> @@ -931,7 +931,7 @@ static int __init dz_init(void)
> if (IOASIC)
> return -ENXIO;
>
> - printk("%s%s\n", dz_name, dz_version);
> + pr_info("%s%s\n", dz_name, dz_version);
Just drop this line, it's not needed and generally just noise.
thanks,
greg k-h
On Sat, Apr 27, 2019 at 02:52:18PM +0200, Enrico Weigelt, metux IT consult wrote:
> Simpily io resource size computation by setting mapsize field.
>
> Some of the special cases handled by serial8250_port_size() can be
> simplified by putting this data to corresponding platform data
> or probe function.
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -105,6 +105,7 @@ struct serial8250_config {
>
> #define SERIAL8250_PORT(_base, _irq) SERIAL8250_PORT_FLAGS(_base, _irq, 0)
>
> +#define SERIAL_RT2880_IOSIZE 0x100
And why this is in the header file and not in corresponding C one?
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index d09af4c..51d6076 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -2833,11 +2833,7 @@ unsigned int serial8250_port_size(struct uart_8250_port *pt)
> {
> if (pt->port.mapsize)
> return pt->port.mapsize;
> - if (pt->port.iotype == UPIO_AU) {
> - if (pt->port.type == PORT_RT2880)
> - return 0x100;
> - return 0x1000;
> - }
> +
> if (is_omap1_8250(pt))
> return 0x16 << pt->port.regshift;
This is good. We definitely need to get rid of custom stuff in generic
8250_port, etc.
--
With Best Regards,
Andy Shevchenko
On Sat, Apr 27, 2019 at 02:52:17PM +0200, Enrico Weigelt, metux IT consult wrote:
> The io resource size is currently recomputed when it's needed but this
> actually needs to be done once (or drivers could specify fixed values)
io -> IO
>
> Simplify that by doing this computation only once and storing the result
> into the mapsize field. serial8250_register_8250_port() is now called
> only once on driver init, the previous call sites now just fetch the
> value from the mapsize field.
Do I understand correctly that this has no side effects?
Which hardware did you test this on?
> @@ -979,6 +979,9 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> if (up->port.uartclk == 0)
> return -EINVAL;
>
> + /* compute the mapsize in case the driver didn't specify one */
> + up->mapsize = serial8250_port_size(up);
I don't know all quirks in 8250 drivers by heart, though can you guarantee that
at this point the device reports correct IO resource size? (If I'm not mistaken
some broken hardware needs some magic to be done before card can be properly
handled)
> - unsigned int size = serial8250_port_size(up);
> struct uart_port *port = &up->port;
> - int ret = 0;
This and Co is a separate change that can be done in its own patch.
> + port->membase = ioremap_nocache(port->mapbase,
> + port->mapsize);
You may increase readability by introducing temporary variables
... mapbase = port->mapbase;
... mapsize = port->mapsize;
...
port->membase = ioremap_nocache(mapbase, mapsize);
...
--
With Best Regards,
Andy Shevchenko
On Sat, Apr 27, 2019 at 02:52:21PM +0200, Enrico Weigelt, metux IT consult wrote:
> Introduce a little helpers for settings the mmio range from an
> struct resource or start/len parameters with less code.
> (also setting iotype to UPIO_MEM)
>
> Also converting drivers to use these new helpers as well as
> fetching mapsize field instead of using hardcoded values.
> (the runtime overhead of that should be negligible)
>
> The idea is moving to a consistent scheme, so later common
> calls like request+ioremap combination can be done by generic
> helpers.
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -134,8 +134,10 @@ static int default_setup(struct exar8250 *priv, struct pci_dev *pcidev,
> const struct exar8250_board *board = priv->board;
> unsigned int bar = 0;
>
> - port->port.iotype = UPIO_MEM;
> - port->port.mapbase = pci_resource_start(pcidev, bar) + offset;
> + uart_memres_set_start_len(&port->port,
> + pci_resource_start(pcidev, bar) + offset,
> + pci_resource_len(pcidev, bar));
> +
I don't see how it's better.
Moreover, the size argument seems wrong here.
> + uart_memres_set_start_len(
> + &port,
> + FRODO_BASE + FRODO_APCI_OFFSET(1), 0);
Please, avoid such splitting, first parameter is quite fit above line.
> port.uartclk = HPDCA_BAUD_BASE * 16;
> - port.mapbase = (pa + UART_OFFSET);
> +
> + uart_memres_set_start_len(&port, (pa + UART_OFFSET));
> port.membase = (char *)(port.mapbase + DIO_VIRADDRBASE);
> port.regshift = 1;
> port.irq = DIO_IPL(pa + DIO_VIRADDRBASE);
Here...
> uart.port.flags = UPF_SKIP_TEST | UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF;
> uart.port.irq = d->ipl;
> uart.port.uartclk = HPDCA_BAUD_BASE * 16;
> - uart.port.mapbase = (d->resource.start + UART_OFFSET);
> + uart_memres_set_start_len(&uart.port,
> + (d->resource.start + UART_OFFSET),
> + resource_size(&d->resource));
> uart.port.membase = (char *)(uart.port.mapbase + DIO_VIRADDRBASE);
> uart.port.regshift = 1;
> uart.port.dev = &d->dev;
...and here, and maybe in other places you split the assignments to the members
in two part. Better to call your function before or after these blocks of
assignments.
> - uport->mapsize = ZS_CHAN_IO_SIZE;
> - uport->mapbase = dec_kn_slot_base +
> - zs_parms.scc[chip] +
> - (side ^ ZS_CHAN_B) * ZS_CHAN_IO_SIZE;
> +
> + uart_memres_set_start_len(dec_kn_slot_base +
> + zs_parms.scc[chip] +
> + (side ^ ZS_CHAN_B) *
> + ZS_CHAN_IO_SIZE,
> + ZS_CHAN_IO_SIZE);
This looks hard to read. Think of temporary variables and better formatting
style.
> /*
> + * set physical io range from struct resource
> + * if resource is NULL, clear the fields
> + * also set the iotype to UPIO_MEM
Something wrong with punctuation and style. Please, use proper casing and
sentences split.
> + */
Shouldn't be kernel-doc formatted?
> +static inline void uart_memres_set_res(struct uart_port *port,
Perhaps better name can be found.
Especially taking into account that it handles IO / MMIO here.
> + struct resource *res)
> +{
> + if (!res) {
It should return an error in such case.
> + port->mapsize = 0;
> + port->mapbase = 0;
> + port->iobase = 0;
> + return;
> + }
> +
> + if (resource_type(res) == IORESOURCE_IO) {
> + port->iotype = UPIO_PORT;
> + port->iobase = resource->start;
> + return;
> + }
> +
> + uart->mapbase = res->start;
> + uart->mapsize = resource_size(res);
> + uart->iotype = UPIO_MEM;
Only one type? Why type is even set here?
> +}
> +
> +/*
> + * set physical io range by start address and length
> + * if resource is NULL, clear the fields
> + * also set the iotype to UPIO_MEM
Should be fixed as told above.
> + */
> +static inline void uart_memres_set_start_len(struct uart_driver *uart,
> + resource_size_t start,
> + resource_size_t len)
The comment doesn't tell why this is needed when we have one for struct
resource.
> +{
> + uart->mapbase = start;
> + uart->mapsize = len;
> + uart->iotype = UPIO_MEM;
Only one type?
> +}
> +
> +/*
--
With Best Regards,
Andy Shevchenko
On 28.04.19 17:21, Andy Shevchenko wrote:
>
>> +#define SERIAL_RT2880_IOSIZE 0x100
>
> And why this is in the header file and not in corresponding C one?
hmm, no particular reason, maybe just an old habit to put definitions
into .h files ;-)
I can move it to 8250_of.c if you like me to.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
"Enrico Weigelt, metux IT consult" <[email protected]> writes:
> @@ -131,7 +133,8 @@ int __init hp300_setup_serial_console(void)
> pr_info("Serial console is HP DCA at select code %d\n", scode);
>
> port.uartclk = HPDCA_BAUD_BASE * 16;
> - port.mapbase = (pa + UART_OFFSET);
> +
> + uart_memres_set_start_len(&port, (pa + UART_OFFSET));
Missing length argument here.
> port.membase = (char *)(port.mapbase + DIO_VIRADDRBASE);
> port.regshift = 1;
> port.irq = DIO_IPL(pa + DIO_VIRADDRBASE);
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index cf8ca66..895c90c 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -1626,8 +1626,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
> * This function also registers this device with the tty layer
> * and triggers invocation of the config_port() entry point.
> */
> - port->mapbase = res->start;
> - port->mapsize = CDNS_UART_REGISTER_SPACE;
> + uart_memres_set_start_len(res->start, CDNS_UART_REGISTER_SPACE);
Missing 1st (port) argument here.
> port->irq = irq;
> port->dev = &pdev->dev;
> port->uartclk = clk_get_rate(cdns_uart_data->uartclk);
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 5fe2b03..d891c8d 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -427,6 +427,46 @@ void uart_console_write(struct uart_port *port, const char *s,
> int uart_match_port(struct uart_port *port1, struct uart_port *port2);
>
> /*
> + * set physical io range from struct resource
> + * if resource is NULL, clear the fields
> + * also set the iotype to UPIO_MEM
> + */
> +static inline void uart_memres_set_res(struct uart_port *port,
> + struct resource *res)
> +{
> + if (!res) {
> + port->mapsize = 0;
> + port->mapbase = 0;
> + port->iobase = 0;
> + return;
> + }
> +
> + if (resource_type(res) == IORESOURCE_IO) {
> + port->iotype = UPIO_PORT;
> + port->iobase = resource->start;
> + return;
> + }
> +
> + uart->mapbase = res->start;
> + uart->mapsize = resource_size(res);
> + uart->iotype = UPIO_MEM;
Hardcoding UPIO_MEM like does not work for drivers using other kind of
memory access, like UPIO_MEM16, UPIO_MEM32 and UPIO_MEM32BE.
Why not leave the control of iotype to drivers as before this patch?
> +}
> +
> +/*
> + * set physical io range by start address and length
> + * if resource is NULL, clear the fields
> + * also set the iotype to UPIO_MEM
> + */
> +static inline void uart_memres_set_start_len(struct uart_driver *uart,
> + resource_size_t start,
> + resource_size_t len)
> +{
> + uart->mapbase = start;
> + uart->mapsize = len;
> + uart->iotype = UPIO_MEM;
Same here, other iotype values must be possible.
> +}
> +
> +/*
> * Power Management
> */
> int uart_suspend_port(struct uart_driver *reg, struct uart_port *port);
/Esben
"Enrico Weigelt, metux IT consult" <[email protected]> writes:
> Introduce a little helpers for settings the mmio range from an
> struct resource or start/len parameters with less code.
> (also setting iotype to UPIO_MEM)
>
> Also converting drivers to use these new helpers as well as
> fetching mapsize field instead of using hardcoded values.
> (the runtime overhead of that should be negligible)
>
> The idea is moving to a consistent scheme, so later common
> calls like request+ioremap combination can be done by generic
> helpers.
Why not simply replace iobase, mapbase and mapsize with a struct
resource value instead?
Incidentally, that would allow to specify a memory resource with a
parent memory resource :)
/Esben
"Enrico Weigelt, metux IT consult" <[email protected]> writes:
> Adding a mapsize field for the 8250 port platform data struct,
> so we can now set the resource size (eg. *1) and don't need
> funny runtime detections like serial8250_port_size() anymore.
>
> For now, serial8250_port_size() is called everytime we need
> the io resource size. That function checks which chip we
> actually have and returns the appropriate size. This approach
> is a bit clumpsy and not entirely easy to understand, and
> it's a violation of layers :p
>
> Obviously, that information cannot change after the driver init,
> so we can safely do that probing once on driver init and just
> use the stored value later.
>
> *1) arch/mips/alchemy/common/platform.c
>
> Signed-off-by: Enrico Weigelt <[email protected]>
> ---
> drivers/tty/serial/8250/8250_core.c | 1 +
> include/linux/serial_8250.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index e441221..71a398b 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -814,6 +814,7 @@ static int serial8250_probe(struct platform_device *dev)
> uart.port.iotype = p->iotype;
> uart.port.flags = p->flags;
> uart.port.mapbase = p->mapbase;
> + uart.port.mapsize = p->mapsize;
> uart.port.hub6 = p->hub6;
> uart.port.private_data = p->private_data;
> uart.port.type = p->type;
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index 5a655ba..8b8183a 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -22,6 +22,7 @@ struct plat_serial8250_port {
> unsigned long iobase; /* io base address */
> void __iomem *membase; /* ioremap cookie or NULL */
> resource_size_t mapbase; /* resource base */
> + resource_size_t mapsize; /* resource size */
> unsigned int irq; /* interrupt number */
> unsigned long irqflags; /* request_irq flags */
> unsigned int uartclk; /* UART clock rate */
Or replace iobase, mapbase and mapsize with a struct resource value?
/Esben
On 27.04.19 15:31, Greg KH wrote:
> On Sat, Apr 27, 2019 at 02:51:42PM +0200, Enrico Weigelt, metux IT consult wrote:
>> Using dev_err() instead of printk() for more consistent output.
>> (prints device name, etc).
>>
>> Signed-off-by: Enrico Weigelt <[email protected]>
>> ---
>> drivers/tty/serial/dz.c | 8 ++++----
>
> Do you have this hardware to test any of these changes with?
Unfortunately not :(
Do you happen to know anybody who has ?
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On 29.04.19 09:03, Esben Haabendal wrote:
> Why not simply replace iobase, mapbase and mapsize with a struct
> resource value instead?
That was actually my original goal, when I started this. But the
situation is a bit more tricky. Many drivers (especially the old ones)
initialize these fields in different ways. And there're many places
accessing these fields.
Drivers for old devices should be handled w/ great care. I don't have
access to all that hardware, so I can't test it. Therefore, I'm trying
to move in small steps. One step ahead another.
One of my next steps would be factoring out more common operations
(eg. mapping, etc) into helpers, up to a point, where someday no driver
is accessing these fields directly anymore.
Then we could easily move everything into struct resource. On that
road, we'd also need to find a way for handling the specialities of
the various UPIO_* modes via struct resource.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On 28.04.19 17:39, Andy Shevchenko wrote:
Hi,
seems I've forgot to add "RFC:" in the subject - I'm not entirely happy
w/ that patch myself, just want to hear your oppinions.
>> - port->port.iotype = UPIO_MEM;>> - port->port.mapbase = pci_resource_start(pcidev, bar) + offset;>> +
uart_memres_set_start_len(&port->port,>> +
pci_resource_start(pcidev, bar) + offset,>> +
pci_resource_len(pcidev, bar));>> +> > I don't see how it's better.>
Moreover, the size argument seems wrong here.
hmm, I'm actually not sure yet, what the correct size really would be,
where the value actually comes from. Just assumed that it would be the
whole area that the BAR tells. But now I recognized that I'd need to
substract 'offset' here.
Rethinking it further, we'd probably could deduce the UPIO_* from the
struct resource, too.
>> + uart_memres_set_start_len(>> + &port,>> + FRODO_BASE + FRODO_APCI_OFFSET(1), 0);> > Please,
avoid such splitting, first parameter is quite fit above line.
Ok. My intention was having both parameters starting at the same line,
but then the second line would get too long again. > ...and here, and
maybe in other places you split the assignments to the members> in two
part. Better to call your function before or after these blocks of>
assignments.
the reason what I've just replaced the exactly the assignments, trying
not to touch too much ;-)
I'll have a closer look on what can be moved w/o side effects.
>> - uport->mapsize = ZS_CHAN_IO_SIZE;
>> - uport->mapbase = dec_kn_slot_base +
>> - zs_parms.scc[chip] +
>> - (side ^ ZS_CHAN_B) * ZS_CHAN_IO_SIZE;
>> +
>> + uart_memres_set_start_len(dec_kn_slot_base +
>> + zs_parms.scc[chip] +
>> + (side ^ ZS_CHAN_B) *
>> + ZS_CHAN_IO_SIZE,
>> + ZS_CHAN_IO_SIZE);
>
> This looks hard to read. Think of temporary variables and better formatting
> style.
Ok.
>
>> /*
>> + * set physical io range from struct resource
>> + * if resource is NULL, clear the fields
>> + * also set the iotype to UPIO_MEM
>
> Something wrong with punctuation and style. Please, use proper casing and
> sentences split.
Ok. fixed it.
>> +static inline void uart_memres_set_res(struct uart_port *port,
>
> Perhaps better name can be found.
> Especially taking into account that it handles IO / MMIO here.
hmm, lacking creativity here ;-)
any suggestions ?
>
>> + struct resource *res)
>> +{
>> + if (!res) {
>
> It should return an error in such case.
It's not an error, but desired behaviour: NULL resource
clears the value.
>> + port->mapsize = 0;
>> + port->mapbase = 0;
>> + port->iobase = 0;
>> + return;
>> + }
>> +
>> + if (resource_type(res) == IORESOURCE_IO) {
>> + port->iotype = UPIO_PORT;
>> + port->iobase = resource->start;
>> + return;
>> + }
>> +
>> + uart->mapbase = res->start;
>> + uart->mapsize = resource_size(res);
>
>> + uart->iotype = UPIO_MEM;
>
> Only one type? Why type is even set here?
It's the default case. The special cases (eg. UPIO_MEM32) need to be
set explicitly, after that call.
Not really nice, but haven't found a better solution yet. I don't like
the idea of passing an UPIO_* parameter to the function, most callers
should not care, if they don't really need to.
>> + */
>
>> +static inline void uart_memres_set_start_len(struct uart_driver *uart,
>> + resource_size_t start,
>> + resource_size_t len)
>
> The comment doesn't tell why this is needed when we have one for struct
> resource.
Renamed it to uart_memres_set_mmio_range().
This helper is meant for drivers that don't work w/ struct resource,
or explicitly set their own len.
Thanks for your review.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On 27.04.19 15:31, Greg KH wrote:
> On Sat, Apr 27, 2019 at 02:51:42PM +0200, Enrico Weigelt, metux IT consult wrote:
>> Using dev_err() instead of printk() for more consistent output.
>> (prints device name, etc).
>>
>> Signed-off-by: Enrico Weigelt <[email protected]>
>> ---
>> drivers/tty/serial/dz.c | 8 ++++----
>
> Do you have this hardware to test any of these changes with?
Unfortunately not :(
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On Mon, Apr 29, 2019 at 02:37:04PM +0200, Enrico Weigelt, metux IT consult wrote:
> On 27.04.19 15:31, Greg KH wrote:
> > On Sat, Apr 27, 2019 at 02:51:42PM +0200, Enrico Weigelt, metux IT consult wrote:
> >> Using dev_err() instead of printk() for more consistent output.
> >> (prints device name, etc).
> >>
> >> Signed-off-by: Enrico Weigelt <[email protected]>
> >> ---
> >> drivers/tty/serial/dz.c | 8 ++++----
> >
> > Do you have this hardware to test any of these changes with?
>
> Unfortunately not :(
Then I can take the "basic" types of patches for the driver (like this
one), but not any others, sorry.
thanks,
greg k-h
On Mon, Apr 29, 2019 at 08:48:53AM +0200, Enrico Weigelt, metux IT consult wrote:
> On 28.04.19 17:21, Andy Shevchenko wrote:
> >> +#define SERIAL_RT2880_IOSIZE 0x100
> >
> > And why this is in the header file and not in corresponding C one?
>
> hmm, no particular reason, maybe just an old habit to put definitions
> into .h files ;-)
>
> I can move it to 8250_of.c if you like me to.
Please, do.
--
With Best Regards,
Andy Shevchenko
On Mon, Apr 29, 2019 at 12:12:35PM +0200, Enrico Weigelt, metux IT consult wrote:
> On 28.04.19 17:39, Andy Shevchenko wrote:
> seems I've forgot to add "RFC:" in the subject - I'm not entirely happy
> w/ that patch myself, just want to hear your oppinions.
>
> Moreover, the size argument seems wrong here.
Something went wrong with quoting style in your reply.
> hmm, I'm actually not sure yet, what the correct size really would be,
> where the value actually comes from. Just assumed that it would be the
> whole area that the BAR tells. But now I recognized that I'd need to
> substract 'offset' here.
It will be still wrong. The driver in question defines resource window based on
several parameters. So, this should be supplied with a real understanding of
all variety of hardware the certain driver serves for.
> Rethinking it further, we'd probably could deduce the UPIO_* from the
> struct resource, too.
>
> >> + uart_memres_set_start_len(>> + &port,>> + FRODO_BASE + FRODO_APCI_OFFSET(1), 0);> > Please,
> avoid such splitting, first parameter is quite fit above line.
>
> Ok. My intention was having both parameters starting at the same line,
> but then the second line would get too long again. > ...and here, and
> maybe in other places you split the assignments to the members> in two
> part. Better to call your function before or after these blocks of>
> assignments.
> the reason what I've just replaced the exactly the assignments, trying
> not to touch too much ;-)
> I'll have a closer look on what can be moved w/o side effects.
Just try to avoid
foo(
bar, ...
-like splitting.
> >> +static inline void uart_memres_set_res(struct uart_port *port,
> >
> > Perhaps better name can be found.
> > Especially taking into account that it handles IO / MMIO here.
>
> hmm, lacking creativity here ;-)
> any suggestions ?
No immediate suggestions.
uart_set_io_resource()
uart_clear_io_resource()
at least sounds more plausible to me.
> >> + struct resource *res)
> >> +{
> >> + if (!res) {
> >
> > It should return an error in such case.
>
> It's not an error, but desired behaviour: NULL resource
> clears the value.
Oh, then why it's in this function, which is *setter* according to its name,
at all?
>
> >> + port->mapsize = 0;
> >> + port->mapbase = 0;
> >> + port->iobase = 0;
> >> + return;
> >> + }
> >> +
> >> + if (resource_type(res) == IORESOURCE_IO) {
> >> + port->iotype = UPIO_PORT;
> >> + port->iobase = resource->start;
> >> + return;
> >> + }
> >> +
> >> + uart->mapbase = res->start;
> >> + uart->mapsize = resource_size(res);
> >
> >> + uart->iotype = UPIO_MEM;
> >
> > Only one type? Why type is even set here?
>
> It's the default case. The special cases (eg. UPIO_MEM32) need to be
> set explicitly, after that call.
Which is weird.
> Not really nice, but haven't found a better solution yet.
Just simple not touching it?
> I don't like
> the idea of passing an UPIO_* parameter to the function, most callers
> should not care, if they don't really need to.
They do care. The driver on its own knows better than any generic code what
type of hardware it serves to.
> >> + */
> >
> >> +static inline void uart_memres_set_start_len(struct uart_driver *uart,
> >> + resource_size_t start,
> >> + resource_size_t len)
> >
> > The comment doesn't tell why this is needed when we have one for struct
> > resource.
>
> Renamed it to uart_memres_set_mmio_range().
See also above about naming patterns.
>
> This helper is meant for drivers that don't work w/ struct resource,
> or explicitly set their own len.
Then why it's not mentioned in the description of the function?
--
With Best Regards,
Andy Shevchenko
On 27.04.19 15:29, Greg KH wrote:
> On Sat, Apr 27, 2019 at 02:51:42PM +0200, Enrico Weigelt, metux IT consult wrote:
>> Using dev_err() instead of printk() for more consistent output.
>> (prints device name, etc).
>>
>> Signed-off-by: Enrico Weigelt <[email protected]>
>
> Your "From:" line does not match the signed-off-by line, so I can't take
> any of these if I wanted to :(
Grmpf. I've manually changed it, as you isisted in having my company
name remove from it ....
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On Mon, Apr 29, 2019 at 04:11:15PM +0200, Enrico Weigelt, metux IT consult wrote:
> On 27.04.19 15:29, Greg KH wrote:
> > On Sat, Apr 27, 2019 at 02:51:42PM +0200, Enrico Weigelt, metux IT consult wrote:
> >> Using dev_err() instead of printk() for more consistent output.
> >> (prints device name, etc).
> >>
> >> Signed-off-by: Enrico Weigelt <[email protected]>
> >
> > Your "From:" line does not match the signed-off-by line, so I can't take
> > any of these if I wanted to :(
>
> Grmpf. I've manually changed it, as you isisted in having my company
> name remove from it ....
Yes, that's fine, but the lines have to match. See the documentation
for how to have a "From:" in the changelog text to override whatever
your email client happens to pollute the email with :)
thanks,
greg k-h
On 28.04.19 17:18, Andy Shevchenko wrote:
> On Sat, Apr 27, 2019 at 02:52:17PM +0200, Enrico Weigelt, metux IT consult wrote:
>> The io resource size is currently recomputed when it's needed but this
>> actually needs to be done once (or drivers could specify fixed values)
>
> io -> IO
fixed.
>> Simplify that by doing this computation only once and storing the result
>> into the mapsize field. serial8250_register_8250_port() is now called
>> only once on driver init, the previous call sites now just fetch the
>> value from the mapsize field.
>
> Do I understand correctly that this has no side effects?
I don't know of any. (except someting changes things like regshift after
the initialization phase ... :o)
>> @@ -979,6 +979,9 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
>> if (up->port.uartclk == 0)
>> return -EINVAL;
>>
>> + /* compute the mapsize in case the driver didn't specify one */
>> + up->mapsize = serial8250_port_size(up);
>
> I don't know all quirks in 8250 drivers by heart, though can you guarantee that
> at this point the device reports correct IO resource size? (If I'm not mistaken
> some broken hardware needs some magic to be done before card can be properly
> handled)
Actually, I don't see anything talking to the hardware at all here.
It's all derived from values that are set up before
serial8250_register_8250_port() is called.
>> - unsigned int size = serial8250_port_size(up);
>> struct uart_port *port = &up->port;
>
>> - int ret = 0;
>
> This and Co is a separate change that can be done in its own patch.
I don't really understand :(
Do you mean the splitting off the retval part from the rest ?
>> + port->membase = ioremap_nocache(port->mapbase,
>> + port->mapsize);
>
> You may increase readability by introducing temporary variables
>
> ... mapbase = port->mapbase;
> ... mapsize = port->mapsize;
> ...
> port->membase = ioremap_nocache(mapbase, mapsize);
> ...
Is that really necessary ? Maybe it's just my personal taste, but I
don't feel the more more verbose one is really easier to read.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
>>>>> "Enrico" == Enrico Weigelt, metux IT consult <[email protected]> writes:
> Fill the struct uart_port->mapsize field and use it, insteaf of
s/insteaf/instead/
> hardcoded values in many places. This makes the code layout a bit
> more consistent and easily allows using generic helpers for the
> io memory handling.
> Candidates for such helpers could be eg. the request+ioremap and
> iounmap+release combinations.
> Signed-off-by: Enrico Weigelt <[email protected]>
Acked-by: Peter Korsgaard <[email protected]>
--
Bye, Peter Korsgaard
>>>>> "Enrico" == Enrico Weigelt, metux IT consult <[email protected]> writes:
> Fix checkpatch warnings:
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #562: FILE: drivers/tty/serial/uartlite.c:562:
> + unsigned retries = 1000000;
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #574: FILE: drivers/tty/serial/uartlite.c:574:
> + const char *s, unsigned n)
s/fix use fix/fix use of/ in Subject. Other than that:
Acked-by: Peter Korsgaard <[email protected]>
--
Bye, Peter Korsgaard
>>>>> "Enrico" == Enrico Weigelt, metux IT consult <[email protected]> writes:
> checkpatch complains:
> WARNING: braces {} are not necessary for any arm of this statement
> #489: FILE: drivers/tty/serial/uartlite.c:489:
> + if (oops_in_progress) {
> [...]
> + } else
> [...]
> Signed-off-by: Enrico Weigelt <[email protected]>
Acked-by: Peter Korsgaard <[email protected]>
--
Bye, Peter Korsgaard
>>>>> "Enrico" == Enrico Weigelt, metux IT consult <[email protected]> writes:
> Fix checkpatch warnings:
> WARNING: line over 80 characters
> #283: FILE: drivers/tty/serial/uartlite.c:283:
> + ret = request_irq(port->irq, ulite_isr, IRQF_SHARED | IRQF_TRIGGER_RISING,
> WARNING: Missing a blank line after declarations
> #577: FILE: drivers/tty/serial/uartlite.c:577:
> + struct earlycon_device *device = console->data;
> + uart_console_write(&device->port, s, n, early_uartlite_putc);
> WARNING: line over 80 characters
> #590: FILE: drivers/tty/serial/uartlite.c:590:
> +OF_EARLYCON_DECLARE(uartlite_b, "xlnx,opb-uartlite-1.00.b", early_uartlite_setup);
> WARNING: line over 80 characters
> #591: FILE: drivers/tty/serial/uartlite.c:591:
> +OF_EARLYCON_DECLARE(uartlite_a, "xlnx,xps-uartlite-1.00.a", early_uartlite_setup);
Given that these are just a few characters more than 80 I don't really
think these changes improve readability.
> Signed-off-by: Enrico Weigelt <[email protected]>
> ---
> drivers/tty/serial/uartlite.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
> diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
> index 6f79353..0140cec 100644
> --- a/drivers/tty/serial/uartlite.c
> +++ b/drivers/tty/serial/uartlite.c
> @@ -280,7 +280,8 @@ static int ulite_startup(struct uart_port *port)
> return ret;
> }
> - ret = request_irq(port->irq, ulite_isr, IRQF_SHARED | IRQF_TRIGGER_RISING,
> + ret = request_irq(port->irq, ulite_isr,
> + IRQF_SHARED | IRQF_TRIGGER_RISING,
> "uartlite", port);
> if (ret)
> return ret;
> @@ -574,6 +575,7 @@ static void early_uartlite_write(struct console *console,
> const char *s, unsigned int n)
> {
> struct earlycon_device *device = console->data;
> +
> uart_console_write(&device->port, s, n, early_uartlite_putc);
Unrelated change?
--
Bye, Peter Korsgaard
>>>>> "Enrico" == Enrico Weigelt, metux IT consult <[email protected]> writes:
> Using dev_dbg() instead of pr_debg() for more consistent output.
> (prints device name, etc).
> Signed-off-by: Enrico Weigelt <[email protected]>
Acked-by: Peter Korsgaard <[email protected]>
--
Bye, Peter Korsgaard
On Mon, Apr 29, 2019 at 04:55:05PM +0200, Enrico Weigelt, metux IT consult wrote:
> On 28.04.19 17:18, Andy Shevchenko wrote:
> > On Sat, Apr 27, 2019 at 02:52:17PM +0200, Enrico Weigelt, metux IT consult wrote:
> >> - int ret = 0;
> >
> > This and Co is a separate change that can be done in its own patch.
>
> I don't really understand :(
> Do you mean the splitting off the retval part from the rest ?
You do two things here: one of them is removing ret and other relative changes.
This should be split to a separate patch.
> > You may increase readability by introducing temporary variables
> >
> > ... mapbase = port->mapbase;
> > ... mapsize = port->mapsize;
> > ...
> > port->membase = ioremap_nocache(mapbase, mapsize);
> > ...
>
> Is that really necessary ? Maybe it's just my personal taste, but I
> don't feel the more more verbose one is really easier to read.
Up to Greg. For me it's harder to read all those port-> in several parameters.
--
With Best Regards,
Andy Shevchenko
Le 27/04/2019 à 14:52, Enrico Weigelt, metux IT consult a écrit :
> Fix checkpatch errors:
What the main purpose of this change ?
If we apply this, any fix to stable will be a nightmare to backport. Is
it really worth it ?
Anyway, a couple of comments in the patch below
[...]
>
>
> Signed-off-by: Enrico Weigelt <[email protected]>
> ---
> drivers/tty/serial/cpm_uart/cpm_uart.h | 10 +--
> drivers/tty/serial/cpm_uart/cpm_uart_core.c | 95 ++++++++++++++++-------------
> drivers/tty/serial/cpm_uart/cpm_uart_cpm1.h | 4 +-
> drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c | 6 +-
> 4 files changed, 64 insertions(+), 51 deletions(-)
[...]
>
> @@ -1048,9 +1058,10 @@ static void cpm_uart_early_write(struct uart_cpm_port *pinfo,
> static int poll_wait_key(char *obuf, struct uart_cpm_port *pinfo)
> {
> u_char c, *cp;
> - volatile cbd_t *bdp;
> int i;
>
> + volatile cbd_t *bdp;
> +
This was likely a false positive from checkpatch. The formatting was
good, and now it is wrong as it adds an unnecessary blank line.
> /* Get the address of the host memory buffer.
> */
> bdp = pinfo->rx_cur;
[...]
> diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> index a0fccda..154ac19 100644
> --- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> +++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> @@ -117,8 +117,7 @@ int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con)
> if (is_con) {
> mem_addr = kzalloc(memsz, GFP_NOWAIT);
> dma_addr = virt_to_bus(mem_addr);
> - }
> - else
> + } else
> mem_addr = dma_alloc_coherent(pinfo->port.dev, memsz, &dma_addr,
> GFP_KERNEL);
Checkpatch should have told you that in case first leg has braces,
second leg must have braces too even if it's a single line.
Christophe
>
> @@ -148,7 +147,8 @@ void cpm_uart_freebuf(struct uart_cpm_port *pinfo)
> dma_free_coherent(pinfo->port.dev, L1_CACHE_ALIGN(pinfo->rx_nrfifos *
> pinfo->rx_fifosize) +
> L1_CACHE_ALIGN(pinfo->tx_nrfifos *
> - pinfo->tx_fifosize), (void __force *)pinfo->mem_addr,
> + pinfo->tx_fifosize),
> + (void __force *)pinfo->mem_addr,
> pinfo->dma_addr);
>
> cpm_dpfree(pinfo->dp_addr);
>
Le 27/04/2019 à 14:52, Enrico Weigelt, metux IT consult a écrit :
> Fix checkpatch warnings by using pr_err():
>
> WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
> #109: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c:109:
> + printk(KERN_ERR
>
> WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
> #128: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c:128:
> + printk(KERN_ERR
>
> WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
> + printk(KERN_ERR
>
> WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
> + printk(KERN_ERR
>
> Signed-off-by: Enrico Weigelt <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
But is that really worth doing those changes ?
If we want to do something useful, wouldn't it make more sense to
introduce the use of dev_err() in order to identify the faulting device
in the message ?
Christophe
> ---
> drivers/tty/serial/cpm_uart/cpm_uart_cpm1.c | 6 ++----
> drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c | 6 ++----
> 2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.c
> index 56fc527..aed61e9 100644
> --- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.c
> +++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm1.c
> @@ -71,8 +71,7 @@ int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con)
> dpmemsz = sizeof(cbd_t) * (pinfo->rx_nrfifos + pinfo->tx_nrfifos);
> dp_offset = cpm_dpalloc(dpmemsz, 8);
> if (IS_ERR_VALUE(dp_offset)) {
> - printk(KERN_ERR
> - "cpm_uart_cpm1.c: could not allocate buffer descriptors\n");
> + pr_err("cpm_uart_cpm1.c: could not allocate buffer descriptors\n");
> return -ENOMEM;
> }
> dp_mem = cpm_dpram_addr(dp_offset);
> @@ -90,8 +89,7 @@ int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con)
>
> if (mem_addr == NULL) {
> cpm_dpfree(dp_offset);
> - printk(KERN_ERR
> - "cpm_uart_cpm1.c: could not allocate coherent memory\n");
> + pr_err("cpm_uart_cpm1.c: could not allocate coherent memory\n");
> return -ENOMEM;
> }
>
> diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> index 40cfcf4..a0fccda 100644
> --- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> +++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> @@ -106,8 +106,7 @@ int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con)
> dpmemsz = sizeof(cbd_t) * (pinfo->rx_nrfifos + pinfo->tx_nrfifos);
> dp_offset = cpm_dpalloc(dpmemsz, 8);
> if (IS_ERR_VALUE(dp_offset)) {
> - printk(KERN_ERR
> - "cpm_uart_cpm.c: could not allocate buffer descriptors\n");
> + pr_err("cpm_uart_cpm.c: could not allocate buffer descriptors\n");
> return -ENOMEM;
> }
>
> @@ -125,8 +124,7 @@ int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con)
>
> if (mem_addr == NULL) {
> cpm_dpfree(dp_offset);
> - printk(KERN_ERR
> - "cpm_uart_cpm.c: could not allocate coherent memory\n");
> + pr_err("cpm_uart_cpm.c: could not allocate coherent memory\n");
> return -ENOMEM;
> }
>
>
On 27.04.19 15:03, John Paul Adrian Glaubitz wrote:
> On 4/27/19 2:52 PM, Enrico Weigelt, metux IT consult wrote:
>> Simpily io resource size computation by setting mapsize field.
> ^^^^
> Here's a typo
thx. fixed.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
Le 27/04/2019 à 14:52, Enrico Weigelt, metux IT consult a écrit :
> Fixing checkpatch warning:
>
> WARNING: Use #include <linux/io.h> instead of <asm/io.h>
> #25: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c:25:
> +#include <asm/io.h>
>
> WARNING: Use #include <linux/io.h> instead of <asm/io.h>
> +#include <asm/io.h>
>
> WARNING: Use #include <linux/delay.h> instead of <asm/delay.h>
> +#include <asm/delay.h>
>
> Signed-off-by: Enrico Weigelt <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
> ---
> drivers/tty/serial/cpm_uart/cpm_uart_core.c | 4 ++--
> drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_core.c b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
> index 374b8bb..c831d31 100644
> --- a/drivers/tty/serial/cpm_uart/cpm_uart_core.c
> +++ b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
> @@ -33,10 +33,10 @@
> #include <linux/gpio.h>
> #include <linux/of_gpio.h>
> #include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/delay.h>
>
> -#include <asm/io.h>
> #include <asm/irq.h>
> -#include <asm/delay.h>
> #include <asm/fs_pd.h>
> #include <asm/udbg.h>
>
> diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> index ef1ae08..40cfcf4 100644
> --- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> +++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> @@ -21,8 +21,8 @@
> #include <linux/device.h>
> #include <linux/memblock.h>
> #include <linux/dma-mapping.h>
> +#include <linux/io.h>
>
> -#include <asm/io.h>
> #include <asm/irq.h>
> #include <asm/fs_pd.h>
> #include <asm/prom.h>
>
Le 27/04/2019 à 14:52, Enrico Weigelt, metux IT consult a écrit :
> Using dev_err()/dev_warn() instead of printk() for more consistent output.
> (prints device name, etc).
>
> Signed-off-by: Enrico Weigelt <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
> ---
> drivers/tty/serial/cpm_uart/cpm_uart_core.c | 6 +++---
> drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_core.c b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
> index b929c7a..374b8bb 100644
> --- a/drivers/tty/serial/cpm_uart/cpm_uart_core.c
> +++ b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
> @@ -265,7 +265,7 @@ static void cpm_uart_int_rx(struct uart_port *port)
> * later, which will be the next rx-interrupt or a timeout
> */
> if (tty_buffer_request_room(tport, i) < i) {
> - printk(KERN_WARNING "No room in flip buffer\n");
> + dev_warn(port->dev, "No room in flip buffer\n");
> return;
> }
>
> @@ -1155,7 +1155,7 @@ static int cpm_uart_init_port(struct device_node *np,
> if (!pinfo->clk) {
> data = of_get_property(np, "fsl,cpm-brg", &len);
> if (!data || len != 4) {
> - printk(KERN_ERR "CPM UART %pOFn has no/invalid "
> + dev_err(port->dev, "CPM UART %pOFn has no/invalid "
> "fsl,cpm-brg property.\n", np);
> return -EINVAL;
> }
> @@ -1164,7 +1164,7 @@ static int cpm_uart_init_port(struct device_node *np,
>
> data = of_get_property(np, "fsl,cpm-command", &len);
> if (!data || len != 4) {
> - printk(KERN_ERR "CPM UART %pOFn has no/invalid "
> + dev_err(port->dev, "CPM UART %pOFn has no/invalid "
> "fsl,cpm-command property.\n", np);
> return -EINVAL;
> }
> diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> index 6a1cd03..ef1ae08 100644
> --- a/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> +++ b/drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c
> @@ -67,7 +67,7 @@ void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port,
> return pram;
>
> if (len != 2) {
> - printk(KERN_WARNING "cpm_uart[%d]: device tree references "
> + dev_warn(port->dev, "cpm_uart[%d]: device tree references "
> "SMC pram, using boot loader/wrapper pram mapping. "
> "Please fix your device tree to reference the pram "
> "base register instead.\n",
>
Hi,
On 04/27/2019 12:51 PM, Enrico Weigelt, metux IT consult wrote:
> Hello folks,
>
>
> here's another attempt of polishing the serial drivers:
>
> * lots of minor cleanups to make checkpatch happier
> (eg. formatting, includes, inttypes, ...)
>
> * use appropriate logging helpers instead of printk()
>
> * consequent use of mapsize/mapbase fields:
> the basic idea is, all drivers should fill mapbase/mapbase fields at
> init time and later only use those fields, instead of hardcoded values
> (later on, we can add generic helpers for the map/unmap stuff, etc)
>
> * untwisting serial8250_port_size() at all:
> move the iomem size probing to initialization time, move out some
> platform specific magic to corresponding platform code, etc.
>
>
> Unfortunately, I don't have the actual hardware to really test all
> the code, so please let me know if there's something broken in here.
>
>
> have fun,
>
> --mtx
>
Got the following build error while compiling for my powerpc board with
your full series applied. No time to investigate though.
CC arch/powerpc/kernel/setup-common.o
In file included from ./include/linux/serial_8250.h:14:0,
from arch/powerpc/kernel/setup-common.c:33:
./include/linux/serial_core.h: In function ‘uart_memres_set_res’:
./include/linux/serial_core.h:446:18: error: ‘resource’ undeclared
(first use in this function)
port->iobase = resource->start;
^
./include/linux/serial_core.h:446:18: note: each undeclared identifier
is reported only once for each function it appears in
./include/linux/serial_core.h:450:2: error: ‘uart’ undeclared (first use
in this function)
uart->mapbase = res->start;
^
./include/linux/serial_core.h: In function ‘uart_memres_set_start_len’:
./include/linux/serial_core.h:464:6: error: ‘struct uart_driver’ has no
member named ‘mapbase’
uart->mapbase = start;
^
./include/linux/serial_core.h:465:6: error: ‘struct uart_driver’ has no
member named ‘mapsize’
uart->mapsize = len;
^
./include/linux/serial_core.h:466:6: error: ‘struct uart_driver’ has no
member named ‘iotype’
uart->iotype = UPIO_MEM;
^
make[3]: *** [arch/powerpc/kernel/setup-common.o] Error 1
Christophe
On 29.04.19 17:59, Christophe Leroy wrote:
> If we want to do something useful, wouldn't it make more sense to
> introduce the use of dev_err() in order to identify the faulting device
> in the message ?
Well, I could get the struct device* pointer via pinfo.port->dev,
but I wasn't entirely sure that it's always defined before these
functions could be called.
Shall I change it to dev_*() ?
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On 29.04.19 18:16, Christophe Leroy wrote:
Hi,
> Got the following build error while compiling for my powerpc board with
> your full series applied. No time to investigate though.
thanks, fixed it. That was the unclean patch where i've forgotten to
add 'rfc' into the title ... turned out that this one needs some
more rework :o
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On 29.04.19 17:19, Peter Korsgaard wrote:
>>>>>> "Enrico" == Enrico Weigelt, metux IT consult <[email protected]> writes:
>
> > Fill the struct uart_port->mapsize field and use it, insteaf of
>
> s/insteaf/instead/
Fixed.
> > hardcoded values in many places. This makes the code layout a bit
> > more consistent and easily allows using generic helpers for the
> > io memory handling.
>
> > Candidates for such helpers could be eg. the request+ioremap and
> > iounmap+release combinations.
>
> > Signed-off-by: Enrico Weigelt <[email protected]>
>
> Acked-by: Peter Korsgaard <[email protected]>
thanks for review.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On Mon, Apr 29, 2019 at 05:59:04PM +0200, Christophe Leroy wrote:
> Le 27/04/2019 ? 14:52, Enrico Weigelt, metux IT consult a ?crit?:
> > Fix checkpatch warnings by using pr_err():
> >
> > WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
> > #109: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c:109:
> > + printk(KERN_ERR
> >
> > WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
> > #128: FILE: drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c:128:
> > + printk(KERN_ERR
> >
> > WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
> > + printk(KERN_ERR
> >
> > WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
> > + printk(KERN_ERR
> >
> > Signed-off-by: Enrico Weigelt <[email protected]>
>
> Reviewed-by: Christophe Leroy <[email protected]>
>
> But is that really worth doing those changes ?
>
> If we want to do something useful, wouldn't it make more sense to introduce
> the use of dev_err() in order to identify the faulting device in the message
> ?
+1 for switching to dev_*().
--
With Best Regards,
Andy Shevchenko
Hi Enrico,
On 04/27/2019 03:52 PM, Enrico Weigelt, metux IT consult wrote:
> Fill the struct uart_port->mapsize field and use it, insteaf of
typo, s/insteaf/instead/
> hardcoded values in many places. This makes the code layout a bit
> more consistent and easily allows using generic helpers for the
> io memory handling.
>
> Candidates for such helpers could be eg. the request+ioremap and
> iounmap+release combinations.
>
> Signed-off-by: Enrico Weigelt <[email protected]>
Acked-by: Vladimir Zapolskiy <[email protected]>
--
Best wishes,
Vladimir
On Mon, 29 Apr 2019, Greg KH wrote:
> > >> drivers/tty/serial/dz.c | 8 ++++----
> > >
> > > Do you have this hardware to test any of these changes with?
> >
> > Unfortunately not :(
>
> Then I can take the "basic" types of patches for the driver (like this
> one), but not any others, sorry.
I can verify changes to dz.c, sb1250-duart.c and zs.c with real hardware,
but regrettably not right away: the hardware is in a remote location and
while I have it wired for remote operation unfortunately its connectivity
has been cut off by an unfriendly ISP.
I'm not sure if all the changes make sense though: if there is a compiler
warning or a usability issue, then a patch is surely welcome, otherwise:
"If it ain't broke, don't fix it".
Maciej
On Sat, 27 Apr 2019, Enrico Weigelt, metux IT consult wrote:
> diff --git a/drivers/tty/serial/sb1250-duart.c b/drivers/tty/serial/sb1250-duart.c
> index 329aced..655961c 100644
> --- a/drivers/tty/serial/sb1250-duart.c
> +++ b/drivers/tty/serial/sb1250-duart.c
> @@ -663,7 +663,6 @@ static void sbd_release_port(struct uart_port *uport)
>
> static int sbd_map_port(struct uart_port *uport)
> {
> - const char *err = KERN_ERR "sbd: Cannot map MMIO\n";
> struct sbd_port *sport = to_sport(uport);
> struct sbd_duart *duart = sport->duart;
>
> @@ -671,7 +670,7 @@ static int sbd_map_port(struct uart_port *uport)
> uport->membase = ioremap_nocache(uport->mapbase,
> DUART_CHANREG_SPACING);
> if (!uport->membase) {
> - printk(err);
> + dev_err(uport->dev, "Cannot map MMIO (base)\n");
> return -ENOMEM;
> }
>
> @@ -679,7 +678,7 @@ static int sbd_map_port(struct uart_port *uport)
> sport->memctrl = ioremap_nocache(duart->mapctrl,
> DUART_CHANREG_SPACING);
> if (!sport->memctrl) {
> - printk(err);
> + dev_err(uport->dev, "Cannot map MMIO (ctrl)\n");
> iounmap(uport->membase);
> uport->membase = NULL;
> return -ENOMEM;
Hmm, what's the point to have separate messages, which consume extra
memory, for a hardly if at all possible error condition?
Maciej