* [PATCH 0/4] media: staging: rkisp1: send cleanups and checkpatch fixes
@ 2020-10-19 20:59 Dafna Hirschfeld
2020-10-19 20:59 ` [PATCH 1/4] media: staging: rkisp1: fix coding style issues Dafna Hirschfeld
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Dafna Hirschfeld @ 2020-10-19 20:59 UTC (permalink / raw)
To: linux-media
Cc: laurent.pinchart, dafna.hirschfeld, helen.koike, ezequiel,
hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip, mchehab,
tfiga
This patchset aims to remove the TODO item "Fix checkpatch errors."
Some cleanups to the rkisp1-params.c code are also sent.
The following false positiv issues are still generated:
* "line length of * exceeds 100 columns" for the topology diagram in rkisp1-dev.c
* "DT compatible string "rockchip,rk3399-cif-isp" appears un-documented" - this will be
solved once the driver will be destaged and the file rockchip-isp1.yaml will move
to the global documentation
* "Prefer using the BIT macro" - in the uapi header 'rkisp1-config.h The BIT() macro is not
available to userspace
Dafna Hirschfeld (4):
media: staging: rkisp1: fix coding style issues
media: staging: rkisp1: params: remove unnecessary "!!"
media: staging: rkisp1: params: remove unnecessary parentheses
media: staging: rkisp1: params: remove extra 'if' conditions
drivers/staging/media/rkisp1/TODO | 1 -
drivers/staging/media/rkisp1/rkisp1-dev.c | 4 +-
drivers/staging/media/rkisp1/rkisp1-isp.c | 1 -
drivers/staging/media/rkisp1/rkisp1-params.c | 420 ++++++++----------
drivers/staging/media/rkisp1/rkisp1-resizer.c | 4 +-
5 files changed, 179 insertions(+), 251 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] media: staging: rkisp1: fix coding style issues
2020-10-19 20:59 [PATCH 0/4] media: staging: rkisp1: send cleanups and checkpatch fixes Dafna Hirschfeld
@ 2020-10-19 20:59 ` Dafna Hirschfeld
2020-10-20 17:13 ` Helen Koike
2020-10-19 20:59 ` [PATCH 2/4] media: staging: rkisp1: params: remove unnecessary "!!" Dafna Hirschfeld
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Dafna Hirschfeld @ 2020-10-19 20:59 UTC (permalink / raw)
To: linux-media
Cc: laurent.pinchart, dafna.hirschfeld, helen.koike, ezequiel,
hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip, mchehab,
tfiga
Fix checkpatch issues:
Blank lines aren't necessary before a close brace '}'
Alignment should match open parenthesis
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-dev.c | 4 ++--
drivers/staging/media/rkisp1/rkisp1-isp.c | 1 -
drivers/staging/media/rkisp1/rkisp1-resizer.c | 4 ++--
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-dev.c b/drivers/staging/media/rkisp1/rkisp1-dev.c
index 91584695804b..4f6ae1a01253 100644
--- a/drivers/staging/media/rkisp1/rkisp1-dev.c
+++ b/drivers/staging/media/rkisp1/rkisp1-dev.c
@@ -254,8 +254,8 @@ static int rkisp1_subdev_notifier(struct rkisp1_device *rkisp1)
struct rkisp1_sensor_async *rk_asd = NULL;
struct fwnode_handle *ep;
- ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(rkisp1->dev),
- 0, next_id, FWNODE_GRAPH_ENDPOINT_NEXT);
+ ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(rkisp1->dev), 0, next_id,
+ FWNODE_GRAPH_ENDPOINT_NEXT);
if (!ep)
break;
diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
index a9715b0b7264..fb23461d865c 100644
--- a/drivers/staging/media/rkisp1/rkisp1-isp.c
+++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
@@ -1157,5 +1157,4 @@ void rkisp1_isp_isr(struct rkisp1_device *rkisp1)
*/
rkisp1_params_isr(rkisp1);
}
-
}
diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
index 1687d82e6c68..a9d537c11ecb 100644
--- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
+++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
@@ -610,8 +610,8 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
RKISP1_ISP_MIN_WIDTH,
RKISP1_ISP_MAX_WIDTH);
sink_fmt->height = clamp_t(u32, format->height,
- RKISP1_ISP_MIN_HEIGHT,
- RKISP1_ISP_MAX_HEIGHT);
+ RKISP1_ISP_MIN_HEIGHT,
+ RKISP1_ISP_MAX_HEIGHT);
*format = *sink_fmt;
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] media: staging: rkisp1: params: remove unnecessary "!!"
2020-10-19 20:59 [PATCH 0/4] media: staging: rkisp1: send cleanups and checkpatch fixes Dafna Hirschfeld
2020-10-19 20:59 ` [PATCH 1/4] media: staging: rkisp1: fix coding style issues Dafna Hirschfeld
@ 2020-10-19 20:59 ` Dafna Hirschfeld
2020-10-20 17:13 ` Helen Koike
2020-10-19 20:59 ` [PATCH 3/4] media: staging: rkisp1: params: remove unnecessary parentheses Dafna Hirschfeld
2020-10-19 20:59 ` [PATCH 4/4] media: staging: rkisp1: params: remove extra 'if' conditions Dafna Hirschfeld
3 siblings, 1 reply; 10+ messages in thread
From: Dafna Hirschfeld @ 2020-10-19 20:59 UTC (permalink / raw)
To: linux-media
Cc: laurent.pinchart, dafna.hirschfeld, helen.koike, ezequiel,
hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip, mchehab,
tfiga
There are several 'if' conditions of the form:
if (!!(module_ens & SOME_FLAG))
Those can be replaced with:
if (module_ens & SOME_FLAG)
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-params.c | 24 ++++++++++----------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
index 986d293201e6..4ac401bc2164 100644
--- a/drivers/staging/media/rkisp1/rkisp1-params.c
+++ b/drivers/staging/media/rkisp1/rkisp1-params.c
@@ -896,7 +896,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
&new_params->others.dpcc_config);
if (module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_DPCC))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_DPCC)
rkisp1_param_set_bits(params,
RKISP1_CIF_ISP_DPCC_MODE,
RKISP1_CIF_ISP_DPCC_ENA);
@@ -915,7 +915,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
&new_params->others.bls_config);
if (module_en_update & RKISP1_CIF_ISP_MODULE_BLS) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_BLS))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_BLS)
rkisp1_param_set_bits(params,
RKISP1_CIF_ISP_BLS_CTRL,
RKISP1_CIF_ISP_BLS_ENA);
@@ -934,7 +934,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
&new_params->others.sdg_config);
if (module_en_update & RKISP1_CIF_ISP_MODULE_SDG) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_SDG))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_SDG)
rkisp1_param_set_bits(params,
RKISP1_CIF_ISP_CTRL,
RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
@@ -953,7 +953,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
&new_params->others.lsc_config);
if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_LSC))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_LSC)
rkisp1_param_set_bits(params,
RKISP1_CIF_ISP_LSC_CTRL,
RKISP1_CIF_ISP_LSC_CTRL_ENA);
@@ -972,7 +972,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
&new_params->others.awb_gain_config);
if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_AWB_GAIN))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
rkisp1_param_set_bits(params,
RKISP1_CIF_ISP_CTRL,
RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
@@ -991,7 +991,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
&new_params->others.bdm_config);
if (module_en_update & RKISP1_CIF_ISP_MODULE_BDM) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_BDM))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_BDM)
rkisp1_param_set_bits(params,
RKISP1_CIF_ISP_DEMOSAIC,
RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
@@ -1010,7 +1010,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
&new_params->others.flt_config);
if (module_en_update & RKISP1_CIF_ISP_MODULE_FLT) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_FLT))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_FLT)
rkisp1_param_set_bits(params,
RKISP1_CIF_ISP_FILT_MODE,
RKISP1_CIF_ISP_FLT_ENA);
@@ -1041,7 +1041,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
&new_params->others.goc_config);
if (module_en_update & RKISP1_CIF_ISP_MODULE_GOC) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_GOC))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_GOC)
rkisp1_param_set_bits(params,
RKISP1_CIF_ISP_CTRL,
RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
@@ -1061,7 +1061,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
}
if (module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_CPROC))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_CPROC)
rkisp1_param_set_bits(params,
RKISP1_CIF_C_PROC_CTRL,
RKISP1_CIF_C_PROC_CTR_ENABLE);
@@ -1092,7 +1092,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
&new_params->others.dpf_config);
if (module_en_update & RKISP1_CIF_ISP_MODULE_DPF) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_DPF))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_DPF)
rkisp1_param_set_bits(params,
RKISP1_CIF_ISP_DPF_MODE,
RKISP1_CIF_ISP_DPF_MODE_EN);
@@ -1141,7 +1141,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
&new_params->meas.afc_config);
if (module_en_update & RKISP1_CIF_ISP_MODULE_AFC) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_AFC))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_AFC)
rkisp1_param_set_bits(params,
RKISP1_CIF_ISP_AFM_CTRL,
RKISP1_CIF_ISP_AFM_ENA);
@@ -1173,7 +1173,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
&new_params->meas.aec_config);
if (module_en_update & RKISP1_CIF_ISP_MODULE_AEC) {
- if (!!(module_ens & RKISP1_CIF_ISP_MODULE_AEC))
+ if (module_ens & RKISP1_CIF_ISP_MODULE_AEC)
rkisp1_param_set_bits(params,
RKISP1_CIF_ISP_EXP_CTRL,
RKISP1_CIF_ISP_EXP_ENA);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] media: staging: rkisp1: params: remove unnecessary parentheses
2020-10-19 20:59 [PATCH 0/4] media: staging: rkisp1: send cleanups and checkpatch fixes Dafna Hirschfeld
2020-10-19 20:59 ` [PATCH 1/4] media: staging: rkisp1: fix coding style issues Dafna Hirschfeld
2020-10-19 20:59 ` [PATCH 2/4] media: staging: rkisp1: params: remove unnecessary "!!" Dafna Hirschfeld
@ 2020-10-19 20:59 ` Dafna Hirschfeld
2020-10-20 17:13 ` Helen Koike
2020-10-19 20:59 ` [PATCH 4/4] media: staging: rkisp1: params: remove extra 'if' conditions Dafna Hirschfeld
3 siblings, 1 reply; 10+ messages in thread
From: Dafna Hirschfeld @ 2020-10-19 20:59 UTC (permalink / raw)
To: linux-media
Cc: laurent.pinchart, dafna.hirschfeld, helen.koike, ezequiel,
hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip, mchehab,
tfiga
There are several 'if' expression where double
parentheses is used when one is enough.
Remove the extra parentheses.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-params.c | 32 ++++++++++----------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
index 4ac401bc2164..a2df6b50c109 100644
--- a/drivers/staging/media/rkisp1/rkisp1-params.c
+++ b/drivers/staging/media/rkisp1/rkisp1-params.c
@@ -891,7 +891,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)) {
/*update dpc config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)
rkisp1_dpcc_config(params,
&new_params->others.dpcc_config);
@@ -910,7 +910,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_BLS) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)) {
/* update bls config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)
rkisp1_bls_config(params,
&new_params->others.bls_config);
@@ -929,7 +929,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_SDG) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)) {
/* update sdg config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)
rkisp1_sdg_config(params,
&new_params->others.sdg_config);
@@ -948,7 +948,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_LSC) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)) {
/* update lsc config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)
rkisp1_lsc_config(params,
&new_params->others.lsc_config);
@@ -967,7 +967,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)) {
/* update awb gains */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
rkisp1_awb_gain_config(params,
&new_params->others.awb_gain_config);
@@ -986,7 +986,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_BDM) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)) {
/* update bdm config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)
rkisp1_bdm_config(params,
&new_params->others.bdm_config);
@@ -1005,7 +1005,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_FLT) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)) {
/* update filter config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)
rkisp1_flt_config(params,
&new_params->others.flt_config);
@@ -1024,7 +1024,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_CTK) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)) {
/* update ctk config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)
rkisp1_ctk_config(params,
&new_params->others.ctk_config);
@@ -1036,7 +1036,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_GOC) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)) {
/* update goc config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)
rkisp1_goc_config(params,
&new_params->others.goc_config);
@@ -1055,7 +1055,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC)) {
/* update cproc config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC)) {
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC) {
rkisp1_cproc_config(params,
&new_params->others.cproc_config);
}
@@ -1075,7 +1075,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_IE) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)) {
/* update ie config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_IE))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)
rkisp1_ie_config(params,
&new_params->others.ie_config);
@@ -1087,7 +1087,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPF) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)) {
/* update dpf config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)
rkisp1_dpf_config(params,
&new_params->others.dpf_config);
@@ -1123,7 +1123,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_AWB) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)) {
/* update awb config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)
rkisp1_awb_meas_config(params,
&new_params->meas.awb_meas_config);
@@ -1136,7 +1136,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_AFC) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)) {
/* update afc config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)
rkisp1_afm_config(params,
&new_params->meas.afc_config);
@@ -1155,7 +1155,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_HST) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)) {
/* update hst config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_HST))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)
rkisp1_hst_config(params,
&new_params->meas.hst_config);
@@ -1168,7 +1168,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
if ((module_en_update & RKISP1_CIF_ISP_MODULE_AEC) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)) {
/* update aec config */
- if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC))
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)
rkisp1_aec_config(params,
&new_params->meas.aec_config);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] media: staging: rkisp1: params: remove extra 'if' conditions
2020-10-19 20:59 [PATCH 0/4] media: staging: rkisp1: send cleanups and checkpatch fixes Dafna Hirschfeld
` (2 preceding siblings ...)
2020-10-19 20:59 ` [PATCH 3/4] media: staging: rkisp1: params: remove unnecessary parentheses Dafna Hirschfeld
@ 2020-10-19 20:59 ` Dafna Hirschfeld
2020-10-20 17:13 ` Helen Koike
3 siblings, 1 reply; 10+ messages in thread
From: Dafna Hirschfeld @ 2020-10-19 20:59 UTC (permalink / raw)
To: linux-media
Cc: laurent.pinchart, dafna.hirschfeld, helen.koike, ezequiel,
hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip, mchehab,
tfiga
There is a repeating code pattern:
if (a || b) {
if (a)
...
if (b)
...
}
In this pattern, the first 'if' is redundant.
The code can be replaced with:
if (a)
...
if (b)
...
In addition, the patch corrects alignment issues
reported by checkpatch.
This solves the TODO item
"Fix checkpatch errors."
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/TODO | 1 -
drivers/staging/media/rkisp1/rkisp1-params.c | 420 ++++++++-----------
2 files changed, 175 insertions(+), 246 deletions(-)
diff --git a/drivers/staging/media/rkisp1/TODO b/drivers/staging/media/rkisp1/TODO
index e7c8398fc2ce..647ab482d032 100644
--- a/drivers/staging/media/rkisp1/TODO
+++ b/drivers/staging/media/rkisp1/TODO
@@ -1,5 +1,4 @@
* Fix pad format size for statistics and parameters entities.
-* Fix checkpatch errors.
* Add uapi docs. Remember to add documentation of how quantization is handled.
* streaming paths (mainpath and selfpath) check if the other path is streaming
in several places of the code, review this, specially that it doesn't seem it
diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
index a2df6b50c109..c8585b6ee918 100644
--- a/drivers/staging/media/rkisp1/rkisp1-params.c
+++ b/drivers/staging/media/rkisp1/rkisp1-params.c
@@ -186,7 +186,7 @@ static void rkisp1_bls_config(struct rkisp1_params *params,
/* ISP LS correction interface function */
static void
rkisp1_lsc_correct_matrix_config(struct rkisp1_params *params,
- const struct rkisp1_cif_isp_lsc_config *pconfig)
+ const struct rkisp1_cif_isp_lsc_config *pconfig)
{
unsigned int isp_lsc_status, sram_addr, isp_lsc_table_sel, i, j, data;
@@ -434,7 +434,7 @@ static void rkisp1_ctk_enable(struct rkisp1_params *params, bool en)
/* ISP White Balance Mode */
static void rkisp1_awb_meas_config(struct rkisp1_params *params,
- const struct rkisp1_cif_isp_awb_meas_config *arg)
+ const struct rkisp1_cif_isp_awb_meas_config *arg)
{
u32 reg_val = 0;
/* based on the mode,configure the awb module */
@@ -888,226 +888,171 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
module_cfg_update = new_params->module_cfg_update;
module_ens = new_params->module_ens;
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)) {
- /*update dpc config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)
- rkisp1_dpcc_config(params,
- &new_params->others.dpcc_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_DPCC)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_ISP_DPCC_MODE,
- RKISP1_CIF_ISP_DPCC_ENA);
- else
- rkisp1_param_clear_bits(params,
+ /*update dpc config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)
+ rkisp1_dpcc_config(params,
+ &new_params->others.dpcc_config);
+
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_DPCC)
+ rkisp1_param_set_bits(params,
+ RKISP1_CIF_ISP_DPCC_MODE,
+ RKISP1_CIF_ISP_DPCC_ENA);
+ else
+ rkisp1_param_clear_bits(params,
RKISP1_CIF_ISP_DPCC_MODE,
RKISP1_CIF_ISP_DPCC_ENA);
- }
}
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_BLS) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)) {
- /* update bls config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)
- rkisp1_bls_config(params,
- &new_params->others.bls_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_BLS) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_BLS)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_ISP_BLS_CTRL,
- RKISP1_CIF_ISP_BLS_ENA);
- else
- rkisp1_param_clear_bits(params,
- RKISP1_CIF_ISP_BLS_CTRL,
- RKISP1_CIF_ISP_BLS_ENA);
- }
+ /* update bls config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)
+ rkisp1_bls_config(params,
+ &new_params->others.bls_config);
+
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_BLS) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_BLS)
+ rkisp1_param_set_bits(params,
+ RKISP1_CIF_ISP_BLS_CTRL,
+ RKISP1_CIF_ISP_BLS_ENA);
+ else
+ rkisp1_param_clear_bits(params,
+ RKISP1_CIF_ISP_BLS_CTRL,
+ RKISP1_CIF_ISP_BLS_ENA);
}
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_SDG) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)) {
- /* update sdg config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)
- rkisp1_sdg_config(params,
- &new_params->others.sdg_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_SDG) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_SDG)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_ISP_CTRL,
- RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
- else
- rkisp1_param_clear_bits(params,
- RKISP1_CIF_ISP_CTRL,
- RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
- }
+ /* update sdg config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)
+ rkisp1_sdg_config(params,
+ &new_params->others.sdg_config);
+
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_SDG) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_SDG)
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
+ else
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
}
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_LSC) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)) {
- /* update lsc config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)
- rkisp1_lsc_config(params,
- &new_params->others.lsc_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_LSC)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_ISP_LSC_CTRL,
- RKISP1_CIF_ISP_LSC_CTRL_ENA);
- else
- rkisp1_param_clear_bits(params,
- RKISP1_CIF_ISP_LSC_CTRL,
+ /* update lsc config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)
+ rkisp1_lsc_config(params,
+ &new_params->others.lsc_config);
+
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_LSC)
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
+ RKISP1_CIF_ISP_LSC_CTRL_ENA);
+ else
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
RKISP1_CIF_ISP_LSC_CTRL_ENA);
- }
}
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)) {
- /* update awb gains */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
- rkisp1_awb_gain_config(params,
- &new_params->others.awb_gain_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_ISP_CTRL,
- RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
- else
- rkisp1_param_clear_bits(params,
- RKISP1_CIF_ISP_CTRL,
- RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
- }
+ /* update awb gains */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
+ rkisp1_awb_gain_config(params, &new_params->others.awb_gain_config);
+
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
+ else
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
}
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_BDM) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)) {
- /* update bdm config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)
- rkisp1_bdm_config(params,
- &new_params->others.bdm_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_BDM) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_BDM)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_ISP_DEMOSAIC,
- RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
- else
- rkisp1_param_clear_bits(params,
- RKISP1_CIF_ISP_DEMOSAIC,
+ /* update bdm config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)
+ rkisp1_bdm_config(params,
+ &new_params->others.bdm_config);
+
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_BDM) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_BDM)
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
+ RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
+ else
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
- }
}
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_FLT) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)) {
- /* update filter config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)
- rkisp1_flt_config(params,
- &new_params->others.flt_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_FLT) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_FLT)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_ISP_FILT_MODE,
- RKISP1_CIF_ISP_FLT_ENA);
- else
- rkisp1_param_clear_bits(params,
- RKISP1_CIF_ISP_FILT_MODE,
+ /* update filter config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)
+ rkisp1_flt_config(params,
+ &new_params->others.flt_config);
+
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_FLT) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_FLT)
+ rkisp1_param_set_bits(params,
+ RKISP1_CIF_ISP_FILT_MODE,
+ RKISP1_CIF_ISP_FLT_ENA);
+ else
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_FILT_MODE,
RKISP1_CIF_ISP_FLT_ENA);
- }
}
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_CTK) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)) {
- /* update ctk config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)
- rkisp1_ctk_config(params,
- &new_params->others.ctk_config);
+ /* update ctk config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)
+ rkisp1_ctk_config(params,
+ &new_params->others.ctk_config);
- if (module_en_update & RKISP1_CIF_ISP_MODULE_CTK)
- rkisp1_ctk_enable(params,
- !!(module_ens & RKISP1_CIF_ISP_MODULE_CTK));
- }
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_CTK)
+ rkisp1_ctk_enable(params, !!(module_ens & RKISP1_CIF_ISP_MODULE_CTK));
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_GOC) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)) {
- /* update goc config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)
- rkisp1_goc_config(params,
- &new_params->others.goc_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_GOC) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_GOC)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_ISP_CTRL,
- RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
- else
- rkisp1_param_clear_bits(params,
- RKISP1_CIF_ISP_CTRL,
- RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
- }
+ /* update goc config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)
+ rkisp1_goc_config(params,
+ &new_params->others.goc_config);
+
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_GOC) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_GOC)
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
+ else
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
}
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC)) {
- /* update cproc config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC) {
- rkisp1_cproc_config(params,
- &new_params->others.cproc_config);
- }
+ /* update cproc config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC) {
+ rkisp1_cproc_config(params,
+ &new_params->others.cproc_config);
+ }
- if (module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_CPROC)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_C_PROC_CTRL,
- RKISP1_CIF_C_PROC_CTR_ENABLE);
- else
- rkisp1_param_clear_bits(params,
- RKISP1_CIF_C_PROC_CTRL,
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_CPROC)
+ rkisp1_param_set_bits(params, RKISP1_CIF_C_PROC_CTRL,
+ RKISP1_CIF_C_PROC_CTR_ENABLE);
+ else
+ rkisp1_param_clear_bits(params, RKISP1_CIF_C_PROC_CTRL,
RKISP1_CIF_C_PROC_CTR_ENABLE);
- }
}
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_IE) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)) {
- /* update ie config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)
- rkisp1_ie_config(params,
- &new_params->others.ie_config);
+ /* update ie config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)
+ rkisp1_ie_config(params,
+ &new_params->others.ie_config);
- if (module_en_update & RKISP1_CIF_ISP_MODULE_IE)
- rkisp1_ie_enable(params,
- !!(module_ens & RKISP1_CIF_ISP_MODULE_IE));
- }
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_IE)
+ rkisp1_ie_enable(params, !!(module_ens & RKISP1_CIF_ISP_MODULE_IE));
+
+ /* update dpf config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)
+ rkisp1_dpf_config(params,
+ &new_params->others.dpf_config);
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPF) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)) {
- /* update dpf config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)
- rkisp1_dpf_config(params,
- &new_params->others.dpf_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_DPF) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_DPF)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_ISP_DPF_MODE,
- RKISP1_CIF_ISP_DPF_MODE_EN);
- else
- rkisp1_param_clear_bits(params,
- RKISP1_CIF_ISP_DPF_MODE,
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_DPF) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_DPF)
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DPF_MODE,
+ RKISP1_CIF_ISP_DPF_MODE_EN);
+ else
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPF_MODE,
RKISP1_CIF_ISP_DPF_MODE_EN);
- }
}
if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPF_STRENGTH) ||
(module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF_STRENGTH)) {
/* update dpf strength config */
- rkisp1_dpf_strength_config(params,
- &new_params->others.dpf_strength_config);
+ rkisp1_dpf_strength_config(params, &new_params->others.dpf_strength_config);
}
}
@@ -1120,68 +1065,53 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
module_cfg_update = new_params->module_cfg_update;
module_ens = new_params->module_ens;
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_AWB) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)) {
- /* update awb config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)
- rkisp1_awb_meas_config(params,
- &new_params->meas.awb_meas_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB)
- rkisp1_awb_meas_enable(params,
- &new_params->meas.awb_meas_config,
- !!(module_ens & RKISP1_CIF_ISP_MODULE_AWB));
- }
+ /* update awb config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)
+ rkisp1_awb_meas_config(params, &new_params->meas.awb_meas_config);
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_AFC) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)) {
- /* update afc config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)
- rkisp1_afm_config(params,
- &new_params->meas.afc_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_AFC) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_AFC)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_ISP_AFM_CTRL,
- RKISP1_CIF_ISP_AFM_ENA);
- else
- rkisp1_param_clear_bits(params,
- RKISP1_CIF_ISP_AFM_CTRL,
- RKISP1_CIF_ISP_AFM_ENA);
- }
- }
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB)
+ rkisp1_awb_meas_enable(params, &new_params->meas.awb_meas_config,
+ !!(module_ens & RKISP1_CIF_ISP_MODULE_AWB));
+
+ /* update afc config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)
+ rkisp1_afm_config(params,
+ &new_params->meas.afc_config);
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_HST) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)) {
- /* update hst config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)
- rkisp1_hst_config(params,
- &new_params->meas.hst_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_HST)
- rkisp1_hst_enable(params,
- &new_params->meas.hst_config,
- !!(module_ens & RKISP1_CIF_ISP_MODULE_HST));
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_AFC) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_AFC)
+ rkisp1_param_set_bits(params,
+ RKISP1_CIF_ISP_AFM_CTRL,
+ RKISP1_CIF_ISP_AFM_ENA);
+ else
+ rkisp1_param_clear_bits(params,
+ RKISP1_CIF_ISP_AFM_CTRL,
+ RKISP1_CIF_ISP_AFM_ENA);
}
- if ((module_en_update & RKISP1_CIF_ISP_MODULE_AEC) ||
- (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)) {
- /* update aec config */
- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)
- rkisp1_aec_config(params,
- &new_params->meas.aec_config);
-
- if (module_en_update & RKISP1_CIF_ISP_MODULE_AEC) {
- if (module_ens & RKISP1_CIF_ISP_MODULE_AEC)
- rkisp1_param_set_bits(params,
- RKISP1_CIF_ISP_EXP_CTRL,
- RKISP1_CIF_ISP_EXP_ENA);
- else
- rkisp1_param_clear_bits(params,
- RKISP1_CIF_ISP_EXP_CTRL,
- RKISP1_CIF_ISP_EXP_ENA);
- }
+ /* update hst config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)
+ rkisp1_hst_config(params,
+ &new_params->meas.hst_config);
+
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_HST)
+ rkisp1_hst_enable(params, &new_params->meas.hst_config,
+ !!(module_ens & RKISP1_CIF_ISP_MODULE_HST));
+
+ /* update aec config */
+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)
+ rkisp1_aec_config(params,
+ &new_params->meas.aec_config);
+
+ if (module_en_update & RKISP1_CIF_ISP_MODULE_AEC) {
+ if (module_ens & RKISP1_CIF_ISP_MODULE_AEC)
+ rkisp1_param_set_bits(params,
+ RKISP1_CIF_ISP_EXP_CTRL,
+ RKISP1_CIF_ISP_EXP_ENA);
+ else
+ rkisp1_param_clear_bits(params,
+ RKISP1_CIF_ISP_EXP_CTRL,
+ RKISP1_CIF_ISP_EXP_ENA);
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] media: staging: rkisp1: fix coding style issues
2020-10-19 20:59 ` [PATCH 1/4] media: staging: rkisp1: fix coding style issues Dafna Hirschfeld
@ 2020-10-20 17:13 ` Helen Koike
0 siblings, 0 replies; 10+ messages in thread
From: Helen Koike @ 2020-10-20 17:13 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media
Cc: laurent.pinchart, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga
Hi Dafna,
On 10/19/20 5:59 PM, Dafna Hirschfeld wrote:
> Fix checkpatch issues:
> Blank lines aren't necessary before a close brace '}'
> Alignment should match open parenthesis
Just a nit, usually, it's one patch per checkpatch error.
With the split:
Acked-by: Helen Koike <helen.koike@collabora.com>
Thanks
Helen
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> drivers/staging/media/rkisp1/rkisp1-dev.c | 4 ++--
> drivers/staging/media/rkisp1/rkisp1-isp.c | 1 -
> drivers/staging/media/rkisp1/rkisp1-resizer.c | 4 ++--
> 3 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-dev.c b/drivers/staging/media/rkisp1/rkisp1-dev.c
> index 91584695804b..4f6ae1a01253 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-dev.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-dev.c
> @@ -254,8 +254,8 @@ static int rkisp1_subdev_notifier(struct rkisp1_device *rkisp1)
> struct rkisp1_sensor_async *rk_asd = NULL;
> struct fwnode_handle *ep;
>
> - ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(rkisp1->dev),
> - 0, next_id, FWNODE_GRAPH_ENDPOINT_NEXT);
> + ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(rkisp1->dev), 0, next_id,
> + FWNODE_GRAPH_ENDPOINT_NEXT);
> if (!ep)
> break;
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
> index a9715b0b7264..fb23461d865c 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-isp.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
> @@ -1157,5 +1157,4 @@ void rkisp1_isp_isr(struct rkisp1_device *rkisp1)
> */
> rkisp1_params_isr(rkisp1);
> }
> -
> }
> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> index 1687d82e6c68..a9d537c11ecb 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> @@ -610,8 +610,8 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
> RKISP1_ISP_MIN_WIDTH,
> RKISP1_ISP_MAX_WIDTH);
> sink_fmt->height = clamp_t(u32, format->height,
> - RKISP1_ISP_MIN_HEIGHT,
> - RKISP1_ISP_MAX_HEIGHT);
> + RKISP1_ISP_MIN_HEIGHT,
> + RKISP1_ISP_MAX_HEIGHT);
>
> *format = *sink_fmt;
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] media: staging: rkisp1: params: remove unnecessary "!!"
2020-10-19 20:59 ` [PATCH 2/4] media: staging: rkisp1: params: remove unnecessary "!!" Dafna Hirschfeld
@ 2020-10-20 17:13 ` Helen Koike
0 siblings, 0 replies; 10+ messages in thread
From: Helen Koike @ 2020-10-20 17:13 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media
Cc: laurent.pinchart, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga
On 10/19/20 5:59 PM, Dafna Hirschfeld wrote:
> There are several 'if' conditions of the form:
>
> if (!!(module_ens & SOME_FLAG))
>
> Those can be replaced with:
>
> if (module_ens & SOME_FLAG)
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Thanks
Helen
> ---
> drivers/staging/media/rkisp1/rkisp1-params.c | 24 ++++++++++----------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
> index 986d293201e6..4ac401bc2164 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-params.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-params.c
> @@ -896,7 +896,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> &new_params->others.dpcc_config);
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_DPCC))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_DPCC)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_ISP_DPCC_MODE,
> RKISP1_CIF_ISP_DPCC_ENA);
> @@ -915,7 +915,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> &new_params->others.bls_config);
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_BLS) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_BLS))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_BLS)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_ISP_BLS_CTRL,
> RKISP1_CIF_ISP_BLS_ENA);
> @@ -934,7 +934,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> &new_params->others.sdg_config);
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_SDG) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_SDG))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_SDG)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_ISP_CTRL,
> RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
> @@ -953,7 +953,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> &new_params->others.lsc_config);
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_LSC))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_LSC)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_ISP_LSC_CTRL,
> RKISP1_CIF_ISP_LSC_CTRL_ENA);
> @@ -972,7 +972,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> &new_params->others.awb_gain_config);
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_AWB_GAIN))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_ISP_CTRL,
> RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
> @@ -991,7 +991,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> &new_params->others.bdm_config);
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_BDM) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_BDM))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_BDM)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_ISP_DEMOSAIC,
> RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> @@ -1010,7 +1010,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> &new_params->others.flt_config);
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_FLT) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_FLT))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_FLT)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_ISP_FILT_MODE,
> RKISP1_CIF_ISP_FLT_ENA);
> @@ -1041,7 +1041,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> &new_params->others.goc_config);
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_GOC) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_GOC))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_GOC)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_ISP_CTRL,
> RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
> @@ -1061,7 +1061,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> }
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_CPROC))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_CPROC)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_C_PROC_CTRL,
> RKISP1_CIF_C_PROC_CTR_ENABLE);
> @@ -1092,7 +1092,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> &new_params->others.dpf_config);
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_DPF) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_DPF))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_DPF)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_ISP_DPF_MODE,
> RKISP1_CIF_ISP_DPF_MODE_EN);
> @@ -1141,7 +1141,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
> &new_params->meas.afc_config);
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_AFC) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_AFC))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_AFC)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_ISP_AFM_CTRL,
> RKISP1_CIF_ISP_AFM_ENA);
> @@ -1173,7 +1173,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
> &new_params->meas.aec_config);
>
> if (module_en_update & RKISP1_CIF_ISP_MODULE_AEC) {
> - if (!!(module_ens & RKISP1_CIF_ISP_MODULE_AEC))
> + if (module_ens & RKISP1_CIF_ISP_MODULE_AEC)
> rkisp1_param_set_bits(params,
> RKISP1_CIF_ISP_EXP_CTRL,
> RKISP1_CIF_ISP_EXP_ENA);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] media: staging: rkisp1: params: remove unnecessary parentheses
2020-10-19 20:59 ` [PATCH 3/4] media: staging: rkisp1: params: remove unnecessary parentheses Dafna Hirschfeld
@ 2020-10-20 17:13 ` Helen Koike
0 siblings, 0 replies; 10+ messages in thread
From: Helen Koike @ 2020-10-20 17:13 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media
Cc: laurent.pinchart, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga
On 10/19/20 5:59 PM, Dafna Hirschfeld wrote:
> There are several 'if' expression where double
> parentheses is used when one is enough.
> Remove the extra parentheses.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Thanks
Helen
> ---
> drivers/staging/media/rkisp1/rkisp1-params.c | 32 ++++++++++----------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
> index 4ac401bc2164..a2df6b50c109 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-params.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-params.c
> @@ -891,7 +891,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)) {
> /*update dpc config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)
> rkisp1_dpcc_config(params,
> &new_params->others.dpcc_config);
>
> @@ -910,7 +910,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_BLS) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)) {
> /* update bls config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)
> rkisp1_bls_config(params,
> &new_params->others.bls_config);
>
> @@ -929,7 +929,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_SDG) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)) {
> /* update sdg config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)
> rkisp1_sdg_config(params,
> &new_params->others.sdg_config);
>
> @@ -948,7 +948,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_LSC) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)) {
> /* update lsc config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)
> rkisp1_lsc_config(params,
> &new_params->others.lsc_config);
>
> @@ -967,7 +967,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)) {
> /* update awb gains */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
> rkisp1_awb_gain_config(params,
> &new_params->others.awb_gain_config);
>
> @@ -986,7 +986,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_BDM) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)) {
> /* update bdm config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)
> rkisp1_bdm_config(params,
> &new_params->others.bdm_config);
>
> @@ -1005,7 +1005,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_FLT) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)) {
> /* update filter config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)
> rkisp1_flt_config(params,
> &new_params->others.flt_config);
>
> @@ -1024,7 +1024,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_CTK) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)) {
> /* update ctk config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)
> rkisp1_ctk_config(params,
> &new_params->others.ctk_config);
>
> @@ -1036,7 +1036,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_GOC) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)) {
> /* update goc config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)
> rkisp1_goc_config(params,
> &new_params->others.goc_config);
>
> @@ -1055,7 +1055,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC)) {
> /* update cproc config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC)) {
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC) {
> rkisp1_cproc_config(params,
> &new_params->others.cproc_config);
> }
> @@ -1075,7 +1075,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_IE) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)) {
> /* update ie config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_IE))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)
> rkisp1_ie_config(params,
> &new_params->others.ie_config);
>
> @@ -1087,7 +1087,7 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPF) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)) {
> /* update dpf config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)
> rkisp1_dpf_config(params,
> &new_params->others.dpf_config);
>
> @@ -1123,7 +1123,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_AWB) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)) {
> /* update awb config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)
> rkisp1_awb_meas_config(params,
> &new_params->meas.awb_meas_config);
>
> @@ -1136,7 +1136,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_AFC) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)) {
> /* update afc config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)
> rkisp1_afm_config(params,
> &new_params->meas.afc_config);
>
> @@ -1155,7 +1155,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_HST) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)) {
> /* update hst config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_HST))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)
> rkisp1_hst_config(params,
> &new_params->meas.hst_config);
>
> @@ -1168,7 +1168,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_AEC) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)) {
> /* update aec config */
> - if ((module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC))
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)
> rkisp1_aec_config(params,
> &new_params->meas.aec_config);
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] media: staging: rkisp1: params: remove extra 'if' conditions
2020-10-19 20:59 ` [PATCH 4/4] media: staging: rkisp1: params: remove extra 'if' conditions Dafna Hirschfeld
@ 2020-10-20 17:13 ` Helen Koike
2020-10-20 17:21 ` Dafna Hirschfeld
0 siblings, 1 reply; 10+ messages in thread
From: Helen Koike @ 2020-10-20 17:13 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media
Cc: laurent.pinchart, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga
Hi Dafna,
On 10/19/20 5:59 PM, Dafna Hirschfeld wrote:
> There is a repeating code pattern:
>
> if (a || b) {
> if (a)
> ...
> if (b)
> ...
> }
>
> In this pattern, the first 'if' is redundant.
> The code can be replaced with:
>
> if (a)
> ...
> if (b)
> ...
>
> In addition, the patch corrects alignment issues
> reported by checkpatch.
> This solves the TODO item
> "Fix checkpatch errors."
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> drivers/staging/media/rkisp1/TODO | 1 -
> drivers/staging/media/rkisp1/rkisp1-params.c | 420 ++++++++-----------
> 2 files changed, 175 insertions(+), 246 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/TODO b/drivers/staging/media/rkisp1/TODO
> index e7c8398fc2ce..647ab482d032 100644
> --- a/drivers/staging/media/rkisp1/TODO
> +++ b/drivers/staging/media/rkisp1/TODO
> @@ -1,5 +1,4 @@
> * Fix pad format size for statistics and parameters entities.
> -* Fix checkpatch errors.
> * Add uapi docs. Remember to add documentation of how quantization is handled.
> * streaming paths (mainpath and selfpath) check if the other path is streaming
> in several places of the code, review this, specially that it doesn't seem it
> diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
> index a2df6b50c109..c8585b6ee918 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-params.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-params.c
> @@ -186,7 +186,7 @@ static void rkisp1_bls_config(struct rkisp1_params *params,
> /* ISP LS correction interface function */
> static void
> rkisp1_lsc_correct_matrix_config(struct rkisp1_params *params,
> - const struct rkisp1_cif_isp_lsc_config *pconfig)
> + const struct rkisp1_cif_isp_lsc_config *pconfig)
I think this should be part of the alignment patch.
> {
> unsigned int isp_lsc_status, sram_addr, isp_lsc_table_sel, i, j, data;
>
> @@ -434,7 +434,7 @@ static void rkisp1_ctk_enable(struct rkisp1_params *params, bool en)
>
> /* ISP White Balance Mode */
> static void rkisp1_awb_meas_config(struct rkisp1_params *params,
> - const struct rkisp1_cif_isp_awb_meas_config *arg)
> + const struct rkisp1_cif_isp_awb_meas_config *arg)
This too.
> {
> u32 reg_val = 0;
> /* based on the mode,configure the awb module */
> @@ -888,226 +888,171 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> module_cfg_update = new_params->module_cfg_update;
> module_ens = new_params->module_ens;
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)) {
> - /*update dpc config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)
> - rkisp1_dpcc_config(params,
> - &new_params->others.dpcc_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_DPCC)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_ISP_DPCC_MODE,
> - RKISP1_CIF_ISP_DPCC_ENA);
> - else
> - rkisp1_param_clear_bits(params,
> + /*update dpc config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)
> + rkisp1_dpcc_config(params,
> + &new_params->others.dpcc_config);
> +
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_DPCC)
> + rkisp1_param_set_bits(params,
> + RKISP1_CIF_ISP_DPCC_MODE,
> + RKISP1_CIF_ISP_DPCC_ENA);
> + else
> + rkisp1_param_clear_bits(params,
> RKISP1_CIF_ISP_DPCC_MODE,
> RKISP1_CIF_ISP_DPCC_ENA);
> - }
> }
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_BLS) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)) {
> - /* update bls config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)
> - rkisp1_bls_config(params,
> - &new_params->others.bls_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_BLS) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_BLS)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_ISP_BLS_CTRL,
> - RKISP1_CIF_ISP_BLS_ENA);
> - else
> - rkisp1_param_clear_bits(params,
> - RKISP1_CIF_ISP_BLS_CTRL,
> - RKISP1_CIF_ISP_BLS_ENA);
> - }
> + /* update bls config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)
> + rkisp1_bls_config(params,
> + &new_params->others.bls_config);
> +
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_BLS) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_BLS)
> + rkisp1_param_set_bits(params,
> + RKISP1_CIF_ISP_BLS_CTRL,
> + RKISP1_CIF_ISP_BLS_ENA);
> + else
> + rkisp1_param_clear_bits(params,
> + RKISP1_CIF_ISP_BLS_CTRL,
> + RKISP1_CIF_ISP_BLS_ENA);
> }
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_SDG) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)) {
> - /* update sdg config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)
> - rkisp1_sdg_config(params,
> - &new_params->others.sdg_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_SDG) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_SDG)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_ISP_CTRL,
> - RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
> - else
> - rkisp1_param_clear_bits(params,
> - RKISP1_CIF_ISP_CTRL,
> - RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
> - }
> + /* update sdg config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)
> + rkisp1_sdg_config(params,
> + &new_params->others.sdg_config);
> +
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_SDG) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_SDG)
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
> + else
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
> }
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_LSC) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)) {
> - /* update lsc config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)
> - rkisp1_lsc_config(params,
> - &new_params->others.lsc_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_LSC)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_ISP_LSC_CTRL,
> - RKISP1_CIF_ISP_LSC_CTRL_ENA);
> - else
> - rkisp1_param_clear_bits(params,
> - RKISP1_CIF_ISP_LSC_CTRL,
> + /* update lsc config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)
> + rkisp1_lsc_config(params,
> + &new_params->others.lsc_config);
> +
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_LSC)
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
> + RKISP1_CIF_ISP_LSC_CTRL_ENA);
> + else
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
> RKISP1_CIF_ISP_LSC_CTRL_ENA);
> - }
> }
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)) {
> - /* update awb gains */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
> - rkisp1_awb_gain_config(params,
> - &new_params->others.awb_gain_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_ISP_CTRL,
> - RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
> - else
> - rkisp1_param_clear_bits(params,
> - RKISP1_CIF_ISP_CTRL,
> - RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
> - }
> + /* update awb gains */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
> + rkisp1_awb_gain_config(params, &new_params->others.awb_gain_config);
> +
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
> + else
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
> }
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_BDM) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)) {
> - /* update bdm config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)
> - rkisp1_bdm_config(params,
> - &new_params->others.bdm_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_BDM) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_BDM)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_ISP_DEMOSAIC,
> - RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> - else
> - rkisp1_param_clear_bits(params,
> - RKISP1_CIF_ISP_DEMOSAIC,
> + /* update bdm config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)
> + rkisp1_bdm_config(params,
> + &new_params->others.bdm_config);
> +
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_BDM) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_BDM)
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> + RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> + else
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> - }
> }
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_FLT) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)) {
> - /* update filter config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)
> - rkisp1_flt_config(params,
> - &new_params->others.flt_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_FLT) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_FLT)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_ISP_FILT_MODE,
> - RKISP1_CIF_ISP_FLT_ENA);
> - else
> - rkisp1_param_clear_bits(params,
> - RKISP1_CIF_ISP_FILT_MODE,
> + /* update filter config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)
> + rkisp1_flt_config(params,
> + &new_params->others.flt_config);
> +
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_FLT) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_FLT)
> + rkisp1_param_set_bits(params,
> + RKISP1_CIF_ISP_FILT_MODE,
> + RKISP1_CIF_ISP_FLT_ENA);
> + else
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_FILT_MODE,
> RKISP1_CIF_ISP_FLT_ENA);
> - }
> }
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_CTK) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)) {
> - /* update ctk config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)
> - rkisp1_ctk_config(params,
> - &new_params->others.ctk_config);
> + /* update ctk config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)
> + rkisp1_ctk_config(params,
> + &new_params->others.ctk_config);
>
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_CTK)
> - rkisp1_ctk_enable(params,
> - !!(module_ens & RKISP1_CIF_ISP_MODULE_CTK));
> - }
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_CTK)
> + rkisp1_ctk_enable(params, !!(module_ens & RKISP1_CIF_ISP_MODULE_CTK));
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_GOC) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)) {
> - /* update goc config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)
> - rkisp1_goc_config(params,
> - &new_params->others.goc_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_GOC) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_GOC)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_ISP_CTRL,
> - RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
> - else
> - rkisp1_param_clear_bits(params,
> - RKISP1_CIF_ISP_CTRL,
> - RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
> - }
> + /* update goc config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)
> + rkisp1_goc_config(params,
> + &new_params->others.goc_config);
> +
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_GOC) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_GOC)
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
> + else
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
> }
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC)) {
> - /* update cproc config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC) {
> - rkisp1_cproc_config(params,
> - &new_params->others.cproc_config);
> - }
> + /* update cproc config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC) {
> + rkisp1_cproc_config(params,
> + &new_params->others.cproc_config);
> + }
No need for the curly braces.
>
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_CPROC)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_C_PROC_CTRL,
> - RKISP1_CIF_C_PROC_CTR_ENABLE);
> - else
> - rkisp1_param_clear_bits(params,
> - RKISP1_CIF_C_PROC_CTRL,
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_CPROC)
> + rkisp1_param_set_bits(params, RKISP1_CIF_C_PROC_CTRL,
> + RKISP1_CIF_C_PROC_CTR_ENABLE);
> + else
> + rkisp1_param_clear_bits(params, RKISP1_CIF_C_PROC_CTRL,
> RKISP1_CIF_C_PROC_CTR_ENABLE);
> - }
> }
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_IE) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)) {
> - /* update ie config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)
> - rkisp1_ie_config(params,
> - &new_params->others.ie_config);
> + /* update ie config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)
> + rkisp1_ie_config(params,
> + &new_params->others.ie_config);
>
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_IE)
> - rkisp1_ie_enable(params,
> - !!(module_ens & RKISP1_CIF_ISP_MODULE_IE));
> - }
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_IE)
> + rkisp1_ie_enable(params, !!(module_ens & RKISP1_CIF_ISP_MODULE_IE));
> +
> + /* update dpf config */
Could you remove this extra space between "dpf" and "config" ?
I'm glad to see a level of identation gone.
With these changes
Acked-by: Helen Koike <helen.koike@collabora.com>
Thanks
Helen
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)
> + rkisp1_dpf_config(params,
> + &new_params->others.dpf_config);
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPF) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)) {
> - /* update dpf config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)
> - rkisp1_dpf_config(params,
> - &new_params->others.dpf_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_DPF) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_DPF)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_ISP_DPF_MODE,
> - RKISP1_CIF_ISP_DPF_MODE_EN);
> - else
> - rkisp1_param_clear_bits(params,
> - RKISP1_CIF_ISP_DPF_MODE,
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_DPF) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_DPF)
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DPF_MODE,
> + RKISP1_CIF_ISP_DPF_MODE_EN);
> + else
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPF_MODE,
> RKISP1_CIF_ISP_DPF_MODE_EN);
> - }
> }
>
> if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPF_STRENGTH) ||
> (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF_STRENGTH)) {
> /* update dpf strength config */
> - rkisp1_dpf_strength_config(params,
> - &new_params->others.dpf_strength_config);
> + rkisp1_dpf_strength_config(params, &new_params->others.dpf_strength_config);
> }
> }
>
> @@ -1120,68 +1065,53 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
> module_cfg_update = new_params->module_cfg_update;
> module_ens = new_params->module_ens;
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_AWB) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)) {
> - /* update awb config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)
> - rkisp1_awb_meas_config(params,
> - &new_params->meas.awb_meas_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB)
> - rkisp1_awb_meas_enable(params,
> - &new_params->meas.awb_meas_config,
> - !!(module_ens & RKISP1_CIF_ISP_MODULE_AWB));
> - }
> + /* update awb config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)
> + rkisp1_awb_meas_config(params, &new_params->meas.awb_meas_config);
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_AFC) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)) {
> - /* update afc config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)
> - rkisp1_afm_config(params,
> - &new_params->meas.afc_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_AFC) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_AFC)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_ISP_AFM_CTRL,
> - RKISP1_CIF_ISP_AFM_ENA);
> - else
> - rkisp1_param_clear_bits(params,
> - RKISP1_CIF_ISP_AFM_CTRL,
> - RKISP1_CIF_ISP_AFM_ENA);
> - }
> - }
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB)
> + rkisp1_awb_meas_enable(params, &new_params->meas.awb_meas_config,
> + !!(module_ens & RKISP1_CIF_ISP_MODULE_AWB));
> +
> + /* update afc config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)
> + rkisp1_afm_config(params,
> + &new_params->meas.afc_config);
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_HST) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)) {
> - /* update hst config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)
> - rkisp1_hst_config(params,
> - &new_params->meas.hst_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_HST)
> - rkisp1_hst_enable(params,
> - &new_params->meas.hst_config,
> - !!(module_ens & RKISP1_CIF_ISP_MODULE_HST));
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_AFC) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_AFC)
> + rkisp1_param_set_bits(params,
> + RKISP1_CIF_ISP_AFM_CTRL,
> + RKISP1_CIF_ISP_AFM_ENA);
> + else
> + rkisp1_param_clear_bits(params,
> + RKISP1_CIF_ISP_AFM_CTRL,
> + RKISP1_CIF_ISP_AFM_ENA);
> }
>
> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_AEC) ||
> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)) {
> - /* update aec config */
> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)
> - rkisp1_aec_config(params,
> - &new_params->meas.aec_config);
> -
> - if (module_en_update & RKISP1_CIF_ISP_MODULE_AEC) {
> - if (module_ens & RKISP1_CIF_ISP_MODULE_AEC)
> - rkisp1_param_set_bits(params,
> - RKISP1_CIF_ISP_EXP_CTRL,
> - RKISP1_CIF_ISP_EXP_ENA);
> - else
> - rkisp1_param_clear_bits(params,
> - RKISP1_CIF_ISP_EXP_CTRL,
> - RKISP1_CIF_ISP_EXP_ENA);
> - }
> + /* update hst config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)
> + rkisp1_hst_config(params,
> + &new_params->meas.hst_config);
> +
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_HST)
> + rkisp1_hst_enable(params, &new_params->meas.hst_config,
> + !!(module_ens & RKISP1_CIF_ISP_MODULE_HST));
> +
> + /* update aec config */
> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)
> + rkisp1_aec_config(params,
> + &new_params->meas.aec_config);
> +
> + if (module_en_update & RKISP1_CIF_ISP_MODULE_AEC) {
> + if (module_ens & RKISP1_CIF_ISP_MODULE_AEC)
> + rkisp1_param_set_bits(params,
> + RKISP1_CIF_ISP_EXP_CTRL,
> + RKISP1_CIF_ISP_EXP_ENA);
> + else
> + rkisp1_param_clear_bits(params,
> + RKISP1_CIF_ISP_EXP_CTRL,
> + RKISP1_CIF_ISP_EXP_ENA);
> }
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] media: staging: rkisp1: params: remove extra 'if' conditions
2020-10-20 17:13 ` Helen Koike
@ 2020-10-20 17:21 ` Dafna Hirschfeld
0 siblings, 0 replies; 10+ messages in thread
From: Dafna Hirschfeld @ 2020-10-20 17:21 UTC (permalink / raw)
To: Helen Koike, linux-media
Cc: laurent.pinchart, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga
Hi,
Am 20.10.20 um 19:13 schrieb Helen Koike:
> Hi Dafna,
>
> On 10/19/20 5:59 PM, Dafna Hirschfeld wrote:
>> There is a repeating code pattern:
>>
>> if (a || b) {
>> if (a)
>> ...
>> if (b)
>> ...
>> }
>>
>> In this pattern, the first 'if' is redundant.
>> The code can be replaced with:
>>
>> if (a)
>> ...
>> if (b)
>> ...
>>
>> In addition, the patch corrects alignment issues
>> reported by checkpatch.
>> This solves the TODO item
>> "Fix checkpatch errors."
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> ---
>> drivers/staging/media/rkisp1/TODO | 1 -
>> drivers/staging/media/rkisp1/rkisp1-params.c | 420 ++++++++-----------
>> 2 files changed, 175 insertions(+), 246 deletions(-)
>>
>> diff --git a/drivers/staging/media/rkisp1/TODO b/drivers/staging/media/rkisp1/TODO
>> index e7c8398fc2ce..647ab482d032 100644
>> --- a/drivers/staging/media/rkisp1/TODO
>> +++ b/drivers/staging/media/rkisp1/TODO
>> @@ -1,5 +1,4 @@
>> * Fix pad format size for statistics and parameters entities.
>> -* Fix checkpatch errors.
>> * Add uapi docs. Remember to add documentation of how quantization is handled.
>> * streaming paths (mainpath and selfpath) check if the other path is streaming
>> in several places of the code, review this, specially that it doesn't seem it
>> diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
>> index a2df6b50c109..c8585b6ee918 100644
>> --- a/drivers/staging/media/rkisp1/rkisp1-params.c
>> +++ b/drivers/staging/media/rkisp1/rkisp1-params.c
>> @@ -186,7 +186,7 @@ static void rkisp1_bls_config(struct rkisp1_params *params,
>> /* ISP LS correction interface function */
>> static void
>> rkisp1_lsc_correct_matrix_config(struct rkisp1_params *params,
>> - const struct rkisp1_cif_isp_lsc_config *pconfig)
>> + const struct rkisp1_cif_isp_lsc_config *pconfig)
>
> I think this should be part of the alignment patch.
>
>> {
>> unsigned int isp_lsc_status, sram_addr, isp_lsc_table_sel, i, j, data;
>>
>> @@ -434,7 +434,7 @@ static void rkisp1_ctk_enable(struct rkisp1_params *params, bool en)
>>
>> /* ISP White Balance Mode */
>> static void rkisp1_awb_meas_config(struct rkisp1_params *params,
>> - const struct rkisp1_cif_isp_awb_meas_config *arg)
>> + const struct rkisp1_cif_isp_awb_meas_config *arg)
>
> This too.
>
>> {
>> u32 reg_val = 0;
>> /* based on the mode,configure the awb module */
>> @@ -888,226 +888,171 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
>> module_cfg_update = new_params->module_cfg_update;
>> module_ens = new_params->module_ens;
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)) {
>> - /*update dpc config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)
>> - rkisp1_dpcc_config(params,
>> - &new_params->others.dpcc_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_DPCC)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_ISP_DPCC_MODE,
>> - RKISP1_CIF_ISP_DPCC_ENA);
>> - else
>> - rkisp1_param_clear_bits(params,
>> + /*update dpc config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)
>> + rkisp1_dpcc_config(params,
>> + &new_params->others.dpcc_config);
>> +
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_DPCC) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_DPCC)
>> + rkisp1_param_set_bits(params,
>> + RKISP1_CIF_ISP_DPCC_MODE,
>> + RKISP1_CIF_ISP_DPCC_ENA);
>> + else
>> + rkisp1_param_clear_bits(params,
>> RKISP1_CIF_ISP_DPCC_MODE,
>> RKISP1_CIF_ISP_DPCC_ENA);
>> - }
>> }
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_BLS) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)) {
>> - /* update bls config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)
>> - rkisp1_bls_config(params,
>> - &new_params->others.bls_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_BLS) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_BLS)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_ISP_BLS_CTRL,
>> - RKISP1_CIF_ISP_BLS_ENA);
>> - else
>> - rkisp1_param_clear_bits(params,
>> - RKISP1_CIF_ISP_BLS_CTRL,
>> - RKISP1_CIF_ISP_BLS_ENA);
>> - }
>> + /* update bls config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)
>> + rkisp1_bls_config(params,
>> + &new_params->others.bls_config);
>> +
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_BLS) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_BLS)
>> + rkisp1_param_set_bits(params,
>> + RKISP1_CIF_ISP_BLS_CTRL,
>> + RKISP1_CIF_ISP_BLS_ENA);
>> + else
>> + rkisp1_param_clear_bits(params,
>> + RKISP1_CIF_ISP_BLS_CTRL,
>> + RKISP1_CIF_ISP_BLS_ENA);
>> }
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_SDG) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)) {
>> - /* update sdg config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)
>> - rkisp1_sdg_config(params,
>> - &new_params->others.sdg_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_SDG) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_SDG)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_ISP_CTRL,
>> - RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
>> - else
>> - rkisp1_param_clear_bits(params,
>> - RKISP1_CIF_ISP_CTRL,
>> - RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
>> - }
>> + /* update sdg config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_SDG)
>> + rkisp1_sdg_config(params,
>> + &new_params->others.sdg_config);
>> +
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_SDG) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_SDG)
>> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
>> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
>> + else
>> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
>> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
>> }
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_LSC) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)) {
>> - /* update lsc config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)
>> - rkisp1_lsc_config(params,
>> - &new_params->others.lsc_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_LSC)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_ISP_LSC_CTRL,
>> - RKISP1_CIF_ISP_LSC_CTRL_ENA);
>> - else
>> - rkisp1_param_clear_bits(params,
>> - RKISP1_CIF_ISP_LSC_CTRL,
>> + /* update lsc config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC)
>> + rkisp1_lsc_config(params,
>> + &new_params->others.lsc_config);
>> +
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_LSC)
>> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
>> + RKISP1_CIF_ISP_LSC_CTRL_ENA);
>> + else
>> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
>> RKISP1_CIF_ISP_LSC_CTRL_ENA);
>> - }
>> }
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)) {
>> - /* update awb gains */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
>> - rkisp1_awb_gain_config(params,
>> - &new_params->others.awb_gain_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_ISP_CTRL,
>> - RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
>> - else
>> - rkisp1_param_clear_bits(params,
>> - RKISP1_CIF_ISP_CTRL,
>> - RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
>> - }
>> + /* update awb gains */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
>> + rkisp1_awb_gain_config(params, &new_params->others.awb_gain_config);
>> +
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_AWB_GAIN)
>> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
>> + RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
>> + else
>> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
>> + RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
>> }
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_BDM) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)) {
>> - /* update bdm config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)
>> - rkisp1_bdm_config(params,
>> - &new_params->others.bdm_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_BDM) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_BDM)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_ISP_DEMOSAIC,
>> - RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
>> - else
>> - rkisp1_param_clear_bits(params,
>> - RKISP1_CIF_ISP_DEMOSAIC,
>> + /* update bdm config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BDM)
>> + rkisp1_bdm_config(params,
>> + &new_params->others.bdm_config);
>> +
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_BDM) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_BDM)
>> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
>> + RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
>> + else
>> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
>> RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
>> - }
>> }
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_FLT) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)) {
>> - /* update filter config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)
>> - rkisp1_flt_config(params,
>> - &new_params->others.flt_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_FLT) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_FLT)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_ISP_FILT_MODE,
>> - RKISP1_CIF_ISP_FLT_ENA);
>> - else
>> - rkisp1_param_clear_bits(params,
>> - RKISP1_CIF_ISP_FILT_MODE,
>> + /* update filter config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_FLT)
>> + rkisp1_flt_config(params,
>> + &new_params->others.flt_config);
>> +
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_FLT) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_FLT)
>> + rkisp1_param_set_bits(params,
>> + RKISP1_CIF_ISP_FILT_MODE,
>> + RKISP1_CIF_ISP_FLT_ENA);
>> + else
>> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_FILT_MODE,
>> RKISP1_CIF_ISP_FLT_ENA);
>> - }
>> }
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_CTK) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)) {
>> - /* update ctk config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)
>> - rkisp1_ctk_config(params,
>> - &new_params->others.ctk_config);
>> + /* update ctk config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CTK)
>> + rkisp1_ctk_config(params,
>> + &new_params->others.ctk_config);
>>
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_CTK)
>> - rkisp1_ctk_enable(params,
>> - !!(module_ens & RKISP1_CIF_ISP_MODULE_CTK));
>> - }
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_CTK)
>> + rkisp1_ctk_enable(params, !!(module_ens & RKISP1_CIF_ISP_MODULE_CTK));
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_GOC) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)) {
>> - /* update goc config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)
>> - rkisp1_goc_config(params,
>> - &new_params->others.goc_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_GOC) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_GOC)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_ISP_CTRL,
>> - RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
>> - else
>> - rkisp1_param_clear_bits(params,
>> - RKISP1_CIF_ISP_CTRL,
>> - RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
>> - }
>> + /* update goc config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_GOC)
>> + rkisp1_goc_config(params,
>> + &new_params->others.goc_config);
>> +
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_GOC) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_GOC)
>> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
>> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
>> + else
>> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
>> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
>> }
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC)) {
>> - /* update cproc config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC) {
>> - rkisp1_cproc_config(params,
>> - &new_params->others.cproc_config);
>> - }
>> + /* update cproc config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_CPROC) {
>> + rkisp1_cproc_config(params,
>> + &new_params->others.cproc_config);
>> + }
>
> No need for the curly braces.
>
>>
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_CPROC)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_C_PROC_CTRL,
>> - RKISP1_CIF_C_PROC_CTR_ENABLE);
>> - else
>> - rkisp1_param_clear_bits(params,
>> - RKISP1_CIF_C_PROC_CTRL,
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_CPROC) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_CPROC)
>> + rkisp1_param_set_bits(params, RKISP1_CIF_C_PROC_CTRL,
>> + RKISP1_CIF_C_PROC_CTR_ENABLE);
>> + else
>> + rkisp1_param_clear_bits(params, RKISP1_CIF_C_PROC_CTRL,
>> RKISP1_CIF_C_PROC_CTR_ENABLE);
>> - }
>> }
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_IE) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)) {
>> - /* update ie config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)
>> - rkisp1_ie_config(params,
>> - &new_params->others.ie_config);
>> + /* update ie config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_IE)
>> + rkisp1_ie_config(params,
>> + &new_params->others.ie_config);
>>
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_IE)
>> - rkisp1_ie_enable(params,
>> - !!(module_ens & RKISP1_CIF_ISP_MODULE_IE));
>> - }
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_IE)
>> + rkisp1_ie_enable(params, !!(module_ens & RKISP1_CIF_ISP_MODULE_IE));
>> +
>> + /* update dpf config */
>
> Could you remove this extra space between "dpf" and "config" ?
I think it is better to add your suggestions on a separate
patch so not to overload the patch.
>
>
> I'm glad to see a level of identation gone.
:-)
Thanks,
Dafna
>
> With these changes
>
> Acked-by: Helen Koike <helen.koike@collabora.com>
>
> Thanks
> Helen
>
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)
>> + rkisp1_dpf_config(params,
>> + &new_params->others.dpf_config);
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPF) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)) {
>> - /* update dpf config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF)
>> - rkisp1_dpf_config(params,
>> - &new_params->others.dpf_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_DPF) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_DPF)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_ISP_DPF_MODE,
>> - RKISP1_CIF_ISP_DPF_MODE_EN);
>> - else
>> - rkisp1_param_clear_bits(params,
>> - RKISP1_CIF_ISP_DPF_MODE,
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_DPF) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_DPF)
>> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DPF_MODE,
>> + RKISP1_CIF_ISP_DPF_MODE_EN);
>> + else
>> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPF_MODE,
>> RKISP1_CIF_ISP_DPF_MODE_EN);
>> - }
>> }
>>
>> if ((module_en_update & RKISP1_CIF_ISP_MODULE_DPF_STRENGTH) ||
>> (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPF_STRENGTH)) {
>> /* update dpf strength config */
>> - rkisp1_dpf_strength_config(params,
>> - &new_params->others.dpf_strength_config);
>> + rkisp1_dpf_strength_config(params, &new_params->others.dpf_strength_config);
>> }
>> }
>>
>> @@ -1120,68 +1065,53 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
>> module_cfg_update = new_params->module_cfg_update;
>> module_ens = new_params->module_ens;
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_AWB) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)) {
>> - /* update awb config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)
>> - rkisp1_awb_meas_config(params,
>> - &new_params->meas.awb_meas_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB)
>> - rkisp1_awb_meas_enable(params,
>> - &new_params->meas.awb_meas_config,
>> - !!(module_ens & RKISP1_CIF_ISP_MODULE_AWB));
>> - }
>> + /* update awb config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB)
>> + rkisp1_awb_meas_config(params, &new_params->meas.awb_meas_config);
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_AFC) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)) {
>> - /* update afc config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)
>> - rkisp1_afm_config(params,
>> - &new_params->meas.afc_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_AFC) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_AFC)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_ISP_AFM_CTRL,
>> - RKISP1_CIF_ISP_AFM_ENA);
>> - else
>> - rkisp1_param_clear_bits(params,
>> - RKISP1_CIF_ISP_AFM_CTRL,
>> - RKISP1_CIF_ISP_AFM_ENA);
>> - }
>> - }
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_AWB)
>> + rkisp1_awb_meas_enable(params, &new_params->meas.awb_meas_config,
>> + !!(module_ens & RKISP1_CIF_ISP_MODULE_AWB));
>> +
>> + /* update afc config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AFC)
>> + rkisp1_afm_config(params,
>> + &new_params->meas.afc_config);
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_HST) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)) {
>> - /* update hst config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)
>> - rkisp1_hst_config(params,
>> - &new_params->meas.hst_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_HST)
>> - rkisp1_hst_enable(params,
>> - &new_params->meas.hst_config,
>> - !!(module_ens & RKISP1_CIF_ISP_MODULE_HST));
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_AFC) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_AFC)
>> + rkisp1_param_set_bits(params,
>> + RKISP1_CIF_ISP_AFM_CTRL,
>> + RKISP1_CIF_ISP_AFM_ENA);
>> + else
>> + rkisp1_param_clear_bits(params,
>> + RKISP1_CIF_ISP_AFM_CTRL,
>> + RKISP1_CIF_ISP_AFM_ENA);
>> }
>>
>> - if ((module_en_update & RKISP1_CIF_ISP_MODULE_AEC) ||
>> - (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)) {
>> - /* update aec config */
>> - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)
>> - rkisp1_aec_config(params,
>> - &new_params->meas.aec_config);
>> -
>> - if (module_en_update & RKISP1_CIF_ISP_MODULE_AEC) {
>> - if (module_ens & RKISP1_CIF_ISP_MODULE_AEC)
>> - rkisp1_param_set_bits(params,
>> - RKISP1_CIF_ISP_EXP_CTRL,
>> - RKISP1_CIF_ISP_EXP_ENA);
>> - else
>> - rkisp1_param_clear_bits(params,
>> - RKISP1_CIF_ISP_EXP_CTRL,
>> - RKISP1_CIF_ISP_EXP_ENA);
>> - }
>> + /* update hst config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_HST)
>> + rkisp1_hst_config(params,
>> + &new_params->meas.hst_config);
>> +
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_HST)
>> + rkisp1_hst_enable(params, &new_params->meas.hst_config,
>> + !!(module_ens & RKISP1_CIF_ISP_MODULE_HST));
>> +
>> + /* update aec config */
>> + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AEC)
>> + rkisp1_aec_config(params,
>> + &new_params->meas.aec_config);
>> +
>> + if (module_en_update & RKISP1_CIF_ISP_MODULE_AEC) {
>> + if (module_ens & RKISP1_CIF_ISP_MODULE_AEC)
>> + rkisp1_param_set_bits(params,
>> + RKISP1_CIF_ISP_EXP_CTRL,
>> + RKISP1_CIF_ISP_EXP_ENA);
>> + else
>> + rkisp1_param_clear_bits(params,
>> + RKISP1_CIF_ISP_EXP_CTRL,
>> + RKISP1_CIF_ISP_EXP_ENA);
>> }
>> }
>>
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-10-20 17:21 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-19 20:59 [PATCH 0/4] media: staging: rkisp1: send cleanups and checkpatch fixes Dafna Hirschfeld
2020-10-19 20:59 ` [PATCH 1/4] media: staging: rkisp1: fix coding style issues Dafna Hirschfeld
2020-10-20 17:13 ` Helen Koike
2020-10-19 20:59 ` [PATCH 2/4] media: staging: rkisp1: params: remove unnecessary "!!" Dafna Hirschfeld
2020-10-20 17:13 ` Helen Koike
2020-10-19 20:59 ` [PATCH 3/4] media: staging: rkisp1: params: remove unnecessary parentheses Dafna Hirschfeld
2020-10-20 17:13 ` Helen Koike
2020-10-19 20:59 ` [PATCH 4/4] media: staging: rkisp1: params: remove extra 'if' conditions Dafna Hirschfeld
2020-10-20 17:13 ` Helen Koike
2020-10-20 17:21 ` Dafna Hirschfeld
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).