[PATCH] serial: ns16550: Fix device-tree probe and add proper teardown
Jules Maselbas
jmaselbas at kalray.eu
Fri Mar 3 05:44:41 PST 2023
This fix a new issue introduced by a previous patch that fixes a
previous issue.
The previous issue is related to the memory resource not being released
in case the probe fails or is deferred, hence if deferred the probe will
always fail to request the same memory resource. The proposed solution
was to move the resource request after the clock initialization which
was causing the driver to be deferred.
This introduced a new issue when using information from the device-tree,
the function ns16550_probe_dt() cannot easily be moved around as it:
- adds offset to mmiobase, thus need to be called after ns16550_init_iomem()
- sets the clock frequency, thus need to be called before clock init
- sets {read,write}_reg callbacks using the width read from the device-tree,
and is expected to override previous callbacks sets when calling
ns16550_init_{iomem,ioport}
To fix this, both ns16550_init_iomem() and ns16550_init_ioport() functions
are fused and return the requested region. In case of an error during the
probe the resource is be properly released and can requested again if needed.
Fixes b32172c2f9 ("serial: ns16550: move iomem/ioport init after clock init")
Signed-off-by: Jules Maselbas <jmaselbas at kalray.eu>
---
drivers/serial/serial_ns16550.c | 120 +++++++++++++++++---------------
1 file changed, 64 insertions(+), 56 deletions(-)
diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c
index 9ce4feed43..b10a26cf21 100644
--- a/drivers/serial/serial_ns16550.c
+++ b/drivers/serial/serial_ns16550.c
@@ -385,23 +385,51 @@ static __maybe_unused struct ns16550_drvdata rpi_drvdata = {
.linux_earlycon_name = "bcm2835aux",
};
-static int ns16550_init_iomem(struct device *dev, struct ns16550_priv *priv)
+/**
+ * @return the requested resource to be properly released in case probe fail
+ */
+static struct resource *ns16550_init_iores(struct device *dev, struct ns16550_priv *priv)
{
- struct resource *iores;
struct resource *res;
- int width;
+ struct resource *iores;
+ unsigned long flags, width;
res = dev_get_resource(dev, IORESOURCE_MEM, 0);
if (IS_ERR(res))
- return PTR_ERR(res);
+ res = dev_get_resource(dev, IORESOURCE_IO, 0);
+ if (IS_ERR(res))
+ return res;
+
+ flags = res->flags & (IORESOURCE_MEM_TYPE_MASK | IORESOURCE_IO);
- iores = dev_request_mem_resource(dev, 0);
+ if (flags & IORESOURCE_IO)
+ iores = request_ioport_region(dev_name(dev), res->start, res->end);
+ else
+ iores = request_iomem_region(dev_name(dev), res->start, res->end);
if (IS_ERR(iores))
- return PTR_ERR(iores);
- priv->mmiobase = IOMEM(iores->start);
+ return iores;
- width = res->flags & IORESOURCE_MEM_TYPE_MASK;
- switch (width) {
+ if (flags & IORESOURCE_IO)
+ priv->iobase = iores->start;
+ else
+ priv->mmiobase = IOMEM(iores->start);
+
+ switch (flags) {
+ case IORESOURCE_IO | IORESOURCE_MEM_8BIT:
+ priv->read_reg = ns16550_read_reg_ioport_8;
+ priv->write_reg = ns16550_write_reg_ioport_8;
+ priv->access_type = "io";
+ break;
+ case IORESOURCE_IO | IORESOURCE_MEM_16BIT:
+ priv->read_reg = ns16550_read_reg_ioport_16;
+ priv->write_reg = ns16550_write_reg_ioport_16;
+ priv->access_type = "io";
+ break;
+ case IORESOURCE_IO | IORESOURCE_MEM_32BIT:
+ priv->read_reg = ns16550_read_reg_ioport_32;
+ priv->write_reg = ns16550_write_reg_ioport_32;
+ priv->access_type = "io";
+ break;
case IORESOURCE_MEM_8BIT:
priv->read_reg = ns16550_read_reg_mmio_8;
priv->write_reg = ns16550_write_reg_mmio_8;
@@ -417,45 +445,13 @@ static int ns16550_init_iomem(struct device *dev, struct ns16550_priv *priv)
priv->write_reg = ns16550_write_reg_mmio_32;
priv->access_type = "mmio32";
break;
- }
-
- return 0;
-}
-
-static int ns16550_init_ioport(struct device *dev, struct ns16550_priv *priv)
-{
- struct resource *res;
- int width;
-
- res = dev_get_resource(dev, IORESOURCE_IO, 0);
- if (IS_ERR(res))
- return PTR_ERR(res);
-
- res = request_ioport_region(dev_name(dev), res->start, res->end);
- if (IS_ERR(res))
- return PTR_ERR(res);
-
- priv->iobase = res->start;
-
- width = res->flags & IORESOURCE_MEM_TYPE_MASK;
- switch (width) {
- case IORESOURCE_MEM_8BIT:
- priv->read_reg = ns16550_read_reg_ioport_8;
- priv->write_reg = ns16550_write_reg_ioport_8;
- break;
- case IORESOURCE_MEM_16BIT:
- priv->read_reg = ns16550_read_reg_ioport_16;
- priv->write_reg = ns16550_write_reg_ioport_16;
- break;
- case IORESOURCE_MEM_32BIT:
- priv->read_reg = ns16550_read_reg_ioport_32;
- priv->write_reg = ns16550_write_reg_ioport_32;
+ default:
+ width = flags & IORESOURCE_MEM_TYPE_MASK;
+ dbg_printf("%s: unsupported width %ld\n", dev->name, width);
break;
}
- priv->access_type = "io";
-
- return 0;
+ return iores;
}
/**
@@ -473,12 +469,19 @@ static int ns16550_probe(struct device *dev)
struct console_device *cdev;
struct NS16550_plat *plat = (struct NS16550_plat *)dev->platform_data;
const struct ns16550_drvdata *devtype;
+ struct resource *iores;
int ret;
devtype = device_get_match_data(dev) ?: &ns16550_drvdata;
priv = xzalloc(sizeof(*priv));
+ iores = ns16550_init_iores(dev, priv);
+ if (IS_ERR(iores)) {
+ ret = PTR_ERR(iores);
+ goto err;
+ }
+
if (plat)
priv->plat = *plat;
else
@@ -492,25 +495,20 @@ static int ns16550_probe(struct device *dev)
if (IS_ERR(priv->clk)) {
ret = PTR_ERR(priv->clk);
dev_err(dev, "failed to get clk (%d)\n", ret);
- goto err;
+ goto release_region;
}
- clk_enable(priv->clk);
+ ret = clk_enable(priv->clk);
+ if (ret)
+ goto clk_put;
priv->plat.clock = clk_get_rate(priv->clk);
}
if (priv->plat.clock == 0) {
dev_err(dev, "no valid clockrate\n");
ret = -EINVAL;
- goto err;
+ goto clk_disable;
}
- ret = ns16550_init_iomem(dev, priv);
- if (ret)
- ret = ns16550_init_ioport(dev, priv);
-
- if (ret)
- goto err;
-
cdev = &priv->cdev;
cdev->dev = dev;
cdev->tstc = ns16550_tstc;
@@ -528,8 +526,18 @@ static int ns16550_probe(struct device *dev)
devtype->init_port(cdev);
- return console_register(cdev);
+ ret = console_register(cdev);
+ if (ret)
+ goto clk_disable;
+
+ return 0;
+clk_disable:
+ clk_disable(priv->clk);
+clk_put:
+ clk_put(priv->clk);
+release_region:
+ release_region(iores);
err:
free(priv);
--
2.17.1
More information about the barebox
mailing list