Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] staging: media: sunxi: Change return type of cedrus_find_format()
@ 2019-07-03  8:13 Nishka Dasgupta
  2019-07-03  8:13 ` [PATCH 2/2] staging: media: sunxi: Replace function cedrus_check_format() Nishka Dasgupta
  2019-07-05 10:26 ` [PATCH 1/2] staging: media: sunxi: Change return type of cedrus_find_format() Paul Kocialkowski
  0 siblings, 2 replies; 7+ messages in thread
From: Nishka Dasgupta @ 2019-07-03  8:13 UTC (permalink / raw)
  To: maxime.ripard, paul.kocialkowski, mchehab, gregkh, wens,
	linux-media, devel, linux-arm-kernel
  Cc: Nishka Dasgupta

Change return type of cedrus_find_format to bool as it is only called
once, by a function whose return value is bool, and the return value of
cedrus_find_format is returned as-is at the call-site.
Issue found with Coccinelle.

Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus_video.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index 9673874ece10..0ec31b9e0aea 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -55,8 +55,8 @@ static inline struct cedrus_ctx *cedrus_file2ctx(struct file *file)
 	return container_of(file->private_data, struct cedrus_ctx, fh);
 }
 
-static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions,
-						unsigned int capabilities)
+static bool cedrus_find_format(u32 pixelformat, u32 directions,
+			       unsigned int capabilities)
 {
 	struct cedrus_format *fmt;
 	unsigned int i;
@@ -70,13 +70,10 @@ static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions,
 
 		if (fmt->pixelformat == pixelformat &&
 		    (fmt->directions & directions) != 0)
-			break;
+			return true;
 	}
 
-	if (i == CEDRUS_FORMATS_COUNT)
-		return NULL;
-
-	return &cedrus_formats[i];
+	return false;
 }
 
 static bool cedrus_check_format(u32 pixelformat, u32 directions,
-- 
2.19.1


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] staging: media: sunxi: Replace function cedrus_check_format()
  2019-07-03  8:13 [PATCH 1/2] staging: media: sunxi: Change return type of cedrus_find_format() Nishka Dasgupta
@ 2019-07-03  8:13 ` Nishka Dasgupta
  2019-07-05 10:26   ` Paul Kocialkowski
  2019-07-05 10:26 ` [PATCH 1/2] staging: media: sunxi: Change return type of cedrus_find_format() Paul Kocialkowski
  1 sibling, 1 reply; 7+ messages in thread
From: Nishka Dasgupta @ 2019-07-03  8:13 UTC (permalink / raw)
  To: maxime.ripard, paul.kocialkowski, mchehab, gregkh, wens,
	linux-media, devel, linux-arm-kernel
  Cc: Nishka Dasgupta

Remove function cedrus_check_format as all it does is call
cedrus_find_format.
Rename cedrus_find_format to cedrus_check_format to maintain
compatibility with call sites.
Issue found with Coccinelle.

Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus_video.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index 0ec31b9e0aea..d5cc9ed04fd2 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -55,8 +55,8 @@ static inline struct cedrus_ctx *cedrus_file2ctx(struct file *file)
 	return container_of(file->private_data, struct cedrus_ctx, fh);
 }
 
-static bool cedrus_find_format(u32 pixelformat, u32 directions,
-			       unsigned int capabilities)
+static bool cedrus_check_format(u32 pixelformat, u32 directions,
+				unsigned int capabilities)
 {
 	struct cedrus_format *fmt;
 	unsigned int i;
@@ -76,12 +76,6 @@ static bool cedrus_find_format(u32 pixelformat, u32 directions,
 	return false;
 }
 
-static bool cedrus_check_format(u32 pixelformat, u32 directions,
-				unsigned int capabilities)
-{
-	return cedrus_find_format(pixelformat, directions, capabilities);
-}
-
 static void cedrus_prepare_format(struct v4l2_pix_format *pix_fmt)
 {
 	unsigned int width = pix_fmt->width;
-- 
2.19.1


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] staging: media: sunxi: Change return type of cedrus_find_format()
  2019-07-03  8:13 [PATCH 1/2] staging: media: sunxi: Change return type of cedrus_find_format() Nishka Dasgupta
  2019-07-03  8:13 ` [PATCH 2/2] staging: media: sunxi: Replace function cedrus_check_format() Nishka Dasgupta
