Re: [PATCH v1 006/107] media: ti-vpe: cal: Merge all status variables in IRQ handler

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

On Tue, Jun 16, 2020 at 01:32:17PM +0300, Tomi Valkeinen wrote:
> On 15/06/2020 02:58, Laurent Pinchart wrote:
> > The cal_irq() function reads three IRQ status registers and stores their
> > values in three different variables. As each value is processed right
> > after reading the corresponding register, a single variable is enough.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >   drivers/media/platform/ti-vpe/cal.c | 35 +++++++++++++----------------
> >   1 file changed, 15 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> > index b04d8cb86977..0a5fb81223da 100644
> > --- a/drivers/media/platform/ti-vpe/cal.c
> > +++ b/drivers/media/platform/ti-vpe/cal.c
> > @@ -1206,19 +1206,18 @@ static irqreturn_t cal_irq(int irq_cal, void *data)
> >   	struct cal_dev *dev = (struct cal_dev *)data;
> >   	struct cal_ctx *ctx;
> >   	struct cal_dmaqueue *dma_q;
> > -	u32 irqst0, irqst1, irqst2;
> > +	unsigned int i;
> > +	u32 status;
> >   
> > -	irqst0 = reg_read(dev, CAL_HL_IRQSTATUS(0));
> > -	if (irqst0) {
> > -		int i;
> 
> I do like to keep local variables inside smaller scopes when possible,
> as it reduces chances for bugs. Any reason you moved i to the whole
> function scope?

I do the same as well, but tend to keep variables used in multiple
smaller scopes as top-level variables. I'm fine multiple instances of
'i' if you prefer.

-- 
Regards,

Laurent Pinchart