Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
* [PATCHv2 5/8] staging: media: atomisp: Refactor ia_css_stream_load()
@ 2021-04-20 15:03 Martiros Shakhzadyan
  2021-04-23  9:26 ` Hans Verkuil
  2021-04-23  9:28 ` Hans Verkuil
  0 siblings, 2 replies; 3+ messages in thread
From: Martiros Shakhzadyan @ 2021-04-20 15:03 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Martiros Shakhzadyan, Mauro Carvalho Chehab, Sakari Ailus, linux-media

Move the support check to the beginning of the function.
Change the logic to avoid multiple nesting blocks.
Move variable assignment outside of the if statement.
Remove an unnecessary check.

Signed-off-by: Martiros Shakhzadyan <vrzh@vrzh.net>
---
 drivers/staging/media/atomisp/pci/sh_css.c | 72 +++++++++++-----------
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 4d4f030e0fe0..bd96c4ab04d0 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -9615,48 +9615,46 @@ ia_css_stream_get_info(const struct ia_css_stream *stream,
 int
 ia_css_stream_load(struct ia_css_stream *stream)
 {
-	if (!IS_ISP2401) {
-		int i;
-		int err;
+	int i, j, err;
 
-		assert(stream);
-		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() enter,\n");
-		for (i = 0; i < MAX_ACTIVE_STREAMS; i++) {
-			if (my_css_save.stream_seeds[i].stream == stream) {
-				int j;
-
-				for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++) {
-					if ((err = ia_css_pipe_create(&my_css_save.stream_seeds[i].pipe_config[j],
-								    &my_css_save.stream_seeds[i].pipes[j])) != 0) {
-						if (j) {
-							int k;
-
-							for (k = 0; k < j; k++)
-								ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[k]);
-						}
-						return err;
-					}
-				}
-				err = ia_css_stream_create(&my_css_save.stream_seeds[i].stream_config,
-							my_css_save.stream_seeds[i].num_pipes,
-							my_css_save.stream_seeds[i].pipes,
-							&my_css_save.stream_seeds[i].stream);
-				if (err) {
-					ia_css_stream_destroy(stream);
-					for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++)
-						ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[j]);
-					return err;
-				}
-				break;
-			}
-		}
-		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() exit,\n");
-		return 0;
-	} else {
+	if (IS_ISP2401) {
 		/* TODO remove function - DEPRECATED */
 		(void)stream;
 		return -ENOTSUPP;
 	}
+
+	assert(stream);
+	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() enter,\n");
+	for (i = 0; i < MAX_ACTIVE_STREAMS; i++) {
+		if (my_css_save.stream_seeds[i].stream != stream)
+			continue;
+
+		for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++) {
+			int k;
+
+			err = ia_css_pipe_create(&my_css_save.stream_seeds[i].pipe_config[j],
+						 &my_css_save.stream_seeds[i].pipes[j]);
+			if (!err)
+				continue;
+
+			for (k = 0; k < j; k++)
+				ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[k]);
+			return err;
+		}
+		err = ia_css_stream_create(&my_css_save.stream_seeds[i].stream_config,
+					   my_css_save.stream_seeds[i].num_pipes,
+					   my_css_save.stream_seeds[i].pipes,
+					   &my_css_save.stream_seeds[i].stream);
+		if (err) {
+			ia_css_stream_destroy(stream);
+			for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++)
+				ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[j]);
+			return err;
+		}
+	}
+
+	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() exit,\n");
+	return 0;
 }
 
 int
-- 
2.31.1


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

* Re: [PATCHv2 5/8] staging: media: atomisp: Refactor ia_css_stream_load()
  2021-04-20 15:03 [PATCHv2 5/8] staging: media: atomisp: Refactor ia_css_stream_load() Martiros Shakhzadyan
@ 2021-04-23  9:26 ` Hans Verkuil
  2021-04-23  9:28 ` Hans Verkuil
  1 sibling, 0 replies; 3+ messages in thread
