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