[PATCH 0/8] hwtracing: Convert to platform remove callback returning void

[PATCH 0/8] hwtracing: Convert to platform remove callback returning void

[Uwe Kleine-König]

Hello,

after a fix for a resource leak this series converts all platform
drivers below drivers/hwtracing to .remove_new(). See commit
5c5a7680e67b ("platform: Provide a remove callback that returns no
value") for an extended explanation and the eventual goal.

Best regards
Uwe

Uwe Kleine-König (8):
  coresight: ultrasoc-smb: Fix resource leak in .remove()
  coresight: dummy: Convert to platform remove callback returning void
  coresight: etm4x: Convert to platform remove callback returning void
  coresight: funnel: Convert to platform remove callback returning void
  coresight: replicator: Convert to platform remove callback returning
    void
  coresight: trbe: Convert to platform remove callback returning void
  intel_th: Convert to platform remove callback returning void
  coresight: ultrasoc-smb: Convert to platform remove callback returning
    void

 drivers/hwtracing/coresight/coresight-dummy.c      |  5 ++---
 drivers/hwtracing/coresight/coresight-etm4x-core.c |  6 ++----
 drivers/hwtracing/coresight/coresight-funnel.c     |  5 ++---
 drivers/hwtracing/coresight/coresight-replicator.c |  5 ++---
 drivers/hwtracing/coresight/coresight-trbe.c       |  5 ++---
 drivers/hwtracing/coresight/ultrasoc-smb.c         | 11 +++--------
 drivers/hwtracing/intel_th/acpi.c                  |  6 ++----
 7 files changed, 15 insertions(+), 28 deletions(-)

base-commit: 5cd631a52568a18b12fd2563418985c8cb63e4b0
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/8] coresight: ultrasoc-smb: Fix resource leak in .remove()

[Uwe Kleine-König]

