[PATCH] staging: media: atomisp: cleaning up sh_css.c

[PATCH] staging: media: atomisp: cleaning up sh_css.c

[Martiros Shakhzadyan]

The following set of patches for sh_css.c aims to resolve coding style issues
and remove redundancies.

Martiros Shakhzadyan (9):
  staging: media: atomisp: [1/6] Fix sh_css.c brace coding style issues
  staging: media: atomisp: [2/6] Fix sh_css.c brace coding style issues
  staging: media: atomisp: [3/6] Fix sh_css.c brace coding style issues
  staging: media: atomisp: [4/6] Fix sh_css.c brace coding style issues
  staging: media: atomisp: [5/6] Fix sh_css.c brace coding style issues
  staging: media: atomisp: [6/6] Fix sh_css.c brace coding style issues
  staging: media: atomisp: Use goto instead of return in ia_css_init()
  staging: media: atomisp: [1/2] Remove redundant assertions in sh_css.c
  staging: media: atomisp: [2/2] Remove redundant assertions in sh_css.c

 drivers/staging/media/atomisp/pci/sh_css.c | 266 ++++++++-------------
 1 file changed, 105 insertions(+), 161 deletions(-)

-- 
2.31.1

[PATCH] staging: media: atomisp: [1/6] Fix sh_css.c brace coding style issues

[Martiros Shakhzadyan]

