mbox series

[0/4] media: staging: rkisp1: send cleanups and checkpatch fixes

Message ID 20201019205956.6980-1-dafna.hirschfeld@collabora.com
Headers show
Series media: staging: rkisp1: send cleanups and checkpatch fixes | expand

Message

Dafna Hirschfeld Oct. 19, 2020, 8:59 p.m. UTC 
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(-)

Comments

Helen Koike Oct. 20, 2020, 5:13 p.m. UTC | #1 
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;

>  

>
Helen Koike Oct. 20, 2020, 5:13 p.m. UTC | #2 
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);

>  

>