* [PATCH] serial: pl011: Fix error handling in pl011_init()
@ 2022-11-09 8:46 Gaosheng Cui
2022-11-09 9:10 ` Greg KH
0 siblings, 1 reply; 2+ messages in thread
From: Gaosheng Cui @ 2022-11-09 8:46 UTC (permalink / raw)
To: linux, gregkh, jirislaby, andre.przywara, cuigaosheng1; +Cc: linux-serial
When amba_driver_register failed, we need to unregister the platform
driver which have been registered, otherwise there maybe resource leak,
so we add error handlings in pl011_init() to fix it.
Fixes: 0dd1e247fd39 ("drivers: PL011: add support for the ARM SBSA generic UART")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
drivers/tty/serial/amba-pl011.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 6d8552506091..ef6d9941f972 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2982,11 +2982,23 @@ static struct amba_driver pl011_driver = {
static int __init pl011_init(void)
{
+ int ret;
+
printk(KERN_INFO "Serial: AMBA PL011 UART driver\n");
- if (platform_driver_register(&arm_sbsa_uart_platform_driver))
+ ret = platform_driver_register(&arm_sbsa_uart_platform_driver);
+ if (unlikely(ret)) {
pr_warn("could not register SBSA UART platform driver\n");
- return amba_driver_register(&pl011_driver);
+ return ret;
+ }
+
+ ret = amba_driver_register(&pl011_driver);
+ if (unlikely(ret)) {
+ platform_driver_unregister(&arm_sbsa_uart_platform_driver);
+ return ret;
+ }
+
+ return 0;
}
static void __exit pl011_exit(void)
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] serial: pl011: Fix error handling in pl011_init()
2022-11-09 8:46 [PATCH] serial: pl011: Fix error handling in pl011_init() Gaosheng Cui
@ 2022-11-09 9:10 ` Greg KH
0 siblings, 0 replies; 2+ messages in thread
From: Greg KH @ 2022-11-09 9:10 UTC (permalink / raw)
To: Gaosheng Cui; +Cc: linux, jirislaby, andre.przywara, linux-serial
On Wed, Nov 09, 2022 at 04:46:26PM +0800, Gaosheng Cui wrote:
> When amba_driver_register failed, we need to unregister the platform
> driver which have been registered, otherwise there maybe resource leak,
> so we add error handlings in pl011_init() to fix it.
>
> Fixes: 0dd1e247fd39 ("drivers: PL011: add support for the ARM SBSA generic UART")
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> ---
> drivers/tty/serial/amba-pl011.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 6d8552506091..ef6d9941f972 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2982,11 +2982,23 @@ static struct amba_driver pl011_driver = {
>
> static int __init pl011_init(void)
> {
> + int ret;
> +
> printk(KERN_INFO "Serial: AMBA PL011 UART driver\n");
>
> - if (platform_driver_register(&arm_sbsa_uart_platform_driver))
> + ret = platform_driver_register(&arm_sbsa_uart_platform_driver);
> + if (unlikely(ret)) {
Only use likely/unlikely if you can prove with a benchmark that it makes
a measurable change to do so.
Here, on the init error path, it will never happen.
And how are you causing this call to fail?
How was this tested?
> pr_warn("could not register SBSA UART platform driver\n");
> - return amba_driver_register(&pl011_driver);
> + return ret;
> + }
> +
> + ret = amba_driver_register(&pl011_driver);
> + if (unlikely(ret)) {
Again, never use unlikely() like this.
And again, how did you get this to fail under normal operation? And how
was this tested?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-11-09 9:11 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-09 8:46 [PATCH] serial: pl011: Fix error handling in pl011_init() Gaosheng Cui
2022-11-09 9:10 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.