[RFT PATCH v2] [media] exynos4-is: Fix fimc_is_parse_sensor_config() nodes handling

[RFT PATCH v2] [media] exynos4-is: Fix fimc_is_parse_sensor_config() nodes handling

[Javier Martinez Canillas]

The same struct device_node * is used for looking up the I2C sensor, OF
graph endpoint and port. So the reference count is incremented but not
decremented for the endpoint and port nodes.

Fix this by having separate pointers for each node looked up.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Hello,

This patch was only build tested because I don't have an Exynos4
board to test. So testing on real HW will be highly appreciated.

Best regards,
Javier

Changes in v2:
- Use the correct node var in the error print out. Suggested by Andreas Färber.

 drivers/media/platform/exynos4-is/fimc-is.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
index 979c388ebf60..a97edd078327 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -165,6 +165,7 @@ static int fimc_is_parse_sensor_config(struct fimc_is *is, unsigned int index,
 						struct device_node *node)
 {
 	struct fimc_is_sensor *sensor = &is->sensor[index];
+	struct device_node *ep, *port;
 	u32 tmp = 0;
 	int ret;
 
@@ -175,22 +176,25 @@ static int fimc_is_parse_sensor_config(struct fimc_is *is, unsigned int index,
 		return -EINVAL;
 	}
 
-	node = of_graph_get_next_endpoint(node, NULL);
-	if (!node)
+	ep = of_graph_get_next_endpoint(node, NULL);
+	if (!ep)
 		return -ENXIO;
 
-	node = of_graph_get_remote_port(node);
-	if (!node)
+	port = of_graph_get_remote_port(ep);
+	of_node_put(ep);
+	if (!port)
 		return -ENXIO;
 
 	/* Use MIPI-CSIS channel id to determine the ISP I2C bus index. */
-	ret = of_property_read_u32(node, "reg", &tmp);
+	ret = of_property_read_u32(port, "reg", &tmp);
 	if (ret < 0) {
 		dev_err(&is->pdev->dev, "reg property not found at: %s\n",
-							 node->full_name);
+							 port->full_name);
+		of_node_put(port);
 		return ret;
 	}
 
+	of_node_put(port);
 	sensor->i2c_bus = tmp - FIMC_INPUT_MIPI_CSI2_0;
 	return 0;
 }
-- 
2.5.0

Re: [RFT PATCH v2] [media] exynos4-is: Fix fimc_is_parse_sensor_config() nodes handling

[Javier Martinez Canillas]

Hello Sylwester,

On 03/23/2016 08:41 PM, Javier Martinez Canillas wrote:
> The same struct device_node * is used for looking up the I2C sensor, OF
> graph endpoint and port. So the reference count is incremented but not
> decremented for the endpoint and port nodes.
> 
> Fix this by having separate pointers for each node looked up.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>

Any comments about this patch?

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [RFT PATCH v2] [media] exynos4-is: Fix fimc_is_parse_sensor_config() nodes handling

[Nicolas Dufresne]

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

Le jeudi 21 avril 2016 à 09:21 -0400, Javier Martinez Canillas a
écrit :
> Hello Sylwester,
> 
> On 03/23/2016 08:41 PM, Javier Martinez Canillas wrote:
> > The same struct device_node * is used for looking up the I2C
> > sensor, OF
> > graph endpoint and port. So the reference count is incremented but
> > not
> > decremented for the endpoint and port nodes.
> > 
> > Fix this by having separate pointers for each node looked up.
> > 
> > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> > 
> 
> Any comments about this patch?

Tested-by: Nicolas Dufresne <nicoas.dufresne@collabora.com>

Note: I could not verify that leak is gone, but I could verify that
this driver is still working properly after the change.

> 
> Best regards,

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [RFT PATCH v2] [media] exynos4-is: Fix fimc_is_parse_sensor_config() nodes handling

[Nicolas Dufresne]

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

Le vendredi 15 juillet 2016 à 12:53 -0400, Nicolas Dufresne a écrit :
> Le jeudi 21 avril 2016 à 09:21 -0400, Javier Martinez Canillas a
> écrit :
> > Hello Sylwester,
> > 
> > On 03/23/2016 08:41 PM, Javier Martinez Canillas wrote:
> > > The same struct device_node * is used for looking up the I2C
> > > sensor, OF
> > > graph endpoint and port. So the reference count is incremented
> > > but
> > > not
> > > decremented for the endpoint and port nodes.
> > > 
> > > Fix this by having separate pointers for each node looked up.
> > > 
> > > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> > > 
> > 
> > Any comments about this patch?
> 
> Tested-by: Nicolas Dufresne <nicoas.dufresne@collabora.com>

There a typo here.

Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> 
> Note: I could not verify that leak is gone, but I could verify that
> this driver is still working properly after the change.
> 
> > 
> > Best regards,

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]