Fix brace coding style issues.

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

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 27dd8ce8ba0a..10b70dfc0528 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -453,15 +453,15 @@ static enum ia_css_frame_format yuv422_copy_formats[] = {
  * by the copy binary given the stream format.
  * */
 static int
-verify_copy_out_frame_format(struct ia_css_pipe *pipe) {
+verify_copy_out_frame_format(struct ia_css_pipe *pipe)
+{
 	enum ia_css_frame_format out_fmt = pipe->output_info[0].format;
 	unsigned int i, found = 0;
 
 	assert(pipe);
 	assert(pipe->stream);
 
-	switch (pipe->stream->config.input_config.format)
-	{
+	switch (pipe->stream->config.input_config.format) {
 	case ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY:
 	case ATOMISP_INPUT_FORMAT_YUV420_8:
 		for (i = 0; i < ARRAY_SIZE(yuv420_copy_formats) && !found; i++)
@@ -528,7 +528,8 @@ ia_css_stream_input_format_bits_per_pixel(struct ia_css_stream *stream)
 
 #if !defined(ISP2401)
 static int
-sh_css_config_input_network(struct ia_css_stream *stream) {
+sh_css_config_input_network(struct ia_css_stream *stream)
+{
 	unsigned int fmt_type;
 	struct ia_css_pipe *pipe = stream->last_pipe;
 	struct ia_css_binary *binary = NULL;
@@ -554,8 +555,7 @@ sh_css_config_input_network(struct ia_css_stream *stream) {
 					stream->config.mode);
 
 	if ((binary && (binary->online || stream->config.continuous)) ||
-	    pipe->config.mode == IA_CSS_PIPE_MODE_COPY)
-	{
+	    pipe->config.mode == IA_CSS_PIPE_MODE_COPY) {
 		err = ia_css_ifmtr_configure(&stream->config,
 					     binary);
 		if (err)
@@ -563,8 +563,7 @@ sh_css_config_input_network(struct ia_css_stream *stream) {
 	}
 
 	if (stream->config.mode == IA_CSS_INPUT_MODE_TPG ||
-	    stream->config.mode == IA_CSS_INPUT_MODE_PRBS)
-	{
+	    stream->config.mode == IA_CSS_INPUT_MODE_PRBS) {
 		unsigned int hblank_cycles = 100,
 		vblank_lines = 6,
 		width,
@@ -723,35 +722,32 @@ static bool sh_css_translate_stream_cfg_to_input_system_input_port_id(
 	switch (stream_cfg->mode) {
 	case IA_CSS_INPUT_MODE_TPG:
 
-		if (stream_cfg->source.tpg.id == IA_CSS_TPG_ID0) {
+		if (stream_cfg->source.tpg.id == IA_CSS_TPG_ID0)
 			isys_stream_descr->input_port_id = INPUT_SYSTEM_PIXELGEN_PORT0_ID;
-		} else if (stream_cfg->source.tpg.id == IA_CSS_TPG_ID1) {
+		else if (stream_cfg->source.tpg.id == IA_CSS_TPG_ID1)
 			isys_stream_descr->input_port_id = INPUT_SYSTEM_PIXELGEN_PORT1_ID;
-		} else if (stream_cfg->source.tpg.id == IA_CSS_TPG_ID2) {
+		else if (stream_cfg->source.tpg.id == IA_CSS_TPG_ID2)
 			isys_stream_descr->input_port_id = INPUT_SYSTEM_PIXELGEN_PORT2_ID;
-		}
 
 		break;
 	case IA_CSS_INPUT_MODE_PRBS:
 
-		if (stream_cfg->source.prbs.id == IA_CSS_PRBS_ID0) {
+		if (stream_cfg->source.prbs.id == IA_CSS_PRBS_ID0)
 			isys_stream_descr->input_port_id = INPUT_SYSTEM_PIXELGEN_PORT0_ID;
-		} else if (stream_cfg->source.prbs.id == IA_CSS_PRBS_ID1) {
+		else if (stream_cfg->source.prbs.id == IA_CSS_PRBS_ID1)
 			isys_stream_descr->input_port_id = INPUT_SYSTEM_PIXELGEN_PORT1_ID;
-		} else if (stream_cfg->source.prbs.id == IA_CSS_PRBS_ID2) {
+		else if (stream_cfg->source.prbs.id == IA_CSS_PRBS_ID2)
 			isys_stream_descr->input_port_id = INPUT_SYSTEM_PIXELGEN_PORT2_ID;
-		}
 
 		break;
 	case IA_CSS_INPUT_MODE_BUFFERED_SENSOR:
 
-		if (stream_cfg->source.port.port == MIPI_PORT0_ID) {
+		if (stream_cfg->source.port.port == MIPI_PORT0_ID)
 			isys_stream_descr->input_port_id = INPUT_SYSTEM_CSI_PORT0_ID;
-		} else if (stream_cfg->source.port.port == MIPI_PORT1_ID) {
+		else if (stream_cfg->source.port.port == MIPI_PORT1_ID)
 			isys_stream_descr->input_port_id = INPUT_SYSTEM_CSI_PORT1_ID;
-		} else if (stream_cfg->source.port.port == MIPI_PORT2_ID) {
+		else if (stream_cfg->source.port.port == MIPI_PORT2_ID)
 			isys_stream_descr->input_port_id = INPUT_SYSTEM_CSI_PORT2_ID;
-		}
 
 		break;
 	default:
@@ -804,15 +800,14 @@ static bool sh_css_translate_stream_cfg_to_input_system_input_port_attr(
 	rc = true;
 	switch (stream_cfg->mode) {
 	case IA_CSS_INPUT_MODE_TPG:
-		if (stream_cfg->source.tpg.mode == IA_CSS_TPG_MODE_RAMP) {
+		if (stream_cfg->source.tpg.mode == IA_CSS_TPG_MODE_RAMP)
 			isys_stream_descr->tpg_port_attr.mode = PIXELGEN_TPG_MODE_RAMP;
-		} else if (stream_cfg->source.tpg.mode == IA_CSS_TPG_MODE_CHECKERBOARD) {
+		else if (stream_cfg->source.tpg.mode == IA_CSS_TPG_MODE_CHECKERBOARD)
 			isys_stream_descr->tpg_port_attr.mode = PIXELGEN_TPG_MODE_CHBO;
-		} else if (stream_cfg->source.tpg.mode == IA_CSS_TPG_MODE_MONO) {
+		else if (stream_cfg->source.tpg.mode == IA_CSS_TPG_MODE_MONO)
 			isys_stream_descr->tpg_port_attr.mode = PIXELGEN_TPG_MODE_MONO;
-		} else {
+		else
 			rc = false;
-		}
 
 		/*
 		 * TODO
@@ -951,12 +946,12 @@ static bool sh_css_translate_stream_cfg_to_input_system_input_port_resolution(
 	     stream_cfg->mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR) &&
 	    stream_cfg->source.port.compression.type != IA_CSS_CSI2_COMPRESSION_TYPE_NONE) {
 		if (stream_cfg->source.port.compression.uncompressed_bits_per_pixel ==
-		    UNCOMPRESSED_BITS_PER_PIXEL_10) {
+		    UNCOMPRESSED_BITS_PER_PIXEL_10)
 			fmt_type = ATOMISP_INPUT_FORMAT_RAW_10;
-		} else if (stream_cfg->source.port.compression.uncompressed_bits_per_pixel ==
-			   UNCOMPRESSED_BITS_PER_PIXEL_12) {
+		else if (stream_cfg->source.port.compression.uncompressed_bits_per_pixel ==
+			   UNCOMPRESSED_BITS_PER_PIXEL_12)
 			fmt_type = ATOMISP_INPUT_FORMAT_RAW_12;
-		} else
+		else
 			return false;
 	}
 
@@ -1045,7 +1040,8 @@ static bool sh_css_translate_binary_info_to_input_system_output_port_attr(
 }
 
 static int
-sh_css_config_input_network(struct ia_css_stream *stream) {
+sh_css_config_input_network(struct ia_css_stream *stream)
+{
 	bool					rc;
 	ia_css_isys_descr_t			isys_stream_descr;
 	unsigned int				sp_thread_id;
@@ -1060,19 +1056,16 @@ sh_css_config_input_network(struct ia_css_stream *stream) {
 	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
 			    "sh_css_config_input_network() enter 0x%p:\n", stream);
 
-	if (stream->config.continuous)
-	{
-		if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_CAPTURE) {
+	if (stream->config.continuous) {
+		if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_CAPTURE)
 			pipe = stream->last_pipe;
-		} else if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_YUVPP) {
+		else if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_YUVPP)
 			pipe = stream->last_pipe;
-		} else if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_PREVIEW) {
+		else if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_PREVIEW)
 			pipe = stream->last_pipe->pipe_settings.preview.copy_pipe;
-		} else if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_VIDEO) {
+		else if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_VIDEO)
 			pipe = stream->last_pipe->pipe_settings.video.copy_pipe;
-		}
-	} else
-	{
+	} else {
 		pipe = stream->last_pipe;
 		if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_CAPTURE) {
 			/*
@@ -1095,8 +1088,7 @@ sh_css_config_input_network(struct ia_css_stream *stream) {
 		if (pipe->pipeline.stages->binary)
 			binary = pipe->pipeline.stages->binary;
 
-	if (binary)
-	{
+	if (binary) {
 		/* this was being done in ifmtr in 2400.
 		 * online and cont bypass the init_in_frameinfo_memory_defaults
 		 * so need to do it here
@@ -1210,11 +1202,10 @@ static inline struct ia_css_pipe *stream_get_target_pipe(
 	struct ia_css_pipe *target_pipe;
 
 	/* get the pipe that consumes the stream */
-	if (stream->config.continuous) {
+	if (stream->config.continuous)
 		target_pipe = stream_get_copy_pipe(stream);
-	} else {
+	else
 		target_pipe = stream_get_last_pipe(stream);
-	}
 
 	return target_pipe;
 }
-- 
2.31.1

[PATCH] staging: media: atomisp: [2/6] Fix sh_css.c brace coding style issues

[Martiros Shakhzadyan]

Fix brace coding style issues.

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

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 10b70dfc0528..7c93fe911d0d 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -1379,7 +1379,8 @@ start_binary(struct ia_css_pipe *pipe,
 /* start the copy function on the SP */
 static int
 start_copy_on_sp(struct ia_css_pipe *pipe,
-		 struct ia_css_frame *out_frame) {
+		 struct ia_css_frame *out_frame)
+{
 	(void)out_frame;
 	assert(pipe);
 	assert(pipe->stream);
@@ -1397,8 +1398,7 @@ start_copy_on_sp(struct ia_css_pipe *pipe,
 	sh_css_sp_start_binary_copy(ia_css_pipe_get_pipe_num(pipe), out_frame, pipe->stream->config.pixels_per_clock == 2);
 
 #if !defined(ISP2401)
-	if (pipe->stream->reconfigure_css_rx)
-	{
+	if (pipe->stream->reconfigure_css_rx) {
 		ia_css_isys_rx_configure(&pipe->stream->csi_rx_config,
 					 pipe->stream->config.mode);
 		pipe->stream->reconfigure_css_rx = false;
@@ -1587,7 +1587,8 @@ ia_css_reset_defaults(struct sh_css *css)
 
 int
 ia_css_load_firmware(struct device *dev, const struct ia_css_env *env,
-		     const struct ia_css_fw  *fw) {
+		     const struct ia_css_fw  *fw)
+{
 	int err;
 
 	if (!env)
@@ -1598,16 +1599,14 @@ ia_css_load_firmware(struct device *dev, const struct ia_css_env *env,
 	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_load_firmware() enter\n");
 
 	/* make sure we initialize my_css */
-	if (my_css.flush != env->cpu_mem_env.flush)
-	{
+	if (my_css.flush != env->cpu_mem_env.flush) {
 		ia_css_reset_defaults(&my_css);
 		my_css.flush = env->cpu_mem_env.flush;
 	}
 
 	ia_css_unload_firmware(); /* in case we are called twice */
 	err = sh_css_load_firmware(dev, fw->data, fw->bytes);
-	if (!err)
-	{
+	if (!err) {
 		err = ia_css_binary_init_infos();
 		if (!err)
 			fw_explicitly_loaded = true;
@@ -1621,7 +1620,8 @@ int
 ia_css_init(struct device *dev, const struct ia_css_env *env,
 	    const struct ia_css_fw  *fw,
 	    u32                 mmu_l1_base,
-	    enum ia_css_irq_type     irq_type) {
+	    enum ia_css_irq_type     irq_type)
+{
 	int err;
 	ia_css_spctrl_cfg spctrl_cfg;
 
@@ -1695,16 +1695,14 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
 	my_css.flush     = flush_func;
 
 	err = ia_css_rmgr_init();
-	if (err)
-	{
+	if (err) {
 		IA_CSS_LEAVE_ERR(err);
 		return err;
 	}
 
 	IA_CSS_LOG("init: %d", my_css_save_initialized);
 
-	if (!my_css_save_initialized)
-	{
+	if (!my_css_save_initialized) {
 		my_css_save_initialized = true;
 		my_css_save.mode = sh_css_mode_working;
 		memset(my_css_save.stream_seeds, 0,
@@ -1732,19 +1730,16 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
 	gpio_reg_store(GPIO0_ID, _gpio_block_reg_do_0, 0);
 
 	err = ia_css_refcount_init(REFCOUNT_SIZE);
-	if (err)
-	{
+	if (err) {
 		IA_CSS_LEAVE_ERR(err);
 		return err;
 	}
 	err = sh_css_params_init();
-	if (err)
-	{
+	if (err) {
 		IA_CSS_LEAVE_ERR(err);
 		return err;
 	}
-	if (fw)
-	{
+	if (fw) {
 		ia_css_unload_firmware(); /* in case we already had firmware loaded */
 		err = sh_css_load_firmware(dev, fw->data, fw->bytes);
 		if (err) {
@@ -1765,23 +1760,20 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
 		return -EINVAL;
 
 	err = ia_css_spctrl_load_fw(SP0_ID, &spctrl_cfg);
-	if (err)
-	{
+	if (err) {
 		IA_CSS_LEAVE_ERR(err);
 		return err;
 	}
 
 #if WITH_PC_MONITORING
-	if (!thread_alive)
-	{
+	if (!thread_alive) {
 		thread_alive++;
 		sh_css_print("PC_MONITORING: %s() -- create thread DISABLED\n",
 			     __func__);
 		spying_thread_create();
 	}
 #endif
-	if (!sh_css_hrt_system_is_idle())
-	{
+	if (!sh_css_hrt_system_is_idle()) {
 		IA_CSS_LEAVE_ERR(-EBUSY);
 		return -EBUSY;
 	}
-- 
2.31.1

[PATCH] staging: media: atomisp: [3/6] Fix sh_css.c brace coding style issues

[Martiros Shakhzadyan]

Fix brace coding style issues.

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

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 7c93fe911d0d..e631944fb6cd 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -1806,7 +1806,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
 }
 
 int
-ia_css_enable_isys_event_queue(bool enable) {
+ia_css_enable_isys_event_queue(bool enable)
+{
 	if (sh_css_sp_is_running())
 		return -EBUSY;
 	sh_css_sp_enable_isys_event_queue(enable);
@@ -1827,7 +1828,8 @@ sh_css_flush(struct ia_css_acc_fw *fw)
  * doing it from stream_create since we could run out of sp threads due to
  * allocation on inactive pipelines. */
 static int
-map_sp_threads(struct ia_css_stream *stream, bool map) {
+map_sp_threads(struct ia_css_stream *stream, bool map)
+{
 	struct ia_css_pipe *main_pipe = NULL;
 	struct ia_css_pipe *copy_pipe = NULL;
 	struct ia_css_pipe *capture_pipe = NULL;
@@ -1839,8 +1841,7 @@ map_sp_threads(struct ia_css_stream *stream, bool map) {
 	IA_CSS_ENTER_PRIVATE("stream = %p, map = %s",
 			     stream, map ? "true" : "false");
 
-	if (!stream)
-	{
+	if (!stream) {
 		IA_CSS_LEAVE_ERR_PRIVATE(-EINVAL);
 		return -EINVAL;
 	}
@@ -1850,8 +1851,7 @@ map_sp_threads(struct ia_css_stream *stream, bool map) {
 
 	ia_css_pipeline_map(main_pipe->pipe_num, map);
 
-	switch (pipe_id)
-	{
+	switch (pipe_id) {
 	case IA_CSS_PIPE_ID_PREVIEW:
 		copy_pipe    = main_pipe->pipe_settings.preview.copy_pipe;
 		capture_pipe = main_pipe->pipe_settings.preview.capture_pipe;
@@ -1870,23 +1870,17 @@ map_sp_threads(struct ia_css_stream *stream, bool map) {
 	}
 
 	if (acc_pipe)
-	{
 		ia_css_pipeline_map(acc_pipe->pipe_num, map);
-	}
 
 	if (capture_pipe)
-	{
 		ia_css_pipeline_map(capture_pipe->pipe_num, map);
-	}
 
 	/* Firmware expects copy pipe to be the last pipe mapped. (if needed) */
 	if (copy_pipe)
-	{
 		ia_css_pipeline_map(copy_pipe->pipe_num, map);
-	}
+
 	/* DH regular multi pipe - not continuous mode: map the next pipes too */
-	if (!stream->config.continuous)
-	{
+	if (!stream->config.continuous) {
 		int i;
 
 		for (i = 1; i < stream->num_pipes; i++)
-- 
2.31.1

[PATCH] staging: media: atomisp: [4/6] Fix sh_css.c brace coding style issues

[Martiros Shakhzadyan]

Fix brace coding style issues.

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

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index e631944fb6cd..03b3b11671a5 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -1894,7 +1894,8 @@ map_sp_threads(struct ia_css_stream *stream, bool map)
 /* creates a host pipeline skeleton for all pipes in a stream. Called during
  * stream_create. */
 static int
-create_host_pipeline_structure(struct ia_css_stream *stream) {
+create_host_pipeline_structure(struct ia_css_stream *stream)
+{
 	struct ia_css_pipe *copy_pipe = NULL, *capture_pipe = NULL;
 	struct ia_css_pipe *acc_pipe = NULL;
 	enum ia_css_pipe_id pipe_id;
@@ -1906,24 +1907,21 @@ create_host_pipeline_structure(struct ia_css_stream *stream) {
 	assert(stream);
 	IA_CSS_ENTER_PRIVATE("stream = %p", stream);
 
-	if (!stream)
-	{
+	if (!stream) {
 		IA_CSS_LEAVE_ERR_PRIVATE(-EINVAL);
 		return -EINVAL;
 	}
 
 	main_pipe	= stream->last_pipe;
 	assert(main_pipe);
-	if (!main_pipe)
-	{
+	if (!main_pipe) {
 		IA_CSS_LEAVE_ERR_PRIVATE(-EINVAL);
 		return -EINVAL;
 	}
 
 	pipe_id	= main_pipe->mode;
 
-	switch (pipe_id)
-	{
+	switch (pipe_id) {
 	case IA_CSS_PIPE_ID_PREVIEW:
 		copy_pipe    = main_pipe->pipe_settings.preview.copy_pipe;
 		copy_pipe_delay = main_pipe->dvs_frame_delay;
@@ -1963,30 +1961,23 @@ create_host_pipeline_structure(struct ia_css_stream *stream) {
 	}
 
 	if (!(err) && copy_pipe)
-	{
 		err = ia_css_pipeline_create(&copy_pipe->pipeline,
 					     copy_pipe->mode,
 					     copy_pipe->pipe_num,
 					     copy_pipe_delay);
-	}
 
 	if (!(err) && capture_pipe)
-	{
 		err = ia_css_pipeline_create(&capture_pipe->pipeline,
 					     capture_pipe->mode,
 					     capture_pipe->pipe_num,
 					     capture_pipe_delay);
-	}
 
 	if (!(err) && acc_pipe)
-	{
 		err = ia_css_pipeline_create(&acc_pipe->pipeline, acc_pipe->mode,
 					     acc_pipe->pipe_num, main_pipe->dvs_frame_delay);
-	}
 
 	/* DH regular multi pipe - not continuous mode: create the next pipelines too */
-	if (!stream->config.continuous)
-	{
+	if (!stream->config.continuous) {
 		int i;
 
 		for (i = 1; i < stream->num_pipes && 0 == err; i++) {
-- 
2.31.1

[PATCH] staging: media: atomisp: [5/6] Fix sh_css.c brace coding style issues

[Martiros Shakhzadyan]

Fix brace coding style issues.

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

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 03b3b11671a5..0c9324684076 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -1996,7 +1996,8 @@ create_host_pipeline_structure(struct ia_css_stream *stream)
 /* creates a host pipeline for all pipes in a stream. Called during
  * stream_start. */
 static int
-create_host_pipeline(struct ia_css_stream *stream) {
+create_host_pipeline(struct ia_css_stream *stream)
+{
 	struct ia_css_pipe *copy_pipe = NULL, *capture_pipe = NULL;
 	struct ia_css_pipe *acc_pipe = NULL;
 	enum ia_css_pipe_id pipe_id;
@@ -2005,8 +2006,7 @@ create_host_pipeline(struct ia_css_stream *stream) {
 	unsigned int max_input_width = 0;
 
 	IA_CSS_ENTER_PRIVATE("stream = %p", stream);
-	if (!stream)
-	{
+	if (!stream) {
 		IA_CSS_LEAVE_ERR_PRIVATE(-EINVAL);
 		return -EINVAL;
 	}
@@ -2017,8 +2017,7 @@ create_host_pipeline(struct ia_css_stream *stream) {
 	/* No continuous frame allocation for capture pipe. It uses the
 	 * "main" pipe's frames. */
 	if ((pipe_id == IA_CSS_PIPE_ID_PREVIEW) ||
-	    (pipe_id == IA_CSS_PIPE_ID_VIDEO))
-	{
+	    (pipe_id == IA_CSS_PIPE_ID_VIDEO)) {
 		/* About pipe_id == IA_CSS_PIPE_ID_PREVIEW && stream->config.mode != IA_CSS_INPUT_MODE_MEMORY:
 		 * The original condition pipe_id == IA_CSS_PIPE_ID_PREVIEW is too strong. E.g. in SkyCam (with memory
 		 * based input frames) there is no continuous mode and thus no need for allocated continuous frames
@@ -2036,24 +2035,21 @@ create_host_pipeline(struct ia_css_stream *stream) {
 
 #if !defined(ISP2401)
 	/* old isys: need to allocate_mipi_frames() even in IA_CSS_PIPE_MODE_COPY */
-	if (pipe_id != IA_CSS_PIPE_ID_ACC)
-	{
+	if (pipe_id != IA_CSS_PIPE_ID_ACC) {
 		err = allocate_mipi_frames(main_pipe, &stream->info);
 		if (err)
 			goto ERR;
 	}
 #elif defined(ISP2401)
 	if ((pipe_id != IA_CSS_PIPE_ID_ACC) &&
-	    (main_pipe->config.mode != IA_CSS_PIPE_MODE_COPY))
-	{
+	    (main_pipe->config.mode != IA_CSS_PIPE_MODE_COPY)) {
 		err = allocate_mipi_frames(main_pipe, &stream->info);
 		if (err)
 			goto ERR;
 	}
 #endif
 
-	switch (pipe_id)
-	{
+	switch (pipe_id) {
 	case IA_CSS_PIPE_ID_PREVIEW:
 		copy_pipe    = main_pipe->pipe_settings.preview.copy_pipe;
 		capture_pipe = main_pipe->pipe_settings.preview.capture_pipe;
@@ -2103,31 +2099,27 @@ create_host_pipeline(struct ia_css_stream *stream) {
 	if (err)
 		goto ERR;
 
-	if (copy_pipe)
-	{
+	if (copy_pipe) {
 		err = create_host_copy_pipeline(copy_pipe, max_input_width,
 						main_pipe->continuous_frames[0]);
 		if (err)
 			goto ERR;
 	}
 
-	if (capture_pipe)
-	{
+	if (capture_pipe) {
 		err = create_host_capture_pipeline(capture_pipe);
 		if (err)
 			goto ERR;
 	}
 
-	if (acc_pipe)
-	{
+	if (acc_pipe) {
 		err = create_host_acc_pipeline(acc_pipe);
 		if (err)
 			goto ERR;
 	}
 
 	/* DH regular multi pipe - not continuous mode: create the next pipelines too */
-	if (!stream->config.continuous)
-	{
+	if (!stream->config.continuous) {
 		int i;
 
 		for (i = 1; i < stream->num_pipes && 0 == err; i++) {
-- 
2.31.1

[PATCH] staging: media: atomisp: [6/6] Fix sh_css.c brace coding style issues

[Martiros Shakhzadyan]

Fix brace coding style issues.

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

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 0c9324684076..bb752d47457c 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -2161,10 +2161,10 @@ static const struct ia_css_yuvpp_settings yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS;
 static int
 init_pipe_defaults(enum ia_css_pipe_mode mode,
 		   struct ia_css_pipe *pipe,
-		   bool copy_pipe) {
+		   bool copy_pipe)
+{
 
-	if (!pipe)
-	{
+	if (!pipe) {
 		IA_CSS_ERROR("NULL pipe parameter");
 		return -EINVAL;
 	}
@@ -2173,18 +2173,17 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
 	memcpy(pipe, &default_pipe, sizeof(default_pipe));
 
 	/* TODO: JB should not be needed, but temporary backward reference */
-	switch (mode)
-	{
+	switch (mode) {
 	case IA_CSS_PIPE_MODE_PREVIEW:
 		pipe->mode = IA_CSS_PIPE_ID_PREVIEW;
 		memcpy(&pipe->pipe_settings.preview, &preview, sizeof(preview));
 		break;
 	case IA_CSS_PIPE_MODE_CAPTURE:
-		if (copy_pipe) {
+		if (copy_pipe)
 			pipe->mode = IA_CSS_PIPE_ID_COPY;
-		} else {
+		else
 			pipe->mode = IA_CSS_PIPE_ID_CAPTURE;
-		}
+
 		memcpy(&pipe->pipe_settings.capture, &capture, sizeof(capture));
 		break;
 	case IA_CSS_PIPE_MODE_VIDEO:
@@ -2214,27 +2213,25 @@ pipe_global_init(void)
 	u8 i;
 
 	my_css.pipe_counter = 0;
-	for (i = 0; i < IA_CSS_PIPELINE_NUM_MAX; i++) {
+	for (i = 0; i < IA_CSS_PIPELINE_NUM_MAX; i++)
 		my_css.all_pipes[i] = NULL;
-	}
 }
 
 static int
 pipe_generate_pipe_num(const struct ia_css_pipe *pipe,
-		       unsigned int *pipe_number) {
+		       unsigned int *pipe_number)
+{
 	const u8 INVALID_PIPE_NUM = (uint8_t)~(0);
 	u8 pipe_num = INVALID_PIPE_NUM;
 	u8 i;
 
-	if (!pipe)
-	{
+	if (!pipe) {
 		IA_CSS_ERROR("NULL pipe parameter");
 		return -EINVAL;
 	}
 
 	/* Assign a new pipe_num .... search for empty place */
-	for (i = 0; i < IA_CSS_PIPELINE_NUM_MAX; i++)
-	{
+	for (i = 0; i < IA_CSS_PIPELINE_NUM_MAX; i++) {
 		if (!my_css.all_pipes[i]) {
 			/*position is reserved */
 			my_css.all_pipes[i] = (struct ia_css_pipe *)pipe;
@@ -2242,8 +2239,7 @@ pipe_generate_pipe_num(const struct ia_css_pipe *pipe,
 			break;
 		}
 	}
-	if (pipe_num == INVALID_PIPE_NUM)
-	{
+	if (pipe_num == INVALID_PIPE_NUM) {
 		/* Max number of pipes already allocated */
 		IA_CSS_ERROR("Max number of pipes already created");
 		return -ENOSPC;
-- 
2.31.1

[PATCH] staging: media: atomisp: Use goto instead of return in ia_css_init()

[Martiros Shakhzadyan]

Replace multiple return statements with goto in ia_css_init(), matching
other functions.

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

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index bb752d47457c..4e3ef68014ec 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -1695,10 +1695,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
 	my_css.flush     = flush_func;
 
 	err = ia_css_rmgr_init();
-	if (err) {
-		IA_CSS_LEAVE_ERR(err);
-		return err;
-	}
+	if (err)
+		goto ERR;
 
 	IA_CSS_LOG("init: %d", my_css_save_initialized);
 
@@ -1730,27 +1728,23 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
 	gpio_reg_store(GPIO0_ID, _gpio_block_reg_do_0, 0);
 
 	err = ia_css_refcount_init(REFCOUNT_SIZE);
-	if (err) {
-		IA_CSS_LEAVE_ERR(err);
-		return err;
-	}
+	if (err)
+		goto ERR;
+
 	err = sh_css_params_init();
-	if (err) {
-		IA_CSS_LEAVE_ERR(err);
-		return err;
-	}
+	if (err)
+		goto ERR;
+
 	if (fw) {
 		ia_css_unload_firmware(); /* in case we already had firmware loaded */
 		err = sh_css_load_firmware(dev, fw->data, fw->bytes);
-		if (err) {
-			IA_CSS_LEAVE_ERR(err);
-			return err;
-		}
+		if (err)
+			goto ERR;
+
 		err = ia_css_binary_init_infos();
-		if (err) {
-			IA_CSS_LEAVE_ERR(err);
-			return err;
-		}
+		if (err)
+			goto ERR;
+
 		fw_explicitly_loaded = false;
 #ifndef ISP2401
 		my_css_save.loaded_fw = (struct ia_css_fw *)fw;
@@ -1760,10 +1754,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
 		return -EINVAL;
 
 	err = ia_css_spctrl_load_fw(SP0_ID, &spctrl_cfg);
-	if (err) {
-		IA_CSS_LEAVE_ERR(err);
-		return err;
-	}
+	if (err)
+		goto ERR;
 
 #if WITH_PC_MONITORING
 	if (!thread_alive) {
@@ -1774,8 +1766,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
 	}
 #endif
 	if (!sh_css_hrt_system_is_idle()) {
-		IA_CSS_LEAVE_ERR(-EBUSY);
-		return -EBUSY;
+		err = -EBUSY;
+		goto ERR;
 	}
 	/* can be called here, queuing works, but:
 	   - when sp is started later, it will wipe queued items
@@ -1801,6 +1793,7 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
 
 	sh_css_params_map_and_store_default_gdc_lut();
 
+ERR:
 	IA_CSS_LEAVE_ERR(err);
 	return err;
 }
-- 
2.31.1

[PATCH] staging: media: atomisp: [1/2] Remove redundant assertions in sh_css.c

[Martiros Shakhzadyan]

Remove assert() in places where the condition is already handled.

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

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 4e3ef68014ec..aebecf650967 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -413,7 +413,6 @@ aspect_ratio_crop(struct ia_css_pipe *curr_pipe,
 static void
 sh_css_pipe_free_shading_table(struct ia_css_pipe *pipe)
 {
-	assert(pipe);
 	if (!pipe) {
 		IA_CSS_ERROR("NULL input parameter");
 		return;
@@ -1080,7 +1079,6 @@ sh_css_config_input_network(struct ia_css_stream *stream)
 		}
 	}
 
-	assert(pipe);
 	if (!pipe)
 		return -EINVAL;
 
@@ -1382,10 +1380,11 @@ start_copy_on_sp(struct ia_css_pipe *pipe,
 		 struct ia_css_frame *out_frame)
 {
 	(void)out_frame;
-	assert(pipe);
-	assert(pipe->stream);
 
-	if ((!pipe) || (!pipe->stream))
+	if (!pipe)
+		return -EINVAL;
+
+	if (!pipe->stream)
 		return -EINVAL;
 
 #if !defined(ISP2401)
-- 
2.31.1

[PATCH] staging: media: atomisp: [2/2] Remove redundant assertions in sh_css.c

[Martiros Shakhzadyan]

Remove assert() in places where the condition is already handled.

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

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index aebecf650967..634e7911d87f 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -1829,7 +1829,6 @@ map_sp_threads(struct ia_css_stream *stream, bool map)
 	int err = 0;
 	enum ia_css_pipe_id pipe_id;
 
-	assert(stream);
 	IA_CSS_ENTER_PRIVATE("stream = %p, map = %s",
 			     stream, map ? "true" : "false");
 
@@ -1896,7 +1895,6 @@ create_host_pipeline_structure(struct ia_css_stream *stream)
 	unsigned int copy_pipe_delay = 0,
 	capture_pipe_delay = 0;
 
-	assert(stream);
 	IA_CSS_ENTER_PRIVATE("stream = %p", stream);
 
 	if (!stream) {
@@ -1905,7 +1903,6 @@ create_host_pipeline_structure(struct ia_css_stream *stream)
 	}
 
 	main_pipe	= stream->last_pipe;
-	assert(main_pipe);
 	if (!main_pipe) {
 		IA_CSS_LEAVE_ERR_PRIVATE(-EINVAL);
 		return -EINVAL;
@@ -2309,7 +2306,6 @@ static void sh_css_pipe_free_acc_binaries(
 	struct ia_css_pipeline *pipeline;
 	struct ia_css_pipeline_stage *stage;
 
-	assert(pipe);
 	if (!pipe) {
 		IA_CSS_ERROR("NULL input pointer");
 		return;
-- 
2.31.1

Re: [PATCH] staging: media: atomisp: [1/2] Remove redundant assertions in sh_css.c

[Hans Verkuil]

On 15/04/2021 02:51, Martiros Shakhzadyan wrote:
> Remove assert() in places where the condition is already handled.
> 
> Signed-off-by: Martiros Shakhzadyan <vrzh@vrzh.net>
> ---
>  drivers/staging/media/atomisp/pci/sh_css.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index 4e3ef68014ec..aebecf650967 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -413,7 +413,6 @@ aspect_ratio_crop(struct ia_css_pipe *curr_pipe,
>  static void
>  sh_css_pipe_free_shading_table(struct ia_css_pipe *pipe)
>  {
> -	assert(pipe);
>  	if (!pipe) {
>  		IA_CSS_ERROR("NULL input parameter");
>  		return;
> @@ -1080,7 +1079,6 @@ sh_css_config_input_network(struct ia_css_stream *stream)
>  		}
>  	}
>  
> -	assert(pipe);
>  	if (!pipe)
>  		return -EINVAL;
>  
> @@ -1382,10 +1380,11 @@ start_copy_on_sp(struct ia_css_pipe *pipe,
>  		 struct ia_css_frame *out_frame)
>  {
>  	(void)out_frame;
> -	assert(pipe);
> -	assert(pipe->stream);
>  
> -	if ((!pipe) || (!pipe->stream))
> +	if (!pipe)
> +		return -EINVAL;
> +
> +	if (!pipe->stream)
>  		return -EINVAL;

Why was this 'if' rewritten? What was wrong with the original?

Also, this change has nothing to do with the removal of assert(), so
that makes me suspect that this change wasn't intended by you.

Regards,

	Hans

>  
>  #if !defined(ISP2401)
> 

Re: [PATCH] staging: media: atomisp: Use goto instead of return in ia_css_init()

[Hans Verkuil]

Hi Martiros,

On 15/04/2021 02:51, Martiros Shakhzadyan wrote:
> Replace multiple return statements with goto in ia_css_init(), matching
> other functions.
> 
> Signed-off-by: Martiros Shakhzadyan <vrzh@vrzh.net>
> ---
>  drivers/staging/media/atomisp/pci/sh_css.c | 45 +++++++++-------------
>  1 file changed, 19 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index bb752d47457c..4e3ef68014ec 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -1695,10 +1695,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
>  	my_css.flush     = flush_func;
>  
>  	err = ia_css_rmgr_init();
> -	if (err) {
> -		IA_CSS_LEAVE_ERR(err);
> -		return err;
> -	}
> +	if (err)
> +		goto ERR;

Sorry, this doesn't work.

First a style issue: goto label are typically lowercase, not uppercase.
I do see that elsewhere in this source they use ERR as well, but that should
really all be changed to lowercase.

But more importantly, if you look up the definition of IA_CSS_LEAVE_ERR
you see:

#define IA_CSS_LEAVE_ERR(__err) \
        ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, \
                "%s() %d: leave: return_err=%d\n", __func__, __LINE__, __err)

I.e., it is used to debug the code and print where the error is returned
(__func__ and __LINE__). By moving this to the end, the __LINE__ is always
the same for all 'error' cases, thus defeating the purpose of this debug line.

So I won't take this patch.

Regards,

	Hans

>  
>  	IA_CSS_LOG("init: %d", my_css_save_initialized);
>  
> @@ -1730,27 +1728,23 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
>  	gpio_reg_store(GPIO0_ID, _gpio_block_reg_do_0, 0);
>  
>  	err = ia_css_refcount_init(REFCOUNT_SIZE);
> -	if (err) {
> -		IA_CSS_LEAVE_ERR(err);
> -		return err;
> -	}
> +	if (err)
> +		goto ERR;
> +
>  	err = sh_css_params_init();
> -	if (err) {
> -		IA_CSS_LEAVE_ERR(err);
> -		return err;
> -	}
> +	if (err)
> +		goto ERR;
> +
>  	if (fw) {
>  		ia_css_unload_firmware(); /* in case we already had firmware loaded */
>  		err = sh_css_load_firmware(dev, fw->data, fw->bytes);
> -		if (err) {
> -			IA_CSS_LEAVE_ERR(err);
> -			return err;
> -		}
> +		if (err)
> +			goto ERR;
> +
>  		err = ia_css_binary_init_infos();
> -		if (err) {
> -			IA_CSS_LEAVE_ERR(err);
> -			return err;
> -		}
> +		if (err)
> +			goto ERR;
> +
>  		fw_explicitly_loaded = false;
>  #ifndef ISP2401
>  		my_css_save.loaded_fw = (struct ia_css_fw *)fw;
> @@ -1760,10 +1754,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
>  		return -EINVAL;
>  
>  	err = ia_css_spctrl_load_fw(SP0_ID, &spctrl_cfg);
> -	if (err) {
> -		IA_CSS_LEAVE_ERR(err);
> -		return err;
> -	}
> +	if (err)
> +		goto ERR;
>  
>  #if WITH_PC_MONITORING
>  	if (!thread_alive) {
> @@ -1774,8 +1766,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
>  	}
>  #endif
>  	if (!sh_css_hrt_system_is_idle()) {
> -		IA_CSS_LEAVE_ERR(-EBUSY);
> -		return -EBUSY;
> +		err = -EBUSY;
> +		goto ERR;
>  	}
>  	/* can be called here, queuing works, but:
>  	   - when sp is started later, it will wipe queued items
> @@ -1801,6 +1793,7 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
>  
>  	sh_css_params_map_and_store_default_gdc_lut();
>  
> +ERR:
>  	IA_CSS_LEAVE_ERR(err);
>  	return err;
>  }
> 

Re: [PATCH] staging: media: atomisp: cleaning up sh_css.c

[Hans Verkuil]

Hi Martiros,

If you look at other patch series, you'll see that they follow this pattern:

[PATCH x/y]

and for the cover letter: [PATCH 0/y]

It's recommended to follow that rule.

On 15/04/2021 02:50, Martiros Shakhzadyan wrote:
> The following set of patches for sh_css.c aims to resolve coding style issues
> and remove redundancies.
> 
> Martiros Shakhzadyan (9):
>   staging: media: atomisp: [1/6] Fix sh_css.c brace coding style issues
>   staging: media: atomisp: [2/6] Fix sh_css.c brace coding style issues
>   staging: media: atomisp: [3/6] Fix sh_css.c brace coding style issues
>   staging: media: atomisp: [4/6] Fix sh_css.c brace coding style issues
>   staging: media: atomisp: [5/6] Fix sh_css.c brace coding style issues
>   staging: media: atomisp: [6/6] Fix sh_css.c brace coding style issues\

Why split this up in 6 patches? Just combine it in one. It's all the same thing
for the same source.

>   staging: media: atomisp: Use goto instead of return in ia_css_init()
>   staging: media: atomisp: [1/2] Remove redundant assertions in sh_css.c
>   staging: media: atomisp: [2/2] Remove redundant assertions in sh_css.c

Ditto for these two.

Regards,

	Hans

> 
>  drivers/staging/media/atomisp/pci/sh_css.c | 266 ++++++++-------------
>  1 file changed, 105 insertions(+), 161 deletions(-)
> 

Re: [PATCH] staging: media: atomisp: cleaning up sh_css.c

[Martiros Shakhzadyan]

On Thu, Apr 15, 2021 at 03:59:25PM +0200, Hans Verkuil wrote:
> Hi Martiros,
> 
> If you look at other patch series, you'll see that they follow this pattern:
> 
> [PATCH x/y]
> 
> and for the cover letter: [PATCH 0/y]
> 
> It's recommended to follow that rule.
> 
> On 15/04/2021 02:50, Martiros Shakhzadyan wrote:
> > The following set of patches for sh_css.c aims to resolve coding style issues
> > and remove redundancies.
> > 
> > Martiros Shakhzadyan (9):
> >   staging: media: atomisp: [1/6] Fix sh_css.c brace coding style issues
> >   staging: media: atomisp: [2/6] Fix sh_css.c brace coding style issues
> >   staging: media: atomisp: [3/6] Fix sh_css.c brace coding style issues
> >   staging: media: atomisp: [4/6] Fix sh_css.c brace coding style issues
> >   staging: media: atomisp: [5/6] Fix sh_css.c brace coding style issues
> >   staging: media: atomisp: [6/6] Fix sh_css.c brace coding style issues\
> 
> Why split this up in 6 patches? Just combine it in one. It's all the same thing
> for the same source.
> 
> >   staging: media: atomisp: Use goto instead of return in ia_css_init()
> >   staging: media: atomisp: [1/2] Remove redundant assertions in sh_css.c
> >   staging: media: atomisp: [2/2] Remove redundant assertions in sh_css.c
> 
> Ditto for these two.
> 
> Regards,
> 
> 	Hans
> 
Gotcha - will resubmit all following this rule (save for the rejected
patch) - Thanks!
> > 
> >  drivers/staging/media/atomisp/pci/sh_css.c | 266 ++++++++-------------
> >  1 file changed, 105 insertions(+), 161 deletions(-)
> > 
> 

Re: [PATCH] staging: media: atomisp: Use goto instead of return in ia_css_init()

[Martiros Shakhzadyan]

On Thu, Apr 15, 2021 at 03:56:35PM +0200, Hans Verkuil wrote:
> Hi Martiros,
> 
> On 15/04/2021 02:51, Martiros Shakhzadyan wrote:
> > Replace multiple return statements with goto in ia_css_init(), matching
> > other functions.
> > 
> > Signed-off-by: Martiros Shakhzadyan <vrzh@vrzh.net>
> > ---
> >  drivers/staging/media/atomisp/pci/sh_css.c | 45 +++++++++-------------
> >  1 file changed, 19 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> > index bb752d47457c..4e3ef68014ec 100644
> > --- a/drivers/staging/media/atomisp/pci/sh_css.c
> > +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> > @@ -1695,10 +1695,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
> >  	my_css.flush     = flush_func;
> >  
> >  	err = ia_css_rmgr_init();
> > -	if (err) {
> > -		IA_CSS_LEAVE_ERR(err);
> > -		return err;
> > -	}
> > +	if (err)
> > +		goto ERR;
> 
> Sorry, this doesn't work.
> 
> First a style issue: goto label are typically lowercase, not uppercase.
> I do see that elsewhere in this source they use ERR as well, but that should
> really all be changed to lowercase.

Will keep that in mind as I continue to work on this file.
> But more importantly, if you look up the definition of IA_CSS_LEAVE_ERR
> you see:
> 
> #define IA_CSS_LEAVE_ERR(__err) \
>         ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, \
>                 "%s() %d: leave: return_err=%d\n", __func__, __LINE__, __err)
> 
> I.e., it is used to debug the code and print where the error is returned
> (__func__ and __LINE__). By moving this to the end, the __LINE__ is always
> the same for all 'error' cases, thus defeating the purpose of this debug line.
Makes sense. I did consider this, however the rest of the error goto
instances have already been implemented with a similar macro
(IS_CSS_LEAVE_ERR_PRIVATE) at exit, so I thought perhaps it was
acceptable. Should I update these functions accordingly (e.g.
create_host_*pipeline() series of functions)?
> So I won't take this patch.
> 
> Regards,
> 
> 	Hans
> 
Thanks for the review! -M
> >  
> >  	IA_CSS_LOG("init: %d", my_css_save_initialized);
> >  
> > @@ -1730,27 +1728,23 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
> >  	gpio_reg_store(GPIO0_ID, _gpio_block_reg_do_0, 0);
> >  
> >  	err = ia_css_refcount_init(REFCOUNT_SIZE);
> > -	if (err) {
> > -		IA_CSS_LEAVE_ERR(err);
> > -		return err;
> > -	}
> > +	if (err)
> > +		goto ERR;
> > +
> >  	err = sh_css_params_init();
> > -	if (err) {
> > -		IA_CSS_LEAVE_ERR(err);
> > -		return err;
> > -	}
> > +	if (err)
> > +		goto ERR;
> > +
> >  	if (fw) {
> >  		ia_css_unload_firmware(); /* in case we already had firmware loaded */
> >  		err = sh_css_load_firmware(dev, fw->data, fw->bytes);
> > -		if (err) {
> > -			IA_CSS_LEAVE_ERR(err);
> > -			return err;
> > -		}
> > +		if (err)
> > +			goto ERR;
> > +
> >  		err = ia_css_binary_init_infos();
> > -		if (err) {
> > -			IA_CSS_LEAVE_ERR(err);
> > -			return err;
> > -		}
> > +		if (err)
> > +			goto ERR;
> > +
> >  		fw_explicitly_loaded = false;
> >  #ifndef ISP2401
> >  		my_css_save.loaded_fw = (struct ia_css_fw *)fw;
> > @@ -1760,10 +1754,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
> >  		return -EINVAL;
> >  
> >  	err = ia_css_spctrl_load_fw(SP0_ID, &spctrl_cfg);
> > -	if (err) {
> > -		IA_CSS_LEAVE_ERR(err);
> > -		return err;
> > -	}
> > +	if (err)
> > +		goto ERR;
> >  
> >  #if WITH_PC_MONITORING
> >  	if (!thread_alive) {
> > @@ -1774,8 +1766,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
> >  	}
> >  #endif
> >  	if (!sh_css_hrt_system_is_idle()) {
> > -		IA_CSS_LEAVE_ERR(-EBUSY);
> > -		return -EBUSY;
> > +		err = -EBUSY;
> > +		goto ERR;
> >  	}
> >  	/* can be called here, queuing works, but:
> >  	   - when sp is started later, it will wipe queued items
> > @@ -1801,6 +1793,7 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
> >  
> >  	sh_css_params_map_and_store_default_gdc_lut();
> >  
> > +ERR:
> >  	IA_CSS_LEAVE_ERR(err);
> >  	return err;
> >  }
> > 
>