[PATCH 0/3] media: ti: vpe: three small fixes

[PATCH 0/3] media: ti: vpe: three small fixes

[Felix Gu]

Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
Felix Gu (3):
      media: ti: vpe: Fix fwnode_handle leak in vip_probe_complete()
      media: ti: vpe: Fix the error code of devm_request_irq()
      media: ti: vpe: Fix the error code of devm_kzalloc() in vip_probe_slice()

 drivers/media/platform/ti/vpe/vip.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
---
base-commit: 95c541ddfb0815a0ea8477af778bb13bb075079a
change-id: 20260317-vip-a2f601930065

Best regards,
-- 
Felix Gu <ustc.gu@gmail.com>

[PATCH 1/3] media: ti: vpe: Fix fwnode_handle leak in vip_probe_complete()

[Felix Gu]

In vip_probe_complete(), the fwnode_handle reference is not released
if the loop continues via the default switch case or if alloc_port()
fails. This results in a reference count leak.

Switch to using the __free(fwnode_handle) cleanup attribute to ensure
the reference is automatically released when the handle goes out of
scope.

Fixes: fc2873aa4a21 ("media: ti: vpe: Add the VIP driver")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/media/platform/ti/vpe/vip.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/ti/vpe/vip.c b/drivers/media/platform/ti/vpe/vip.c
index a4b616a5ece7..01d27769cd10 100644
--- a/drivers/media/platform/ti/vpe/vip.c
+++ b/drivers/media/platform/ti/vpe/vip.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
@@ -3389,7 +3390,6 @@ static int vip_probe_complete(struct platform_device *pdev)
 	struct vip_port *port;
 	struct vip_dev *dev;
 	struct device_node *parent = pdev->dev.of_node;
-	struct fwnode_handle *ep = NULL;
 	unsigned int syscon_args[5];
 	int ret, i, slice_id, port_id, p;
 
@@ -3411,8 +3411,9 @@ static int vip_probe_complete(struct platform_device *pdev)
 		ctrl->syscon_bit_field[i] = syscon_args[i + 1];
 
 	for (p = 0; p < (VIP_NUM_PORTS * VIP_NUM_SLICES); p++) {
-		ep = fwnode_graph_get_next_endpoint_by_regs(of_fwnode_handle(parent),
-							    p, 0);
+		struct fwnode_handle *ep __free(fwnode_handle) =
+			fwnode_graph_get_next_endpoint_by_regs(
+				of_fwnode_handle(parent), p, 0);
 		if (!ep)
 			continue;
 
@@ -3447,7 +3448,6 @@ static int vip_probe_complete(struct platform_device *pdev)
 		port = dev->ports[port_id];
 
 		vip_register_subdev_notify(port, ep);
-		fwnode_handle_put(ep);
 	}
 	return 0;
 }

-- 
2.43.0

[PATCH 2/3] media: ti: vpe: Fix the error code of devm_request_irq()

[Felix Gu]

Return the actual error code from devm_request_irq() instead of
incorrectly returning -ENOMEM.

Fixes: fc2873aa4a21 ("media: ti: vpe: Add the VIP driver")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/media/platform/ti/vpe/vip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/ti/vpe/vip.c b/drivers/media/platform/ti/vpe/vip.c
index 01d27769cd10..f0354b52048e 100644
--- a/drivers/media/platform/ti/vpe/vip.c
+++ b/drivers/media/platform/ti/vpe/vip.c
@@ -3472,7 +3472,7 @@ static int vip_probe_slice(struct platform_device *pdev, int slice)
 	ret = devm_request_irq(&pdev->dev, dev->irq, vip_irq,
 			       0, VIP_MODULE_NAME, dev);
 	if (ret < 0)
-		return -ENOMEM;
+		return ret;
 
 	spin_lock_init(&dev->slock);
 	mutex_init(&dev->mutex);

-- 
2.43.0

Re: [PATCH 2/3] media: ti: vpe: Fix the error code of devm_request_irq()

[Markus Elfring]

> Return the actual error code from devm_request_irq() instead of
> incorrectly returning -ENOMEM.

Would a summary phrase like “Propagate error code of a devm_request_irq() call
in vip_probe_slice()” be nicer?

Regards,
Markus

[PATCH 3/3] media: ti: vpe: Fix the error code of devm_kzalloc() in vip_probe_slice()

[Felix Gu]

In vip_probe_slice(), the error check for devm_kzalloc() incorrectly
uses PTR_ERR_OR_ZERO() which returns 0 for NULL pointer.

Return -ENOMEM for devm_kzalloc() failure.

Fixes: fc2873aa4a21 ("media: ti: vpe: Add the VIP driver")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/media/platform/ti/vpe/vip.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/ti/vpe/vip.c b/drivers/media/platform/ti/vpe/vip.c
index f0354b52048e..aec9dbd1320a 100644
--- a/drivers/media/platform/ti/vpe/vip.c
+++ b/drivers/media/platform/ti/vpe/vip.c
@@ -3490,7 +3490,7 @@ static int vip_probe_slice(struct platform_device *pdev, int slice)
 
 	parser = devm_kzalloc(&pdev->dev, sizeof(*dev->parser), GFP_KERNEL);
 	if (!parser)
-		return PTR_ERR_OR_ZERO(parser);
+		return -ENOMEM;
 
 	parser->base = dev->base + (slice ? VIP_SLICE1_PARSER : VIP_SLICE0_PARSER);
 	if (IS_ERR(parser->base))
@@ -3502,7 +3502,7 @@ static int vip_probe_slice(struct platform_device *pdev, int slice)
 	dev->sc_assigned = VIP_NOT_ASSIGNED;
 	sc = devm_kzalloc(&pdev->dev, sizeof(*dev->sc), GFP_KERNEL);
 	if (!sc)
-		return PTR_ERR_OR_ZERO(sc);
+		return -ENOMEM;
 
 	sc->base = dev->base + (slice ? VIP_SLICE1_SC : VIP_SLICE0_SC);
 	if (IS_ERR(sc->base))
@@ -3514,7 +3514,7 @@ static int vip_probe_slice(struct platform_device *pdev, int slice)
 	dev->csc_assigned = VIP_NOT_ASSIGNED;
 	csc = devm_kzalloc(&pdev->dev, sizeof(*dev->csc), GFP_KERNEL);
 	if (!csc)
-		return PTR_ERR_OR_ZERO(csc);
+		return -ENOMEM;
 
 	csc->base = dev->base + (slice ? VIP_SLICE1_CSC : VIP_SLICE0_CSC);
 	if (IS_ERR(csc->base))

-- 
2.43.0

Re: [PATCH 3/3] media: ti: vpe: Fix the error code of devm_kzalloc() in vip_probe_slice()

[Markus Elfring]

> In vip_probe_slice(), the error check for devm_kzalloc() incorrectly
> uses PTR_ERR_OR_ZERO() which returns 0 for NULL pointer.
…

I find such a change description improvable.
Return statements should be corrected because a corresponding failure predicate
is known already for the called function.
https://elixir.bootlin.com/linux/v7.0-rc4/source/include/linux/device/devres.h#L48-L51
https://elixir.bootlin.com/linux/v7.0-rc4/source/drivers/media/platform/ti/vpe/vip.c#L3455-L3527

Regards,
Markus