media: staging/intel-ipu3: css - possible typo in array being assigned

Hi,

I believe there maybe a typo in the name of an array being assigned, introduced in commit:

commit e11110a5b74487cfc99dad8a5744fe26782f5d9c

Author: Yong Zhi <[email protected]>
Date:   Thu Dec 6 20:03:33 2018 -0500

media: staging/intel-ipu3: css: Compute and program ccs

The issue is as follows in function imgu_css_acc_process_lines in drivers/staging/media/ipu3/ipu3-css-params.c

                        if (pl_idx == 0)
                                /* First process line */
                                p_pl[pl_idx].lines = first_process_lines;
                        else if (pl_idx == num_of_sets - 1)
                                /* Last in grid */
                                p_pl[pl_idx].lines = 
last_process_lines_in_grid;
                        else if (pl_idx == num_of_process_lines - 1)
                                /* After the grid */
                                p_pl[pl_idx].lines = 
process_lines_after_grid;
                        else
                                /* Inside the grid */
                                p_pl[pl_idx].lines = process_lines;

                        if (p_tr) {
                                p_pl[pl_idx].cfg_set = pl_cfg_set;
                                pl_cfg_set = 1 - pl_cfg_set;
                        }
                        pl_idx++;

The non-null check 'if (p_tr)' seem suspect, should it be 'if (p_pl)' instead since p_pl is being assigned (this maybe a cut-n-paste issue from a previous hunk of code that does:

                        op_idx++;
                        if (p_tr) {
                                p_tr[tr_idx].set_number = tr_set_num;
                                tr_set_num = 1 - tr_set_num;
                        }
                        tr_idx++;

Also, if it is meant to be a check on p_pl then surely the assignments to p_pl[] also need a non-null check in the cascaded if/else checks too.

Colin

Yeah... It probably should be the below patch. The commit message would say something like:

Subject: Fix copy and paste bug.

This code is copy and pasted from earlier in the function and the NULL check should be if (p_pl) instead of if (p_tr). Except that p_pl is never NULL and the check can be removed.

The impact of this bug is that for several callers the "p_pl[pl_idx].cfg_set = pl_cfg_set" value is never set. (I wonder why this didn't show up in testing?)"

regards,
dan carpenter

index 2c48d57a3180..2d1c38119258 100644
@@ -1617,10 +1617,9 @@ imgu_css_acc_process_lines(const struct process_lines *pl,
 				/* Inside the grid */
 				p_pl[pl_idx].lines = process_lines;
 
-			if (p_tr) {
-				p_pl[pl_idx].cfg_set = pl_cfg_set;
-				pl_cfg_set = 1 - pl_cfg_set;
-			}
+			p_pl[pl_idx].cfg_set = pl_cfg_set;
+			pl_cfg_set = 1 - pl_cfg_set;
+
 			pl_idx++;
 		}
 	}