@ 2019-07-05 10:26 ` Paul Kocialkowski
  2019-07-05 11:11   ` Nishka Dasgupta
  1 sibling, 1 reply; 7+ messages in thread
From: Paul Kocialkowski @ 2019-07-05 10:26 UTC (permalink / raw)
  To: Nishka Dasgupta
  Cc: devel, maxime.ripard, gregkh, wens, mchehab, linux-arm-kernel,
	linux-media

Hi,

On Wed 03 Jul 19, 13:43, Nishka Dasgupta wrote:
> Change return type of cedrus_find_format to bool as it is only called
> once, by a function whose return value is bool, and the return value of
> cedrus_find_format is returned as-is at the call-site.
> Issue found with Coccinelle.

The purpose of this function (although definitely under-used at this point),
was to return the pointer to the element structure, not to indicate whether
the format format is part of the list or not.

In spite of that, this change reduces the use case for the function, so I do
not think it is beneficial, sorry.

Cheers,

Paul

> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus_video.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index 9673874ece10..0ec31b9e0aea 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -55,8 +55,8 @@ static inline struct cedrus_ctx *cedrus_file2ctx(struct file *file)
>  	return container_of(file->private_data, struct cedrus_ctx, fh);
>  }
>  
> -static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions,
> -						unsigned int capabilities)
> +static bool cedrus_find_format(u32 pixelformat, u32 directions,
> +			       unsigned int capabilities)
>  {
>  	struct cedrus_format *fmt;
>  	unsigned int i;
> @@ -70,13 +70,10 @@ static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions,
>  
>  		if (fmt->pixelformat == pixelformat &&
>  		    (fmt->directions & directions) != 0)
> -			break;
> +			return true;
>  	}
>  
> -	if (i == CEDRUS_FORMATS_COUNT)
> -		return NULL;
> -
> -	return &cedrus_formats[i];
> +	return false;
>  }
>  
>  static bool cedrus_check_format(u32 pixelformat, u32 directions,
> -- 
> 2.19.1
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] staging: media: sunxi: Replace function cedrus_check_format()
  2019-07-03  8:13 ` [PATCH 2/2] staging: media: sunxi: Replace function cedrus_check_format() Nishka Dasgupta
@ 2019-07-05 10:26   ` Paul Kocialkowski
  2019-07-05 12:13     ` Nishka Dasgupta
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Kocialkowski @ 2019-07-05 10:26 UTC (permalink / raw)
  To: Nishka Dasgupta
  Cc: devel, maxime.ripard, gregkh, wens, mchehab, linux-arm-kernel,
	linux-media

Hi,

On Wed 03 Jul 19, 13:43, Nishka Dasgupta wrote:
> Remove function cedrus_check_format as all it does is call
> cedrus_find_format.
> Rename cedrus_find_format to cedrus_check_format to maintain
> compatibility with call sites.
> Issue found with Coccinelle.

Maybe we could have a !! or a bool cast to make coccinelle happy here?

Cheers,

Paul

> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus_video.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index 0ec31b9e0aea..d5cc9ed04fd2 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -55,8 +55,8 @@ static inline struct cedrus_ctx *cedrus_file2ctx(struct file *file)
>  	return container_of(file->private_data, struct cedrus_ctx, fh);
>  }
>  
> -static bool cedrus_find_format(u32 pixelformat, u32 directions,
> -			       unsigned int capabilities)
> +static bool cedrus_check_format(u32 pixelformat, u32 directions,
> +				unsigned int capabilities)
>  {
>  	struct cedrus_format *fmt;
>  	unsigned int i;
> @@ -76,12 +76,6 @@ static bool cedrus_find_format(u32 pixelformat, u32 directions,
>  	return false;
>  }
>  
> -static bool cedrus_check_format(u32 pixelformat, u32 directions,
> -				unsigned int capabilities)
> -{
> -	return cedrus_find_format(pixelformat, directions, capabilities);
> -}
> -
>  static void cedrus_prepare_format(struct v4l2_pix_format *pix_fmt)
>  {
>  	unsigned int width = pix_fmt->width;
> -- 
> 2.19.1
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] staging: media: sunxi: Change return type of cedrus_find_format()
  2019-07-05 10:26 ` [PATCH 1/2] staging: media: sunxi: Change return type of cedrus_find_format() Paul Kocialkowski