If smb_config_inport() fails it's still necessary to unregister all
resources. As smb_config_inport() already emits an error message on
failure, there is no need to add another one. By not returning the error
code, a second error message (about the return value being ignored) is
suppressed.

Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwtracing/coresight/ultrasoc-smb.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c
index e9a32a97fbee..10e7d4852112 100644
--- a/drivers/hwtracing/coresight/ultrasoc-smb.c
+++ b/drivers/hwtracing/coresight/ultrasoc-smb.c
@@ -613,11 +613,8 @@ static int smb_probe(struct platform_device *pdev)
 static int smb_remove(struct platform_device *pdev)
 {
 	struct smb_drv_data *drvdata = platform_get_drvdata(pdev);
-	int ret;
 
-	ret = smb_config_inport(&pdev->dev, false);
-	if (ret)
-		return ret;
+	smb_config_inport(&pdev->dev, false);
 
 	smb_unregister_sink(drvdata);
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/8] coresight: dummy: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwtracing/coresight/coresight-dummy.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/hwtracing/coresight/coresight-dummy.c
index e4deafae7bc2..ac70c0b491be 100644
--- a/drivers/hwtracing/coresight/coresight-dummy.c
+++ b/drivers/hwtracing/coresight/coresight-dummy.c
@@ -122,14 +122,13 @@ static int dummy_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int dummy_remove(struct platform_device *pdev)
+static void dummy_remove(struct platform_device *pdev)
 {
 	struct dummy_drvdata *drvdata = platform_get_drvdata(pdev);
 	struct device *dev = &pdev->dev;
 
 	pm_runtime_disable(dev);
 	coresight_unregister(drvdata->csdev);
-	return 0;
 }
 
 static const struct of_device_id dummy_match[] = {
@@ -140,7 +139,7 @@ static const struct of_device_id dummy_match[] = {
 
 static struct platform_driver dummy_driver = {
 	.probe	= dummy_probe,
-	.remove	= dummy_remove,
+	.remove_new = dummy_remove,
 	.driver	= {
 		.name   = "coresight-dummy",
 		.of_match_table = dummy_match,
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/8] coresight: etm4x: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 285539104bcc..ce1995a2827f 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -2303,7 +2303,7 @@ static void etm4_remove_amba(struct amba_device *adev)
 		etm4_remove_dev(drvdata);
 }
 
-static int etm4_remove_platform_dev(struct platform_device *pdev)
+static void etm4_remove_platform_dev(struct platform_device *pdev)
 {
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(&pdev->dev);
 
@@ -2313,8 +2313,6 @@ static int etm4_remove_platform_dev(struct platform_device *pdev)
 
 	if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk))
 		clk_put(drvdata->pclk);
-
-	return 0;
 }
 
 static const struct amba_id etm4_ids[] = {
@@ -2400,7 +2398,7 @@ MODULE_DEVICE_TABLE(acpi, etm4x_acpi_ids);
 
 static struct platform_driver etm4_platform_driver = {
 	.probe		= etm4_probe_platform_dev,
-	.remove		= etm4_remove_platform_dev,
+	.remove_new	= etm4_remove_platform_dev,
 	.driver			= {
 		.name			= "coresight-etm4x",
 		.of_match_table		= etm4_sysreg_match,
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/8] coresight: funnel: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwtracing/coresight/coresight-funnel.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
index b8e150e45b27..a5b1fc787766 100644
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -335,11 +335,10 @@ static int static_funnel_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int static_funnel_remove(struct platform_device *pdev)
+static void static_funnel_remove(struct platform_device *pdev)
 {
 	funnel_remove(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-	return 0;
 }
 
 static const struct of_device_id static_funnel_match[] = {
@@ -360,7 +359,7 @@ MODULE_DEVICE_TABLE(acpi, static_funnel_ids);
 
 static struct platform_driver static_funnel_driver = {
 	.probe          = static_funnel_probe,
-	.remove          = static_funnel_remove,
+	.remove_new      = static_funnel_remove,
 	.driver         = {
 		.name   = "coresight-static-funnel",
 		/* THIS_MODULE is taken care of by platform_driver_register() */
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/8] coresight: replicator: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwtracing/coresight/coresight-replicator.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index b6be73034996..91d93060dda5 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -320,11 +320,10 @@ static int static_replicator_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int static_replicator_remove(struct platform_device *pdev)
+static void static_replicator_remove(struct platform_device *pdev)
 {
 	replicator_remove(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-	return 0;
 }
 
 #ifdef CONFIG_PM
@@ -373,7 +372,7 @@ MODULE_DEVICE_TABLE(acpi, static_replicator_acpi_ids);
 
 static struct platform_driver static_replicator_driver = {
 	.probe          = static_replicator_probe,
-	.remove         = static_replicator_remove,
+	.remove_new     = static_replicator_remove,
 	.driver         = {
 		.name   = "coresight-static-replicator",
 		/* THIS_MODULE is taken care of by platform_driver_register() */
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/8] coresight: trbe: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwtracing/coresight/coresight-trbe.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index a3954be7b1f3..6136776482e6 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -1530,14 +1530,13 @@ static int arm_trbe_device_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int arm_trbe_device_remove(struct platform_device *pdev)
+static void arm_trbe_device_remove(struct platform_device *pdev)
 {
 	struct trbe_drvdata *drvdata = platform_get_drvdata(pdev);
 
 	arm_trbe_remove_cpuhp(drvdata);
 	arm_trbe_remove_coresight(drvdata);
 	arm_trbe_remove_irq(drvdata);
-	return 0;
 }
 
 static const struct of_device_id arm_trbe_of_match[] = {
@@ -1562,7 +1561,7 @@ static struct platform_driver arm_trbe_driver = {
 		.suppress_bind_attrs = true,
 	},
 	.probe	= arm_trbe_device_probe,
-	.remove	= arm_trbe_device_remove,
+	.remove_new = arm_trbe_device_remove,
 };
 
 static int __init arm_trbe_init(void)
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 7/8] intel_th: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwtracing/intel_th/acpi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/intel_th/acpi.c b/drivers/hwtracing/intel_th/acpi.c
index 87f9024e4bbb..503620e9fd10 100644
--- a/drivers/hwtracing/intel_th/acpi.c
+++ b/drivers/hwtracing/intel_th/acpi.c
@@ -60,18 +60,16 @@ static int intel_th_acpi_probe(struct platform_device *pdev)
        return 0;
 }

-static int intel_th_acpi_remove(struct platform_device *pdev)
+static void intel_th_acpi_remove(struct platform_device *pdev)
 {
        struct intel_th *th = platform_get_drvdata(pdev);

        intel_th_free(th);
-
-       return 0;
 }

 static struct platform_driver intel_th_acpi_driver = {
        .probe          = intel_th_acpi_probe,
-       .remove         = intel_th_acpi_remove,
+       .remove_new     = intel_th_acpi_remove,
        .driver         = {
                .name                   = DRIVER_NAME,
                .acpi_match_table       = intel_th_acpi_ids,
--
2.42.0

[PATCH 8/8] coresight: ultrasoc-smb: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/hwtracing/coresight/ultrasoc-smb.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c
index 10e7d4852112..35be22d66417 100644
--- a/drivers/hwtracing/coresight/ultrasoc-smb.c
+++ b/drivers/hwtracing/coresight/ultrasoc-smb.c
@@ -610,15 +610,13 @@ static int smb_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int smb_remove(struct platform_device *pdev)
+static void smb_remove(struct platform_device *pdev)
 {
 	struct smb_drv_data *drvdata = platform_get_drvdata(pdev);
 
 	smb_config_inport(&pdev->dev, false);
 
 	smb_unregister_sink(drvdata);
-
-	return 0;
 }
 
 #ifdef CONFIG_ACPI
@@ -636,7 +634,7 @@ static struct platform_driver smb_driver = {
 		.suppress_bind_attrs = true,
 	},
 	.probe = smb_probe,
-	.remove = smb_remove,
+	.remove_new = smb_remove,
 };
 module_platform_driver(smb_driver);
 
-- 
2.42.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/8] coresight: ultrasoc-smb: Fix resource leak in .remove()

[hejunhao]

On 2023/11/8 4:28, Uwe Kleine-König wrote:
> If smb_config_inport() fails it's still necessary to unregister all
> resources. As smb_config_inport() already emits an error message on
> failure, there is no need to add another one. By not returning the error
> code, a second error message (about the return value being ignored) is
> suppressed.
>
> Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Hi,

thanks, for the report. Can you try this patch and help review it?

https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/ 


This patch should have fixed this issue.

Best regards,
Junhao.


> ---
>   drivers/hwtracing/coresight/ultrasoc-smb.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c
> index e9a32a97fbee..10e7d4852112 100644
> --- a/drivers/hwtracing/coresight/ultrasoc-smb.c
> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c
> @@ -613,11 +613,8 @@ static int smb_probe(struct platform_device *pdev)
>   static int smb_remove(struct platform_device *pdev)
>   {
>   	struct smb_drv_data *drvdata = platform_get_drvdata(pdev);
> -	int ret;
>   
> -	ret = smb_config_inport(&pdev->dev, false);
> -	if (ret)
> -		return ret;
> +	smb_config_inport(&pdev->dev, false);
>   
>   	smb_unregister_sink(drvdata);
>   


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/8] coresight: ultrasoc-smb: Fix resource leak in .remove()

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 1096 bytes --]

Hello,

On Thu, Nov 09, 2023 at 05:41:55PM +0800, hejunhao wrote:
> On 2023/11/8 4:28, Uwe Kleine-König wrote:
> > If smb_config_inport() fails it's still necessary to unregister all
> > resources. As smb_config_inport() already emits an error message on
> > failure, there is no need to add another one. By not returning the error
> > code, a second error message (about the return value being ignored) is
> > suppressed.
> > 
> > Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver")
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> thanks, for the report. Can you try this patch and help review it?
> 
> https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/

Well, I cannot try it, but it improves things a bit. The remaining
problem is that smb_remove() should return 0 even if smb_config_inport()
fails.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/8] hwtracing: Convert to platform remove callback returning void

[James Clark]

On 07/11/2023 20:28, Uwe Kleine-König wrote:
> Hello,
> 
> after a fix for a resource leak this series converts all platform
> drivers below drivers/hwtracing to .remove_new(). See commit
> 5c5a7680e67b ("platform: Provide a remove callback that returns no
> value") for an extended explanation and the eventual goal.
> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (8):
>   coresight: ultrasoc-smb: Fix resource leak in .remove()
>   coresight: dummy: Convert to platform remove callback returning void
>   coresight: etm4x: Convert to platform remove callback returning void
>   coresight: funnel: Convert to platform remove callback returning void
>   coresight: replicator: Convert to platform remove callback returning
>     void
>   coresight: trbe: Convert to platform remove callback returning void
>   intel_th: Convert to platform remove callback returning void
>   coresight: ultrasoc-smb: Convert to platform remove callback returning
>     void
> 
>  drivers/hwtracing/coresight/coresight-dummy.c      |  5 ++---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c |  6 ++----
>  drivers/hwtracing/coresight/coresight-funnel.c     |  5 ++---
>  drivers/hwtracing/coresight/coresight-replicator.c |  5 ++---
>  drivers/hwtracing/coresight/coresight-trbe.c       |  5 ++---
>  drivers/hwtracing/coresight/ultrasoc-smb.c         | 11 +++--------
>  drivers/hwtracing/intel_th/acpi.c                  |  6 ++----
>  7 files changed, 15 insertions(+), 28 deletions(-)
> 
> base-commit: 5cd631a52568a18b12fd2563418985c8cb63e4b0

For the whole set:

Reviewed-by: James Clark <james.clark@arm.com>

Although I'm not sure what the resolution should be on patch 1 with
Junhao's other change.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/8] coresight: ultrasoc-smb: Fix resource leak in .remove()

[hejunhao]

On 2023/11/9 17:53, Uwe Kleine-König wrote:
> Hello,
>
> On Thu, Nov 09, 2023 at 05:41:55PM +0800, hejunhao wrote:
>> On 2023/11/8 4:28, Uwe Kleine-König wrote:
>>> If smb_config_inport() fails it's still necessary to unregister all
>>> resources. As smb_config_inport() already emits an error message on
>>> failure, there is no need to add another one. By not returning the error
>>> code, a second error message (about the return value being ignored) is
>>> suppressed.
>>>
>>> Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver")
>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> thanks, for the report. Can you try this patch and help review it?
>>
>> https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/
> Well, I cannot try it, but it improves things a bit. The remaining
> problem is that smb_remove() should return 0 even if smb_config_inport()
> fails.
>
> Best regards
> Uwe

Yes, I will do that.
By the way, Can I add patch 8.8 to my patchset? I think this is a good 
way to resolve patch conflicts.

Best regards,
Junhao.
>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/8] coresight: ultrasoc-smb: Fix resource leak in .remove()

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 1687 bytes --]

On Thu, Nov 09, 2023 at 08:14:34PM +0800, hejunhao wrote:
> 
> On 2023/11/9 17:53, Uwe Kleine-König wrote:
> > Hello,
> > 
> > On Thu, Nov 09, 2023 at 05:41:55PM +0800, hejunhao wrote:
> > > On 2023/11/8 4:28, Uwe Kleine-König wrote:
> > > > If smb_config_inport() fails it's still necessary to unregister all
> > > > resources. As smb_config_inport() already emits an error message on
> > > > failure, there is no need to add another one. By not returning the error
> > > > code, a second error message (about the return value being ignored) is
> > > > suppressed.
> > > > 
> > > > Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver")
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > thanks, for the report. Can you try this patch and help review it?
> > > 
> > > https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/
> > Well, I cannot try it, but it improves things a bit. The remaining
> > problem is that smb_remove() should return 0 even if smb_config_inport()
> > fails.
> > 
> > Best regards
> > Uwe
> 
> Yes, I will do that.
> By the way, Can I add patch 8.8 to my patchset? I think this is a good way
> to resolve patch conflicts.

I don't know how patch application works in drivers/hwtracing. I will
probably check if these patches are in next in a few weeks and resend if
they are not. If you adding patch #8 to your patch set needs more
coordination, please tell me.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/8] coresight: ultrasoc-smb: Fix resource leak in .remove()

[hejunhao]

Hi Uwe,

On 2023/11/9 21:11, Uwe Kleine-König wrote:
> On Thu, Nov 09, 2023 at 08:14:34PM +0800, hejunhao wrote:
>> On 2023/11/9 17:53, Uwe Kleine-König wrote:
>>> Hello,
>>>
>>> On Thu, Nov 09, 2023 at 05:41:55PM +0800, hejunhao wrote:
>>>> On 2023/11/8 4:28, Uwe Kleine-König wrote:
>>>>> If smb_config_inport() fails it's still necessary to unregister all
>>>>> resources. As smb_config_inport() already emits an error message on
>>>>> failure, there is no need to add another one. By not returning the error
>>>>> code, a second error message (about the return value being ignored) is
>>>>> suppressed.
>>>>>
>>>>> Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver")
>>>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>>> thanks, for the report. Can you try this patch and help review it?
>>>>
>>>> https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/
>>> Well, I cannot try it, but it improves things a bit. The remaining
>>> problem is that smb_remove() should return 0 even if smb_config_inport()
>>> fails.
>>>
>>> Best regards
>>> Uwe
>> Yes, I will do that.
>> By the way, Can I add patch 8.8 to my patchset? I think this is a good way
>> to resolve patch conflicts.
> I don't know how patch application works in drivers/hwtracing. I will
> probably check if these patches are in next in a few weeks and resend if
> they are not. If you adding patch #8 to your patch set needs more
> coordination, please tell me.
>
> Best regards
> Uwe

Forgive me for interrupting.
I sent v3 and it ignored the return value of smb_config_inport(). Can 
you rebase to that?:-)

