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 0E257C48BC3 for ; Tue, 20 Feb 2024 02:22:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Content-ID:In-Reply-To: References:Message-ID:Date:Subject:CC:To:From:Reply-To:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=2ECoofn1zVPdf7qIuA/fJJVO34TadFLztPSFrWZZRbU=; b=gophGdV23uLHwV 1NO/2OEoPpXUWXj+hmKAujN1p5ZqeE9CVfH6cHa2iKlA9ETQ4FPyHA4aN7RSykY2eICZRJ9tuU5II CEbyfiiN8319dvgPMGFP6765t5ymqMAb3MseEneTM5FsESGTrYwy57DBYLDLjEssegyam49xHcOWY m7XBDg/3BZ1fny/nK+jLVl5/+UZa2TzpYNR9s1NJAX813csshlJfYyezYqBxQFkb447/apPJ7YFUu 4ACrhMpdhqA/25ykYyZJDcjh7yA6xWpPch/UHH2Fgi3A6tsYQQU6x44FRA2yCMaixE0RXiGWlveV2 oXV/ZYN3X7GcPJ/3JmMA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rcFlp-0000000Cqz2-3085; Tue, 20 Feb 2024 02:22:09 +0000 Received: from mailgw01.mediatek.com ([216.200.240.184]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rcFll-0000000CqyK-3vio; Tue, 20 Feb 2024 02:22:07 +0000 X-UUID: d57e5f3ccf9611ee9a263b4415211400-20240219 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=MIME-Version:Content-Transfer-Encoding:Content-ID:Content-Type:In-Reply-To:References:Message-ID:Date:Subject:CC:To:From; bh=oJM1og04qpp04fBo97wOy+7kq/t4/spX/1VRMFAYmSM=; b=l/0B8Agm3RT1puV84IcUNPZT8NRS6gc6KnOnfNHR/Ko0hnpdSjtksu4/5b4yMlFpB1onM1bYYoLF/5jvC4xeixrImzFMBLRh3VtY9cafd8sX05fVNOB3EYkPHY/E9g4ctjsR2iThiQVfI84cfRPXDQ+0kwqk4CI6NmLXhsQ4AHw=; X-CID-P-RULE: Release_Ham X-CID-O-INFO: VERSION:1.1.37,REQID:c8468296-cf4e-4780-8e48-a15fb8c96a24,IP:0,U RL:0,TC:0,Content:0,EDM:0,RT:0,SF:0,FILE:0,BULK:0,RULE:Release_Ham,ACTION: release,TS:0 X-CID-META: VersionHash:6f543d0,CLOUDID:11687b8f-e2c0-40b0-a8fe-7c7e47299109,B ulkID:nil,BulkQuantity:0,Recheck:0,SF:102,TC:nil,Content:0,EDM:-3,IP:nil,U RL:11|1,File:nil,RT:nil,Bulk:nil,QS:nil,BEC:nil,COL:0,OSI:0,OSA:0,AV:0,LES :1,SPR:NO,DKR:0,DKP:0,BRR:0,BRE:0 X-CID-BVR: 0 X-CID-BAS: 0,_,0,_ X-CID-FACTOR: TF_CID_SPAM_SNR,TF_CID_SPAM_ULN X-UUID: d57e5f3ccf9611ee9a263b4415211400-20240219 Received: from mtkmbs14n2.mediatek.inc [(172.21.101.76)] by mailgw01.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLSv1.2 ECDHE-RSA-AES256-GCM-SHA384 256/256) with ESMTP id 365136891; Mon, 19 Feb 2024 19:22:02 -0700 Received: from mtkmbs10n1.mediatek.inc (172.21.101.34) by mtkmbs13n2.mediatek.inc (172.21.101.108) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.26; Tue, 20 Feb 2024 10:21:26 +0800 Received: from APC01-SG2-obe.outbound.protection.outlook.com (172.21.101.237) by mtkmbs10n1.mediatek.inc (172.21.101.34) with Microsoft SMTP Server id 15.2.1118.26 via Frontend Transport; Tue, 20 Feb 2024 10:21:25 +0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EGCOpLY+se9M5KqIbfHRx0Pdim/kqQbj+crDNr9VDGnlwcukbPPeIY6TL2wHuYIq/D5XglJKQUa94LMtfLemqbEnlBdEwMQBoY64JvZyRgU61uQHOAJ7aqOIkx57P4uZ9Vfa5zJfhbQhdJMo79Vg8cDwZ1Fk+MQm8CU5o2WgS69fOSgaEkZjT709Tn9lFk7m1MujYUwqAmoc96SLgVsxC/9Eca8fT9AR4+jIYqlpKZLvflgeEGqgmsspfrM/2yXVScE7Ay8sKH0osWJSvoAo11o8hZd++8oBhZRkT5JcB+iln/QdY6tJaxgYNOqRhuyk1oWkc0O4I42RIj/JKQIDpA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=oJM1og04qpp04fBo97wOy+7kq/t4/spX/1VRMFAYmSM=; b=hZEVHrY7oQbjsluqjEhztRSqujWsXjWo5Tr8iZy7ulAcd46ja/6+60JXqFJ8vfFjE4xljklhfJu7S/vlgYnLBc9iRRK6L6u5CwLDVs3KTtcJc3awEl+PwAr6QX0/izy6Orz5K2ohOsCVxTeq2ubg8wZ6G55LRwTxj0Z+4Ykw+jwQk+j8AGtO9SaC8Gic+LILFNYKCY64xMGq+rpsZfvBPD57HLeA1jKni4H0OlFi38Lj5YYjvKvcEPNqKgldM/AaYDr3lR1yPmKZLoIiye/9FHjVGgp0B5AlYx8Qs12LeVokDQjv6PPo/Qzm579xt4j9WjeqsRz2tVOXLT7fhoSjGA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=mediatek.com; dmarc=pass action=none header.from=mediatek.com; dkim=pass header.d=mediatek.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mediateko365.onmicrosoft.com; s=selector2-mediateko365-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=oJM1og04qpp04fBo97wOy+7kq/t4/spX/1VRMFAYmSM=; b=S7YgrBFmwQ/fCCA7wZwdHkejP5cGcJmt1NTA271Qt6mXpZr+nzPHmGMylJlolOgj1mz9Rs5flPxBRQGHrqx9moP0AQXLt6AxvhT0rPCzxfc0kdVPrL8bmM0iXXgRrskmkUQUxhBGU45NK6O7nKoepUyun3Avkvey2zy03NqxUnY= Received: from TYZPR03MB5566.apcprd03.prod.outlook.com (2603:1096:400:53::7) by SEYPR03MB8006.apcprd03.prod.outlook.com (2603:1096:101:176::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7292.38; Tue, 20 Feb 2024 02:21:21 +0000 Received: from TYZPR03MB5566.apcprd03.prod.outlook.com ([fe80::c435:bad8:87ad:994c]) by TYZPR03MB5566.apcprd03.prod.outlook.com ([fe80::c435:bad8:87ad:994c%5]) with mapi id 15.20.7292.036; Tue, 20 Feb 2024 02:21:21 +0000 From: =?utf-8?B?WmhpIE1hbyAo5q+b5pm6KQ==?= To: "mchehab@kernel.org" , "sakari.ailus@linux.intel.com" , "angelogioacchino.delregno@collabora.com" , "robh+dt@kernel.org" , "krzysztof.kozlowski+dt@linaro.org" CC: "heiko@sntech.de" , "gerald.loacker@wolfvision.net" , "linux-kernel@vger.kernel.org" , "yunkec@chromium.org" , "linux-mediatek@lists.infradead.org" , "dan.scally@ideasonboard.com" , "linux-media@vger.kernel.org" , =?utf-8?B?U2hlbmduYW4gV2FuZyAo546L5Zyj55S3KQ==?= , "hdegoede@redhat.com" , "linus.walleij@linaro.org" , "andy.shevchenko@gmail.com" , =?utf-8?B?WWF5YSBDaGFuZyAo5by16ZuF5riFKQ==?= , "bingbu.cao@intel.com" , "jacopo.mondi@ideasonboard.com" , "jernej.skrabec@gmail.com" , "devicetree@vger.kernel.org" , "conor+dt@kernel.org" , Project_Global_Chrome_Upstream_Group , "10572168@qq.com" <10572168@qq.com>, "hverkuil-cisco@xs4all.nl" , "tomi.valkeinen@ideasonboard.com" , "linux-arm-kernel@lists.infradead.org" , "matthias.bgg@gmail.com" , "laurent.pinchart@ideasonboard.com" , "macromorgan@hotmail.com" Subject: Re: [PATCH v4 2/2] media: i2c: Add GC08A3 image sensor driver Thread-Topic: [PATCH v4 2/2] media: i2c: Add GC08A3 image sensor driver Thread-Index: AQHaVzH+Ve+4uX+wTUqBqQq2qHHPDrD++V6AgBOfOQA= Date: Tue, 20 Feb 2024 02:21:21 +0000 Message-ID: <1a9cb6c04de90cde777e30e15b0bced4c1d002f9.camel@mediatek.com> References: <20240204061538.2105-1-zhi.mao@mediatek.com> <20240204061538.2105-3-zhi.mao@mediatek.com> <21370bd8-502b-4b4e-8c9b-6d13c60685d5@collabora.com> In-Reply-To: <21370bd8-502b-4b4e-8c9b-6d13c60685d5@collabora.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=mediatek.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: TYZPR03MB5566:EE_|SEYPR03MB8006:EE_ x-ms-office365-filtering-correlation-id: 1f20e6ab-6209-440a-48cd-08dc31baa0c7 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: qeqkXuvFkMMnzGxg8WpN8SmetM9Lm80n+u6t0dmUxA2FOLUnK6lJdRY5ZZN4mcWDPMfraE2otJTPD3Z0GFsNct/bfuXgkKMgA605bd8hU4RibElfNMPZbEO55PVTLEqBhiQ0Ycx6RVdvCILPlt6n/JkHqdHmYrC8qO4UFow053S9YpComkc8OJxwUyOq+Wdghht6l6hi27mawQDUBMb791hI+aT4FacZqAu/d0gcfLgdMqIx8baU0QY7m2K722nVAVHdmASzltOgBmEllYxoidw0WKGxrwNfSlQ702ljuIm9EDkRBk5C15C3dY5bPB9TKJSMfTHFxrLD3L6OO/bXmOWE8GX/UBrpAh72C2N+ya1qFbbIlFfY8I2w6z6fHcbfZD/+76bxPCvRtqw67BnlFhxsYk/wcsrrUe8zp64BWtLDZT+rpX+9EWfgfHpCBM3r1XQ8034cHhYkoWhWrSUOYedWPvmAUdmRehmcPPzOIxKF7Aix4+TojOxRWn8GSREwdPzYFnsKdDfJ3jPwvtuSRNraq+ZDC4f8oTkQaWgqniS8BH10MVtgxOBQDYsu7IlZe2a5dxCmBHEeN4eQol1ODxcWaL2GGxWVx69wrX2JtiYDFtTBG8UoyTNFu46J4AH5 x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:TYZPR03MB5566.apcprd03.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(38070700009);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?utf-8?B?K3FJS0k0QnkxWXdLZGNGaml1TGc2UXFYbXJPcGdRMEFGODcwcDkxQTd6Q0pL?= =?utf-8?B?NTM4a1cxQUhKbkIrV3ErK0oxYWMyK2FNNlVPMjB6RHpCTjZTWGxwWUVtc25O?= =?utf-8?B?SnRNUVlTeld0eDc0K2lRbC9TRHQ0Mkh0aHNHRlAwNlZnYi9hcjRHRmlRNy9B?= =?utf-8?B?Z0EyeW1mdklOdHhseEdTYTVRcDNLUGswSXp6Sk04eXRCcEIxbVpMT0VreElr?= =?utf-8?B?eFI0L1hMenJuR0trbDJEQmVYSlNsdjFWRDk1OGdpWnptMW1VdnkzdnA0Rzk0?= =?utf-8?B?Y1h1SytnVUw1dVNiWCtJQzNQZVZkUFVvOUY5UnhzT1dzVlZaREgrWkhManh1?= =?utf-8?B?dU05a1FoOWdtUVRHSnpaMzdjbVp2Y0ZkUXJzVjMxRlFURngvT0JWVzdjS1Q0?= =?utf-8?B?K0tTVlBuaXEzaERERW1qOHl1WFA1cm56WCtaSWE1Z3RFbE1KVm92YnVrUlNX?= =?utf-8?B?Nmc1emVSWEEwdHN3UXJqUEgxaTR1THB6WU1wbzRkbmFLWjRXMjVXV0sySUpT?= =?utf-8?B?OTB2eE1uZElaSlhVbk9IRHprQ2VBTGpiVGZDWnRQQURCMFJ4TUlZVzNGclQ4?= =?utf-8?B?RjJUQnA0c1VPMlUwS1p6TEhnSloxN1pzdEFlVjRMRm9wUnI5WThkT2NYR2VI?= =?utf-8?B?YThPVGRjLy9hd1F2cVgxaUYraG92Z3JDaHRWTSs3QWdRMFhMbkVuVWVsSjJn?= =?utf-8?B?NHkwajFPb2Z1cTc3NmV1ZUtkNmUyVlJTQjl5TTFhOVhsa0V0Sjhhd2pQYmFs?= =?utf-8?B?MGhhaDI4SWhwZktPQkhxNmR4Yi9rWFllU2psdHgrWkt0YmRXWW9zYnd0Zzk3?= =?utf-8?B?dVRaQXRsY3ZZSGIwbXpUazFhbXVlWVhDU04zdGNRL2hIWFZpeFZ4bU90aG1q?= =?utf-8?B?dFhpNWJ2U3VNU1ZBajUwa3JXc1FPOHY0NzE0K0IvWDRRZisvdFJuUVN2eDdS?= =?utf-8?B?NzhzTGhPZ3ppTnBJQVhvOExJNWxNV0VsQzJsZkpvUys2RllYZ3dKaXZDTS9J?= =?utf-8?B?WmhZRFNzNVBMTjJRQ3dqaTg1TU1BSnZHekJ2SjB6RjYyU1pRZ1NyM0EvTU9T?= =?utf-8?B?N3h5Ty9xKy9nYjZsYjhZOFNvUC9vRktjUE8vVXhRS0NyWGRURlRTaWwwRksx?= =?utf-8?B?QW5YYmZ5aGQzTnJuRWd5ZkszNnZpeksyY2tYVlNsZVRaZVRIcVA4REpwanc2?= =?utf-8?B?VzJKdWVOZkErRWhkMmlIY0wzOHQvNXYyWGxVaWFvdFJPdy9xWlR3V3VmZ0Q3?= =?utf-8?B?dnc2TTNpdDdYUEYzdStXMEVvbHU3bDVmWjRNdm9SZWdpZmtWbmJ5S0ZSSTJh?= =?utf-8?B?cWVPWFpnQXFHK3JMWUIyYUh2c1RZTVJyOXZ3Wk5nL0hEbUwyTmFHSmZqTUo4?= =?utf-8?B?MzR4cHhGb3dRU3RWTk85NnEwM3pWdmkrSENDa1E3Rk9mMVF0eWpwRFI5bDhh?= =?utf-8?B?WW5PYXdPUTlIWVZQR3R6WDdVK3E1WGEwbXJSZEw0cW1CNW05WUdqbWJWbnJn?= =?utf-8?B?NzJURUFkd2o5bnFGcHc5THhBL3hURi82OXdkSDNNZUkzY3VDcHY5a3A1d05N?= =?utf-8?B?dWNWa0xTcVlaUDc3TFNUZDlkN1M2cGgrNUlmWUNkUkE0RXRWcWJtRVlLWVE4?= =?utf-8?B?QnpvcGVJTlJBd0RsMXBZMDhteVlBVDlVV2lTelJOcG5jczZ5ODYwQ1NCZW9r?= =?utf-8?B?Y2ZKYithRVBmT0dwaWVyV1RqM2JMdElRQzU3WTJicHBXOHdzSE1XdGVrL2dW?= =?utf-8?B?dmVrTC9VQmhmZWtueWRuK1BMTWZvbzljTklMVDhPNk91OHowQ0J4cU9TVEds?= =?utf-8?B?MnpsYzlyTnMya0l0MUpVRy93TUxucDVNM2ZCZlduVEVyV2U0b1A4Y3JIV1pF?= =?utf-8?B?WGMrdXVGZ1VEbmRpcEk1cUR6L1ZZaXYxRzJkV09iWThrR3BEMW12T3VGeXFT?= =?utf-8?B?SkFmRDVqektjRjRWME51WUxXSW5GVVR3QkNpWjl3WWpiVjBlTVJjbFdUT3E0?= =?utf-8?B?Ukg2TXQzeWFwQzVrbzQvdDhhMnQwdnF4bm1YemdFeHdFdWZrclR6RHE0Njlo?= =?utf-8?B?ZlVmY3B2eTJxU0FORFF6VExXL1hzaHgwUFhMOFkvajNGQmYvUFRRMjhldEZC?= =?utf-8?B?TTdaZmUwV01adTg4ZDNmQmR3NGhTM0paRVMzT3N4dlovSnZvSXM5T1lUUkxI?= =?utf-8?B?elE9PQ==?= Content-ID: <66846D4343AD0640A8CA1B7AC916A858@apcprd03.prod.outlook.com> MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: TYZPR03MB5566.apcprd03.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 1f20e6ab-6209-440a-48cd-08dc31baa0c7 X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Feb 2024 02:21:21.0451 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a7687ede-7a6b-4ef6-bace-642f677fbe31 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: RkcR11wp00Dr4agm7QAps0S5Wc+dEQkiwd2xsJA88Ruonox1AskFaRTVphZMvf4xgHK9ZzkaHUpDKI02hLKnqg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SEYPR03MB8006 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240219_182206_007839_9B7385A2 X-CRM114-Status: GOOD ( 22.53 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi AngeloGioacchino, Thanks for your review. On Wed, 2024-02-07 at 15:42 +0100, AngeloGioacchino Del Regno wrote: > Il 04/02/24 07:15, Zhi Mao ha scritto: > > Add a V4L2 sub-device driver for Galaxycore GC08A3 image sensor. > > > > Signed-off-by: Zhi Mao > > --- > > drivers/media/i2c/Kconfig | 10 + > > drivers/media/i2c/Makefile | 1 + > > drivers/media/i2c/gc08a3.c | 1448 > > ++++++++++++++++++++++++++++++++++++ > > 3 files changed, 1459 insertions(+) > > create mode 100644 drivers/media/i2c/gc08a3.c > > > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > > index 56f276b920ab..e4da68835683 100644 > > --- a/drivers/media/i2c/Kconfig > > +++ b/drivers/media/i2c/Kconfig > > @@ -70,6 +70,16 @@ config VIDEO_GC0308 > > To compile this driver as a module, choose M here: the > > module will be called gc0308. > > > > +config VIDEO_GC08A3 > > + tristate "GalaxyCore gc08a3 sensor support" > > + select V4L2_CCI_I2C > > + help > > + This is a Video4Linux2 sensor driver for the GalaxyCore > > gc08a3 > > + camera. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called gc08a3. > > + > > config VIDEO_GC2145 > > select V4L2_CCI_I2C > > tristate "GalaxyCore GC2145 sensor support" > > diff --git a/drivers/media/i2c/Makefile > > b/drivers/media/i2c/Makefile > > index dfbe6448b549..b82e99ca7578 100644 > > --- a/drivers/media/i2c/Makefile > > +++ b/drivers/media/i2c/Makefile > > @@ -38,6 +38,7 @@ obj-$(CONFIG_VIDEO_DW9768) += dw9768.o > > obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o > > obj-$(CONFIG_VIDEO_ET8EK8) += et8ek8/ > > obj-$(CONFIG_VIDEO_GC0308) += gc0308.o > > +obj-$(CONFIG_VIDEO_GC08A3) += gc08a3.o > > obj-$(CONFIG_VIDEO_GC2145) += gc2145.o > > obj-$(CONFIG_VIDEO_HI556) += hi556.o > > obj-$(CONFIG_VIDEO_HI846) += hi846.o > > diff --git a/drivers/media/i2c/gc08a3.c > > b/drivers/media/i2c/gc08a3.c > > new file mode 100644 > > index 000000000000..3fc7fffb815c > > --- /dev/null > > +++ b/drivers/media/i2c/gc08a3.c > > @@ -0,0 +1,1448 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * gc08a3.c - gc08a3 sensor driver > > + * > > + * Copyright 2023 MediaTek > > + * > > + * Zhi Mao > > + */ > > + > > ..snip.. > fixed in patch:v5. > > + > > +static const struct gc08a3_link_freq_config > > gc08a3_link_freq_336m_configs = { > > + .reg_list = { > > + .num_of_regs = ARRAY_SIZE(mode_table_common), > > + .regs = mode_table_common, > > + } > > +}; > > + > > +static const struct gc08a3_link_freq_config > > gc08a3_link_freq_207m_configs = { > > + .reg_list = { > > + .num_of_regs = ARRAY_SIZE(mode_table_common), > > + .regs = mode_table_common, > > + } > > +}; > > + > > Since you're documenting this structure anyway, why not kerneldoc? :- > ) > As these registers are the same, I followed Mr.Laurent's comments and droped this "gc08a3_link_freq_config" structure. > > +struct gc08a3_mode { > > + u32 width; > > + u32 height; > > + const struct gc08a3_reg_list reg_list; > > + > > + u32 hts; /* Horizontal timining size */ > > + u32 vts_def; /* Default vertical timining size */ > > + u32 vts_min; /* Min vertical timining size */ > > + u32 max_framerate; > > + const struct gc08a3_link_freq_config *link_freq_configs; > > +}; > > + > > +/* > > + * Declare modes in order, from biggest > > + * to smallest height. > > + */ > > one line is enough for this comment. > fixed in patch:v5. > > +static const struct gc08a3_mode gc08a3_modes[] = { > > + { > > + .width = GC08A3_NATIVE_WIDTH, > > + .height = GC08A3_NATIVE_HEIGHT, > > + .reg_list = { > > + .num_of_regs = ARRAY_SIZE(mode_3264x2448), > > + .regs = mode_3264x2448, > > + }, > > + .link_freq_configs = &gc08a3_link_freq_336m_configs, > > + > > + .hts = GC08A3_HTS_30FPS, > > + .vts_def = GC08A3_VTS_30FPS, > > + .vts_min = GC08A3_VTS_30FPS_MIN, > > + .max_framerate = 300, > > + }, > > + { > > + .width = 1920, > > + .height = 1080, > > + .reg_list = { > > + .num_of_regs = ARRAY_SIZE(mode_1920x1080), > > + .regs = mode_1920x1080, > > + }, > > + .link_freq_configs = &gc08a3_link_freq_207m_configs, > > + > > + .hts = GC08A3_HTS_60FPS, > > + .vts_def = GC08A3_VTS_60FPS, > > + .vts_min = GC08A3_VTS_60FPS_MIN, > > + .max_framerate = 600, > > + }, > > +}; > > + > > +static u64 to_pixel_rate(u32 f_index) > > +{ > > + u64 pixel_rate = link_freq_menu_items[f_index] * 2 * > > GC08A3_DATA_LANES; > > + > > + do_div(pixel_rate, GC08A3_RGB_DEPTH); > > The divisor is (less than) 32 bits and the dividend is always 64 > bits: that will > break on builds for 32-bits CPUs. > > Just do.... > > return div_u64(pixel_rate, GB08A3_RGB_DEPTH); > fixed in patch:v5. > > + > > + return pixel_rate; > > +} > > + > > +static int gc08a3_identify_module(struct gc08a3 *gc08a3) > > +{ > > + struct i2c_client *client = v4l2_get_subdevdata(&gc08a3->sd); > > + u64 val = 0; > > u64 val; > > > + int ret; > > + > > Either log here or in the probe function, otherwise it's just > redudant. > please review patch:v5. > > + ret = cci_read(gc08a3->regmap, GC08A3_REG_CHIP_ID, &val, NULL); > > + if (ret) { > > + dev_err(&client->dev, > > + "failed to read chip id: 0x%x", > > GC08A3_CHIP_ID); > > + return ret; > > + } > > + > > + if (val != GC08A3_CHIP_ID) { > > + dev_err(&client->dev, "chip id mismatch: 0x%x!=0x%llx", > > + GC08A3_CHIP_ID, val); > > + return -ENXIO; > > + } > > + > > + return 0; > > +} > > + > > +static inline struct gc08a3 *to_gc08a3(struct v4l2_subdev *sd) > > +{ > > + return container_of(sd, struct gc08a3, sd); > > +} > > + > > ..snip.. > fixed in patch:v5. > > + > > +static int gc08a3_update_cur_mode_controls(struct gc08a3 *gc08a3, > > + const struct gc08a3_mode > > *mode) > > +{ > > + s64 exposure_max, h_blank; > > + int ret = 0; > > + > > + ret = __v4l2_ctrl_modify_range(gc08a3->vblank, > > + mode->vts_min - mode->height, > > + GC08A3_VTS_MAX - mode->height, > > 1, > > + mode->vts_def - mode->height); > > + if (ret) > > + dev_err(gc08a3->dev, "VB ctrl range update failed\n"); > > + > > + h_blank = mode->hts - mode->width; > > + ret = __v4l2_ctrl_modify_range(gc08a3->hblank, h_blank, > > h_blank, 1, > > + h_blank); > > + if (ret) > > + dev_err(gc08a3->dev, "HB ctrl range update failed\n"); > > + > > + exposure_max = mode->vts_def - GC08A3_EXP_MARGIN; > > + ret = __v4l2_ctrl_modify_range(gc08a3->exposure, > > GC08A3_EXP_MIN, > > + exposure_max, GC08A3_EXP_STEP, > > + exposure_max); > > + if (ret) > > + dev_err(gc08a3->dev, "exposure ctrl range update > > failed\n"); > > No. You're not returning anywhere for error. That's not okay. > Besides... > > if (ret) { > dev_err.. > return ret; > } > > return 0; > fixed in patch:v5. > > + > > + return ret; > > +} > > + > > +static void gc08a3_update_pad_format(struct gc08a3 *gc08a3, > > + const struct gc08a3_mode *mode, > > + struct v4l2_mbus_framefmt *fmt) > > +{ > > + fmt->width = mode->width; > > + fmt->height = mode->height; > > + fmt->code = GC08A3_MBUS_CODE; > > + fmt->field = V4L2_FIELD_NONE; > > + fmt->colorspace = V4L2_COLORSPACE_SRGB; > > + fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace); > > + fmt->quantization = > > + V4L2_MAP_QUANTIZATION_DEFAULT(true, > > + fmt->colorspace, > > + fmt->ycbcr_enc); > > + fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace); > > +} > > + > > +static int gc08a3_set_format(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *state, > > + struct v4l2_subdev_format *fmt) > > +{ > > + struct gc08a3 *gc08a3 = to_gc08a3(sd); > > + struct v4l2_mbus_framefmt *mbus_fmt; > > + struct v4l2_rect *crop; > > + const struct gc08a3_mode *mode; > > + > > + mode = v4l2_find_nearest_size(gc08a3_modes, > > ARRAY_SIZE(gc08a3_modes), > > + width, height, fmt->format.width, > > + fmt->format.height); > > + > > + /*update crop info to subdev state*/ > > + crop = v4l2_subdev_state_get_crop(state, 0); > > + crop->width = mode->width; > > + crop->height = mode->height; > > + > > + /*update fmt info to subdev state*/ > > + gc08a3_update_pad_format(gc08a3, mode, &fmt->format); > > + mbus_fmt = v4l2_subdev_state_get_format(state, 0); > > + *mbus_fmt = fmt->format; > > + > > + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) > > + return 0; > > + > > + gc08a3->cur_mode = mode; > > + gc08a3_update_cur_mode_controls(gc08a3, mode); > > + > > + return 0; > > +} > > + > > +static int gc08a3_get_selection(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *state, > > + struct v4l2_subdev_selection *sel) > > +{ > > + struct gc08a3 *gc08a3 = to_gc08a3(sd); > > + > > + switch (sel->target) { > > + case V4L2_SEL_TGT_CROP: > > + sel->r = *v4l2_subdev_state_get_crop(state, 0); > > + break; > > + case V4L2_SEL_TGT_CROP_BOUNDS: > > + sel->r.top = 0; > > + sel->r.left = 0; > > + sel->r.width = GC08A3_NATIVE_WIDTH; > > + sel->r.height = GC08A3_NATIVE_HEIGHT; > > + break; > > + case V4L2_SEL_TGT_CROP_DEFAULT: > > + if (gc08a3->cur_mode->width == GC08A3_NATIVE_WIDTH) { > > + sel->r.top = 0; > > + sel->r.left = 0; > > + sel->r.width = gc08a3_modes[0].width; > > + sel->r.height = gc08a3_modes[0].height; > > + } else { > > + sel->r.top = 0; > > + sel->r.left = 0; > > + sel->r.width = gc08a3_modes[1].width; > > + sel->r.height = gc08a3_modes[1].height; > > + } > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +static int gc08a3_init_state(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *state) > > +{ > > + struct v4l2_subdev_format fmt = { > > + .which = V4L2_SUBDEV_FORMAT_TRY, > > + .pad = 0, > > + .format = { > > + .code = GC08A3_MBUS_CODE, > > + .width = gc08a3_modes[0].width, > > + .height = gc08a3_modes[0].height, > > + }, > > + }; > > + > > + gc08a3_set_format(sd, state, &fmt); > > + > > + return 0; > > +} > > + > > +static int gc08a3_set_ctrl_hflip(struct gc08a3 *gc08a3, u32 > > ctrl_val) > > +{ > > + int ret; > > + u64 val; > > + > > + ret = cci_read(gc08a3->regmap, GC08A3_FLIP_REG, &val, NULL); > > + if (ret) { > > + dev_err(gc08a3->dev, "read hflip register failed: > > %d\n", ret); > > + return ret; > > + } > > + > > + val = ctrl_val ? (val | GC08A3_FLIP_H_MASK) : > > + (val & ~GC08A3_FLIP_H_MASK); > > + ret = cci_write(gc08a3->regmap, GC08A3_FLIP_REG, val, NULL); > > + if (ret < 0) > > + dev_err(gc08a3->dev, "Error %d\n", ret); > > + > > + return ret; > > +} > > + > > +static int gc08a3_set_ctrl_vflip(struct gc08a3 *gc08a3, u32 > > ctrl_val) > > +{ > > + int ret; > > + u64 val; > > + > > + ret = cci_read(gc08a3->regmap, GC08A3_FLIP_REG, &val, NULL); > > + if (ret) { > > + dev_err(gc08a3->dev, "read vflip register failed: > > %d\n", ret); > > + return ret; > > + } > > + > > + val = ctrl_val ? (val | GC08A3_FLIP_V_MASK) : > > + (val & ~GC08A3_FLIP_V_MASK); > > + ret = cci_write(gc08a3->regmap, GC08A3_FLIP_REG, val, NULL); > > + if (ret < 0) > > + dev_err(gc08a3->dev, "Error %d\n", ret); > > + > > + return ret; > > +} > > + > > +static int gc08a3_test_pattern(struct gc08a3 *gc08a3, u32 > > pattern_menu) > > +{ > > + u32 pattern = 0; > > + int ret; > > ret not initialized is ok here; > > > + > > + if (pattern_menu) { > > + switch (pattern_menu) { > > + case 1: > > + pattern = 0x00; > > + break; > > + case 2: > > + pattern = 0x10; > > + break; > > + case 3: > > + case 4: > > + case 5: > > + case 6: > > + case 7: > > + pattern = pattern_menu + 1; > > + break; > > + } > > + > > + ret = cci_write(gc08a3->regmap, > > GC08A3_REG_TEST_PATTERN_EN, > > + GC08A3_TEST_PATTERN_EN, NULL); > > + if (ret) > > + return ret; > > + > > + ret = cci_write(gc08a3->regmap, > > GC08A3_REG_TEST_PATTERN_IDX, > > + pattern, NULL); > > if (ret) > return ret; > > > + > > + } else { > > + ret = cci_write(gc08a3->regmap, > > GC08A3_REG_TEST_PATTERN_EN, > > + 0x00, NULL); > > if (ret) > return ret; > > > + } > > + > > return 0; > fixed in patch:v5. > > + return ret; > > +} > > + > > +static int gc08a3_set_ctrl(struct v4l2_ctrl *ctrl) > > +{ > > + struct gc08a3 *gc08a3 = > > + container_of(ctrl->handler, struct gc08a3, ctrls); > > + int ret = 0; > > int ret; > > > + s64 exposure_max; > > + > > + if (ctrl->id == V4L2_CID_VBLANK) { > > + /* Update max exposure while meeting expected vblanking > > */ > > + exposure_max = gc08a3->cur_mode->height + ctrl->val - > > + GC08A3_EXP_MARGIN; > > + __v4l2_ctrl_modify_range(gc08a3->exposure, > > + gc08a3->exposure->minimum, > > + exposure_max, gc08a3- > > >exposure->step, > > + exposure_max); > > + } > > + > > + /* > > + * Applying V4L2 control value only happens > > + * when power is on for streaming > > + */ > > + if (!pm_runtime_get_if_in_use(gc08a3->dev)) > > + return 0; > > + > > + switch (ctrl->id) { > > + case V4L2_CID_EXPOSURE: > > + ret = cci_write(gc08a3->regmap, GC08A3_EXP_REG, > > + ctrl->val, NULL); > > + break; > > + > > + case V4L2_CID_ANALOGUE_GAIN: > > + ret = cci_write(gc08a3->regmap, GC08A3_AGAIN_REG, > > + ctrl->val, NULL); > > + break; > > + > > + case V4L2_CID_VBLANK: > > + ret = cci_write(gc08a3->regmap, > > GC08A3_FRAME_LENGTH_REG, > > + gc08a3->cur_mode->height + ctrl->val, > > NULL); > > + break; > > + > > + case V4L2_CID_HFLIP: > > + ret = gc08a3_set_ctrl_hflip(gc08a3, ctrl->val); > > + break; > > + > > + case V4L2_CID_VFLIP: > > + ret = gc08a3_set_ctrl_vflip(gc08a3, ctrl->val); > > + break; > > + > > + case V4L2_CID_TEST_PATTERN: > > + ret = gc08a3_test_pattern(gc08a3, ctrl->val); > > + break; > > + > > + default: > > + break; > > + } > > + > > + pm_runtime_put(gc08a3->dev); > > if (ret) > return ret; > > return 0; > As "default" case is not assign the variable "ret", this return value is not expected, so we need initialize the variable "ret=0". Can we keep this coding style? > > + > > + return ret; > > +} > > + > > ...and I've ignored some more stuff as other reviewers already gave > you feedback. > > Regards, > Angelo > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel