| Message ID | 20211230102110.3861-3-yu.tu@amlogic.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | the UART driver compatible with the Amlogic Meson | expand |
Hi Greg, On Thu, Dec 30, 2021 at 1:29 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: [...] > > static void meson_uart_release_port(struct uart_port *port) > > { > > - devm_iounmap(port->dev, port->membase); > > - port->membase = NULL; > > - devm_release_mem_region(port->dev, port->mapbase, port->mapsize); > > + /* nothing to do */ > > } > > Are you sure a release call like this can be "empty"? That goes against > the normal way the driver model works. If it is empty, why have it at > all? In patch #4 from this series some logic is added here again. I can think of three options here (in no particular order): - keep this patch as-is - remove the empty function from this patch and it back in patch #4 from this series - try to split some of the logic from patch #4 so the relevant clk_prepare_enable/clk_disable_unpepare calls are added here (meaning that this function has logic inside it at all times throughout this series) - (if you have another suggestion then please let us know) Best regards, Martin
diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c index c9a37602ffd0..99efe62a1507 100644 --- a/drivers/tty/serial/meson_uart.c +++ b/drivers/tty/serial/meson_uart.c @@ -397,24 +397,11 @@ static int meson_uart_verify_port(struct uart_port *port, static void meson_uart_release_port(struct uart_port *port) { - devm_iounmap(port->dev, port->membase); - port->membase = NULL; - devm_release_mem_region(port->dev, port->mapbase, port->mapsize); + /* nothing to do */ } static int meson_uart_request_port(struct uart_port *port) { - if (!devm_request_mem_region(port->dev, port->mapbase, port->mapsize, - dev_name(port->dev))) { - dev_err(port->dev, "Memory region busy\n"); - return -EBUSY; - } - - port->membase = devm_ioremap(port->dev, port->mapbase, - port->mapsize); - if (!port->membase) - return -ENOMEM; - return 0; } @@ -728,6 +715,10 @@ static int meson_uart_probe(struct platform_device *pdev) if (!port) return -ENOMEM; + port->membase = devm_ioremap_resource(&pdev->dev, res_mem); + if (IS_ERR(port->membase)) + return PTR_ERR(port->membase); + ret = meson_uart_probe_clocks(pdev, port); if (ret) return ret; @@ -749,10 +740,7 @@ static int meson_uart_probe(struct platform_device *pdev) platform_set_drvdata(pdev, port); /* reset port before registering (and possibly registering console) */ - if (meson_uart_request_port(port) >= 0) { - meson_uart_reset(port); - meson_uart_release_port(port); - } + meson_uart_reset(port); ret = uart_add_one_port(&meson_uart_driver, port); if (ret)
This simplifies resetting the UART controller during probe and will make it easier to integrate the common clock code which will require the registers at probe time as well. Signed-off-by: Yu Tu <yu.tu@amlogic.com> --- drivers/tty/serial/meson_uart.c | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-)