[{"id":17670,"web_url":"https://patchwork.libcamera.org/comment/17670/","msgid":"<36c519dc-03d0-c648-8731-ce334b0a08d4@ideasonboard.com>","date":"2021-06-21T19:53:02","subject":"Re: [libcamera-devel] [PATCH v3] libcamera: Add OV5647 sensor\n\tproperties","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Vedant,\n\nOn 21/06/2021 15:17, Vedant Paranjape wrote:\n> Brief specifications available at\n> https://cdn.sparkfun.com/datasheets/Dev/RaspberryPi/ov5647_full.pdf\n> \n>> pixel size: 1.4 μm x 1.4 μm\n> \n> Change in this patch is referenced from Page 5: key specifications\n> section of the above linked pdf\n> \n> Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> ---\n>  src/libcamera/camera_sensor_properties.cpp | 5 +++++\n>  1 file changed, 5 insertions(+)\n> \n> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> index f660743a..43030e8b 100644\n> --- a/src/libcamera/camera_sensor_properties.cpp\n> +++ b/src/libcamera/camera_sensor_properties.cpp\n> @@ -81,6 +81,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>  \t\t\t\t{ 1, controls::draft::TestPatternModeColorBars },\n>  \t\t\t},\n>  \t\t} },\n> +\t\t{ \"ov5647\", {\n> +\t\t\t.unitCellSize = { 1400, 1400 },\n> +\t\t\t/* \\todo fill test pattern modes for ov5647. */\n\nThere was discussion on this:\n\n>> So the todo is likely sufficient, but I'd be more explicit saying there\n>> are no current supported test modes in the kernel.\n> \n> I'd say there's no \\todo needed in that case, if the driver doesn't\n> support test patterns, the implementation here is enough.\n\n\nSo - it's either, remove the todo, or make the comment explicit that\nthere is no current test pattern support in the kernel, which ever feels\nmost appropriate to you.\n\n\nEither of those is fine. But a todo that won't get done due to lacking\nkernel support is a bit meaningless otherwise.\n\nAlso, I had given a tag on your first patch.\n\nWhen posting a new revision, you should collect the tags given and add\nthem to your commit so they don't get lost.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n\n> +\t\t\t.testPatternModes = {},\n> +\t\t} },\n>  \t\t{ \"ov5693\", {\n>  \t\t\t.unitCellSize = { 1400, 1400 },\n>  \t\t\t.testPatternModes = {\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 F27F0BE58C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Jun 2021 19:53:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3FF7068935;\n\tMon, 21 Jun 2021 21:53:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 54F2E60295\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Jun 2021 21:53:05 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DE1AA5E17;\n\tMon, 21 Jun 2021 21:53:04 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BivMxemE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624305185;\n\tbh=PcY/X1dufIRR1HO3ff5RZODVigAkSiYOIhlSRzVM+d0=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=BivMxemEq6Y3rXfPfXmjmNxQ4KQVUuas46uletEYfiAtspza6sNoyX/8Y8k1WuKyl\n\tFGbIoLbiuh/xeGOTOjkuWsoSqgqu3O7nEUOXh2l/nxhqQqullAqML6QeiSvj+X7EYz\n\tjIR8KaAEE4bJEGqsXvWyys1VilH/rDST1IUmYg0c=","To":"Vedant Paranjape <vedantparanjape160201@gmail.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210621141738.153329-1-vedantparanjape160201@gmail.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<36c519dc-03d0-c648-8731-ce334b0a08d4@ideasonboard.com>","Date":"Mon, 21 Jun 2021 20:53:02 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.8.1","MIME-Version":"1.0","In-Reply-To":"<20210621141738.153329-1-vedantparanjape160201@gmail.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v3] libcamera: Add OV5647 sensor\n\tproperties","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]