Run scripts/checkpatch.pl --strict on your patch.
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#15:
In the execution logic of the lpss8250_probe() function, the function may directly return via a return statement at either line 347 or line 351.
WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: 12dadc409c2b ("Linux 6.8.7")'
#19:
Fixes: e88c4cfcb7b888ac374916806f86c17d8ecaeb67
On Fri, Apr 26, 2024 at 12:47:16PM +0100, lumingyindetect@xxxxxxx wrote:
> From: LuMingYin <11570291+yin-luming@xxxxxxxxxxxxxxxxxxxxxx>
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Fix this email address.
>
> In the execution logic of the lpss8250_probe() function, the function may directly return via a return statement at either line 347 or line 351.
The line numbers are not important and they change all the time.
Instead say "if pcim_iomap() or lpss->board->setup() fail then ...".
> Unlike lines 357 or 361, where the return statement is used directly without releasing the dynamically allocated memory region pointed to by the variable pdev, causing a memory leak of the variable pdev.
> In the lpss8250_probe() function, I added a label named "free_irq_vectors" to release the dynamically allocated memory region pointed to by the variable pdev, and replaced the two return statements mentioned above with goto statements to this label.
Just say "Use a goto to release this memory". No need to explain
further.
>
> Fixes: e88c4cfcb7b888ac374916806f86c17d8ecaeb67
This is the wrong hash and the format is wrong. It should be:
Fixes: 254cc7743e84 ("serial: 8250_lpss: Switch over to MSI interrupts")
>
> Signed-off-by: LuMingYin <11570291+yin-luming@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/tty/serial/8250/8250_lpss.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
> index c3cd6cb9ac80..fa9fd4dc86c7 100644
> --- a/drivers/tty/serial/8250/8250_lpss.c
> +++ b/drivers/tty/serial/8250/8250_lpss.c
> @@ -344,11 +344,11 @@ static int lpss8250_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> uart.port.mapbase = pci_resource_start(pdev, 0);
> uart.port.membase = pcim_iomap(pdev, 0, 0);
> if (!uart.port.membase)
> - return -ENOMEM;
> + goto free_irq_vectors;
This needs to be:
if (!uart.port.membase) {
ret = -ENOMEM;
goto free_irq_vectors;
}
regards,
dan carpenter
>
> ret = lpss->board->setup(lpss, &uart.port);
> if (ret)
> - return ret;
> + goto free_irq_vectors;
>
> dw8250_setup_port(&uart.port);
>
> @@ -367,6 +367,7 @@ static int lpss8250_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>
> err_exit:
> lpss->board->exit(lpss);
> +free_irq_vectors:
> pci_free_irq_vectors(pdev);
> return ret;
> }
