From: Hugues FRUCHET <hugues.fruchet@st.com>
To: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
Jacopo Mondi <jacopo+renesas@jmondi.org>,
Sakari Ailus <sakari.ailus@linux.intel.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Steve Longerbeam <slongerbeam@gmail.com>,
Paul <paul.kocialkowski@bootlin.com>
Cc: "linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-renesas-soc@vger.kernel.org"
<linux-renesas-soc@vger.kernel.org>,
"Biju Das" <biju.das.jz@bp.renesas.com>,
Prabhakar <prabhakar.csengg@gmail.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>
Subject: Re: [PATCH v4 1/6] media: i2c: ov5640: Remain in power down for DVP mode unless streaming
Date: Wed, 9 Sep 2020 16:16:01 +0000 [thread overview]
Message-ID: <8b5d4928-2921-b876-7d1e-04bb42eff4fa@st.com> (raw)
In-Reply-To: <20200904201835.5958-2-prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi Prabhakar,
As discussed separately I would prefer to better understand issue before
going to this patch.
Nevertheless I have some remarks in code in case we'll need it at the end.
On 9/4/20 10:18 PM, Lad Prabhakar wrote:
> Keep the sensor in software power down mode and wake up only in
> ov5640_set_stream_dvp() callback.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> Tested-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
> drivers/media/i2c/ov5640.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 2fe4a7ac0592..880fde73a030 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -34,6 +34,8 @@
> #define OV5640_REG_SYS_RESET02 0x3002
> #define OV5640_REG_SYS_CLOCK_ENABLE02 0x3006
> #define OV5640_REG_SYS_CTRL0 0x3008
> +#define OV5640_REG_SYS_CTRL0_SW_PWDN 0x42
> +#define OV5640_REG_SYS_CTRL0_SW_PWUP 0x02
For the time being this section was only referring to registers
addresses and bit details was explained into a comment right before
affectation, see OV5640_REG_IO_MIPI_CTRL00 for example.
> #define OV5640_REG_CHIP_ID 0x300a
> #define OV5640_REG_IO_MIPI_CTRL00 0x300e
> #define OV5640_REG_PAD_OUTPUT_ENABLE01 0x3017
> @@ -1120,6 +1122,12 @@ static int ov5640_load_regs(struct ov5640_dev *sensor,
> val = regs->val;
> mask = regs->mask;
>
> + /* remain in power down mode for DVP */
> + if (regs->reg_addr == OV5640_REG_SYS_CTRL0 &&
> + val == OV5640_REG_SYS_CTRL0_SW_PWUP &&
> + sensor->ep.bus_type != V4L2_MBUS_CSI2_DPHY)
> + continue;
> +
I understand that more or less register OV5640_REG_SYS_CTRL0 (0x3008)
has been partially removed from big init sequence: for power-up part,
but power-dwn remains at very beginning of sequence.
We should completely remove 0x3008 from init sequence, including
power-dwn, and introduce a new function ov5640_sw_powerdown(on/off) that
should be called at the right place instead.
> if (mask)
> ret = ov5640_mod_reg(sensor, reg_addr, mask, val);
> else
> @@ -1297,9 +1305,14 @@ static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
> * PAD OUTPUT ENABLE 02
> * - [7:2]: D[5:0] output enable
> */
> - return ov5640_write_reg(sensor,
> - OV5640_REG_PAD_OUTPUT_ENABLE02,
> - on ? 0xfc : 0);
> + ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE02,
> + on ? 0xfc : 0);
> + if (ret)
> + return ret;
> +
> + return ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, on ?
> + OV5640_REG_SYS_CTRL0_SW_PWUP :
> + OV5640_REG_SYS_CTRL0_SW_PWDN);
> }
>
> static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on)
>
BR,
Hugues.
next prev parent reply other threads:[~2020-09-09 16:42 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-04 20:18 [PATCH v4 0/6] media: i2c: ov5640 feature enhancement and fixes Lad Prabhakar
2020-09-04 20:18 ` [PATCH v4 1/6] media: i2c: ov5640: Remain in power down for DVP mode unless streaming Lad Prabhakar
2020-09-09 16:16 ` Hugues FRUCHET [this message]
2021-06-29 10:47 ` Eugen.Hristev
2021-12-21 7:37 ` Eugen.Hristev
2021-12-21 12:06 ` Sakari Ailus
2021-12-21 14:47 ` Lad, Prabhakar
2021-12-21 15:01 ` Eugen.Hristev
2021-12-21 15:11 ` Lad, Prabhakar
2022-01-03 11:29 ` Eugen.Hristev
2022-01-04 9:57 ` Lad, Prabhakar
2022-01-07 15:49 ` Lad, Prabhakar
2020-09-04 20:18 ` [PATCH v4 2/6] media: i2c: ov5640: Separate out mipi configuration from s_power Lad Prabhakar
2020-09-04 20:18 ` [PATCH v4 3/6] media: i2c: ov5640: Enable data pins on poweron for DVP mode Lad Prabhakar
2020-09-07 9:44 ` Hugues FRUCHET
2020-09-07 14:35 ` Lad, Prabhakar
2020-09-09 15:48 ` Hugues FRUCHET
2020-09-04 20:18 ` [PATCH v4 4/6] media: i2c: ov5640: Configure HVP lines in s_power callback Lad Prabhakar
2020-09-04 20:18 ` [PATCH v4 5/6] media: i2c: ov5640: Add support for BT656 mode Lad Prabhakar
2020-09-04 20:18 ` [PATCH v4 6/6] media: i2c: ov5640: Fail probe on unsupported bus_type Lad Prabhakar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8b5d4928-2921-b876-7d1e-04bb42eff4fa@st.com \
--to=hugues.fruchet@st.com \
--cc=biju.das.jz@bp.renesas.com \
--cc=jacopo+renesas@jmondi.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=paul.kocialkowski@bootlin.com \
--cc=prabhakar.csengg@gmail.com \
--cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
--cc=sakari.ailus@linux.intel.com \
--cc=slongerbeam@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.