From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F1834C5DF62 for ; Wed, 26 Jan 2022 00:28:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235273AbiAZA25 (ORCPT ); Tue, 25 Jan 2022 19:28:57 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34918 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235260AbiAZA24 (ORCPT ); Tue, 25 Jan 2022 19:28:56 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 440B1C06161C for ; Tue, 25 Jan 2022 16:28:56 -0800 (PST) Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3716871; Wed, 26 Jan 2022 01:28:53 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1643156933; bh=NueSv6vKG4GXBykEREe7r07qfG2X/NP6wWAzN47j+K8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LHerEtGPt1tGTLWokPTyjf3+TMVDbsaOYX6Sb+nIDyPTviyfcp+cNfXwr2QiNSrSy tiau4A886jlInTHkHtG916bJRnpxJ667PcHmhuO9eZG/ReBKNn2OGhTCoULJAQP/+x TntIZ8m8+06iI9DMvrJzX9wTbwHKOR1tn/HbL9f4= Date: Wed, 26 Jan 2022 02:28:34 +0200 From: Laurent Pinchart To: Jacopo Mondi Cc: Jerome Brunet , Rui Miguel Silva , Steve Longerbeam , Philipp Zabel , Mauro Carvalho Chehab , linux-media@vger.kernel.org Subject: Re: [PATCH 1/2] staging: media: imx: imx7-mipi-csic: Resume on debug Message-ID: References: <20220119112024.11339-1-jacopo@jmondi.org> <20220119112024.11339-2-jacopo@jmondi.org> <20220125102257.7kziixej2c5k7fss@uno.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20220125102257.7kziixej2c5k7fss@uno.localdomain> Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hi Jacopo, On Tue, Jan 25, 2022 at 11:22:57AM +0100, Jacopo Mondi wrote: > On Tue, Jan 25, 2022 at 05:18:26AM +0200, Laurent Pinchart wrote: > > 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 > > > > --- > > > > 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 > > > > 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. > > Tbh, I would drop this custom sysfs attribute completely. > It should serve the purpose to easily dump the reg value, but it is > either accessed at the right time (ie during the streaming session) > otherwise all you get is POR default values (or a hang, without this > patch) The debugfs interface has served me before to diagnose problems, as it allows checking how the register values change during streaming, in particular the DPHY status. I'd like to keep it if possible. > > > > + > > > > 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