From: Hans Verkuil @ 2021-04-23  9:26 UTC (permalink / raw)
  To: Martiros Shakhzadyan; +Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media

On 20/04/2021 17:03, Martiros Shakhzadyan wrote:
> Move the support check to the beginning of the function.
> Change the logic to avoid multiple nesting blocks.
> Move variable assignment outside of the if statement.
> Remove an unnecessary check.
> 
> Signed-off-by: Martiros Shakhzadyan <vrzh@vrzh.net>
> ---
>  drivers/staging/media/atomisp/pci/sh_css.c | 72 +++++++++++-----------
>  1 file changed, 35 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index 4d4f030e0fe0..bd96c4ab04d0 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -9615,48 +9615,46 @@ ia_css_stream_get_info(const struct ia_css_stream *stream,
>  int
>  ia_css_stream_load(struct ia_css_stream *stream)
>  {
> -	if (!IS_ISP2401) {
> -		int i;
> -		int err;
> +	int i, j, err;
>  
> -		assert(stream);
> -		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() enter,\n");
> -		for (i = 0; i < MAX_ACTIVE_STREAMS; i++) {
> -			if (my_css_save.stream_seeds[i].stream == stream) {
> -				int j;
> -
> -				for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++) {
> -					if ((err = ia_css_pipe_create(&my_css_save.stream_seeds[i].pipe_config[j],
> -								    &my_css_save.stream_seeds[i].pipes[j])) != 0) {
> -						if (j) {
> -							int k;
> -
> -							for (k = 0; k < j; k++)
> -								ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[k]);
> -						}
> -						return err;
> -					}
> -				}
> -				err = ia_css_stream_create(&my_css_save.stream_seeds[i].stream_config,
> -							my_css_save.stream_seeds[i].num_pipes,
> -							my_css_save.stream_seeds[i].pipes,
> -							&my_css_save.stream_seeds[i].stream);
> -				if (err) {
> -					ia_css_stream_destroy(stream);
> -					for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++)
> -						ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[j]);
> -					return err;
> -				}
> -				break;
> -			}
> -		}
> -		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() exit,\n");
> -		return 0;
> -	} else {
> +	if (IS_ISP2401) {
>  		/* TODO remove function - DEPRECATED */
>  		(void)stream;
>  		return -ENOTSUPP;
>  	}
> +
> +	assert(stream);
> +	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() enter,\n");
> +	for (i = 0; i < MAX_ACTIVE_STREAMS; i++) {
> +		if (my_css_save.stream_seeds[i].stream != stream)
> +			continue;
> +
> +		for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++) {
> +			int k;
> +
> +			err = ia_css_pipe_create(&my_css_save.stream_seeds[i].pipe_config[j],
> +						 &my_css_save.stream_seeds[i].pipes[j]);
> +			if (!err)
> +				continue;
> +
> +			for (k = 0; k < j; k++)
> +				ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[k]);
> +			return err;
> +		}
> +		err = ia_css_stream_create(&my_css_save.stream_seeds[i].stream_config,
> +					   my_css_save.stream_seeds[i].num_pipes,
> +					   my_css_save.stream_seeds[i].pipes,
> +					   &my_css_save.stream_seeds[i].stream);
> +		if (err) {
> +			ia_css_stream_destroy(stream);
> +			for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++)
> +				ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[j]);
> +			return err;
> +		}

You dropped a 'break' here!

Regards,

	Hans

> +	}
> +
> +	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() exit,\n");
> +	return 0;
>  }
>  
>  int
> 


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

* Re: [PATCHv2 5/8] staging: media: atomisp: Refactor ia_css_stream_load()
  2021-04-20 15:03 [PATCHv2 5/8] staging: media: atomisp: Refactor ia_css_stream_load() Martiros Shakhzadyan
  2021-04-23  9:26 ` Hans Verkuil
@ 2021-04-23  9:28 ` Hans Verkuil
  1 sibling, 0 replies; 3+ messages in thread
