Re: [PATCH 1/2] staging: media: imx: imx7-mipi-csic: Resume on debug

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

Hi Jacopo,

On Wed, Jan 19, 2022 at 04:24:51PM +0200, Laurent Pinchart wrote:
> On Wed, Jan 19, 2022 at 12:20:23PM +0100, Jacopo Mondi wrote:
> > The mipi_csis_dump_regs() function reads and printout the interface
> > registers for debugging purposes.
> > 
> > Trying to access the registers without proper powering up the interface
> > causes the chip to hang.
> > 
> > Fix that by increasing the pm runtime usage count which, if necessary,
> > resumes the interface.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  drivers/staging/media/imx/imx7-mipi-csis.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
> > index 2b73fa55c938..cb54bb7491d9 100644
> > --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> > @@ -780,11 +780,15 @@ static int mipi_csis_dump_regs(struct csi_state *state)
> >  
> >  	dev_info(state->dev, "--- REGISTERS ---\n");
> >  
> > +	pm_runtime_resume_and_get(state->dev);
> 
> Should this have an error check ? With that,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I just noticed that the call to mipi_csis_dump_regs() in
mipi_csis_log_status() is conditioned by state->state & ST_POWERED. An
alternative would thus be to add the same condition to
mipi_csis_dump_regs_show() (or to move it to mipi_csis_dump_regs()), as
dumping the register when then hardware is turned off is quite
pointeless. Up to you.

> > +
> >  	for (i = 0; i < ARRAY_SIZE(registers); i++) {
> >  		cfg = mipi_csis_read(state, registers[i].offset);
> >  		dev_info(state->dev, "%14s: 0x%08x\n", registers[i].name, cfg);
> >  	}
> >  
> > +	pm_runtime_put(state->dev);
> > +
> >  	return 0;
> >  }
> >  

-- 
Regards,

Laurent Pinchart