https://lore.kernel.org/lkml/20231114133346.30489-1-hejunhao3@huawei.com/

Best regards,
Junhao.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/8] coresight: ultrasoc-smb: Fix resource leak in .remove()

[Suzuki K Poulose]

On 16/11/2023 12:35, hejunhao wrote:
> Hi Uwe,
> 
> On 2023/11/9 21:11, Uwe Kleine-K�nig wrote:
>> On Thu, Nov 09, 2023 at 08:14:34PM +0800, hejunhao wrote:
>>> On 2023/11/9 17:53, Uwe Kleine-K�nig wrote:
>>>> Hello,
>>>>
>>>> On Thu, Nov 09, 2023 at 05:41:55PM +0800, hejunhao wrote:
>>>>> On 2023/11/8 4:28, Uwe Kleine-K�nig wrote:
>>>>>> If smb_config_inport() fails it's still necessary to unregister all
>>>>>> resources. As smb_config_inport() already emits an error message on
>>>>>> failure, there is no need to add another one. By not returning the 
>>>>>> error
>>>>>> code, a second error message (about the return value being 
>>>>>> ignored) is
>>>>>> suppressed.
>>>>>>
>>>>>> Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System 
>>>>>> Memory Buffer driver")
>>>>>> Signed-off-by: Uwe Kleine-K�nig <u.kleine-koenig@pengutronix.de>
>>>>> thanks, for the report. Can you try this patch and help review it?
>>>>>
>>>>> https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/
>>>> Well, I cannot try it, but it improves things a bit. The remaining
>>>> problem is that smb_remove() should return 0 even if 
>>>> smb_config_inport()
>>>> fails.
>>>>
>>>> Best regards
>>>> Uwe
>>> Yes, I will do that.
>>> By the way, Can I add patch 8.8 to my patchset? I think this is a 
>>> good way
>>> to resolve patch conflicts.
>> I don't know how patch application works in drivers/hwtracing. I will
>> probably check if these patches are in next in a few weeks and resend if
>> they are not. If you adding patch #8 to your patch set needs more
>> coordination, please tell me.
>>
>> Best regards
>> Uwe
> 
> Forgive me for interrupting.
> I sent v3 and it ignored the return value of smb_config_inport(). Can 
> you rebase to that?:-)
> 
> https://lore.kernel.org/lkml/20231114133346.30489-1-hejunhao3@huawei.com/

