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@xxxxxxxxxx>
> ---
> 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