@ 2019-07-05 11:11   ` Nishka Dasgupta
  0 siblings, 0 replies; 7+ messages in thread
From: Nishka Dasgupta @ 2019-07-05 11:11 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: devel, maxime.ripard, gregkh, wens, mchehab, linux-arm-kernel,
	linux-media

On 05/07/19 3:56 PM, Paul Kocialkowski wrote:
> Hi,
> 
> On Wed 03 Jul 19, 13:43, Nishka Dasgupta wrote:
>> Change return type of cedrus_find_format to bool as it is only called
>> once, by a function whose return value is bool, and the return value of
>> cedrus_find_format is returned as-is at the call-site.
>> Issue found with Coccinelle.
> 
> The purpose of this function (although definitely under-used at this point),
> was to return the pointer to the element structure, not to indicate whether
> the format format is part of the list or not.
> 
> In spite of that, this change reduces the use case for the function, so I do
> not think it is beneficial, sorry.

Okay, thank you for the clarification.

Nishka

> Cheers,
> 
> Paul
> 
>> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
>> ---
>>   drivers/staging/media/sunxi/cedrus/cedrus_video.c | 11 ++++-------
>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>> index 9673874ece10..0ec31b9e0aea 100644
>> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>> @@ -55,8 +55,8 @@ static inline struct cedrus_ctx *cedrus_file2ctx(struct file *file)
>>   	return container_of(file->private_data, struct cedrus_ctx, fh);
>>   }
>>   
>> -static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions,
>> -						unsigned int capabilities)
>> +static bool cedrus_find_format(u32 pixelformat, u32 directions,
>> +			       unsigned int capabilities)
>>   {
>>   	struct cedrus_format *fmt;
>>   	unsigned int i;
>> @@ -70,13 +70,10 @@ static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions,
>>   
>>   		if (fmt->pixelformat == pixelformat &&
>>   		    (fmt->directions & directions) != 0)
>> -			break;
>> +			return true;
>>   	}
>>   
>> -	if (i == CEDRUS_FORMATS_COUNT)
>> -		return NULL;
>> -
>> -	return &cedrus_formats[i];
>> +	return false;
>>   }
>>   
>>   static bool cedrus_check_format(u32 pixelformat, u32 directions,
>> -- 
>> 2.19.1
>>
> 


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] staging: media: sunxi: Replace function cedrus_check_format()
  2019-07-05 10:26   ` Paul Kocialkowski
@ 2019-07-05 12:13     ` Nishka Dasgupta
  2019-07-17  9:33       ` Paul Kocialkowski
  0 siblings, 1 reply; 7+ messages in thread
From: Nishka Dasgupta @ 2019-07-05 12:13 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: devel, maxime.ripard, gregkh, wens, mchehab, linux-arm-kernel,
	linux-media

On 05/07/19 3:56 PM, Paul Kocialkowski wrote:
> Hi,
> 
> On Wed 03 Jul 19, 13:43, Nishka Dasgupta wrote:
>> Remove function cedrus_check_format as all it does is call
>> cedrus_find_format.
>> Rename cedrus_find_format to cedrus_check_format to maintain
>> compatibility with call sites.
>> Issue found with Coccinelle.
> 
> Maybe we could have a !! or a bool cast to make coccinelle happy here?

Coccinelle didn't flag the type mismatch, just the single-line 
functions. I could add the bool cast then?

Thanking you,
Nishka

