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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 080AAD3B7FF for ; Mon, 25 Nov 2024 10:24:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=zA7xM9K6ZZSQhBmqRXo54R3Bob+g4FvLBnSSMPtVfLY=; b=roVpPiQY1AuSaUgmo+W5hcxp5U hoINXVQcktzdBhvs9gcM3v1j6NVyf/lTr/1E6vR9elCajTEdlJ/OnXAbOsDo3jyu62L2A9UCRauPN lkOEqeW6cRz+NQtf8D/48ksgNDnMJLsUBRcx2D5wCTQ0qS9jyQnSHF4zqaSLEeehp65iaDrcCK2jq AhE3K3e+8ol0E/NwgO6OrCzqkMY8U/SCwJkpVhn4P0+lJsbhgjOhTZ63LSXZaZt/w0es0l8nOQA1p XeAtVBu/AeYFQwGPi7CrCdS9F2AlE/jzwliDq3P+dFU3SbJT8nv6Rk3XdPCU3o+eMG1wj6JDL+36E Mm5iCE9g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tFWGA-00000007kat-0uTD; Mon, 25 Nov 2024 10:24:02 +0000 Received: from perceval.ideasonboard.com ([213.167.242.64]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1tFWFC-00000007kVe-3gqr; Mon, 25 Nov 2024 10:23:04 +0000 Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F03B86B5; Mon, 25 Nov 2024 11:22:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1732530159; bh=vEktPLBDDcD6K+Ywf5tGYAWq1SEiHS7jR7yItctdw1k=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=cC2d+P8eBtYJZnAFLVQwSXuk8HKXOpxgMwPY3VYIEmgd0pcostPigM1+qqL0vGtSa KNo4PJzzSIzAaHge+E16wTllKnKDapBMN1EneVGXxdNsoStd9TdJ2eFk+0zoSCFRZr wFMwWgFkKmZk8BTGHVEbLVgwjsAIIoWnoY+ia/3o= Date: Mon, 25 Nov 2024 12:22:50 +0200 From: Laurent Pinchart To: CK Hu =?utf-8?B?KOiDoeS/iuWFiSk=?= Cc: "linux-kernel@vger.kernel.org" , "linux-mediatek@lists.infradead.org" , "linux-media@vger.kernel.org" , "devicetree@vger.kernel.org" , "paul.elder@ideasonboard.com" , "mchehab@kernel.org" , "conor+dt@kernel.org" , "robh@kernel.org" , Andy Hsieh =?utf-8?B?KOisneaZuueakyk=?= , "linux-arm-kernel@lists.infradead.org" , Julien Stephan , "matthias.bgg@gmail.com" , "krzk+dt@kernel.org" , "fsylvestre@baylibre.com" , AngeloGioacchino Del Regno Subject: Re: [PATCH v7 4/5] media: platform: mediatek: isp: add mediatek ISP3.0 camsv Message-ID: <20241125102250.GO19381@pendragon.ideasonboard.com> References: <20241121-add-mtk-isp-3-0-support-v7-0-b04dc9610619@baylibre.com> <20241121-add-mtk-isp-3-0-support-v7-4-b04dc9610619@baylibre.com> <20241125093953.GM19381@pendragon.ideasonboard.com> <25f70693c81eb86c832378fee89792f6754b7ca0.camel@mediatek.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <25f70693c81eb86c832378fee89792f6754b7ca0.camel@mediatek.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241125_022303_055140_B4AC6A25 X-CRM114-Status: GOOD ( 26.45 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Mon, Nov 25, 2024 at 09:56:54AM +0000, CK Hu (胡俊光) wrote: > On Mon, 2024-11-25 at 11:39 +0200, Laurent Pinchart wrote: > > On Mon, Nov 25, 2024 at 06:59:59AM +0000, CK Hu (胡俊光) wrote: > > > On Thu, 2024-11-21 at 09:53 +0100, Julien Stephan wrote: > > > > > > > > From: Phi-bang Nguyen > > > > > > > > This driver provides a path to bypass the SoC ISP so that image data > > > > coming from the SENINF can go directly into memory without any image > > > > processing. This allows the use of an external ISP. > > > > > > > > Signed-off-by: Phi-bang Nguyen > > > > Signed-off-by: Florian Sylvestre > > > > [Paul Elder fix irq locking] > > > > Signed-off-by: Paul Elder > > > > Co-developed-by: Laurent Pinchart > > > > Signed-off-by: Laurent Pinchart > > > > Co-developed-by: Julien Stephan > > > > Signed-off-by: Julien Stephan > > > > --- > > > > > > [snip] > > > > > > > +static const struct mtk_cam_conf camsv30_conf = { > > > > + .tg_sen_mode = 0x00010002U, /* TIME_STP_EN = 1. DBL_DATA_BUS = 1 */ > > > > + .module_en = 0x40000001U, /* enable double buffer and TG */ > > > > + .imgo_con = 0x80000080U, /* DMA FIFO depth and burst */ > > > > + .imgo_con2 = 0x00020002U, /* DMA priority */ > > > > > > Now support only one SoC, so it's not necessary make these SoC variable. > > > They could be constant symbol now. > > > > This I would keep as a mtk_cam_conf structure instance, as I think it > > makes it clear what we consider to be model-specific without hindering > > readability. I don't have a very strong opinion though. > > If this is a configuration table, I would like it to be > > { > .time_stp_en = true; > .dbl_data_bus = 1; > .double_buffer_en = true; > .tg = 0x4; > ... > } I like that too, it's more readable than raw register values. > If next SoC has only one parameter is different, we duplicate all > other parameter? That's what we usually do when the amount of parameters is not too large. When it becomes larger, we sometimes split the configuration data in multiple chunks. For instance, static const char * const family_a_clks[] = { "core", "io", }; static sont char * const family_b_clks[] = { "main", "ram", "bus", }; static const foo_dev_info soc_1_info = { .has_time_machine = false, .clks = family_a_clks, .num_clks = ARRAY_SIZE(family_a_clks), }; static const foo_dev_info soc_2_info = { .has_time_machine = false, .clks = family_b_clks, .num_clks = ARRAY_SIZE(family_b_clks), }; static const foo_dev_info soc_3_info = { .has_time_machine = true, .clks = family_b_clks, .num_clks = ARRAY_SIZE(family_b_clks), }; There's no clear rule, it's on a case-by-case basis. > > > > +}; > > > > + -- Regards, Laurent Pinchart