fyi,

I have queued all the patches in the link above, to fixes branch on 
coresight and it would soon appear on next branch too. So you should
be able to rebase it on coresight/next.

Suzuki

> 
> Best regards,
> Junhao.
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 7/8] intel_th: Convert to platform remove callback returning void

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]

Hello,

On Tue, Nov 07, 2023 at 09:28:26PM +0100, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is ignored (apart
> from emitting a warning) and this typically results in resource leaks.
> 
> To improve here there is a quest to make the remove callback return
> void. In the first step of this quest all drivers are converted to
> .remove_new(), which already returns void. Eventually after all drivers
> are converted, .remove_new() will be renamed to .remove().
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

I didn't get any feedback to this patch and it didn't make it into next
up to now.

Is this still on someone's radar?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 7/8] intel_th: Convert to platform remove callback returning void

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1517 bytes --]

Hello Alexander,

On Wed, Jan 10, 2024 at 09:41:54AM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> On Tue, Nov 07, 2023 at 09:28:26PM +0100, Uwe Kleine-König wrote:
> > The .remove() callback for a platform driver returns an int which makes
> > many driver authors wrongly assume it's possible to do error handling by
> > returning an error code. However the value returned is ignored (apart
> > from emitting a warning) and this typically results in resource leaks.
> > 
> > To improve here there is a quest to make the remove callback return
> > void. In the first step of this quest all drivers are converted to
> > .remove_new(), which already returns void. Eventually after all drivers
> > are converted, .remove_new() will be renamed to .remove().
> > 
> > Trivially convert this driver from always returning zero in the remove
> > callback to the void returning variant.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> I didn't get any feedback to this patch and it didn't make it into next
> up to now.
> 
> Is this still on someone's radar?

