[PATCH] media: v4l2-fwnode: use the cached value instead of getting again

[PATCH] media: v4l2-fwnode: use the cached value instead of getting again

[Mauro Carvalho Chehab]

There is a get/put operation in order to get firmware is_available
data there at the __v4l2_async_notifier_parse_fwnode_endpoints()
function. However, instead of using it, the code just reads again
without the lock. That's probably a mistake, as a similar code on
another function use the cached value.

This solves this smatch warning:

drivers/media/v4l2-core/v4l2-fwnode.c:453:8: warning: variable 'is_available' set but not used [-Wunused-but-set-variable]
   bool is_available;
        ^~~~~~~~~~~~

Fixes: 9ca465312132 ("media: v4l: fwnode: Support generic parsing of graph endpoints in a device")
Cc: Sakari Ailus <sakari.ailus@iki.fi>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 3b9c6afb49a3..681b192420d9 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -455,8 +455,7 @@ static int __v4l2_async_notifier_parse_fwnode_endpoints(
 		dev_fwnode = fwnode_graph_get_port_parent(fwnode);
 		is_available = fwnode_device_is_available(dev_fwnode);
 		fwnode_handle_put(dev_fwnode);
-
-		if (!fwnode_device_is_available(dev_fwnode))
+		if (!is_available)
 			continue;
 
 		if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
-- 
2.13.6

Re: [PATCH] media: v4l2-fwnode: use the cached value instead of getting again

[Sakari Ailus]

Hi Mauro,

On Tue, Oct 31, 2017 at 02:22:59PM -0400, Mauro Carvalho Chehab wrote:
> There is a get/put operation in order to get firmware is_available
> data there at the __v4l2_async_notifier_parse_fwnode_endpoints()
> function. However, instead of using it, the code just reads again
> without the lock. That's probably a mistake, as a similar code on
> another function use the cached value.
> 
> This solves this smatch warning:
> 
> drivers/media/v4l2-core/v4l2-fwnode.c:453:8: warning: variable 'is_available' set but not used [-Wunused-but-set-variable]
>    bool is_available;
>         ^~~~~~~~~~~~
> 
> Fixes: 9ca465312132 ("media: v4l: fwnode: Support generic parsing of graph endpoints in a device")
> Cc: Sakari Ailus <sakari.ailus@iki.fi>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 3b9c6afb49a3..681b192420d9 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -455,8 +455,7 @@ static int __v4l2_async_notifier_parse_fwnode_endpoints(
>  		dev_fwnode = fwnode_graph_get_port_parent(fwnode);
>  		is_available = fwnode_device_is_available(dev_fwnode);
>  		fwnode_handle_put(dev_fwnode);
> -
> -		if (!fwnode_device_is_available(dev_fwnode))
> +		if (!is_available)
>  			continue;
>  
>  		if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

This is actually a bug: dev_fwnode isn't guaranteed to be there once
fwnode_handle_put() has been called on it. Good catch!

-- 
Cheers,

Sakari Ailus
e-mail: sakari.ailus@iki.fi

Re: [PATCH] media: v4l2-fwnode: use the cached value instead of getting again

[Sebastian Reichel]

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

Hi,

On Tue, Oct 31, 2017 at 02:22:59PM -0400, Mauro Carvalho Chehab wrote:
> There is a get/put operation in order to get firmware is_available
> data there at the __v4l2_async_notifier_parse_fwnode_endpoints()
> function. However, instead of using it, the code just reads again
> without the lock. That's probably a mistake, as a similar code on
> another function use the cached value.
> 
> This solves this smatch warning:
> 
> drivers/media/v4l2-core/v4l2-fwnode.c:453:8: warning: variable 'is_available' set but not used [-Wunused-but-set-variable]
>    bool is_available;
>         ^~~~~~~~~~~~
> 
> Fixes: 9ca465312132 ("media: v4l: fwnode: Support generic parsing of graph endpoints in a device")
> Cc: Sakari Ailus <sakari.ailus@iki.fi>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

-- Sebastian

>  drivers/media/v4l2-core/v4l2-fwnode.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 3b9c6afb49a3..681b192420d9 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -455,8 +455,7 @@ static int __v4l2_async_notifier_parse_fwnode_endpoints(
>  		dev_fwnode = fwnode_graph_get_port_parent(fwnode);
>  		is_available = fwnode_device_is_available(dev_fwnode);
>  		fwnode_handle_put(dev_fwnode);
> -
> -		if (!fwnode_device_is_available(dev_fwnode))
> +		if (!is_available)
>  			continue;
>  
>  		if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
> -- 
> 2.13.6
> 

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