Re: [Outreachy kernel] [PATCH] media: staging/intel-ipu3: css: simplify expression

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 17 Mar 2020 02:05:58 +0100
Stefano Brivio <sbrivio@redhat.com> wrote:

> On Fri, 13 Mar 2020 15:21:28 -0300
> Helen Koike <helen@koikeco.de> wrote:
> 
> > Hi Deepak,
> > 
> > On 3/13/20 3:13 PM, Deepak R Varma wrote:  
> > > An array index computed inside square brackets leading to complexity
> > > and code line exceeding 80 character. Add new variable to compute  
> 
> The first statement isn't really a statement ("leads" instead of
> "leading"?)
> 
> > > array index separately and use it as an index during assignment.
> > > 
> > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > ---
> > >  drivers/staging/media/ipu3/ipu3-css-params.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c
> > > index 4533dacad4be..4b15e767bf32 100644
> > > --- a/drivers/staging/media/ipu3/ipu3-css-params.c
> > > +++ b/drivers/staging/media/ipu3/ipu3-css-params.c
> > > @@ -50,12 +50,13 @@ imgu_css_scaler_setup_lut(unsigned int taps, unsigned int input_width,
> > >  	int exponent = imgu_css_scaler_get_exp(output_width, input_width);
> > >  	int mantissa = (1 << exponent) * output_width;
> > >  	unsigned int phase_step;
> > > +	int phase_taps = 0;    
> > 
> > No need to initialized it to zero.
> > 
> > Also, it seems phase_taps can't be negative, so you can use unsigned int
> >   
> > >  
> > >  	if (input_width == output_width) {
> > >  		for (phase = 0; phase < IMGU_SCALER_PHASES; phase++) {
> > > +			phase_taps = phase * IMGU_SCALER_FILTER_TAPS;    
> > 
> > You can also declare it here:
> >  +			unsigned int offset = phase * IMGU_SCALER_FILTER_TAPS;
> >   
> > >  			for (tap = 0; tap < taps; tap++) {  
> 
> Also, at that point, you should extend this to the loop for
> input_width != output_width below, for consistency.

Sorry, I missed v2 and v3 of this, never mind.

-- 
Stefano