[{"id":11300,"web_url":"https://patchwork.libcamera.org/comment/11300/","msgid":"<5dbb468e-db42-6ac2-db40-196dc0f8a09f@collabora.com>","date":"2020-07-09T20:04:14","subject":"Re: [libcamera-devel] [PATCH 03/25] media: ov5647: Add support for\n\tPWDN GPIO.","submitter":{"id":46,"url":"https://patchwork.libcamera.org/api/people/46/","name":"Dafna Hirschfeld","email":"dafna.hirschfeld@collabora.com"},"content":"On 23.06.20 12:07, Jacopo Mondi wrote:\n> From: Dave Stevenson \n> \n> Add support for an optional GPIO connected to PWDN on the sensor. This\n> allows the use of hardware standby mode where internal device clock\n> and circuit activities are halted.\n> \n> Please note that power is off when PWDN is high.\n> \n> Signed-off-by: Dave Stevenson \n> Signed-off-by: Roman Kovalivskyi \n> Signed-off-by: Jacopo Mondi \n> ---\n> drivers/media/i2c/ov5647.c | 28 ++++++++++++++++++++++++++++\n> 1 file changed, 28 insertions(+)\n> \n> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c\n> index e7d2e5b4ad4b9..105ff7f899b34 100644\n> --- a/drivers/media/i2c/ov5647.c\n> +++ b/drivers/media/i2c/ov5647.c\n> @@ -21,6 +21,7 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -35,6 +36,13 @@\n> \n> #define SENSOR_NAME \"ov5647\"\n> \n> +/*\n> + * From the datasheet, \"20ms after PWDN goes low or 20ms after RESETB goes\n> + * high if reset is inserted after PWDN goes high, host can access sensor's\n> + * SCCB to initialize sensor.\"\n> + */\n> +#define PWDN_ACTIVE_DELAY_MS\t20\n> +\n> #define MIPI_CTRL00_CLOCK_LANE_GATE\t\tBIT(5)\n> #define MIPI_CTRL00_BUS_IDLE\t\t\tBIT(2)\n> #define MIPI_CTRL00_CLOCK_LANE_DISABLE\t\tBIT(0)\n> @@ -86,6 +94,7 @@ struct ov5647 {\n> \tunsigned int\t\t\theight;\n> \tint\t\t\t\tpower_count;\n> \tstruct clk\t\t\t*xclk;\n> +\tstruct gpio_desc\t\t*pwdn;\n> };\n> \n> static inline struct ov5647 *to_state(struct v4l2_subdev *sd)\n> @@ -355,6 +364,11 @@ static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)\n> \tif (on && !ov5647->power_count)\t{\n> \t\tdev_dbg(&client->dev, \"OV5647 power on\\n\");\n> \n> +\t\tif (ov5647->pwdn) {\n> +\t\t\tgpiod_set_value_cansleep(ov5647->pwdn, 0);\n> +\t\t\tmsleep(PWDN_ACTIVE_DELAY_MS);\n> +\t\t}\n> +\n> \t\tret = clk_prepare_enable(ov5647->xclk);\n> \t\tif (ret < 0) {\n> \t\t\tdev_err(&client->dev, \"clk prepare enable failed\\n\");\n> @@ -392,6 +406,8 @@ static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)\n> \t\t\tdev_dbg(&client->dev, \"soft stby failed\\n\");\n> \n> \t\tclk_disable_unprepare(ov5647->xclk);\n> +\n> +\t\tgpiod_set_value_cansleep(ov5647->pwdn, 1);\n> \t}\n> \n> \t/* Update the power count. */\n> @@ -581,6 +597,10 @@ static int ov5647_probe(struct i2c_client *client)\n> \t\treturn -EINVAL;\n> \t}\n> \n> +\t/* Request the power down GPIO asserted */\n> +\tsensor->pwdn = devm_gpiod_get_optional(&client->dev, \"pwdn\",\n> +\t\t\t\t\t GPIOD_OUT_HIGH);\n\nHi,\nI see that other drivers check for error from 'devm_gpiod_get_optional'\nusing 'IS_ERR'\n\nThanks,\nDafna\n\n> +\n> \tmutex_init(&sensor->lock);\n> \n> \tsd = &sensor->sd;\n> @@ -594,7 +614,15 @@ static int ov5647_probe(struct i2c_client *client)\n> \tif (ret < 0)\n> \t\tgoto mutex_remove;\n> \n> +\tif (sensor->pwdn) {\n> +\t\tgpiod_set_value_cansleep(sensor->pwdn, 0);\n> +\t\tmsleep(PWDN_ACTIVE_DELAY_MS);\n> +\t}\n> +\n> \tret = ov5647_detect(sd);\n> +\n> +\tgpiod_set_value_cansleep(sensor->pwdn, 1);\n> +\n> \tif (ret < 0)\n> \t\tgoto error;\n> \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EADE7BD792\n\tfor ;\n\tThu, 9 Jul 2020 20:04:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94FC961370;\n\tThu, 9 Jul 2020 22:04:18 +0200 (CEST)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E69D06123A\n\tfor ;\n\tThu, 9 Jul 2020 22:04:17 +0200 (CEST)","from [IPv6:2003:cb:8737:cf00:f5ff:2fff:89f:f3f4]\n\t(p200300cb8737cf00f5ff2fff089ff3f4.dip0.t-ipconnect.de\n\t[IPv6:2003:cb:8737:cf00:f5ff:2fff:89f:f3f4])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits))\n\t(No client certificate requested) (Authenticated sender: dafna)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id 03A072A663A;\n\tThu, 9 Jul 2020 21:04:16 +0100 (BST)"],"To":"Jacopo Mondi , mchehab@kernel.org,\n\tsakari.ailus@linux.intel.com, hverkuil@xs4all.nl,\n\tlaurent.pinchart@ideasonboard.com, roman.kovalivskyi@globallogic.com, \n\tdave.stevenson@raspberrypi.org, naush@raspberrypi.com","References":"<20200623100815.10674-1-jacopo@jmondi.org>\n\t<20200623100815.10674-4-jacopo@jmondi.org>","From":"Dafna Hirschfeld ","Message-ID":"<5dbb468e-db42-6ac2-db40-196dc0f8a09f@collabora.com>","Date":"Thu, 9 Jul 2020 22:04:14 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200623100815.10674-4-jacopo@jmondi.org>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 03/25] media: ov5647: Add support for\n\tPWDN GPIO.","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"andrew_gabbasov@mentor.com, mrodin@de.adit-jv.com, mripard@kernel.org,\n\tlibcamera-devel@lists.libcamera.org, sudipi@jp.adit-jv.com,\n\thugues.fruchet@st.com, erosca@de.adit-jv.com, aford173@gmail.com,\n\tlinux-media@vger.kernel.org","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11381,"web_url":"https://patchwork.libcamera.org/comment/11381/","msgid":"<20200714124515.ptliaxjrscieqsdi@uno.localdomain>","date":"2020-07-14T12:45:15","subject":"Re: [libcamera-devel] [PATCH 03/25] media: ov5647: Add support for\n\tPWDN GPIO.","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Dafna,\n\nOn Thu, Jul 09, 2020 at 10:04:14PM +0200, Dafna Hirschfeld wrote:\n>\n>\n> On 23.06.20 12:07, Jacopo Mondi wrote:\n> > From: Dave Stevenson \n> >\n> > Add support for an optional GPIO connected to PWDN on the sensor. This\n> > allows the use of hardware standby mode where internal device clock\n> > and circuit activities are halted.\n> >\n> > Please note that power is off when PWDN is high.\n> >\n> > Signed-off-by: Dave Stevenson \n> > Signed-off-by: Roman Kovalivskyi \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > drivers/media/i2c/ov5647.c | 28 ++++++++++++++++++++++++++++\n> > 1 file changed, 28 insertions(+)\n> >\n> > diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c\n> > index e7d2e5b4ad4b9..105ff7f899b34 100644\n> > --- a/drivers/media/i2c/ov5647.c\n> > +++ b/drivers/media/i2c/ov5647.c\n> > @@ -21,6 +21,7 @@\n> > #include \n> > #include \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -35,6 +36,13 @@\n> > #define SENSOR_NAME \"ov5647\"\n> > +/*\n> > + * From the datasheet, \"20ms after PWDN goes low or 20ms after RESETB goes\n> > + * high if reset is inserted after PWDN goes high, host can access sensor's\n> > + * SCCB to initialize sensor.\"\n> > + */\n> > +#define PWDN_ACTIVE_DELAY_MS\t20\n> > +\n> > #define MIPI_CTRL00_CLOCK_LANE_GATE\t\tBIT(5)\n> > #define MIPI_CTRL00_BUS_IDLE\t\t\tBIT(2)\n> > #define MIPI_CTRL00_CLOCK_LANE_DISABLE\t\tBIT(0)\n> > @@ -86,6 +94,7 @@ struct ov5647 {\n> > \tunsigned int\t\t\theight;\n> > \tint\t\t\t\tpower_count;\n> > \tstruct clk\t\t\t*xclk;\n> > +\tstruct gpio_desc\t\t*pwdn;\n> > };\n> > static inline struct ov5647 *to_state(struct v4l2_subdev *sd)\n> > @@ -355,6 +364,11 @@ static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)\n> > \tif (on && !ov5647->power_count)\t{\n> > \t\tdev_dbg(&client->dev, \"OV5647 power on\\n\");\n> > +\t\tif (ov5647->pwdn) {\n> > +\t\t\tgpiod_set_value_cansleep(ov5647->pwdn, 0);\n> > +\t\t\tmsleep(PWDN_ACTIVE_DELAY_MS);\n> > +\t\t}\n> > +\n> > \t\tret = clk_prepare_enable(ov5647->xclk);\n> > \t\tif (ret < 0) {\n> > \t\t\tdev_err(&client->dev, \"clk prepare enable failed\\n\");\n> > @@ -392,6 +406,8 @@ static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)\n> > \t\t\tdev_dbg(&client->dev, \"soft stby failed\\n\");\n> > \t\tclk_disable_unprepare(ov5647->xclk);\n> > +\n> > +\t\tgpiod_set_value_cansleep(ov5647->pwdn, 1);\n> > \t}\n> > \t/* Update the power count. */\n> > @@ -581,6 +597,10 @@ static int ov5647_probe(struct i2c_client *client)\n> > \t\treturn -EINVAL;\n> > \t}\n> > +\t/* Request the power down GPIO asserted */\n> > +\tsensor->pwdn = devm_gpiod_get_optional(&client->dev, \"pwdn\",\n> > +\t\t\t\t\t GPIOD_OUT_HIGH);\n>\n> Hi,\n> I see that other drivers check for error from 'devm_gpiod_get_optional'\n> using 'IS_ERR'\n\nThanks for noticing. It's probably worth adding a check for this.\nI'll fix in v2.\n\nThanks\n j\n\n>\n> Thanks,\n> Dafna\n>\n> > +\n> > \tmutex_init(&sensor->lock);\n> > \tsd = &sensor->sd;\n> > @@ -594,7 +614,15 @@ static int ov5647_probe(struct i2c_client *client)\n> > \tif (ret < 0)\n> > \t\tgoto mutex_remove;\n> > +\tif (sensor->pwdn) {\n> > +\t\tgpiod_set_value_cansleep(sensor->pwdn, 0);\n> > +\t\tmsleep(PWDN_ACTIVE_DELAY_MS);\n> > +\t}\n> > +\n> > \tret = ov5647_detect(sd);\n> > +\n> > +\tgpiod_set_value_cansleep(sensor->pwdn, 1);\n> > +\n> > \tif (ret < 0)\n> > \t\tgoto error;\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6EF85BD790\n\tfor ;\n\tTue, 14 Jul 2020 12:41:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0833B60832;\n\tTue, 14 Jul 2020 14:41:51 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0B286605B2\n\tfor ;\n\tTue, 14 Jul 2020 14:41:49 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 5E71B100006;\n\tTue, 14 Jul 2020 12:41:42 +0000 (UTC)"],"Date":"Tue, 14 Jul 2020 14:45:15 +0200","From":"Jacopo Mondi ","To":"Dafna Hirschfeld ","Message-ID":"<20200714124515.ptliaxjrscieqsdi@uno.localdomain>","References":"<20200623100815.10674-1-jacopo@jmondi.org>\n\t<20200623100815.10674-4-jacopo@jmondi.org>\n\t<5dbb468e-db42-6ac2-db40-196dc0f8a09f@collabora.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<5dbb468e-db42-6ac2-db40-196dc0f8a09f@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH 03/25] media: ov5647: Add support for\n\tPWDN GPIO.","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"andrew_gabbasov@mentor.com, dave.stevenson@raspberrypi.org,\n\tmrodin@de.adit-jv.com, erosca@de.adit-jv.com, mripard@kernel.org,\n\troman.kovalivskyi@globallogic.com, libcamera-devel@lists.libcamera.org,\n\tlinux-media@vger.kernel.org, sakari.ailus@linux.intel.com,\n\thugues.fruchet@st.com, mchehab@kernel.org, aford173@gmail.com,\n\tsudipi@jp.adit-jv.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]