> Cheers,
> 
> Paul
> 
>> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
>> ---
>>   drivers/staging/media/sunxi/cedrus/cedrus_video.c | 10 ++--------
>>   1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>> index 0ec31b9e0aea..d5cc9ed04fd2 100644
>> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>> @@ -55,8 +55,8 @@ static inline struct cedrus_ctx *cedrus_file2ctx(struct file *file)
>>   	return container_of(file->private_data, struct cedrus_ctx, fh);
>>   }
>>   
>> -static bool cedrus_find_format(u32 pixelformat, u32 directions,
>> -			       unsigned int capabilities)
>> +static bool cedrus_check_format(u32 pixelformat, u32 directions,
>> +				unsigned int capabilities)
>>   {
>>   	struct cedrus_format *fmt;
>>   	unsigned int i;
>> @@ -76,12 +76,6 @@ static bool cedrus_find_format(u32 pixelformat, u32 directions,
>>   	return false;
>>   }
>>   
>> -static bool cedrus_check_format(u32 pixelformat, u32 directions,
>> -				unsigned int capabilities)
>> -{
>> -	return cedrus_find_format(pixelformat, directions, capabilities);
>> -}
>> -
>>   static void cedrus_prepare_format(struct v4l2_pix_format *pix_fmt)
>>   {
>>   	unsigned int width = pix_fmt->width;
>> -- 
>> 2.19.1
>>
> 


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] staging: media: sunxi: Replace function cedrus_check_format()
  2019-07-05 12:13     ` Nishka Dasgupta
@ 2019-07-17  9:33       ` Paul Kocialkowski
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Kocialkowski @ 2019-07-17  9:33 UTC (permalink / raw)
  To: Nishka Dasgupta
  Cc: devel, maxime.ripard, gregkh, wens, mchehab, linux-arm-kernel,
	linux-media

Hi,

On Fri 05 Jul 19, 17:43, Nishka Dasgupta wrote:
> On 05/07/19 3:56 PM, Paul Kocialkowski wrote:
> > Hi,
> > 
> > On Wed 03 Jul 19, 13:43, Nishka Dasgupta wrote:
> > > Remove function cedrus_check_format as all it does is call
> > > cedrus_find_format.
> > > Rename cedrus_find_format to cedrus_check_format to maintain
> > > compatibility with call sites.
> > > Issue found with Coccinelle.
> > 
> > Maybe we could have a !! or a bool cast to make coccinelle happy here?
> 
> Coccinelle didn't flag the type mismatch, just the single-line functions. I
> could add the bool cast then?

Looks like I failed to follow-up on this in due time, sorry.

Yes a bool cast would definitely be welcome :)

Cheers,

Paul

> Thanking you,
> Nishka
> 
> > Cheers,
> > 
> > Paul
> > 
> > > Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> > > ---
> > >   drivers/staging/media/sunxi/cedrus/cedrus_video.c | 10 ++--------
> > >   1 file changed, 2 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > > index 0ec31b9e0aea..d5cc9ed04fd2 100644
> > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > > @@ -55,8 +55,8 @@ static inline struct cedrus_ctx *cedrus_file2ctx(struct file *file)
> > >   	return container_of(file->private_data, struct cedrus_ctx, fh);
> > >   }
> > > -static bool cedrus_find_format(u32 pixelformat, u32 directions,
> > > -			       unsigned int capabilities)
> > > +static bool cedrus_check_format(u32 pixelformat, u32 directions,
> > > +				unsigned int capabilities)
> > >   {
> > >   	struct cedrus_format *fmt;
> > >   	unsigned int i;
> > > @@ -76,12 +76,6 @@ static bool cedrus_find_format(u32 pixelformat, u32 directions,
> > >   	return false;
> > >   }
> > > -static bool cedrus_check_format(u32 pixelformat, u32 directions,
> > > -				unsigned int capabilities)
> > > -{
> > > -	return cedrus_find_format(pixelformat, directions, capabilities);
> > > -}
> > > -
> > >   static void cedrus_prepare_format(struct v4l2_pix_format *pix_fmt)
> > >   {
> > >   	unsigned int width = pix_fmt->width;
> > > -- 
> > > 2.19.1
> > > 
> > 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-03  8:13 [PATCH 1/2] staging: media: sunxi: Change return type of cedrus_find_format() Nishka Dasgupta
2019-07-03  8:13 ` [PATCH 2/2] staging: media: sunxi: Replace function cedrus_check_format() Nishka Dasgupta
2019-07-05 10:26   ` Paul Kocialkowski
2019-07-05 12:13     ` Nishka Dasgupta
2019-07-17  9:33       ` Paul Kocialkowski
2019-07-05 10:26 ` [PATCH 1/2] staging: media: sunxi: Change return type of cedrus_find_format() Paul Kocialkowski
2019-07-05 11:11   ` Nishka Dasgupta

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org infradead-linux-arm-kernel@archiver.kernel.org
	public-inbox-index linux-arm-kernel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox