* Re: [PATCH v2] soc: ti: knav_qmss_queue: free resources in remove callback (v2)
2025-11-10 22:44 ` [PATCH v2] soc: ti: knav_qmss_queue: free resources in remove callback (v2) Md Shofiqul Islam
@ 2025-11-12 15:16 ` Nishanth Menon
0 siblings, 0 replies; 2+ messages in thread
From: Nishanth Menon @ 2025-11-12 15:16 UTC (permalink / raw)
To: Md Shofiqul Islam; +Cc: ssantosh, linux-arm-kernel, linux-kernel
On 00:44-20251111, Md Shofiqul Islam wrote:
>
> Hi Nishanth,
>
> Thanks for reviewing!
>
> This is version 2 (v2) of the patch, not the same as before.
> I have fixed the formatting and indentation issues you pointed out.
> The patch now passes checkpatch.pl without any warnings or errors.
>
> Changes in v2:
> - Fixed indentation and alignment issues reported by checkpatch
> - Wrapped commit message lines to within 75 characters
>
> Thanks again for your feedback!
>
> Regards,
> Md Shofiqul Islam
>
> Implement the TODO in knav_queue_remove() by stopping PDSPs and
> freeing queue regions and queue ranges before disabling runtime PM,
> mirroring the allocations performed in the probe path.
>
> This ensures resources are released on driver unbind and avoids
> leaking queue/region state.
>
> Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
Please fix the commit message. I use b4 to apply patches.
Documentation/process/submitting-patches.rst
or https://docs.kernel.org/process/submitting-patches.html
https://docs.kernel.org/process/email-clients.html
Also you can see how patches are reviewed, the style used
https://lore.kernel.org/linux-arm-kernel/
(hint: don't add (v2) as postfix in $subject)
> ---
> drivers/soc/ti/knav_qmss_queue.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
> index 1e5f3e9faa99..7d9f3570ecf3 100644
> --- a/drivers/soc/ti/knav_qmss_queue.c
> +++ b/drivers/soc/ti/knav_qmss_queue.c
> @@ -1,11 +1,11 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/*
> +/**
> * Keystone Queue Manager subsystem driver
> *
> * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com
> - * Authors: Sandeep Nair <sandeep_n@ti.com>
> - * Cyril Chemparathy <cyril@ti.com>
> - * Santosh Shilimkar <santosh.shilimkar@ti.com>
> + * Authors: Sandeep Nair <sandeep_n@ti.com>
> + * Cyril Chemparathy <cyril@ti.com>
> + * Santosh Shilimkar <santosh.shilimkar@ti.com>
un-necessary change, drop.
> */
>
> #include <linux/debugfs.h>
> @@ -1884,14 +1884,13 @@ static int knav_queue_probe(struct platform_device *pdev)
>
> static void knav_queue_remove(struct platform_device *pdev)
> {
> - struct knav_device *kdev = platform_get_drvdata(pdev);
> -
> - knav_queue_stop_pdsps(kdev);
> - knav_queue_free_regions(kdev);
> - knav_free_queue_ranges(kdev);
> + struct knav_device *kdev = platform_get_drvdata(pdev);
>
> - pm_runtime_put_sync(&pdev->dev);
> - pm_runtime_disable(&pdev->dev);
> + knav_queue_stop_pdsps(kdev);
> + knav_queue_free_regions(kdev);
> + knav_free_queue_ranges(kdev);
> + pm_runtime_put_sync(&pdev->dev);
> + pm_runtime_disable(&pdev->dev);
When submitting a patch, please rebase and squash local changes.. and
make sure the patch applies cleanly.
> }
>
> static struct platform_driver keystone_qmss_driver = {
> --
> 2.51.1
>
>
PS: if you are not comfortable with upstream patch flow, I suggest
online training/in-person training from various sources such as bootlin
etc..
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
https://ti.com/opensource
^ permalink raw reply [flat|nested] 2+ messages in thread