Is there a chance to get this patch into v6.9-rc1? Are you the right one
to talk to about this patch? (According to MAINTAINERS you are.)

The patch was sent during the 6.7 merge window and now already missed
the 6.8 one :-\

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 7/8] intel_th: Convert to platform remove callback returning void

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]

Hello Greg,

On Thu, Feb 15, 2024 at 10:16:41PM +0100, Uwe Kleine-König wrote:
> On Wed, Jan 10, 2024 at 09:41:54AM +0100, Uwe Kleine-König wrote:
> > On Tue, Nov 07, 2023 at 09:28:26PM +0100, Uwe Kleine-König wrote:
> > > The .remove() callback for a platform driver returns an int which makes
> > > many driver authors wrongly assume it's possible to do error handling by
> > > returning an error code. However the value returned is ignored (apart
> > > from emitting a warning) and this typically results in resource leaks.
> > > 
> > > To improve here there is a quest to make the remove callback return
> > > void. In the first step of this quest all drivers are converted to
> > > .remove_new(), which already returns void. Eventually after all drivers
> > > are converted, .remove_new() will be renamed to .remove().
> > > 
> > > Trivially convert this driver from always returning zero in the remove
> > > callback to the void returning variant.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 
> > I didn't get any feedback to this patch and it didn't make it into next
> > up to now.
> > 
> > Is this still on someone's radar?
> 
> Is there a chance to get this patch into v6.9-rc1? Are you the right one
> to talk to about this patch? (According to MAINTAINERS you are.)
> 
> The patch was sent during the 6.7 merge window and now already missed
> the 6.8 one :-\

