* [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers
@ 2016-01-07 18:27 Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 1/8] [media] v4l: of: Correct v4l2_of_parse_endpoint() kernel-doc Javier Martinez Canillas
` (8 more replies)
0 siblings, 9 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2016-01-07 18:27 UTC (permalink / raw
To: linux-kernel
Cc: Javier Martinez Canillas, Sakari Ailus, Nikhil Devshatwar,
Kukjin Kim, Krzysztof Kozlowski, Andrzej Hajda,
Mauro Carvalho Chehab, Guennadi Liakhovetski, linux-samsung-soc,
Kyungmin Park, Sylwester Nawrocki, Sakari Ailus, Prabhakar",
Ricardo Ribalda Delgado, Laurent Pinchart, Hans Verkuil,
linux-arm-kernel, linux-media
Hello,
When discussing a patch [0] with Laurent Pinchart for another series I
mentioned to him that most callers of v4l2_of_parse_endpoint() weren't
checking the return value. This is likely due the function kernel-doc
stating incorrectly that the return value is always 0 but can return a
negative error code on failure.
This trivial patch series fixes the function kernel-doc and add proper
error checking in all the drivers that are currently not doing so.
[0]: https://lkml.org/lkml/2016/1/6/307
Best regards,
Javier
Javier Martinez Canillas (8):
[media] v4l: of: Correct v4l2_of_parse_endpoint() kernel-doc
[media] adv7604: Check v4l2_of_parse_endpoint() return value
[media] s5c73m3: Check v4l2_of_parse_endpoint() return value
[media] s5k5baf: Check v4l2_of_parse_endpoint() return value
[media] tvp514x: Check v4l2_of_parse_endpoint() return value
[media] tvp7002: Check v4l2_of_parse_endpoint() return value
[media] exynos4-is: Check v4l2_of_parse_endpoint() return value
[media] omap3isp: Check v4l2_of_parse_endpoint() return value
drivers/media/i2c/adv7604.c | 7 ++++++-
drivers/media/i2c/s5c73m3/s5c73m3-core.c | 4 +++-
drivers/media/i2c/s5k5baf.c | 5 ++++-
drivers/media/i2c/tvp514x.c | 4 +++-
drivers/media/i2c/tvp7002.c | 4 +++-
drivers/media/platform/exynos4-is/media-dev.c | 8 +++++++-
drivers/media/platform/exynos4-is/mipi-csis.c | 10 +++++++---
drivers/media/platform/omap3isp/isp.c | 5 ++++-
drivers/media/v4l2-core/v4l2-of.c | 2 +-
9 files changed, 38 insertions(+), 11 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/8] [media] v4l: of: Correct v4l2_of_parse_endpoint() kernel-doc
2016-01-07 18:27 [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Javier Martinez Canillas
@ 2016-01-07 18:27 ` Javier Martinez Canillas
2016-01-07 19:56 ` Sakari Ailus
2016-01-11 1:57 ` Laurent Pinchart
2016-01-07 18:27 ` [PATCH 2/8] [media] adv7604: Check v4l2_of_parse_endpoint() return value Javier Martinez Canillas
` (7 subsequent siblings)
8 siblings, 2 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2016-01-07 18:27 UTC (permalink / raw
To: linux-kernel
Cc: Javier Martinez Canillas, Sakari Ailus, Nikhil Devshatwar,
Mauro Carvalho Chehab, Sylwester Nawrocki, Laurent Pinchart,
Hans Verkuil, linux-media
The v4l2_of_parse_endpoint function kernel-doc says that the return value
is always 0. But that is not true since the function can fail and a error
negative code is returned on failure. So correct the kernel-doc to match.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/media/v4l2-core/v4l2-of.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
index b27cbb1f5afe..93b33681776c 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -146,7 +146,7 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
* variable without a low fixed limit. Please use
* v4l2_of_alloc_parse_endpoint() in new drivers instead.
*
- * Return: 0.
+ * Return: 0 on success or a negative error code on failure.
*/
int v4l2_of_parse_endpoint(const struct device_node *node,
struct v4l2_of_endpoint *endpoint)
--
2.4.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/8] [media] adv7604: Check v4l2_of_parse_endpoint() return value
2016-01-07 18:27 [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 1/8] [media] v4l: of: Correct v4l2_of_parse_endpoint() kernel-doc Javier Martinez Canillas
@ 2016-01-07 18:27 ` Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 3/8] [media] s5c73m3: " Javier Martinez Canillas
` (6 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2016-01-07 18:27 UTC (permalink / raw
To: linux-kernel
Cc: Javier Martinez Canillas, Mauro Carvalho Chehab, Hans Verkuil,
linux-media
The v4l2_of_parse_endpoint() function can fail so check the return value.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/media/i2c/adv7604.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index f8dd7505b529..ed3eccd94285 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -2799,6 +2799,7 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
struct device_node *endpoint;
struct device_node *np;
unsigned int flags;
+ int ret;
u32 v;
np = state->i2c_clients[ADV76XX_PAGE_IO]->dev.of_node;
@@ -2808,7 +2809,11 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
if (!endpoint)
return -EINVAL;
- v4l2_of_parse_endpoint(endpoint, &bus_cfg);
+ ret = v4l2_of_parse_endpoint(endpoint, &bus_cfg);
+ if (ret) {
+ of_node_put(endpoint);
+ return ret;
+ }
if (!of_property_read_u32(endpoint, "default-input", &v))
state->pdata.default_input = v;
--
2.4.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/8] [media] s5c73m3: Check v4l2_of_parse_endpoint() return value
2016-01-07 18:27 [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 1/8] [media] v4l: of: Correct v4l2_of_parse_endpoint() kernel-doc Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 2/8] [media] adv7604: Check v4l2_of_parse_endpoint() return value Javier Martinez Canillas
@ 2016-01-07 18:27 ` Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 4/8] [media] s5k5baf: " Javier Martinez Canillas
` (5 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2016-01-07 18:27 UTC (permalink / raw
To: linux-kernel
Cc: Javier Martinez Canillas, Mauro Carvalho Chehab, Andrzej Hajda,
Kyungmin Park, linux-media
The v4l2_of_parse_endpoint() function can fail so check the return value.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/media/i2c/s5c73m3/s5c73m3-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index 57b3d27993a4..08af58fb8e7d 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -1639,8 +1639,10 @@ static int s5c73m3_get_platform_data(struct s5c73m3 *state)
return 0;
}
- v4l2_of_parse_endpoint(node_ep, &ep);
+ ret = v4l2_of_parse_endpoint(node_ep, &ep);
of_node_put(node_ep);
+ if (ret)
+ return ret;
if (ep.bus_type != V4L2_MBUS_CSI2) {
dev_err(dev, "unsupported bus type\n");
--
2.4.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/8] [media] s5k5baf: Check v4l2_of_parse_endpoint() return value
2016-01-07 18:27 [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Javier Martinez Canillas
` (2 preceding siblings ...)
2016-01-07 18:27 ` [PATCH 3/8] [media] s5c73m3: " Javier Martinez Canillas
@ 2016-01-07 18:27 ` Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 5/8] [media] tvp514x: " Javier Martinez Canillas
` (4 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2016-01-07 18:27 UTC (permalink / raw
To: linux-kernel
Cc: Javier Martinez Canillas, Mauro Carvalho Chehab, Andrzej Hajda,
Kyungmin Park, linux-media
The v4l2_of_parse_endpoint() function can fail so check the return value.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/media/i2c/s5k5baf.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index fc3a5a8e6c9c..db82ed05792e 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -1868,8 +1868,11 @@ static int s5k5baf_parse_device_node(struct s5k5baf *state, struct device *dev)
return -EINVAL;
}
- v4l2_of_parse_endpoint(node_ep, &ep);
+ ret = v4l2_of_parse_endpoint(node_ep, &ep);
of_node_put(node_ep);
+ if (ret)
+ return ret;
+
state->bus_type = ep.bus_type;
switch (state->bus_type) {
--
2.4.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/8] [media] tvp514x: Check v4l2_of_parse_endpoint() return value
2016-01-07 18:27 [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Javier Martinez Canillas
` (3 preceding siblings ...)
2016-01-07 18:27 ` [PATCH 4/8] [media] s5k5baf: " Javier Martinez Canillas
@ 2016-01-07 18:27 ` Javier Martinez Canillas
2016-01-09 23:01 ` Sakari Ailus
2016-01-07 18:27 ` [PATCH 6/8] [media] tvp7002: " Javier Martinez Canillas
` (3 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Javier Martinez Canillas @ 2016-01-07 18:27 UTC (permalink / raw
To: linux-kernel
Cc: Javier Martinez Canillas, Mauro Carvalho Chehab, Sakari Ailus,
Prabhakar", Ricardo Ribalda Delgado, Laurent Pinchart,
Hans Verkuil, linux-media
The v4l2_of_parse_endpoint() function can fail so check the return value.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/media/i2c/tvp514x.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index 7fa5f1e4fe37..7a1e20feade9 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -1013,11 +1013,13 @@ tvp514x_get_pdata(struct i2c_client *client)
if (!endpoint)
return NULL;
+ if (v4l2_of_parse_endpoint(endpoint, &bus_cfg))
+ goto done;
+
pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
goto done;
- v4l2_of_parse_endpoint(endpoint, &bus_cfg);
flags = bus_cfg.bus.parallel.flags;
if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
--
2.4.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 6/8] [media] tvp7002: Check v4l2_of_parse_endpoint() return value
2016-01-07 18:27 [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Javier Martinez Canillas
` (4 preceding siblings ...)
2016-01-07 18:27 ` [PATCH 5/8] [media] tvp514x: " Javier Martinez Canillas
@ 2016-01-07 18:27 ` Javier Martinez Canillas
2016-01-09 23:01 ` Sakari Ailus
2016-01-07 18:27 ` [PATCH 7/8] [media] exynos4-is: " Javier Martinez Canillas
` (2 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Javier Martinez Canillas @ 2016-01-07 18:27 UTC (permalink / raw
To: linux-kernel
Cc: Javier Martinez Canillas, Mauro Carvalho Chehab, Sakari Ailus,
Prabhakar", Ricardo Ribalda Delgado, Guennadi Liakhovetski,
Hans Verkuil, linux-media
The v4l2_of_parse_endpoint() function can fail so check the return value.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/media/i2c/tvp7002.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index 83c79fa5f61d..4aac303da5d4 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -905,11 +905,13 @@ tvp7002_get_pdata(struct i2c_client *client)
if (!endpoint)
return NULL;
+ if (v4l2_of_parse_endpoint(endpoint, &bus_cfg))
+ goto done;
+
pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
goto done;
- v4l2_of_parse_endpoint(endpoint, &bus_cfg);
flags = bus_cfg.bus.parallel.flags;
if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
--
2.4.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 7/8] [media] exynos4-is: Check v4l2_of_parse_endpoint() return value
2016-01-07 18:27 [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Javier Martinez Canillas
` (5 preceding siblings ...)
2016-01-07 18:27 ` [PATCH 6/8] [media] tvp7002: " Javier Martinez Canillas
@ 2016-01-07 18:27 ` Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 8/8] [media] omap3isp: " Javier Martinez Canillas
2016-01-09 23:03 ` [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Sakari Ailus
8 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2016-01-07 18:27 UTC (permalink / raw
To: linux-kernel
Cc: Javier Martinez Canillas, Kukjin Kim, Mauro Carvalho Chehab,
linux-samsung-soc, Kyungmin Park, Krzysztof Kozlowski,
Sylwester Nawrocki, linux-arm-kernel, linux-media
The v4l2_of_parse_endpoint() function can fail so check the return value.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/media/platform/exynos4-is/media-dev.c | 8 +++++++-
drivers/media/platform/exynos4-is/mipi-csis.c | 10 +++++++---
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index f3b2dd30ec77..662d4a5c584e 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -332,13 +332,19 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
struct fimc_source_info *pd = &fmd->sensor[index].pdata;
struct device_node *rem, *ep, *np;
struct v4l2_of_endpoint endpoint;
+ int ret;
/* Assume here a port node can have only one endpoint node. */
ep = of_get_next_child(port, NULL);
if (!ep)
return 0;
- v4l2_of_parse_endpoint(ep, &endpoint);
+ ret = v4l2_of_parse_endpoint(ep, &endpoint);
+ if (ret) {
+ of_node_put(ep);
+ return ret;
+ }
+
if (WARN_ON(endpoint.base.port == 0) || index >= FIMC_MAX_SENSORS)
return -EINVAL;
diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
index ac5e50e595be..bd5c46c3d4b7 100644
--- a/drivers/media/platform/exynos4-is/mipi-csis.c
+++ b/drivers/media/platform/exynos4-is/mipi-csis.c
@@ -736,6 +736,7 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
{
struct device_node *node = pdev->dev.of_node;
struct v4l2_of_endpoint endpoint;
+ int ret;
if (of_property_read_u32(node, "clock-frequency",
&state->clk_frequency))
@@ -751,7 +752,9 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
return -EINVAL;
}
/* Get port node and validate MIPI-CSI channel id. */
- v4l2_of_parse_endpoint(node, &endpoint);
+ ret = v4l2_of_parse_endpoint(node, &endpoint);
+ if (ret)
+ goto err;
state->index = endpoint.base.port - FIMC_INPUT_MIPI_CSI2_0;
if (state->index >= CSIS_MAX_ENTITIES)
@@ -764,9 +767,10 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
"samsung,csis-wclk");
state->num_lanes = endpoint.bus.mipi_csi2.num_data_lanes;
- of_node_put(node);
- return 0;
+err:
+ of_node_put(node);
+ return ret;
}
static int s5pcsis_pm_resume(struct device *dev, bool runtime);
--
2.4.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 8/8] [media] omap3isp: Check v4l2_of_parse_endpoint() return value
2016-01-07 18:27 [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Javier Martinez Canillas
` (6 preceding siblings ...)
2016-01-07 18:27 ` [PATCH 7/8] [media] exynos4-is: " Javier Martinez Canillas
@ 2016-01-07 18:27 ` Javier Martinez Canillas
2016-01-11 1:58 ` Laurent Pinchart
2016-01-09 23:03 ` [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Sakari Ailus
8 siblings, 1 reply; 16+ messages in thread
From: Javier Martinez Canillas @ 2016-01-07 18:27 UTC (permalink / raw
To: linux-kernel
Cc: Javier Martinez Canillas, Mauro Carvalho Chehab, Laurent Pinchart,
linux-media
The v4l2_of_parse_endpoint() function can fail so check the return value.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
drivers/media/platform/omap3isp/isp.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 79a0b953bba3..891e54394a1c 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2235,8 +2235,11 @@ static int isp_of_parse_node(struct device *dev, struct device_node *node,
struct isp_bus_cfg *buscfg = &isd->bus;
struct v4l2_of_endpoint vep;
unsigned int i;
+ int ret;
- v4l2_of_parse_endpoint(node, &vep);
+ ret = v4l2_of_parse_endpoint(node, &vep);
+ if (ret)
+ return ret;
dev_dbg(dev, "parsing endpoint %s, interface %u\n", node->full_name,
vep.base.port);
--
2.4.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/8] [media] v4l: of: Correct v4l2_of_parse_endpoint() kernel-doc
2016-01-07 18:27 ` [PATCH 1/8] [media] v4l: of: Correct v4l2_of_parse_endpoint() kernel-doc Javier Martinez Canillas
@ 2016-01-07 19:56 ` Sakari Ailus
2016-01-11 1:57 ` Laurent Pinchart
1 sibling, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2016-01-07 19:56 UTC (permalink / raw
To: Javier Martinez Canillas
Cc: linux-kernel, Nikhil Devshatwar, Mauro Carvalho Chehab,
Sylwester Nawrocki, Laurent Pinchart, Hans Verkuil, linux-media
On Thu, Jan 07, 2016 at 03:27:15PM -0300, Javier Martinez Canillas wrote:
> The v4l2_of_parse_endpoint function kernel-doc says that the return value
> is always 0. But that is not true since the function can fail and a error
> negative code is returned on failure. So correct the kernel-doc to match.
>
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
>
> drivers/media/v4l2-core/v4l2-of.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
> index b27cbb1f5afe..93b33681776c 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -146,7 +146,7 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
> * variable without a low fixed limit. Please use
> * v4l2_of_alloc_parse_endpoint() in new drivers instead.
> *
> - * Return: 0.
> + * Return: 0 on success or a negative error code on failure.
> */
> int v4l2_of_parse_endpoint(const struct device_node *node,
> struct v4l2_of_endpoint *endpoint)
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 6/8] [media] tvp7002: Check v4l2_of_parse_endpoint() return value
2016-01-07 18:27 ` [PATCH 6/8] [media] tvp7002: " Javier Martinez Canillas
@ 2016-01-09 23:01 ` Sakari Ailus
0 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2016-01-09 23:01 UTC (permalink / raw
To: Javier Martinez Canillas
Cc: linux-kernel, Mauro Carvalho Chehab, Sakari Ailus, Prabhakar",
Ricardo Ribalda Delgado, Guennadi Liakhovetski, Hans Verkuil,
linux-media
Hi Javier,
On Thu, Jan 07, 2016 at 03:27:20PM -0300, Javier Martinez Canillas wrote:
> The v4l2_of_parse_endpoint() function can fail so check the return value.
>
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
>
> drivers/media/i2c/tvp7002.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
> index 83c79fa5f61d..4aac303da5d4 100644
> --- a/drivers/media/i2c/tvp7002.c
> +++ b/drivers/media/i2c/tvp7002.c
> @@ -905,11 +905,13 @@ tvp7002_get_pdata(struct i2c_client *client)
> if (!endpoint)
> return NULL;
>
> + if (v4l2_of_parse_endpoint(endpoint, &bus_cfg))
pdata is uninitialised here. There are many ways to fix this but I think I'd
just assign it to NULL in variable declaration.
> + goto done;
> +
> pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
> if (!pdata)
> goto done;
>
> - v4l2_of_parse_endpoint(endpoint, &bus_cfg);
> flags = bus_cfg.bus.parallel.flags;
>
> if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/8] [media] tvp514x: Check v4l2_of_parse_endpoint() return value
2016-01-07 18:27 ` [PATCH 5/8] [media] tvp514x: " Javier Martinez Canillas
@ 2016-01-09 23:01 ` Sakari Ailus
0 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2016-01-09 23:01 UTC (permalink / raw
To: Javier Martinez Canillas
Cc: linux-kernel, Mauro Carvalho Chehab, Sakari Ailus, Prabhakar",
Ricardo Ribalda Delgado, Laurent Pinchart, Hans Verkuil,
linux-media
On Thu, Jan 07, 2016 at 03:27:19PM -0300, Javier Martinez Canillas wrote:
> The v4l2_of_parse_endpoint() function can fail so check the return value.
>
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
>
> drivers/media/i2c/tvp514x.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
> index 7fa5f1e4fe37..7a1e20feade9 100644
> --- a/drivers/media/i2c/tvp514x.c
> +++ b/drivers/media/i2c/tvp514x.c
> @@ -1013,11 +1013,13 @@ tvp514x_get_pdata(struct i2c_client *client)
> if (!endpoint)
> return NULL;
>
> + if (v4l2_of_parse_endpoint(endpoint, &bus_cfg))
Same as in patch 6.
> + goto done;
> +
> pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
> if (!pdata)
> goto done;
>
> - v4l2_of_parse_endpoint(endpoint, &bus_cfg);
> flags = bus_cfg.bus.parallel.flags;
>
> if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers
2016-01-07 18:27 [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Javier Martinez Canillas
` (7 preceding siblings ...)
2016-01-07 18:27 ` [PATCH 8/8] [media] omap3isp: " Javier Martinez Canillas
@ 2016-01-09 23:03 ` Sakari Ailus
2016-01-11 16:49 ` Javier Martinez Canillas
8 siblings, 1 reply; 16+ messages in thread
From: Sakari Ailus @ 2016-01-09 23:03 UTC (permalink / raw
To: Javier Martinez Canillas
Cc: linux-kernel, Nikhil Devshatwar, Kukjin Kim, Krzysztof Kozlowski,
Andrzej Hajda, Mauro Carvalho Chehab, Guennadi Liakhovetski,
linux-samsung-soc, Kyungmin Park, Sylwester Nawrocki,
Sakari Ailus, Prabhakar", Ricardo Ribalda Delgado,
Laurent Pinchart, Hans Verkuil, linux-arm-kernel, linux-media
Hi Javier,
On Thu, Jan 07, 2016 at 03:27:14PM -0300, Javier Martinez Canillas wrote:
> Hello,
>
> When discussing a patch [0] with Laurent Pinchart for another series I
> mentioned to him that most callers of v4l2_of_parse_endpoint() weren't
> checking the return value. This is likely due the function kernel-doc
> stating incorrectly that the return value is always 0 but can return a
> negative error code on failure.
>
> This trivial patch series fixes the function kernel-doc and add proper
> error checking in all the drivers that are currently not doing so.
After fixing patches 5 and 6,
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/8] [media] v4l: of: Correct v4l2_of_parse_endpoint() kernel-doc
2016-01-07 18:27 ` [PATCH 1/8] [media] v4l: of: Correct v4l2_of_parse_endpoint() kernel-doc Javier Martinez Canillas
2016-01-07 19:56 ` Sakari Ailus
@ 2016-01-11 1:57 ` Laurent Pinchart
1 sibling, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2016-01-11 1:57 UTC (permalink / raw
To: Javier Martinez Canillas
Cc: linux-kernel, Sakari Ailus, Nikhil Devshatwar,
Mauro Carvalho Chehab, Sylwester Nawrocki, Hans Verkuil,
linux-media
Hi Javier,
Thank you for the patch.
On Thursday 07 January 2016 15:27:15 Javier Martinez Canillas wrote:
> The v4l2_of_parse_endpoint function kernel-doc says that the return value
> is always 0. But that is not true since the function can fail and a error
> negative code is returned on failure. So correct the kernel-doc to match.
>
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>
> drivers/media/v4l2-core/v4l2-of.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-of.c
> b/drivers/media/v4l2-core/v4l2-of.c index b27cbb1f5afe..93b33681776c 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -146,7 +146,7 @@ static void v4l2_of_parse_parallel_bus(const struct
> device_node *node, * variable without a low fixed limit. Please use
> * v4l2_of_alloc_parse_endpoint() in new drivers instead.
> *
> - * Return: 0.
> + * Return: 0 on success or a negative error code on failure.
> */
> int v4l2_of_parse_endpoint(const struct device_node *node,
> struct v4l2_of_endpoint *endpoint)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 8/8] [media] omap3isp: Check v4l2_of_parse_endpoint() return value
2016-01-07 18:27 ` [PATCH 8/8] [media] omap3isp: " Javier Martinez Canillas
@ 2016-01-11 1:58 ` Laurent Pinchart
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2016-01-11 1:58 UTC (permalink / raw
To: Javier Martinez Canillas; +Cc: linux-kernel, Mauro Carvalho Chehab, linux-media
Hi Javier,
Thank you for the patch.
On Thursday 07 January 2016 15:27:22 Javier Martinez Canillas wrote:
> The v4l2_of_parse_endpoint() function can fail so check the return value.
>
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>
> drivers/media/platform/omap3isp/isp.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 79a0b953bba3..891e54394a1c
> 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2235,8 +2235,11 @@ static int isp_of_parse_node(struct device *dev,
> struct device_node *node, struct isp_bus_cfg *buscfg = &isd->bus;
> struct v4l2_of_endpoint vep;
> unsigned int i;
> + int ret;
>
> - v4l2_of_parse_endpoint(node, &vep);
> + ret = v4l2_of_parse_endpoint(node, &vep);
> + if (ret)
> + return ret;
>
> dev_dbg(dev, "parsing endpoint %s, interface %u\n", node->full_name,
> vep.base.port);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers
2016-01-09 23:03 ` [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Sakari Ailus
@ 2016-01-11 16:49 ` Javier Martinez Canillas
0 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2016-01-11 16:49 UTC (permalink / raw
To: Sakari Ailus
Cc: linux-kernel, Nikhil Devshatwar, Kukjin Kim, Krzysztof Kozlowski,
Andrzej Hajda, Mauro Carvalho Chehab, Guennadi Liakhovetski,
linux-samsung-soc, Kyungmin Park, Sylwester Nawrocki,
Sakari Ailus, Prabhakar", Ricardo Ribalda Delgado,
Laurent Pinchart, Hans Verkuil, linux-arm-kernel, linux-media
Hello Sakari,
On 01/09/2016 08:03 PM, Sakari Ailus wrote:
> Hi Javier,
>
> On Thu, Jan 07, 2016 at 03:27:14PM -0300, Javier Martinez Canillas wrote:
>> Hello,
>>
>> When discussing a patch [0] with Laurent Pinchart for another series I
>> mentioned to him that most callers of v4l2_of_parse_endpoint() weren't
>> checking the return value. This is likely due the function kernel-doc
>> stating incorrectly that the return value is always 0 but can return a
>> negative error code on failure.
>>
>> This trivial patch series fixes the function kernel-doc and add proper
>> error checking in all the drivers that are currently not doing so.
>
> After fixing patches 5 and 6,
>
Done, posted a v2 fixing the issues you pointed out.
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
Thanks a lot for your feedback and review!
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-01-11 16:49 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-07 18:27 [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 1/8] [media] v4l: of: Correct v4l2_of_parse_endpoint() kernel-doc Javier Martinez Canillas
2016-01-07 19:56 ` Sakari Ailus
2016-01-11 1:57 ` Laurent Pinchart
2016-01-07 18:27 ` [PATCH 2/8] [media] adv7604: Check v4l2_of_parse_endpoint() return value Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 3/8] [media] s5c73m3: " Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 4/8] [media] s5k5baf: " Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 5/8] [media] tvp514x: " Javier Martinez Canillas
2016-01-09 23:01 ` Sakari Ailus
2016-01-07 18:27 ` [PATCH 6/8] [media] tvp7002: " Javier Martinez Canillas
2016-01-09 23:01 ` Sakari Ailus
2016-01-07 18:27 ` [PATCH 7/8] [media] exynos4-is: " Javier Martinez Canillas
2016-01-07 18:27 ` [PATCH 8/8] [media] omap3isp: " Javier Martinez Canillas
2016-01-11 1:58 ` Laurent Pinchart
2016-01-09 23:03 ` [PATCH 0/8] [media] Check v4l2_of_parse_endpoint() ret val in all drivers Sakari Ailus
2016-01-11 16:49 ` Javier Martinez Canillas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).