RFC: cleaning up the serial drivers and use struct resource

RFC: cleaning up the serial drivers and use struct resource

[Enrico Weigelt, metux IT consult]

Hello folks,

I'm currently working on some cleanups in drivers/tty/serial.

There're several cases where new helpers, like devm_platform_ioremap_resource
can be used, other places can use devm_ioremap_resource() for a bit
cleaner code.

Another topic here is using struct resource, instead of separate fields
(BTW: struct uart_port->mapsize doesn't seem to be used consequently):
in struct uart_port, I'm adding a struct resource field for holding the
port's iomem range, and helpers for it. Then, I'm step by step patching
the individual drivers to use that, instead of the old fields, directly.

For now, this all is early, untested and not yet meant for mainline.
I'd just like to have your oppions on my approach.

What do you think about it ?


thx
--mtx

[PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()

[Enrico Weigelt, metux IT consult]

---
 drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c
index bd53661..0738d14 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -25,7 +25,6 @@ struct bcm2835aux_data {
 static int bcm2835aux_serial_probe(struct platform_device *pdev)
 {
 	struct bcm2835aux_data *data;
-	struct resource *res;
 	int ret;
 
 	/* allocate the custom structure */
@@ -63,15 +62,12 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
 	data->uart.port.irq = ret;
 
 	/* map the main registers */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "memory resource not found");
-		return -EINVAL;
-	}
-	data->uart.port.membase = devm_ioremap_resource(&pdev->dev, res);
+	data->uart.port.membase = devm_platform_ioremap_resource(pdev, 0);
 	ret = PTR_ERR_OR_ZERO(data->uart.port.membase);
-	if (ret)
+	if (ret) {
+		dev_err(&pdev->dev, "could not map memory resource");
 		return ret;
+	}
 
 	/* Check for a fixed line number */
 	ret = of_alias_get_id(pdev->dev.of_node, "serial");
-- 
1.9.1

[PATCH 2/8] drivers: tty: serial: 8250_dw: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

Use helper devm_ioremap_resource() to make the code a little bit
shorter and easier to read.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/8250/8250_dw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index d31b975..f0a294d 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -526,7 +526,7 @@ static int dw8250_probe(struct platform_device *pdev)
 	p->set_ldisc	= dw8250_set_ldisc;
 	p->set_termios	= dw8250_set_termios;
 
-	p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
+	p->membase = devm_ioremap_resource(dev, regs);
 	if (!p->membase)
 		return -ENOMEM;
 
-- 
1.9.1

[PATCH 3/8] drivers: tty: serial: 8250_ingenic: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

Use helper devm_ioremap_resource() to make the code a little bit
shorter and easier to read.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/8250/8250_ingenic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
index 424c07c..90ae3e2 100644
--- a/drivers/tty/serial/8250/8250_ingenic.c
+++ b/drivers/tty/serial/8250/8250_ingenic.c
@@ -249,8 +249,7 @@ static int ingenic_uart_probe(struct platform_device *pdev)
 	if (line >= 0)
 		uart.port.line = line;
 
-	uart.port.membase = devm_ioremap(&pdev->dev, regs->start,
-					 resource_size(regs));
+	uart.port.membase = devm_ioremap_resource(&pdev->dev, regs);
 	if (!uart.port.membase)
 		return -ENOMEM;
 
-- 
1.9.1

[PATCH 4/8] drivers: tty: serial: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

instead of fetching out start and len from a struct resource for
passing it to devm_ioremap(), directly use devm_ioremap_resource()

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/8250/8250_lpc18xx.c  | 3 +--
 drivers/tty/serial/8250/8250_mtk.c      | 3 +--
 drivers/tty/serial/8250/8250_uniphier.c | 2 +-
 drivers/tty/serial/amba-pl010.c         | 3 +--
 drivers/tty/serial/samsung.c            | 2 +-
 drivers/tty/serial/sirfsoc_uart.c       | 3 +--
 6 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index eddf119..f36902b 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -119,8 +119,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
 
 	memset(&uart, 0, sizeof(uart));
 
-	uart.port.membase = devm_ioremap(&pdev->dev, res->start,
-					 resource_size(res));
+	uart.port.membase = devm_ioremap_resource(&pdev->dev, res);
 	if (!uart.port.membase)
 		return -ENOMEM;
 
diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index c1fdbc0..bf6eb8d 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -383,8 +383,7 @@ static int mtk8250_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	uart.port.membase = devm_ioremap(&pdev->dev, regs->start,
-					 resource_size(regs));
+	uart.port.membase = devm_ioremap_resource(&pdev->dev, regs);
 	if (!uart.port.membase)
 		return -ENOMEM;
 
diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
index 164ba13..9c1244e 100644
--- a/drivers/tty/serial/8250/8250_uniphier.c
+++ b/drivers/tty/serial/8250/8250_uniphier.c
@@ -171,7 +171,7 @@ static int uniphier_uart_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	membase = devm_ioremap(dev, regs->start, resource_size(regs));
+	membase = devm_ioremap_resource(dev, regs);
 	if (!membase)
 		return -ENOMEM;
 
diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index 2c37d11..109b8df 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -713,8 +713,7 @@ static int pl010_probe(struct amba_device *dev, const struct amba_id *id)
 	if (!uap)
 		return -ENOMEM;
 
-	base = devm_ioremap(&dev->dev, dev->res.start,
-			    resource_size(&dev->res));
+	base = devm_ioremap_resource(&dev->dev, &dev->res);
 	if (!base)
 		return -ENOMEM;
 
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 83fd516..a49cf15 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1775,7 +1775,7 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
 
 	dbg("resource %pR)\n", res);
 
-	port->membase = devm_ioremap(port->dev, res->start, resource_size(res));
+	port->membase = devm_ioremap_resource(port->dev, res);
 	if (!port->membase) {
 		dev_err(port->dev, "failed to remap controller address\n");
 		return -EBUSY;
diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
index 38622f2..db2e08d 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -1359,8 +1359,7 @@ static int sirfsoc_uart_probe(struct platform_device *pdev)
 		goto err;
 	}
 	port->mapbase = res->start;
-	port->membase = devm_ioremap(&pdev->dev,
-			res->start, resource_size(res));
+	port->membase = devm_ioremap_resource(&pdev->dev, res);
 	if (!port->membase) {
 		dev_err(&pdev->dev, "Cannot remap resource.\n");
 		ret = -ENOMEM;
-- 
1.9.1

[PATCH 5/8] drivers: tty: serial: introduce struct resource

[Enrico Weigelt, metux IT consult]

The standard data structure for holding io resources in the
kernel is struct resource. Serial drivers yet don't really
use it (except when retrieving from oftree).

This patch introduces a new field in struct uart_port for that,
plus several helpers. Yet it's up to the individual drivers for
using it - serial_core doesn't care yet. Therefore, the old fields
mapbase and mapsize need to be filled properly by the drivers.
---
 include/linux/serial_core.h | 49 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 5fe2b03..9b07a40 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -24,6 +24,7 @@
 #include <linux/compiler.h>
 #include <linux/console.h>
 #include <linux/interrupt.h>
+#include <linux/ioport.h>
 #include <linux/circ_buf.h>
 #include <linux/spinlock.h>
 #include <linux/sched.h>
@@ -256,6 +257,7 @@ struct uart_port {
 	unsigned int		minor;
 	resource_size_t		mapbase;		/* for ioremap */
 	resource_size_t		mapsize;
+	struct resource		memres;
 	struct device		*dev;			/* parent device */
 	unsigned char		hub6;			/* this should be in the 8250 driver */
 	unsigned char		suspended;
@@ -427,6 +429,53 @@ void uart_console_write(struct uart_port *port, const char *s,
 int uart_match_port(struct uart_port *port1, struct uart_port *port2);
 
 /*
+ * Port resource management
+ */
+static inline void uart_memres_set(struct uart_port *port,
+				     struct resource res)
+{
+	port->memres = res;
+	port->mapbase = res.start;
+}
+
+static inline void uart_memres_clear(struct uart_port *port)
+{
+	port->memres = DEFINE_RES_MEM(0, 0);
+}
+
+static inline int uart_memres_valid(struct uart_port *port)
+{
+	return (port->memres.start != 0);
+}
+
+static inline struct resource *uart_memres_request(struct uart_port *port,
+				     const char *name)
+{
+	return request_mem_region(
+		port->memres.start,
+		resource_size(&port->memres),
+		name);
+}
+
+static inline void uart_memres_release(struct uart_port *port)
+{
+	return release_mem_region(port->memres.start,
+				  resource_size(&port->memres));
+}
+
+static inline void __iomem *uart_memres_ioremap_nocache(struct uart_port *port)
+{
+	return ioremap_nocache(port->memres.start,
+			       resource_size(&port->memres.start));
+}
+
+static inline void __iomem *uart_memres_ioremap(struct uart_port *port)
+{
+	return ioremap(port->memres.start,
+		       resource_size(&port->memres.start));
+}
+
+/*
  * Power Management
  */
 int uart_suspend_port(struct uart_driver *reg, struct uart_port *port);
-- 
1.9.1

[PATCH 6/8] drivers: tty: serial: vt8500: use memres

[Enrico Weigelt, metux IT consult]

---
 drivers/tty/serial/vt8500_serial.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index 3d58e9b..331a9dd 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -696,13 +696,13 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 				      );
 	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;
 	vt8500_port->uart.line = port;
 	vt8500_port->uart.dev = &pdev->dev;
 	vt8500_port->uart.flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF;
+	uart_memres_set(&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) /
-- 
1.9.1

[PATCH 7/8] drivers: tty: serial: use memres

[Enrico Weigelt, metux IT consult]

---
 drivers/tty/serial/zs.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/zs.c b/drivers/tty/serial/zs.c
index b03d3e4..2fd4821 100644
--- a/drivers/tty/serial/zs.c
+++ b/drivers/tty/serial/zs.c
@@ -986,14 +986,13 @@ static void zs_release_port(struct uart_port *uport)
 {
 	iounmap(uport->membase);
 	uport->membase = 0;
-	release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
+	uart_memres_release(uport);
 }
 
 static int zs_map_port(struct uart_port *uport)
 {
 	if (!uport->membase)
-		uport->membase = ioremap_nocache(uport->mapbase,
-						 ZS_CHAN_IO_SIZE);
+		uport->membase = uart_memres_ioremap_nocache(uport);
 	if (!uport->membase) {
 		printk(KERN_ERR "zs: Cannot map MMIO\n");
 		return -ENOMEM;
@@ -1005,13 +1004,13 @@ static int zs_request_port(struct uart_port *uport)
 {
 	int ret;
 
-	if (!request_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE, "scc")) {
+	if (!uart_memres_request(uport, "scc")) {
 		printk(KERN_ERR "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);
+		uart_release_memres(uport);
 		return ret;
 	}
 	return 0;
@@ -1113,9 +1112,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->mapbase	= dec_kn_slot_base +
-					  zs_parms.scc[chip] +
-					  (side ^ ZS_CHAN_B) * ZS_CHAN_IO_SIZE;
+			uart_memres_set(
+				DEFINE_RES_MEM(
+					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];
-- 
1.9.1

[PATCH 8/8] drivers: tty: serial: xilinx_uartps: use helpers

[Enrico Weigelt, metux IT consult]

---
 drivers/tty/serial/xilinx_uartps.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 74089f5..92aff38 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -953,15 +953,14 @@ 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,
-					 CDNS_UART_NAME)) {
+	if (!uart_memres_request(port, CDNS_UART_NAME)) {
 		return -ENOMEM;
 	}
 
-	port->membase = ioremap(port->mapbase, CDNS_UART_REGISTER_SPACE);
+	port->membase = uart_memres_ioremap(port);
 	if (!port->membase) {
 		dev_err(port->dev, "Unable to map registers\n");
-		release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+		uart_memres_release(port);
 		return -ENOMEM;
 	}
 	return 0;
@@ -976,7 +975,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);
+	uart_memres_release(port);
 	iounmap(port->membase);
 	port->membase = NULL;
 }
@@ -1626,7 +1625,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;
+	uart_memres_set(*res);
 	port->irq = irq;
 	port->dev = &pdev->dev;
 	port->uartclk = clk_get_rate(cdns_uart_data->uartclk);
@@ -1707,7 +1706,7 @@ static int cdns_uart_remove(struct platform_device *pdev)
 			&cdns_uart_data->clk_rate_change_nb);
 #endif
 	rc = uart_remove_one_port(cdns_uart_data->cdns_uart_driver, port);
-	port->mapbase = 0;
+	uart_memres_clear(port);
 	mutex_lock(&bitmap_lock);
 	if (cdns_uart_data->id < MAX_UART_INSTANCES)
 		clear_bit(cdns_uart_data->id, bitmap);
-- 
1.9.1

Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()

[Greg KH]

On Tue, Mar 12, 2019 at 03:57:33PM +0100, Enrico Weigelt, metux IT consult wrote:
> ---
>  drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)

No changelog or signed-off-by, sorry, please fix up the series and
resend.

thanks,

greg k-h

Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()

[Enrico Weigelt, metux IT consult]

On 12.03.19 17:33, Greg KH wrote:
> On Tue, Mar 12, 2019 at 03:57:33PM +0100, Enrico Weigelt, metux IT consult wrote:
>> ---
>>  drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> No changelog or signed-off-by, sorry, please fix up the series and
> resend.

Maybe the threading got messed up somehow (at least my tbird doesn't
show it correctly), so you've probably didn't see my intro letter: there
I wrote that this is yet WIP, not meant for actual submission, instead
just to ask you folks whether my approach in general would be work.

Sorry for the confusion.

Of course, when the stuff becomes ready for submission, all these
things will be cleaned up.


OTOH, if you think some of the patches indeed were ready, but yet need
some cleanups (like mentioned missing description), just let me know.


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()

[Miguel Ojeda]

On Wed, Mar 13, 2019 at 8:03 AM Enrico Weigelt, metux IT consult
<lkml@metux.net> wrote:
>
> On 12.03.19 17:33, Greg KH wrote:
> > On Tue, Mar 12, 2019 at 03:57:33PM +0100, Enrico Weigelt, metux IT consult wrote:
> >> ---
> >>  drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
> >>  1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > No changelog or signed-off-by, sorry, please fix up the series and
> > resend.
>
> Maybe the threading got messed up somehow (at least my tbird doesn't
> show it correctly), so you've probably didn't see my intro letter: there

The intro letter seems to be an independent message with a different
subject line -- not sure what threading you expected (?).

> I wrote that this is yet WIP, not meant for actual submission, instead
> just to ask you folks whether my approach in general would be work.

That will indeed cause confusion. If you are requesting comments only,
please do so placing [RFC] or similar in the subject of each patch
(you did in the letter, but that had a different title, and it is
not how cover letters are usually sent, they are normally patch 0).

Another option is sending a message with the repo URL where
development is happening, so interested parties can take a look.

However:

> Of course, when the stuff becomes ready for submission, all these
> things will be cleaned up.

Even if you are sending some draft patches, it is still a bad idea to
send them incomplete. Basically you are making it harder for early
reviewers simply by not having written an explanation or at least a
reference to the explanation.

Cheers,
Miguel

Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()

[Enrico Weigelt, metux IT consult]

On 13.03.19 12:28, Miguel Ojeda wrote:

Hi,

> The intro letter seems to be an independent message with a different> subject line -- not sure what threading you expected (?).
hmm, I've sent it via git-send-email --compose. IMHO, this should
send the intro as the first message and the actual patches as reply.

But it seems that didn't work as I hoped.

>> I wrote that this is yet WIP, not meant for actual submission, instead>> just to ask you folks whether my approach in general would be work.>
> That will indeed cause confusion. If you are requesting comments
only,> please do so placing [RFC] or similar in the subject of each patch

Ah, good idea. So I also have this tag in the git repo.
BTW: are there more such conventions I should be aware of ?

> Another option is sending a message with the repo URL where> development is happening, so interested parties can take a look.
Ok. I had the impression that people here don't really like to look
at some git repos for reviews.

> Even if you are sending some draft patches, it is still a bad idea to> send them incomplete. Basically you are making it harder for early>
reviewers simply by not having written an explanation or at least a>
reference to the explanation.
Understood.


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()

[Greg KH]

On Wed, Mar 13, 2019 at 07:59:53AM +0100, Enrico Weigelt, metux IT consult wrote:
> On 12.03.19 17:33, Greg KH wrote:
> > On Tue, Mar 12, 2019 at 03:57:33PM +0100, Enrico Weigelt, metux IT consult wrote:
> >> ---
> >>  drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
> >>  1 file changed, 4 insertions(+), 8 deletions(-)
> > 
> > No changelog or signed-off-by, sorry, please fix up the series and
> > resend.
> 
> Maybe the threading got messed up somehow (at least my tbird doesn't
> show it correctly), so you've probably didn't see my intro letter: there
> I wrote that this is yet WIP, not meant for actual submission, instead
> just to ask you folks whether my approach in general would be work.
> 

Threading worked just fine, but you still shouldn't send out patches
like this without any independant information in the patch itself saying
what it does and what it is for.  That's just bad engineering :)

Also, usually RFC like patches and series are ignored, I know I almost
always ignore them because if the author doesn't think they are ready to
be reviewed, why would I spend time on them compared to other patches
that their authors think are ready for review.

Remember, patch review is the most limited resource we have.

thanks,

greg k-h

Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()

[Enrico Weigelt, metux IT consult]

On 13.03.19 15:36, Greg KH wrote:

> Also, usually RFC like patches and series are ignored, I know I almost
> always ignore them because if the author doesn't think they are ready to
> be reviewed, why would I spend time on them compared to other patches
> that their authors think are ready for review.

Okay, but then, how is the best way to ask for help ?

In my case here, there're several things still unclear to me, eg:

* struct uart_port -> mapbase:

=> seems to be used by most drivers, but some also clear it later, eg.
   uartlite does so when uart_add_one_port() call failed. is that
   really necessary ?

* struct uart_port -> mapsize:

=> seems to be used only rarely. most drivers use either fixed params
   on the memory mapping functions, or take it from some function.
   wouldn't it be better if all would fill out that field and later
   use it consistently ?


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287