I failed in several attempts to get feedback on this patch. You applied
the last two patches for this driver (that is all patches since the
driver was born). Would you care for that one, too? Tell me if you want
a resend. Note that the other 7 patches from this series are already
cared for, so if you're using b4 am or shazam, make use of -P7.

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 7/8] intel_th: Convert to platform remove callback returning void

[Alexander Shishkin]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:

> Hello Greg,
>
> On Thu, Feb 15, 2024 at 10:16:41PM +0100, Uwe Kleine-König wrote:
>> On Wed, Jan 10, 2024 at 09:41:54AM +0100, Uwe Kleine-König wrote:
>> > On Tue, Nov 07, 2023 at 09:28:26PM +0100, Uwe Kleine-König wrote:
>> > > The .remove() callback for a platform driver returns an int which makes
>> > > many driver authors wrongly assume it's possible to do error handling by
>> > > returning an error code. However the value returned is ignored (apart
>> > > from emitting a warning) and this typically results in resource leaks.
>> > > 
>> > > To improve here there is a quest to make the remove callback return
>> > > void. In the first step of this quest all drivers are converted to
>> > > .remove_new(), which already returns void. Eventually after all drivers
>> > > are converted, .remove_new() will be renamed to .remove().
>> > > 
>> > > Trivially convert this driver from always returning zero in the remove
>> > > callback to the void returning variant.
>> > > 
>> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> > 
>> > I didn't get any feedback to this patch and it didn't make it into next
>> > up to now.
>> > 
>> > Is this still on someone's radar?
>> 
>> Is there a chance to get this patch into v6.9-rc1? Are you the right one
>> to talk to about this patch? (According to MAINTAINERS you are.)
>> 
>> The patch was sent during the 6.7 merge window and now already missed
>> the 6.8 one :-\
>
> I failed in several attempts to get feedback on this patch. You applied
> the last two patches for this driver (that is all patches since the
> driver was born). Would you care for that one, too? Tell me if you want
> a resend. Note that the other 7 patches from this series are already
> cared for, so if you're using b4 am or shazam, make use of -P7.

