* [Linux-kernel-mentees] [PATCH] media: atomisp: replace boolean comparison of values with bool variables
@ 2020-12-14 13:27 Aditya Srivastava
0 siblings, 0 replies; 6+ messages in thread
From: Aditya Srivastava @ 2020-12-14 13:27 UTC (permalink / raw)
To: linux-media; +Cc: yashsri421, sakari.ailus, mchehab, linux-kernel-mentees
There are certain expressions in a condition in atomisp, where a boolean
variable is compared with true/false in forms such as (foo == true)
or (false != bar), which does not comply with the coding style rule by
checkpatch.pl (CHK: BOOL_COMPARISON), according to which the boolean
variables should be themselves used in the condition, rather than
comparing with true or false.
E.g. In drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:
if (asd->stream_prepared == false) {
Can be replaced with:
if (!asd->stream_prepared) {
Replace such expressions with boolean variables appropriately.
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
* The changes made are compile tested
* The patch applies perfectly over next-20201204
.../staging/media/atomisp/pci/atomisp_compat_css20.c | 2 +-
.../atomisp/pci/runtime/isys/src/virtual_isys.c | 12 ++++++------
drivers/staging/media/atomisp/pci/sh_css.c | 12 ++++++------
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index faa0935e536a..6a80c676f668 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -1142,7 +1142,7 @@ int atomisp_css_start(struct atomisp_sub_device *asd,
* Thus the stream created in set_fmt get destroyed and need to be
* recreated in the next stream on.
*/
- if (asd->stream_prepared == false) {
+ if (!asd->stream_prepared) {
if (__create_pipes(asd)) {
dev_err(isp->dev, "create pipe error.\n");
return -EINVAL;
diff --git a/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c b/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
index 317ea30ede7a..82f3c19dc455 100644
--- a/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
+++ b/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
@@ -179,12 +179,12 @@ ia_css_isys_error_t ia_css_isys_stream_create(
isys_stream->linked_isys_stream_id = isys_stream_descr->linked_isys_stream_id;
rc = create_input_system_input_port(isys_stream_descr,
&isys_stream->input_port);
- if (rc == false)
+ if (!rc)
return false;
rc = create_input_system_channel(isys_stream_descr, false,
&isys_stream->channel);
- if (rc == false) {
+ if (!rc) {
destroy_input_system_input_port(&isys_stream->input_port);
return false;
}
@@ -204,7 +204,7 @@ ia_css_isys_error_t ia_css_isys_stream_create(
if (isys_stream_descr->metadata.enable) {
rc = create_input_system_channel(isys_stream_descr, true,
&isys_stream->md_channel);
- if (rc == false) {
+ if (!rc) {
destroy_input_system_input_port(&isys_stream->input_port);
destroy_input_system_channel(&isys_stream->channel);
return false;
@@ -248,7 +248,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
isys_stream_descr,
&isys_stream_cfg->channel_cfg,
false);
- if (rc == false)
+ if (!rc)
return false;
/* configure metadata channel */
@@ -260,7 +260,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
isys_stream_descr,
&isys_stream_cfg->md_channel_cfg,
true);
- if (rc == false)
+ if (!rc)
return false;
}
@@ -269,7 +269,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
&isys_stream->input_port,
isys_stream_descr,
&isys_stream_cfg->input_port_cfg);
- if (rc == false)
+ if (!rc)
return false;
isys_stream->valid = 1;
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index ddee04c8248d..a71c1bbd984b 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -1063,7 +1063,7 @@ 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 == true)
+ if (stream->config.continuous)
{
if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_CAPTURE) {
pipe = stream->last_pipe;
@@ -5626,7 +5626,7 @@ static int load_video_binaries(struct ia_css_pipe *pipe)
} else {
/* output from main binary is not yuv line. currently this is
* possible only when bci is enabled on vfpp output */
- assert(pipe->config.enable_vfpp_bci == true);
+ assert(pipe->config.enable_vfpp_bci);
ia_css_pipe_get_yuvscaler_binarydesc(pipe, &vf_pp_descr,
&mycs->video_binary.vf_frame_info,
pipe_vf_out_info, NULL, NULL);
@@ -8072,7 +8072,7 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe) {
struct ia_css_frame *tmp_out_frame = NULL;
for (i = 0; i < num_yuv_scaler; i++) {
- if (is_output_stage[i] == true)
+ if (is_output_stage[i])
tmp_out_frame = out_frame;
else
tmp_out_frame = NULL;
@@ -8464,7 +8464,7 @@ sh_css_pipeline_add_acc_stage(struct ia_css_pipeline *pipeline,
/* In QoS case, load_extension already called, so skipping */
int err = 0;
- if (fw->loaded == false)
+ if (!fw->loaded)
err = acc_load_extension(fw);
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
@@ -9701,8 +9701,8 @@ ia_css_stream_destroy(struct ia_css_stream *stream) {
assert(entry);
if (entry) {
/* get the SP thread id */
- if (ia_css_pipeline_get_sp_thread_id(
- ia_css_pipe_get_pipe_num(entry), &sp_thread_id) != true)
+ if (!ia_css_pipeline_get_sp_thread_id(
+ ia_css_pipe_get_pipe_num(entry), &sp_thread_id))
return -EINVAL;
/* get the target input terminal */
sp_pipeline_input_terminal =
--
2.17.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] media: atomisp: replace boolean comparison of values with bool variables
2020-12-14 5:41 ` Lukas Bulwahn
@ 2020-12-14 5:45 ` Lukas Bulwahn
0 siblings, 0 replies; 6+ messages in thread
From: Lukas Bulwahn @ 2020-12-14 5:45 UTC (permalink / raw)
To: Aditya; +Cc: linux-kernel-mentees
>
> Other than that, the change looks good. It is probably a driver that
> is on its way to removal anyway. So, do not be surprised if even if
> the maintainers thinks the change generally improves the code, they
> simply do not want to touch code that is soon to be deleted.
>
Here is some reading material on staging:
https://lwn.net/Articles/798258/
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] media: atomisp: replace boolean comparison of values with bool variables
2020-12-13 17:30 ` Aditya
@ 2020-12-14 5:41 ` Lukas Bulwahn
2020-12-14 5:45 ` Lukas Bulwahn
0 siblings, 1 reply; 6+ messages in thread
From: Lukas Bulwahn @ 2020-12-14 5:41 UTC (permalink / raw)
To: Aditya; +Cc: linux-kernel-mentees
On Sun, Dec 13, 2020 at 6:30 PM Aditya <yashsri421@gmail.com> wrote:
>
> On 13/12/20 10:51 pm, Aditya Srivastava wrote:
> > There are certain relational expressions in atomisp where a boolean
> > variable is compared with true/false in forms such as (foo == true)
> > or (false != bar), which does not comply with the coding style rule by
> > checkpatch.pl (CHK: BOOL_COMPARISON), according to which the boolean
> > variables should be themselves used in relational expression, rather
> > than comparing with true or false.
> >
> > E.g. In drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:
> >
> > if (asd->stream_prepared == false) {
> >
> > Can be replaced with:
> > if (!asd->stream_prepared) {
> >
> > Replace such relational expressions with boolean variables appropriately.
> >
> > Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> > ---
> > .../staging/media/atomisp/pci/atomisp_compat_css20.c | 2 +-
> > .../atomisp/pci/runtime/isys/src/virtual_isys.c | 12 ++++++------
> > drivers/staging/media/atomisp/pci/sh_css.c | 12 ++++++------
> > 3 files changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> > index faa0935e536a..6a80c676f668 100644
> > --- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> > +++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> > @@ -1142,7 +1142,7 @@ int atomisp_css_start(struct atomisp_sub_device *asd,
> > * Thus the stream created in set_fmt get destroyed and need to be
> > * recreated in the next stream on.
> > */
> > - if (asd->stream_prepared == false) {
> > + if (!asd->stream_prepared) {
> > if (__create_pipes(asd)) {
> > dev_err(isp->dev, "create pipe error.\n");
> > return -EINVAL;
> > diff --git a/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c b/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
> > index 317ea30ede7a..82f3c19dc455 100644
> > --- a/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
> > +++ b/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
> > @@ -179,12 +179,12 @@ ia_css_isys_error_t ia_css_isys_stream_create(
> > isys_stream->linked_isys_stream_id = isys_stream_descr->linked_isys_stream_id;
> > rc = create_input_system_input_port(isys_stream_descr,
> > &isys_stream->input_port);
> > - if (rc == false)
> > + if (!rc)
> > return false;
> >
> > rc = create_input_system_channel(isys_stream_descr, false,
> > &isys_stream->channel);
> > - if (rc == false) {
> > + if (!rc) {
> > destroy_input_system_input_port(&isys_stream->input_port);
> > return false;
> > }
> > @@ -204,7 +204,7 @@ ia_css_isys_error_t ia_css_isys_stream_create(
> > if (isys_stream_descr->metadata.enable) {
> > rc = create_input_system_channel(isys_stream_descr, true,
> > &isys_stream->md_channel);
> > - if (rc == false) {
> > + if (!rc) {
> > destroy_input_system_input_port(&isys_stream->input_port);
> > destroy_input_system_channel(&isys_stream->channel);
> > return false;
> > @@ -248,7 +248,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
> > isys_stream_descr,
> > &isys_stream_cfg->channel_cfg,
> > false);
> > - if (rc == false)
> > + if (!rc)
> > return false;
> >
> > /* configure metadata channel */
> > @@ -260,7 +260,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
> > isys_stream_descr,
> > &isys_stream_cfg->md_channel_cfg,
> > true);
> > - if (rc == false)
> > + if (!rc)
> > return false;
> > }
> >
> > @@ -269,7 +269,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
> > &isys_stream->input_port,
> > isys_stream_descr,
> > &isys_stream_cfg->input_port_cfg);
> > - if (rc == false)
> > + if (!rc)
> > return false;
> >
> > isys_stream->valid = 1;
> > diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> > index ddee04c8248d..a71c1bbd984b 100644
> > --- a/drivers/staging/media/atomisp/pci/sh_css.c
> > +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> > @@ -1063,7 +1063,7 @@ 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 == true)
> > + if (stream->config.continuous)
> > {
> > if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_CAPTURE) {
> > pipe = stream->last_pipe;
> > @@ -5626,7 +5626,7 @@ static int load_video_binaries(struct ia_css_pipe *pipe)
> > } else {
> > /* output from main binary is not yuv line. currently this is
> > * possible only when bci is enabled on vfpp output */
> > - assert(pipe->config.enable_vfpp_bci == true);
> > + assert(pipe->config.enable_vfpp_bci);
> > ia_css_pipe_get_yuvscaler_binarydesc(pipe, &vf_pp_descr,
> > &mycs->video_binary.vf_frame_info,
> > pipe_vf_out_info, NULL, NULL);
> > @@ -8072,7 +8072,7 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe) {
> > struct ia_css_frame *tmp_out_frame = NULL;
> >
> > for (i = 0; i < num_yuv_scaler; i++) {
> > - if (is_output_stage[i] == true)
> > + if (is_output_stage[i])
> > tmp_out_frame = out_frame;
> > else
> > tmp_out_frame = NULL;
> > @@ -8464,7 +8464,7 @@ sh_css_pipeline_add_acc_stage(struct ia_css_pipeline *pipeline,
> > /* In QoS case, load_extension already called, so skipping */
> > int err = 0;
> >
> > - if (fw->loaded == false)
> > + if (!fw->loaded)
> > err = acc_load_extension(fw);
> >
> > ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
> > @@ -9701,8 +9701,8 @@ ia_css_stream_destroy(struct ia_css_stream *stream) {
> > assert(entry);
> > if (entry) {
> > /* get the SP thread id */
> > - if (ia_css_pipeline_get_sp_thread_id(
> > - ia_css_pipe_get_pipe_num(entry), &sp_thread_id) != true)
> > + if (!ia_css_pipeline_get_sp_thread_id(
> > + ia_css_pipe_get_pipe_num(entry), &sp_thread_id))
> > return -EINVAL;
> > /* get the target input terminal */
> > sp_pipeline_input_terminal =
> >
>
> Hi Lukas
>
> I have tested these changes by running Makefile in the root directory,
> where I did not get any errors. Can you please review it quickly
> before I send it to maintainers?
>
Well, two things are helpful even if you do not have the hardware.
First, determine the required kernel configs so that the code you
touched is actually compiled.
Second, compile ite and compare the objdump of the created object file
before and after the change.
Other than that, the change looks good. It is probably a driver that
is on its way to removal anyway. So, do not be surprised if even if
the maintainers thinks the change generally improves the code, they
simply do not want to touch code that is soon to be deleted.
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] media: atomisp: replace boolean comparison of values with bool variables
2020-12-13 17:21 Aditya Srivastava
2020-12-13 17:30 ` Aditya
@ 2020-12-14 5:37 ` Lukas Bulwahn
1 sibling, 0 replies; 6+ messages in thread
From: Lukas Bulwahn @ 2020-12-14 5:37 UTC (permalink / raw)
To: Aditya Srivastava; +Cc: linux-kernel-mentees
On Sun, Dec 13, 2020 at 6:21 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> There are certain relational expressions in atomisp where a boolean
relational expressions? Do you mean expressions in a condition?
> variable is compared with true/false in forms such as (foo == true)
> or (false != bar), which does not comply with the coding style rule by
> checkpatch.pl (CHK: BOOL_COMPARISON), according to which the boolean
> variables should be themselves used in relational expression, rather
in the condition.
> than comparing with true or false.
>
> E.g. In drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:
>
> if (asd->stream_prepared == false) {
>
> Can be replaced with:
> if (!asd->stream_prepared) {
>
> Replace such relational expressions with boolean variables appropriately.
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> .../staging/media/atomisp/pci/atomisp_compat_css20.c | 2 +-
> .../atomisp/pci/runtime/isys/src/virtual_isys.c | 12 ++++++------
> drivers/staging/media/atomisp/pci/sh_css.c | 12 ++++++------
> 3 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> index faa0935e536a..6a80c676f668 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> @@ -1142,7 +1142,7 @@ int atomisp_css_start(struct atomisp_sub_device *asd,
> * Thus the stream created in set_fmt get destroyed and need to be
> * recreated in the next stream on.
> */
> - if (asd->stream_prepared == false) {
> + if (!asd->stream_prepared) {
> if (__create_pipes(asd)) {
> dev_err(isp->dev, "create pipe error.\n");
> return -EINVAL;
> diff --git a/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c b/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
> index 317ea30ede7a..82f3c19dc455 100644
> --- a/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
> +++ b/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
> @@ -179,12 +179,12 @@ ia_css_isys_error_t ia_css_isys_stream_create(
> isys_stream->linked_isys_stream_id = isys_stream_descr->linked_isys_stream_id;
> rc = create_input_system_input_port(isys_stream_descr,
> &isys_stream->input_port);
> - if (rc == false)
> + if (!rc)
> return false;
>
> rc = create_input_system_channel(isys_stream_descr, false,
> &isys_stream->channel);
> - if (rc == false) {
> + if (!rc) {
> destroy_input_system_input_port(&isys_stream->input_port);
> return false;
> }
> @@ -204,7 +204,7 @@ ia_css_isys_error_t ia_css_isys_stream_create(
> if (isys_stream_descr->metadata.enable) {
> rc = create_input_system_channel(isys_stream_descr, true,
> &isys_stream->md_channel);
> - if (rc == false) {
> + if (!rc) {
> destroy_input_system_input_port(&isys_stream->input_port);
> destroy_input_system_channel(&isys_stream->channel);
> return false;
> @@ -248,7 +248,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
> isys_stream_descr,
> &isys_stream_cfg->channel_cfg,
> false);
> - if (rc == false)
> + if (!rc)
> return false;
>
> /* configure metadata channel */
> @@ -260,7 +260,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
> isys_stream_descr,
> &isys_stream_cfg->md_channel_cfg,
> true);
> - if (rc == false)
> + if (!rc)
> return false;
> }
>
> @@ -269,7 +269,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
> &isys_stream->input_port,
> isys_stream_descr,
> &isys_stream_cfg->input_port_cfg);
> - if (rc == false)
> + if (!rc)
> return false;
>
> isys_stream->valid = 1;
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index ddee04c8248d..a71c1bbd984b 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -1063,7 +1063,7 @@ 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 == true)
> + if (stream->config.continuous)
> {
> if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_CAPTURE) {
> pipe = stream->last_pipe;
> @@ -5626,7 +5626,7 @@ static int load_video_binaries(struct ia_css_pipe *pipe)
> } else {
> /* output from main binary is not yuv line. currently this is
> * possible only when bci is enabled on vfpp output */
> - assert(pipe->config.enable_vfpp_bci == true);
> + assert(pipe->config.enable_vfpp_bci);
> ia_css_pipe_get_yuvscaler_binarydesc(pipe, &vf_pp_descr,
> &mycs->video_binary.vf_frame_info,
> pipe_vf_out_info, NULL, NULL);
> @@ -8072,7 +8072,7 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe) {
> struct ia_css_frame *tmp_out_frame = NULL;
>
> for (i = 0; i < num_yuv_scaler; i++) {
> - if (is_output_stage[i] == true)
> + if (is_output_stage[i])
> tmp_out_frame = out_frame;
> else
> tmp_out_frame = NULL;
> @@ -8464,7 +8464,7 @@ sh_css_pipeline_add_acc_stage(struct ia_css_pipeline *pipeline,
> /* In QoS case, load_extension already called, so skipping */
> int err = 0;
>
> - if (fw->loaded == false)
> + if (!fw->loaded)
> err = acc_load_extension(fw);
>
> ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
> @@ -9701,8 +9701,8 @@ ia_css_stream_destroy(struct ia_css_stream *stream) {
> assert(entry);
> if (entry) {
> /* get the SP thread id */
> - if (ia_css_pipeline_get_sp_thread_id(
> - ia_css_pipe_get_pipe_num(entry), &sp_thread_id) != true)
> + if (!ia_css_pipeline_get_sp_thread_id(
> + ia_css_pipe_get_pipe_num(entry), &sp_thread_id))
> return -EINVAL;
> /* get the target input terminal */
> sp_pipeline_input_terminal =
> --
> 2.17.1
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] media: atomisp: replace boolean comparison of values with bool variables
2020-12-13 17:21 Aditya Srivastava
@ 2020-12-13 17:30 ` Aditya
2020-12-14 5:41 ` Lukas Bulwahn
2020-12-14 5:37 ` Lukas Bulwahn
1 sibling, 1 reply; 6+ messages in thread
From: Aditya @ 2020-12-13 17:30 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: linux-kernel-mentees, yashsri421
On 13/12/20 10:51 pm, Aditya Srivastava wrote:
> There are certain relational expressions in atomisp where a boolean
> variable is compared with true/false in forms such as (foo == true)
> or (false != bar), which does not comply with the coding style rule by
> checkpatch.pl (CHK: BOOL_COMPARISON), according to which the boolean
> variables should be themselves used in relational expression, rather
> than comparing with true or false.
>
> E.g. In drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:
>
> if (asd->stream_prepared == false) {
>
> Can be replaced with:
> if (!asd->stream_prepared) {
>
> Replace such relational expressions with boolean variables appropriately.
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> .../staging/media/atomisp/pci/atomisp_compat_css20.c | 2 +-
> .../atomisp/pci/runtime/isys/src/virtual_isys.c | 12 ++++++------
> drivers/staging/media/atomisp/pci/sh_css.c | 12 ++++++------
> 3 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> index faa0935e536a..6a80c676f668 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> @@ -1142,7 +1142,7 @@ int atomisp_css_start(struct atomisp_sub_device *asd,
> * Thus the stream created in set_fmt get destroyed and need to be
> * recreated in the next stream on.
> */
> - if (asd->stream_prepared == false) {
> + if (!asd->stream_prepared) {
> if (__create_pipes(asd)) {
> dev_err(isp->dev, "create pipe error.\n");
> return -EINVAL;
> diff --git a/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c b/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
> index 317ea30ede7a..82f3c19dc455 100644
> --- a/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
> +++ b/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
> @@ -179,12 +179,12 @@ ia_css_isys_error_t ia_css_isys_stream_create(
> isys_stream->linked_isys_stream_id = isys_stream_descr->linked_isys_stream_id;
> rc = create_input_system_input_port(isys_stream_descr,
> &isys_stream->input_port);
> - if (rc == false)
> + if (!rc)
> return false;
>
> rc = create_input_system_channel(isys_stream_descr, false,
> &isys_stream->channel);
> - if (rc == false) {
> + if (!rc) {
> destroy_input_system_input_port(&isys_stream->input_port);
> return false;
> }
> @@ -204,7 +204,7 @@ ia_css_isys_error_t ia_css_isys_stream_create(
> if (isys_stream_descr->metadata.enable) {
> rc = create_input_system_channel(isys_stream_descr, true,
> &isys_stream->md_channel);
> - if (rc == false) {
> + if (!rc) {
> destroy_input_system_input_port(&isys_stream->input_port);
> destroy_input_system_channel(&isys_stream->channel);
> return false;
> @@ -248,7 +248,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
> isys_stream_descr,
> &isys_stream_cfg->channel_cfg,
> false);
> - if (rc == false)
> + if (!rc)
> return false;
>
> /* configure metadata channel */
> @@ -260,7 +260,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
> isys_stream_descr,
> &isys_stream_cfg->md_channel_cfg,
> true);
> - if (rc == false)
> + if (!rc)
> return false;
> }
>
> @@ -269,7 +269,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
> &isys_stream->input_port,
> isys_stream_descr,
> &isys_stream_cfg->input_port_cfg);
> - if (rc == false)
> + if (!rc)
> return false;
>
> isys_stream->valid = 1;
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index ddee04c8248d..a71c1bbd984b 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -1063,7 +1063,7 @@ 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 == true)
> + if (stream->config.continuous)
> {
> if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_CAPTURE) {
> pipe = stream->last_pipe;
> @@ -5626,7 +5626,7 @@ static int load_video_binaries(struct ia_css_pipe *pipe)
> } else {
> /* output from main binary is not yuv line. currently this is
> * possible only when bci is enabled on vfpp output */
> - assert(pipe->config.enable_vfpp_bci == true);
> + assert(pipe->config.enable_vfpp_bci);
> ia_css_pipe_get_yuvscaler_binarydesc(pipe, &vf_pp_descr,
> &mycs->video_binary.vf_frame_info,
> pipe_vf_out_info, NULL, NULL);
> @@ -8072,7 +8072,7 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe) {
> struct ia_css_frame *tmp_out_frame = NULL;
>
> for (i = 0; i < num_yuv_scaler; i++) {
> - if (is_output_stage[i] == true)
> + if (is_output_stage[i])
> tmp_out_frame = out_frame;
> else
> tmp_out_frame = NULL;
> @@ -8464,7 +8464,7 @@ sh_css_pipeline_add_acc_stage(struct ia_css_pipeline *pipeline,
> /* In QoS case, load_extension already called, so skipping */
> int err = 0;
>
> - if (fw->loaded == false)
> + if (!fw->loaded)
> err = acc_load_extension(fw);
>
> ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
> @@ -9701,8 +9701,8 @@ ia_css_stream_destroy(struct ia_css_stream *stream) {
> assert(entry);
> if (entry) {
> /* get the SP thread id */
> - if (ia_css_pipeline_get_sp_thread_id(
> - ia_css_pipe_get_pipe_num(entry), &sp_thread_id) != true)
> + if (!ia_css_pipeline_get_sp_thread_id(
> + ia_css_pipe_get_pipe_num(entry), &sp_thread_id))
> return -EINVAL;
> /* get the target input terminal */
> sp_pipeline_input_terminal =
>
Hi Lukas
I have tested these changes by running Makefile in the root directory,
where I did not get any errors. Can you please review it quickly
before I send it to maintainers?
Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Linux-kernel-mentees] [PATCH] media: atomisp: replace boolean comparison of values with bool variables
@ 2020-12-13 17:21 Aditya Srivastava
2020-12-13 17:30 ` Aditya
2020-12-14 5:37 ` Lukas Bulwahn
0 siblings, 2 replies; 6+ messages in thread
From: Aditya Srivastava @ 2020-12-13 17:21 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: linux-kernel-mentees, yashsri421
There are certain relational expressions in atomisp where a boolean
variable is compared with true/false in forms such as (foo == true)
or (false != bar), which does not comply with the coding style rule by
checkpatch.pl (CHK: BOOL_COMPARISON), according to which the boolean
variables should be themselves used in relational expression, rather
than comparing with true or false.
E.g. In drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:
if (asd->stream_prepared == false) {
Can be replaced with:
if (!asd->stream_prepared) {
Replace such relational expressions with boolean variables appropriately.
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
.../staging/media/atomisp/pci/atomisp_compat_css20.c | 2 +-
.../atomisp/pci/runtime/isys/src/virtual_isys.c | 12 ++++++------
drivers/staging/media/atomisp/pci/sh_css.c | 12 ++++++------
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index faa0935e536a..6a80c676f668 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -1142,7 +1142,7 @@ int atomisp_css_start(struct atomisp_sub_device *asd,
* Thus the stream created in set_fmt get destroyed and need to be
* recreated in the next stream on.
*/
- if (asd->stream_prepared == false) {
+ if (!asd->stream_prepared) {
if (__create_pipes(asd)) {
dev_err(isp->dev, "create pipe error.\n");
return -EINVAL;
diff --git a/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c b/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
index 317ea30ede7a..82f3c19dc455 100644
--- a/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
+++ b/drivers/staging/media/atomisp/pci/runtime/isys/src/virtual_isys.c
@@ -179,12 +179,12 @@ ia_css_isys_error_t ia_css_isys_stream_create(
isys_stream->linked_isys_stream_id = isys_stream_descr->linked_isys_stream_id;
rc = create_input_system_input_port(isys_stream_descr,
&isys_stream->input_port);
- if (rc == false)
+ if (!rc)
return false;
rc = create_input_system_channel(isys_stream_descr, false,
&isys_stream->channel);
- if (rc == false) {
+ if (!rc) {
destroy_input_system_input_port(&isys_stream->input_port);
return false;
}
@@ -204,7 +204,7 @@ ia_css_isys_error_t ia_css_isys_stream_create(
if (isys_stream_descr->metadata.enable) {
rc = create_input_system_channel(isys_stream_descr, true,
&isys_stream->md_channel);
- if (rc == false) {
+ if (!rc) {
destroy_input_system_input_port(&isys_stream->input_port);
destroy_input_system_channel(&isys_stream->channel);
return false;
@@ -248,7 +248,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
isys_stream_descr,
&isys_stream_cfg->channel_cfg,
false);
- if (rc == false)
+ if (!rc)
return false;
/* configure metadata channel */
@@ -260,7 +260,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
isys_stream_descr,
&isys_stream_cfg->md_channel_cfg,
true);
- if (rc == false)
+ if (!rc)
return false;
}
@@ -269,7 +269,7 @@ ia_css_isys_error_t ia_css_isys_stream_calculate_cfg(
&isys_stream->input_port,
isys_stream_descr,
&isys_stream_cfg->input_port_cfg);
- if (rc == false)
+ if (!rc)
return false;
isys_stream->valid = 1;
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index ddee04c8248d..a71c1bbd984b 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -1063,7 +1063,7 @@ 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 == true)
+ if (stream->config.continuous)
{
if (stream->last_pipe->config.mode == IA_CSS_PIPE_MODE_CAPTURE) {
pipe = stream->last_pipe;
@@ -5626,7 +5626,7 @@ static int load_video_binaries(struct ia_css_pipe *pipe)
} else {
/* output from main binary is not yuv line. currently this is
* possible only when bci is enabled on vfpp output */
- assert(pipe->config.enable_vfpp_bci == true);
+ assert(pipe->config.enable_vfpp_bci);
ia_css_pipe_get_yuvscaler_binarydesc(pipe, &vf_pp_descr,
&mycs->video_binary.vf_frame_info,
pipe_vf_out_info, NULL, NULL);
@@ -8072,7 +8072,7 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe) {
struct ia_css_frame *tmp_out_frame = NULL;
for (i = 0; i < num_yuv_scaler; i++) {
- if (is_output_stage[i] == true)
+ if (is_output_stage[i])
tmp_out_frame = out_frame;
else
tmp_out_frame = NULL;
@@ -8464,7 +8464,7 @@ sh_css_pipeline_add_acc_stage(struct ia_css_pipeline *pipeline,
/* In QoS case, load_extension already called, so skipping */
int err = 0;
- if (fw->loaded == false)
+ if (!fw->loaded)
err = acc_load_extension(fw);
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
@@ -9701,8 +9701,8 @@ ia_css_stream_destroy(struct ia_css_stream *stream) {
assert(entry);
if (entry) {
/* get the SP thread id */
- if (ia_css_pipeline_get_sp_thread_id(
- ia_css_pipe_get_pipe_num(entry), &sp_thread_id) != true)
+ if (!ia_css_pipeline_get_sp_thread_id(
+ ia_css_pipe_get_pipe_num(entry), &sp_thread_id))
return -EINVAL;
/* get the target input terminal */
sp_pipeline_input_terminal =
--
2.17.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-14 13:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-14 13:27 [Linux-kernel-mentees] [PATCH] media: atomisp: replace boolean comparison of values with bool variables Aditya Srivastava
-- strict thread matches above, loose matches on Subject: below --
2020-12-13 17:21 Aditya Srivastava
2020-12-13 17:30 ` Aditya
2020-12-14 5:41 ` Lukas Bulwahn
2020-12-14 5:45 ` Lukas Bulwahn
2020-12-14 5:37 ` Lukas Bulwahn
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 NNTP newsgroup(s).