From: Hans Verkuil @ 2021-04-23  9:28 UTC (permalink / raw)
  To: Martiros Shakhzadyan; +Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media

On 20/04/2021 17:03, Martiros Shakhzadyan wrote:
> Move the support check to the beginning of the function.
> Change the logic to avoid multiple nesting blocks.
> Move variable assignment outside of the if statement.

Which variable? Mention it explicitly: 'Move 'err' variable assignment...'

> Remove an unnecessary check.

Which check?

Regards,

	Hans

> 
> Signed-off-by: Martiros Shakhzadyan <vrzh@vrzh.net>
> ---
>  drivers/staging/media/atomisp/pci/sh_css.c | 72 +++++++++++-----------
>  1 file changed, 35 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index 4d4f030e0fe0..bd96c4ab04d0 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -9615,48 +9615,46 @@ ia_css_stream_get_info(const struct ia_css_stream *stream,
>  int
>  ia_css_stream_load(struct ia_css_stream *stream)
>  {
> -	if (!IS_ISP2401) {
> -		int i;
> -		int err;
> +	int i, j, err;
>  
> -		assert(stream);
> -		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() enter,\n");
> -		for (i = 0; i < MAX_ACTIVE_STREAMS; i++) {
> -			if (my_css_save.stream_seeds[i].stream == stream) {
> -				int j;
> -
> -				for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++) {
> -					if ((err = ia_css_pipe_create(&my_css_save.stream_seeds[i].pipe_config[j],
> -								    &my_css_save.stream_seeds[i].pipes[j])) != 0) {
> -						if (j) {
> -							int k;
> -
> -							for (k = 0; k < j; k++)
> -								ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[k]);
> -						}
> -						return err;
> -					}
> -				}
> -				err = ia_css_stream_create(&my_css_save.stream_seeds[i].stream_config,
> -							my_css_save.stream_seeds[i].num_pipes,
> -							my_css_save.stream_seeds[i].pipes,
> -							&my_css_save.stream_seeds[i].stream);
> -				if (err) {
> -					ia_css_stream_destroy(stream);
> -					for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++)
> -						ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[j]);
> -					return err;
> -				}
> -				break;
> -			}
> -		}
> -		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() exit,\n");
> -		return 0;
> -	} else {
> +	if (IS_ISP2401) {
>  		/* TODO remove function - DEPRECATED */
>  		(void)stream;
>  		return -ENOTSUPP;
>  	}
> +
> +	assert(stream);
> +	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() enter,\n");
> +	for (i = 0; i < MAX_ACTIVE_STREAMS; i++) {
> +		if (my_css_save.stream_seeds[i].stream != stream)
> +			continue;
> +
> +		for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++) {
> +			int k;
> +
> +			err = ia_css_pipe_create(&my_css_save.stream_seeds[i].pipe_config[j],
> +						 &my_css_save.stream_seeds[i].pipes[j]);
> +			if (!err)
> +				continue;
> +
> +			for (k = 0; k < j; k++)
> +				ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[k]);
> +			return err;
> +		}
> +		err = ia_css_stream_create(&my_css_save.stream_seeds[i].stream_config,
> +					   my_css_save.stream_seeds[i].num_pipes,
> +					   my_css_save.stream_seeds[i].pipes,
> +					   &my_css_save.stream_seeds[i].stream);
> +		if (err) {
> +			ia_css_stream_destroy(stream);
> +			for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++)
> +				ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[j]);
> +			return err;
> +		}
> +	}
> +
> +	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() exit,\n");
> +	return 0;
>  }
>  
>  int
> 


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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-20 15:03 [PATCHv2 5/8] staging: media: atomisp: Refactor ia_css_stream_load() Martiros Shakhzadyan
2021-04-23  9:26 ` Hans Verkuil
2021-04-23  9:28 ` Hans Verkuil

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.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-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


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