Apologies. This looks good to me, I will pick it up for my next
submission to Greg unless somebody objects.

Regards,
--
Alex

Re: [PATCH 7/8] intel_th: Convert to platform remove callback returning void

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2606 bytes --]

Hello Alex,

On Wed, Feb 21, 2024 at 08:50:32PM +0200, Alexander Shishkin wrote:
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> > On Thu, Feb 15, 2024 at 10:16:41PM +0100, Uwe Kleine-König wrote:
> >> On Wed, Jan 10, 2024 at 09:41:54AM +0100, Uwe Kleine-König wrote:
> >> > On Tue, Nov 07, 2023 at 09:28:26PM +0100, Uwe Kleine-König wrote:
> >> > > The .remove() callback for a platform driver returns an int which makes
> >> > > many driver authors wrongly assume it's possible to do error handling by
> >> > > returning an error code. However the value returned is ignored (apart
> >> > > from emitting a warning) and this typically results in resource leaks.
> >> > > 
> >> > > To improve here there is a quest to make the remove callback return
> >> > > void. In the first step of this quest all drivers are converted to
> >> > > .remove_new(), which already returns void. Eventually after all drivers
> >> > > are converted, .remove_new() will be renamed to .remove().
> >> > > 
> >> > > Trivially convert this driver from always returning zero in the remove
> >> > > callback to the void returning variant.
> >> > > 
> >> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >> > 
> >> > I didn't get any feedback to this patch and it didn't make it into next
> >> > up to now.
> >> > 
> >> > Is this still on someone's radar?
> >> 
> >> Is there a chance to get this patch into v6.9-rc1? Are you the right one
> >> to talk to about this patch? (According to MAINTAINERS you are.)
> >> 
> >> The patch was sent during the 6.7 merge window and now already missed
> >> the 6.8 one :-\
> >
> > I failed in several attempts to get feedback on this patch. You applied
> > the last two patches for this driver (that is all patches since the
> > driver was born). Would you care for that one, too? Tell me if you want
> > a resend. Note that the other 7 patches from this series are already
> > cared for, so if you're using b4 am or shazam, make use of -P7.
> 
> Apologies. This looks good to me, I will pick it up for my next
> submission to Greg unless somebody objects.

Given there are not that many patches left (~50) as of today's next, I'd
like to see this patch going in during the next merge window, that I can
finalize the (next step of the) quest around platform_driver::remove.

Is sending this patch to Greg still on your todo list?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 7/8] intel_th: Convert to platform remove callback returning void

[Alexander Shishkin]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:

> Hello Alex,

Hi Uwe,

> Given there are not that many patches left (~50) as of today's next, I'd
> like to see this patch going in during the next merge window, that I can
> finalize the (next step of the) quest around platform_driver::remove.
>
> Is sending this patch to Greg still on your todo list?

Yes, as soon as I finalize other patches on my series. Should be soon.

Regards,
--
Alex

Re: [PATCH 7/8] intel_th: Convert to platform remove callback returning void

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]

Hello Alex,

On Mon, Apr 15, 2024 at 09:48:27AM +0300, Alexander Shishkin wrote:
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> > Given there are not that many patches left (~50) as of today's next, I'd
> > like to see this patch going in during the next merge window, that I can
> > finalize the (next step of the) quest around platform_driver::remove.
> >
> > Is sending this patch to Greg still on your todo list?
> 
> Yes, as soon as I finalize other patches on my series. Should be soon.

I know neither your nor Greg's plans, but I wonder if this patch will
make it into v6.10-rc1 given that it still didn't land in next yet.

I intend to change the prototype of platform_driver::remove() after the
merge window leading to v6.10-rc1 closes to cook in next for some time.
If this patch doesn't make it into the mainline before, I'll send it
together with the change adapting the remove callback. That will go in
via Greg's tree, too.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 7/8] intel_th: Convert to platform remove callback returning void

[Alexander Shishkin]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:

> Hello Alex,

Hi Uwe,

> On Mon, Apr 15, 2024 at 09:48:27AM +0300, Alexander Shishkin wrote:
>> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
>> > Given there are not that many patches left (~50) as of today's next, I'd
>> > like to see this patch going in during the next merge window, that I can
>> > finalize the (next step of the) quest around platform_driver::remove.
>> >
>> > Is sending this patch to Greg still on your todo list?
>> 
>> Yes, as soon as I finalize other patches on my series. Should be soon.
>
> I know neither your nor Greg's plans, but I wonder if this patch will
> make it into v6.10-rc1 given that it still didn't land in next yet.

I've just sent my series [0].

> I intend to change the prototype of platform_driver::remove() after the
> merge window leading to v6.10-rc1 closes to cook in next for some time.
> If this patch doesn't make it into the mainline before, I'll send it
> together with the change adapting the remove callback. That will go in
> via Greg's tree, too.

I saw that you included this patch with your series earlier today, so I
dropped it from my series [0]. FWIW, you can add

Acked-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>

to that patch if it helps. If you want me to pick it up again, let me
know. Apologies for the repeated tardiness in handling this.

[0] https://lore.kernel.org/all/20240429120908.3723458-1-alexander.shishkin@linux.intel.com/

Regards,
--
Alex

Re: [PATCH 7/8] intel_th: Convert to platform remove callback returning void

[Alexander Shishkin]

Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:

> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
>> I intend to change the prototype of platform_driver::remove() after the
>> merge window leading to v6.10-rc1 closes to cook in next for some time.
>> If this patch doesn't make it into the mainline before, I'll send it
>> together with the change adapting the remove callback. That will go in
>> via Greg's tree, too.
>
> I saw that you included this patch with your series earlier today, so I

Wait, did I hallucinate this? I can't find your new series now. Ugh.
If so, I'll put your patch back in and re-send my series right away.

Regards,
--
Alex

Re: [PATCH 7/8] intel_th: Convert to platform remove callback returning void

[Alexander Shishkin]

Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:

> Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:
>
>> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
>>> I intend to change the prototype of platform_driver::remove() after the
>>> merge window leading to v6.10-rc1 closes to cook in next for some time.
>>> If this patch doesn't make it into the mainline before, I'll send it
>>> together with the change adapting the remove callback. That will go in
>>> via Greg's tree, too.
>>
>> I saw that you included this patch with your series earlier today, so I
>
> Wait, did I hallucinate this? I can't find your new series now. Ugh.
> If so, I'll put your patch back in and re-send my series right away.

Series re-sent [0]. Sorry for the mess.

[0] https://lore.kernel.org/all/20240429130119.1518073-1-alexander.shishkin@linux.intel.com/

Regards,
--
Alex