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 05EEFC433F5 for ; Fri, 10 Dec 2021 11:09:39 +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:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Content-ID:In-Reply-To: References:Message-ID:Date:Subject:To:From:Reply-To:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Flcq9r1FIEeVsBdHPwBjJBC/qo1VwGUWMd9bVpdDLdE=; b=yT+lET3eFcAgG/ 0ohVFDFU7SC9rYrre6Ab+D6H852OLAn3AcuF070Nh1+SPiKx3SAaGViecqLd39Hgh0Pcn82OgWUcZ 3u26UE9s1v9Umi08APoRCKvco84S2wMwh6YYyJUR0RDOVS7rWszo1s66jtt8nrCmDZ2eqC4ow5lKI 89Ty+tWYLp5jhGiGL3hr6AfJyn1PPQmmTSwZtPLio8XilLBmAXjVmqlea4G7sjl/lb9CnrnmiXkka PsA+Vc8PNiK8QcD0mqNuqWZvCLLbRIGqnXJj7UlVVzOiiUiQAyjqsnAOxhaHVm1Pd67MsTNKjX/rQ UPsX4Os1pxok/PUQYFoA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mvdkT-001arw-UA; Fri, 10 Dec 2021 11:07:34 +0000 Received: from esa.microchip.iphmx.com ([68.232.153.233]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mvdkK-001ap3-Qg for linux-arm-kernel@lists.infradead.org; Fri, 10 Dec 2021 11:07:32 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1639134444; x=1670670444; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=IrstbjfMfDKwR6xg/iG9ROJUuKIhc+XiibJy83BOCAw=; b=n1vFL4w4f3YSPRJPF5C3kAshu93HmB+tNVl0J9p1a1izvvQKeFaJXRMQ Qq2hlc8rX4HVfaQGjEI7KXGw/xh4UVaBOx8xHb87MfoA38lqywadrHyD6 nRfgDBY7z1Om/XnW4Q+Bg280/SqfAftnAQlRSYTc2xPckp3NnJwFWeLOD q+grT7NDiHPo26vm+R+F2sPF13vSwQsLKSNlG6l61k2sJCXSAl/qIYuto 1i/W95OYFCJ+wOboB4Ne29urdo3WoUYNRcYg1UgV53M/gzdiPahLK2+mb e1E+oyULmDKVDwYIgurPRzQABQbmvqKd1jo5mFkG2WwDaZCEOUVs9D1sC A==; IronPort-SDR: 4aU95doKbQAIdvJbLnSH+7KDuz8u3ldYEhtnmJOf4yAPQ1fco6eFIbPO0C1ubxrsvOOBhaGKCM GIFfFLph8kvNJtvOWZfln9ZlBQTsMG9Y+Z8oaPlkU1jOu5mlnWstONlHGM+F7KKMyGwZo86D4u CR2PuHLTF47zJ5PiiBRme8hkS+Klmzhvx4BO8vWvVWz2GwQWfClMqH3EXO4a1IgIQV4yrmOhL7 U+/3agU2d1d0pxwX8P0NbtUvNAfvLQ6GqYUWdNyiWde6cqB22CJUtcQbOE7mznM6bpkOaqSPLv tv7d6pqU02YgUIAwJqmuZiMJ X-IronPort-AV: E=Sophos;i="5.88,195,1635231600"; d="scan'208";a="146798894" Received: from smtpout.microchip.com (HELO email.microchip.com) ([198.175.253.82]) by esa3.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 10 Dec 2021 04:07:21 -0700 Received: from chn-vm-ex03.mchp-main.com (10.10.85.151) by chn-vm-ex04.mchp-main.com (10.10.85.152) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.14; Fri, 10 Dec 2021 04:07:20 -0700 Received: from NAM11-DM6-obe.outbound.protection.outlook.com (10.10.215.89) by email.microchip.com (10.10.87.152) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.17 via Frontend Transport; Fri, 10 Dec 2021 04:07:20 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=XXFsHuqPlKWz2hP83ayoMMMmKkqT3FgMV+9MrfJg0vWGvebSY8ke43RlzEqi9GM8umh8wSNj+mFyL09uLJV1yJK24G22ZgkQ27W6P5td8nF8qQjBIE429ysKnlAsLI9mUGv9ibH5vdwwT5I3UEFXpJJ+xlru397SyZp6EX3XhNM1t0CiffF4VckDvd5axdMC3bUzQLt4x6QSbAoAw2A79bOx2Zm5JTp1wrxlGcEtmk/Rf3sSWp3nAznwiGi2b9n1PfOGv2vlemjAHV5FNqD13Rx2j07P7R6iaaGZk4Mu6XPezLQ8xdQ9o9mGjRBMZ2FNHTpr0zu6yFmzsCd76uM7Zw== 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=IrstbjfMfDKwR6xg/iG9ROJUuKIhc+XiibJy83BOCAw=; b=Wz4JI/7ni1M+Ez84HdPpFsL0L7EIPNRl8LiGQfk4eiS7qI5k6om1CrBhxbdCMISlt+AHVQyiR7xLMIn97TgngqCz0k/aj6r32iYn24OR/456oa/T8EjXAu0K0r2A1RuV51KK4xa5KOD/4zw7A0vv6qIYlimWBb6dguR/s3UBCxJQ5eXDISGVyFDmgq4mqeqW9ejZcSGySvjpe84cusDmunC26c2RAs7DeGATA/TP/P+1Lzdn3ZvMcvwxYZZGafy45BG393ZAnL6X9HcGbN+hb3EBvURlxHSjeYLCr4alVrancWNpEPVe4a786m6+NYcV3DtX3NrKBDmbgI51aEqFTw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microchip.com; dmarc=pass action=none header.from=microchip.com; dkim=pass header.d=microchip.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microchiptechnology.onmicrosoft.com; s=selector2-microchiptechnology-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=IrstbjfMfDKwR6xg/iG9ROJUuKIhc+XiibJy83BOCAw=; b=oqFzOcEiZ6Xkc3D3VsDSf/jDwmZ+cfnjTqZWT5hm1LC8H5EAP0rzGnjYbWW5fZNq8nf/N07aoYgffatgEo/wmVbcJx+sVQDyLFINjgaLdzta/7gLdTMDP0xRla1cQUDIT8C3//oaA+ePr9l7YLvlo1Gj8GwucMRUgPcNIY243sg= Received: from PH0PR11MB4920.namprd11.prod.outlook.com (2603:10b6:510:41::22) by PH0PR11MB4949.namprd11.prod.outlook.com (2603:10b6:510:31::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4755.24; Fri, 10 Dec 2021 11:07:15 +0000 Received: from PH0PR11MB4920.namprd11.prod.outlook.com ([fe80::1433:9dcf:4912:b8f]) by PH0PR11MB4920.namprd11.prod.outlook.com ([fe80::1433:9dcf:4912:b8f%9]) with mapi id 15.20.4778.016; Fri, 10 Dec 2021 11:07:15 +0000 From: To: Subject: Re: [PATCH v2 03/25] media: atmel: introduce microchip csi2dc driver Thread-Topic: [PATCH v2 03/25] media: atmel: introduce microchip csi2dc driver Thread-Index: AQHX19FOge8HD4Ymu0y44q0z2b5AxKwlbt8AgAAOKgCAAAa8AIAGOL8A Date: Fri, 10 Dec 2021 11:07:15 +0000 Message-ID: <3d5cf51e-3ea0-9fff-b9f8-cd0eadb4ea2b@microchip.com> References: <20211112142509.2230884-1-eugen.hristev@microchip.com> <20211112142509.2230884-4-eugen.hristev@microchip.com> <20211206105142.chmrks6sueohw7sx@uno.localdomain> <1fca04a4-ab5c-15cc-61ab-829d39a63ec8@microchip.com> <20211206120630.mlciq2662azznesu@uno.localdomain> In-Reply-To: <20211206120630.mlciq2662azznesu@uno.localdomain> Accept-Language: en-US, ro-RO Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=microchip.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 52c99f50-e2ca-4ec0-f8f5-08d9bbcd396f x-ms-traffictypediagnostic: PH0PR11MB4949:EE_ x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:5236; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: SNdQw0NwCZyqgM+IRr36xtoaGpLYCIuocOGBH4U6NpsPeVIn8uUD6G8a78hc1N6e7LubLVCe3vF1cFY/SAMDHcpTX3f6hiZRevGd/ZFf1+g5NN5fntiZbdiMmubhIakx0+pM8GuhW7M4Y8VHM0CQtgCpeyerAom25yP9/a8V2XejN7M6InPP+it2n8H4JfCG33RbFzs36YOR8MqrnDCOysu7VX/hPN1gq8OXHgrlUje06NqtkTshs7YihYoAFLx2DG1XLbKzQIciVN5FFL3CDS3l4l7p5aSzkBg0TFv20poqSSw7UML79QBd3Dy66Qj7sOCiqPqxojzi9C/+ztw+GtIahgSW09GHQ+7/En1eB6h+NEN4V46i7CVLAOQZYgxtKC5gF1dPpeK5fY+gZuNgb/zAcGKCPvLB32BmqDhZ+jSRNBXY8V1pNsKnBhNkYIZvc192D3YClAlaCgr77NxWioRANxPjgcdCjBqlZUkEDHZL7QJI/CNEk1W6ckW/Gt8dhCrubpkOOCsKN5od28AjbyFZvJvL+yceZHE2Y+omhOTbSk++MrL7AgMaCLmhVy9kQTsFntuUnrT3AeaQl8gACRlcO63DuF4EL3wz+FrSGO3DSQFJ/P6vHUfsIL5VGFdrTJPHjRMWvd/JMPrZWXRz/jOFqX+eTb1dURptU4VlnJLTry3oIfDyQubKbJLryBwAICCUJQ77jsOPMfCqAGoKp0AFWbAqcsHMZ0xujuZCFvZXFCNsqe9O9BfBaDgJo2RM35+9S93CHEfvug5UlrAGg6sNAYyVY5H6SPWHryul5YAV3ETRYAC0Xqv1mN7PswDxlpGhlkSWFEnB2qsukdmDqBUE7dYSy+cNIikf1TuJsus= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:PH0PR11MB4920.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(366004)(26005)(2906002)(83380400001)(107886003)(91956017)(30864003)(54906003)(31686004)(4326008)(5660300002)(508600001)(31696002)(38070700005)(316002)(6916009)(186003)(53546011)(36756003)(6506007)(66556008)(66476007)(2616005)(6512007)(71200400001)(66946007)(8936002)(64756008)(8676002)(76116006)(86362001)(122000001)(966005)(38100700002)(66446008)(45080400002)(6486002)(45980500001)(43740500002)(579004)(559001); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?utf-8?B?R3M0dDZBcWo4SHdEc2RsakM5MGlGSThNSlhHbUplbk82TEZQNWxONGtJMzlW?= =?utf-8?B?MEd6Rm9rNGJOaStqMnF3d09ac2ZKTHFtc0h2SUNHNnFoNi9zcVBwT0JTZFRi?= =?utf-8?B?dXQrRldmSmNrdEd4YmJXYmMyUkRsbU0xcmJBTlNleXJxcy9wRnNuc3FwamlO?= =?utf-8?B?ZkI0bkJPRElwMGgxcHpVNFNyaFVGemJpcUFKREYzWFhXUFpWVzQrbUo3aGpT?= =?utf-8?B?SVljcDdzUVdPZnlGN3RpWjh0MVlQU1FDY0FGNlQzb0tmbUU5d3UxMHdvdVRK?= =?utf-8?B?SkY1TU91MC93VVhRelh4czNsRHMwcFpudGVva0gyNEJuVGQ5a1NQTS9oQzFH?= =?utf-8?B?Y3lnVGt1bDg1bE1JNFBTMXhKa2NxRVc2N09qTyt1bDdxYlRSaUpEK3NxSjYx?= =?utf-8?B?N1VWanU0NzBGRS9RZGppWkRuZU5PK1dmZVJMa2plZ3JlRThsVGszZ3d4Ympi?= =?utf-8?B?U3VUMFNHc2YwQW5zTlFaYnJBNzJnN3RLT2h3MlVqa0dDMXhNWU1VTmJIcDBw?= =?utf-8?B?WUoyRFZPbUQzb0QzNmRpSDZpNnJxbkFKM2hYYkpUclN0ejY5c0ZWSTR5Zitr?= =?utf-8?B?Qm5WWG5YVVh1eHloMFhGTWFlWXNqcTVxc2VQVEl0RHVoK096VmZQSnNzTE1K?= =?utf-8?B?VERrdlJST0ZBT0l6UG9BQ2g1YkJOdCtGS2c4cE1QcUliNHkxelk0UklVelJ0?= =?utf-8?B?TnduZXdDRU1xZ2VOVDBkbVp2UTFKbnZkVjBLdmlVaEluZDRpRHUzOG1mY1d1?= =?utf-8?B?Vnd1QWV5NlY3SWxVOHN1b1BnNlpCUlRJRzREUm4raEhBUEJMaml2djg4dGpG?= =?utf-8?B?SHVJR01lUGYvaDM2RzgvZzZIQ3YwZ01PeDFQVjI3TEtwRG40aHJGTXk3cjFV?= =?utf-8?B?UTVvMUlhRVNoNWlnMnJrS1lIRjMrK1JpLzdYKzZ3ODBPNVo4Q1F0YU1WT3lJ?= =?utf-8?B?aUNNL0VpNVcrWTRSVy92VzVsQ3hoZ2NKcitnYWJBN2FwM243c2p6TUpWK0Zm?= =?utf-8?B?ZFNZcXF0RG9QS1JVVW4rRkUxV2lNV3BpNnNnNTczQjNYYjgxMmZFcUxFSlFj?= =?utf-8?B?TlVkNmxSUWlmOVZRN2kxM05tdTc5bDFJUjZJZnk5WnRKbTh2M1MwUWlTYU4z?= =?utf-8?B?RnRCcTZIT2NvRnBvdENyM09hU3I1RVArWjRTQ2NKazJrd2RPUHljWStRQ2lW?= =?utf-8?B?clBhK3I0aytMemZFbkRHSmFKaGZObmxuRDZEL2h3eWZKRlN5Kzc0cjdqVi9s?= =?utf-8?B?Wi93UDhaT1pCWEF2QVZzOS9DcWs2b0g4OGhoWHFRdXZEZXZKTXhoMHlDMVpI?= =?utf-8?B?VmJ6LzhqeFZoekc3M0NVR2V0azFKMWRudFk3alJ1Z1E1Q2NWMzNZbFBLbXVv?= =?utf-8?B?U0wyYVdEcUNpZGptV05jSDFDWENkYkdxUTd1Sis0T3VHOUFiZCsxNVhiemVB?= =?utf-8?B?U3M2NHR2amZSQitDVUd4dzhHL2pxQnkxU3o5WVRYK3NuUDV1cDZoR2xsZldB?= =?utf-8?B?bmJtd0tLcyt1a1BYSFJPSkxhZmI4bWZrWHFhYXVYSnZwR0lOSlhES3UxdGlh?= =?utf-8?B?d3FCcDQ5SXJaamZGTHYrVkFoRkNtKzFzK0ZqbUxaR1FsWWV1cnlwT0h6UDNi?= =?utf-8?B?Y0FYdHVlV29ZNHA3THZjeDBQKzdIZFBCVmZKaHdYR0FJdWFzVGtPNnp4WGJM?= =?utf-8?B?c09pSE9tZ2NxT3R5ZGE4Y0VKclpJRHB3dUdjUjYzQklRNTY1VDllRkdseHlG?= =?utf-8?B?WTNiRVZpWkZPTXdoLzFtaHFPSUQ4dUphdXNrSzBBcGg3UUpieDhNTDZ6ZG00?= =?utf-8?B?cld2RjYxeFRnZjQzZEs3RnQ3Wk12US9Ob1o0b3FuNzgyM2hNRHphTHp2UVlR?= =?utf-8?B?ODFFSTBVSGw3MUdzclBRM2NJZG9aeUIrL1lYQXZVczRTaWJSTitlT2RPNHlP?= =?utf-8?B?cjhQb25NWGVWcXZvV01RejVuVEJwWDFmZjlKSEZLL3dkLy9wTHI2bnJmamdD?= =?utf-8?B?MzVpc0xieUM0N0lRMHlmaSsweEplcEhoeVBJN0ZxME5RV1JZeHFwSXp2bEZt?= =?utf-8?B?MlJMS2I1djdjSWY2UFVueGNyaWU1eEh1NWtwSGpSVTl0djM4c3o3RXlFQWtJ?= =?utf-8?B?WkZmalhXdjNwTzRWVVRGUzBjUjUxZUo4MmNmLzVZMkV3WTM2NFNnd1ZsNEZr?= =?utf-8?B?TllBU2s1R0lGeVBydkQwOXpTMUg1ZjZ6SHQrS3cyWnBJdE1TQTErSmpzTE1q?= =?utf-8?B?aHVkWFd0Y3ZpcTJoNGZpc2x0MmlBPT0=?= Content-ID: MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4920.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 52c99f50-e2ca-4ec0-f8f5-08d9bbcd396f X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Dec 2021 11:07:15.4282 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: lu+EAa3a/180+2AsK31DOTPEt/5NwwyuWoiYLCMWcXlftbvs0Mk51hUZRqgmixkmxEiFjwTJm/gPx898XuYFH03OPGPHN2T7mLP44NjdGvY= X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4949 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211210_030724_988499_BD867D51 X-CRM114-Status: GOOD ( 20.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: , Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, sakari.ailus@iki.fi, laurent.pinchart@ideasonboard.com, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org 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 On 12/6/21 2:06 PM, Jacopo Mondi wrote: > Hi Eugen > > On Mon, Dec 06, 2021 at 11:42:25AM +0000, Eugen.Hristev@microchip.com wrote: >> On 12/6/21 12:51 PM, Jacopo Mondi wrote: >>> Hello Eugen >>> >>> thanks for addressing all my previous comments, just a few more >>> things and the driver looks good to me >> >> Thanks for reviewing. I tried my best to accomodate your suggestions. >>> >>> On Fri, Nov 12, 2021 at 04:24:47PM +0200, Eugen Hristev wrote: >>>> Microchip CSI2DC (CSI2 Demultiplexer Controller) is a misc bridge device >>>> that converts a byte stream in IDI Synopsys format (coming from a CSI2HOST) >>>> to a pixel stream that can be captured by a sensor controller. >>>> >>>> Signed-off-by: Eugen Hristev >>>> --- >>>> Changes in v2: >>>> - implement try formats >>>> - added parallel mode >>>> - moved to fwlink endpoints >>>> - many other minor corrections from Jacopo's review >>>> >>>> Changes in this revision: >>>> - addressed comments by Jacopo and Laurent as in this thread: >>>> https://www.spinics.net/lists/linux-media/msg181044.html >>>> >>>> Previous change log : >>>> Changes in v5: >>>> - only in bindings >>>> >>>> Changes in v4: >>>> - now using get_mbus_config ops to get data from the subdevice, like the >>>> virtual channel id, and the clock type. >>>> - now having possibility to select any of the RAW10 data modes >>>> - at completion time, select which formats are also available in the subdevice, >>>> and move to the dynamic list accordingly >>>> - changed the pipeline integration, do not advertise subdev ready at probe time. >>>> wait until completion is done, and then start a workqueue that will register >>>> this device as a subdevice for the next element in pipeline. >>>> - moved the s_power code into a different function called now csi2dc_power >>>> that is called with CONFIG_PM functions. This is also called at completion, >>>> to have the device ready in case CONFIG_PM is not selected on the platform. >>>> - merged try_fmt into set_fmt >>>> - driver cleanup, wrapped lines over 80 characters >>>> >>>> Changes in v2: >>>> - moved driver to platform/atmel >>>> - fixed minor things as per Sakari's review >>>> - still some things from v2 review are not yet addressed, to be followed up >>>> >>>> >>>> drivers/media/platform/Makefile | 1 + >>>> drivers/media/platform/atmel/Kconfig | 15 + >>>> drivers/media/platform/atmel/Makefile | 1 + >>>> .../media/platform/atmel/microchip-csi2dc.c | 797 ++++++++++++++++++ >>>> 4 files changed, 814 insertions(+) >>>> create mode 100644 drivers/media/platform/atmel/microchip-csi2dc.c >>>> >>>> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile >>>> index 832357240e89..b18e5f704145 100644 >>>> --- a/drivers/media/platform/Makefile >>>> +++ b/drivers/media/platform/Makefile >>>> @@ -69,6 +69,7 @@ obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar-vin/ >>>> obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel/ >>>> obj-$(CONFIG_VIDEO_ATMEL_ISI) += atmel/ >>>> obj-$(CONFIG_VIDEO_ATMEL_XISC) += atmel/ >>>> +obj-$(CONFIG_VIDEO_MICROCHIP_CSI2DC) += atmel/ >>>> >>>> obj-$(CONFIG_VIDEO_STM32_DCMI) += stm32/ >>>> >>>> diff --git a/drivers/media/platform/atmel/Kconfig b/drivers/media/platform/atmel/Kconfig >>>> index dda2f27da317..f83bee373d82 100644 >>>> --- a/drivers/media/platform/atmel/Kconfig >>>> +++ b/drivers/media/platform/atmel/Kconfig >>>> @@ -40,3 +40,18 @@ config VIDEO_ATMEL_ISI >>>> help >>>> This module makes the ATMEL Image Sensor Interface available >>>> as a v4l2 device. >>>> + >>>> +config VIDEO_MICROCHIP_CSI2DC >>>> + tristate "Microchip CSI2 Demux Controller" >>>> + depends on VIDEO_V4L2 && COMMON_CLK && OF >>>> + depends on ARCH_AT91 || COMPILE_TEST >>>> + select MEDIA_CONTROLLER >>>> + select VIDEO_V4L2_SUBDEV_API >>>> + select V4L2_FWNODE >>>> + help >>>> + CSI2 Demux Controller driver. CSI2DC is a helper chip >>>> + that converts IDI interface byte stream to a parallel pixel stream. >>>> + It supports various RAW formats as input. >>>> + >>>> + To compile this driver as a module, choose M here: the >>>> + module will be called microchip-csi2dc. >>>> diff --git a/drivers/media/platform/atmel/Makefile b/drivers/media/platform/atmel/Makefile >>>> index 46d264ab7948..39f0a7eba702 100644 >>>> --- a/drivers/media/platform/atmel/Makefile >>>> +++ b/drivers/media/platform/atmel/Makefile >>>> @@ -6,3 +6,4 @@ obj-$(CONFIG_VIDEO_ATMEL_ISI) += atmel-isi.o >>>> obj-$(CONFIG_VIDEO_ATMEL_ISC_BASE) += atmel-isc-base.o >>>> obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc.o >>>> obj-$(CONFIG_VIDEO_ATMEL_XISC) += atmel-xisc.o >>>> +obj-$(CONFIG_VIDEO_MICROCHIP_CSI2DC) += microchip-csi2dc.o >>>> diff --git a/drivers/media/platform/atmel/microchip-csi2dc.c b/drivers/media/platform/atmel/microchip-csi2dc.c >>>> new file mode 100644 >>>> index 000000000000..2b106f6fd5d0 >>>> --- /dev/null >>>> +++ b/drivers/media/platform/atmel/microchip-csi2dc.c >>>> @@ -0,0 +1,797 @@ >>>> +// SPDX-License-Identifier: GPL-2.0-only >>>> +/* >>>> + * Microchip CSI2 Demux Controller (CSI2DC) driver >>>> + * >>>> + * Copyright (C) 2018 Microchip Technology, Inc. >>>> + * >>>> + * Author: Eugen Hristev >>>> + * >>>> + */ >>>> + >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> + >>>> +#include >>>> +#include >>>> + >>>> +/* Global configuration register */ >>>> +#define CSI2DC_GCFG 0x0 >>>> + >>>> +/* MIPI sensor pixel clock is free running */ >>>> +#define CSI2DC_GCFG_MIPIFRN BIT(0) >>>> +/* GPIO parallel interface selection */ >>>> +#define CSI2DC_GCFG_GPIOSEL BIT(1) >>>> +/* Output waveform inter-line minimum delay */ >>>> +#define CSI2DC_GCFG_HLC(v) ((v) << 4) >>>> +#define CSI2DC_GCFG_HLC_MASK GENMASK(7, 4) >>>> +/* SAMA7G5 requires a HLC delay of 15 */ >>>> +#define SAMA7G5_HLC (15) >>>> + >>>> +/* Global control register */ >>>> +#define CSI2DC_GCTLR 0x04 >>>> +#define CSI2DC_GCTLR_SWRST BIT(0) >>>> + >>>> +/* Global status register */ >>>> +#define CSI2DC_GS 0x08 >>>> + >>>> +/* SSP interrupt status register */ >>>> +#define CSI2DC_SSPIS 0x28 >>>> +/* Pipe update register */ >>>> +#define CSI2DC_PU 0xc0 >>>> +/* Video pipe attributes update */ >>>> +#define CSI2DC_PU_VP BIT(0) >>>> + >>>> +/* Pipe update status register */ >>>> +#define CSI2DC_PUS 0xc4 >>>> + >>>> +/* Video pipeline enable register */ >>>> +#define CSI2DC_VPE 0xf8 >>>> +#define CSI2DC_VPE_ENABLE BIT(0) >>>> + >>>> +/* Video pipeline configuration register */ >>>> +#define CSI2DC_VPCFG 0xfc >>>> +/* Data type */ >>>> +#define CSI2DC_VPCFG_DT(v) ((v) << 0) >>>> +#define CSI2DC_VPCFG_DT_MASK GENMASK(5, 0) >>>> +/* Virtual channel identifier */ >>>> +#define CSI2DC_VPCFG_VC(v) ((v) << 6) >>>> +#define CSI2DC_VPCFG_VC_MASK GENMASK(7, 6) >>>> +/* Decompression enable */ >>>> +#define CSI2DC_VPCFG_DE BIT(8) >>>> +/* Decoder mode */ >>>> +#define CSI2DC_VPCFG_DM(v) ((v) << 9) >>>> +#define CSI2DC_VPCFG_DM_DECODER8TO12 0 >>>> +/* Decoder predictor 2 selection */ >>>> +#define CSI2DC_VPCFG_DP2 BIT(12) >>>> +/* Recommended memory storage */ >>>> +#define CSI2DC_VPCFG_RMS BIT(13) >>>> +/* Post adjustment */ >>>> +#define CSI2DC_VPCFG_PA BIT(14) >>>> + >>>> +/* Video pipeline column register */ >>>> +#define CSI2DC_VPCOL 0x100 >>>> +/* Column number */ >>>> +#define CSI2DC_VPCOL_COL(v) ((v) << 0) >>>> +#define CSI2DC_VPCOL_COL_MASK GENMASK(15, 0) >>>> + >>>> +/* Video pipeline row register */ >>>> +#define CSI2DC_VPROW 0x104 >>>> +/* Row number */ >>>> +#define CSI2DC_VPROW_ROW(v) ((v) << 0) >>>> +#define CSI2DC_VPROW_ROW_MASK GENMASK(15, 0) >>>> + >>>> +/* Version register */ >>>> +#define CSI2DC_VERSION 0x1fc >>>> + >>>> +/* register read/write helpers */ >>>> +#define csi2dc_readl(st, reg) readl_relaxed((st)->base + (reg)) >>>> +#define csi2dc_writel(st, reg, val) writel_relaxed((val), \ >>>> + (st)->base + (reg)) >>>> + >>>> +/* supported RAW data types */ >>>> +#define CSI2DC_DT_RAW6 0x28 >>>> +#define CSI2DC_DT_RAW7 0x29 >>>> +#define CSI2DC_DT_RAW8 0x2a >>>> +#define CSI2DC_DT_RAW10 0x2b >>>> +#define CSI2DC_DT_RAW12 0x2c >>>> +#define CSI2DC_DT_RAW14 0x2d >>>> + >>>> +/* >>>> + * struct csi2dc_format - CSI2DC format type struct >>>> + * @mbus_code: Media bus code for the format >>>> + * @dt: Data type constant for this format >>>> + */ >>>> +struct csi2dc_format { >>>> + u32 mbus_code; >>>> + u32 dt; >>>> +}; >>>> + >>>> +static const struct csi2dc_format csi2dc_formats[] = { >>>> + { >>>> + .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8, >>>> + .dt = CSI2DC_DT_RAW8, >>>> + }, { >>>> + .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8, >>>> + .dt = CSI2DC_DT_RAW8, >>>> + }, { >>>> + .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8, >>>> + .dt = CSI2DC_DT_RAW8, >>>> + }, { >>>> + .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8, >>>> + .dt = CSI2DC_DT_RAW8, >>>> + }, { >>>> + .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10, >>>> + .dt = CSI2DC_DT_RAW10, >>>> + }, { >>>> + .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10, >>>> + .dt = CSI2DC_DT_RAW10, >>>> + }, { >>>> + .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10, >>>> + .dt = CSI2DC_DT_RAW10, >>>> + }, { >>>> + .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10, >>>> + .dt = CSI2DC_DT_RAW10, >>>> + }, { >>>> + .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8, >>> >>> Is this intentionally not associated with a CSI-2 DT code ? >> >> Yes. It is not a raw format. >> I do not have a sensor that could stream YUYV8_2X8 with CSI-2 . >> I tested this mode with parallel sensor, and in this case, CSI-2 DT >> value does not make sense and it's not used. >> The idea would be that when and if I get my hands on a sensor that could >> actually do this with CSI2, I would update the driver here (if it works. >> I am not sure it actually does ...) > > I understand, but the first user of your driver that connects a > YUV-capable sensor will be very disappointed, as debugging the fact > that the DT identifier is silently set to 0 will be quite painful (as > if I'm not mistaken fields not initialized in a designated initializer > list are set to 0). > > As the DT identifiers are defined by the CSI-2 specification and will > not change, I think you can safely add it here. > > Thanks > j > >>> >>>> + }, >>>> +}; >>>> + >>>> +enum mipi_csi_pads { >>>> + CSI2DC_PAD_SINK = 0, >>>> + CSI2DC_PAD_SOURCE = 1, >>>> + CSI2DC_PADS_NUM = 2, >>>> +}; >>>> + >>>> +/* >>>> + * struct csi2dc_device - CSI2DC device driver data/config struct >>>> + * @base: Register map base address >>>> + * @csi2dc_sd: v4l2 subdevice for the csi2dc device >>>> + * This is the subdevice that the csi2dc device itself >>>> + * registers in v4l2 subsystem >>>> + * @dev: struct device for this csi2dc device >>>> + * @pclk: Peripheral clock reference >>>> + * Input clock that clocks the hardware block internal >>>> + * logic >>>> + * @scck: Sensor Controller clock reference >>>> + * Input clock that is used to generate the pixel clock >>>> + * @format: Current saved format used in g/s fmt >>>> + * @cur_fmt: Current state format >>>> + * @try_fmt: Try format that is being tried >>>> + * @pads: Media entity pads for the csi2dc subdevice >>>> + * @clk_gated: Whether the clock is gated or free running >>>> + * @video_pipe: Whether video pipeline is configured >>>> + * @parallel_mode: The underlying subdevice is connected on a parallel bus >>>> + * @vc: Current set virtual channel >>>> + * @notifier: Async notifier that is used to bound the underlying >>>> + * subdevice to the csi2dc subdevice >>>> + * @input_sd: Reference to the underlying subdevice bound to the >>>> + * csi2dc subdevice >>>> + * @remote_pad: Pad number of the underlying subdevice that is linked >>>> + * to the csi2dc subdevice sink pad. >>>> + */ >>>> +struct csi2dc_device { >>>> + void __iomem *base; >>>> + struct v4l2_subdev csi2dc_sd; >>>> + struct device *dev; >>>> + struct clk *pclk; >>>> + struct clk *scck; >>>> + >>>> + struct v4l2_mbus_framefmt format; >>>> + >>>> + const struct csi2dc_format *cur_fmt; >>>> + const struct csi2dc_format *try_fmt; >>>> + >>>> + struct media_pad pads[CSI2DC_PADS_NUM]; >>>> + >>>> + bool clk_gated; >>>> + bool video_pipe; >>>> + bool parallel_mode; >>>> + u32 vc; >>>> + >>>> + struct v4l2_async_notifier notifier; >>>> + >>>> + struct v4l2_subdev *input_sd; >>>> + >>>> + u32 remote_pad; >>>> +}; >>>> + >>>> +static inline struct csi2dc_device * >>>> +csi2dc_sd_to_csi2dc_device(struct v4l2_subdev *csi2dc_sd) >>>> +{ >>>> + return container_of(csi2dc_sd, struct csi2dc_device, csi2dc_sd); >>>> +} >>>> + >>>> +static int csi2dc_enum_mbus_code(struct v4l2_subdev *csi2dc_sd, >>>> + struct v4l2_subdev_state *sd_state, >>>> + struct v4l2_subdev_mbus_code_enum *code) >>>> +{ >>>> + if (code->index >= ARRAY_SIZE(csi2dc_formats)) >>>> + return -EINVAL; >>>> + >>>> + code->code = csi2dc_formats[code->index].mbus_code; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int csi2dc_get_fmt(struct v4l2_subdev *csi2dc_sd, >>>> + struct v4l2_subdev_state *sd_state, >>>> + struct v4l2_subdev_format *format) >>>> +{ >>>> + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); >>>> + struct v4l2_mbus_framefmt *v4l2_try_fmt; >>>> + >>>> + if (format->which == V4L2_SUBDEV_FORMAT_TRY) { >>>> + v4l2_try_fmt = v4l2_subdev_get_try_format(csi2dc_sd, sd_state, >>>> + format->pad); >>>> + format->format = *v4l2_try_fmt; >>>> + >>>> + return 0; >>>> + } >>>> + >>>> + format->format = csi2dc->format; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int csi2dc_set_fmt(struct v4l2_subdev *csi2dc_sd, >>>> + struct v4l2_subdev_state *sd_state, >>>> + struct v4l2_subdev_format *req_fmt) >>>> +{ >>>> + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); >>>> + const struct csi2dc_format *fmt, *try_fmt = NULL; >>>> + struct v4l2_mbus_framefmt *v4l2_try_fmt; >>>> + unsigned int i; >>>> + >>>> + for (i = 0; i < ARRAY_SIZE(csi2dc_formats); i++) { >>>> + fmt = &csi2dc_formats[i]; >>>> + if (req_fmt->format.code == fmt->mbus_code) >>>> + try_fmt = fmt; >>>> + fmt++; >>>> + } >>>> + >>>> + /* in case we could not find the desired format, default to something */ >>>> + if (!try_fmt) { >>>> + try_fmt = &csi2dc_formats[0]; >>>> + >>>> + dev_dbg(csi2dc->dev, >>>> + "CSI2DC unsupported format 0x%x, defaulting to 0x%x\n", >>>> + req_fmt->format.code, csi2dc_formats[0].mbus_code); >>>> + } >>>> + >>>> + req_fmt->format.code = try_fmt->mbus_code; >>>> + req_fmt->format.colorspace = V4L2_COLORSPACE_SRGB; >>>> + req_fmt->format.field = V4L2_FIELD_NONE; >>>> + >>>> + if (req_fmt->which == V4L2_SUBDEV_FORMAT_TRY) { >>>> + v4l2_try_fmt = v4l2_subdev_get_try_format(csi2dc_sd, sd_state, >>>> + req_fmt->pad); >>>> + *v4l2_try_fmt = req_fmt->format; >>>> + /* Trying on the pad sink makes the source sink change too */ >>>> + if (req_fmt->pad == CSI2DC_PAD_SINK) { >>> >>> s/source sink/source pad/ >>> >>>> + v4l2_try_fmt = >>>> + v4l2_subdev_get_try_format(csi2dc_sd, >>>> + sd_state, >>>> + CSI2DC_PAD_SOURCE); >>>> + *v4l2_try_fmt = req_fmt->format; >>> >>> Shouldn't this be the same for the active format then ? If this device >>> operates by transporting the same exact format from the sink to the >>> source, shouldn't you disable setting a format on the source, and >>> always propagate to the sink ? I would have done so, but maybe >>> Sakari/Laurent could tell you if that's fine. >> >> Hm. I will try. To see if it works and v4l2-compliance is happy. ( I did >> this propagation for try format because it wasn't ) >> >>> >>>> + } >>>> + /* if we are just trying, we are done */ >>>> + return 0; >>>> + } >>>> + >>>> + /* save the format for later requests */ >>>> + csi2dc->format = req_fmt->format; >>>> + >>>> + /* update config */ >>>> + csi2dc->cur_fmt = try_fmt; >>>> + >>>> + dev_dbg(csi2dc->dev, "new format set: 0x%x @%dx%d\n", >>>> + csi2dc->format.code, csi2dc->format.width, >>>> + csi2dc->format.height); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int csi2dc_power(struct csi2dc_device *csi2dc, int on) >>>> +{ >>>> + int ret = 0; >>>> + >>>> + if (on) { >>>> + ret = clk_prepare_enable(csi2dc->pclk); >>>> + if (ret) { >>>> + dev_err(csi2dc->dev, "failed to enable pclk:%d\n", ret); >>>> + return ret; >>>> + } >>>> + >>>> + ret = clk_prepare_enable(csi2dc->scck); >>>> + if (ret) { >>>> + dev_err(csi2dc->dev, "failed to enable scck:%d\n", ret); >>>> + clk_disable_unprepare(csi2dc->pclk); >>>> + return ret; >>>> + } >>>> + >>>> + /* if powering up, deassert reset line */ >>>> + csi2dc_writel(csi2dc, CSI2DC_GCTLR, CSI2DC_GCTLR_SWRST); >>>> + } else { >>>> + /* if powering down, assert reset line */ >>>> + csi2dc_writel(csi2dc, CSI2DC_GCTLR, 0); >>>> + >>>> + clk_disable_unprepare(csi2dc->scck); >>>> + clk_disable_unprepare(csi2dc->pclk); >>>> + } >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int csi2dc_get_mbus_config(struct csi2dc_device *csi2dc) >>>> +{ >>>> + struct v4l2_mbus_config mbus_config = { 0 }; >>>> + int ret; >>>> + >>>> + ret = v4l2_subdev_call(csi2dc->input_sd, pad, get_mbus_config, >>>> + csi2dc->remote_pad, &mbus_config); >>>> + if (ret == -ENOIOCTLCMD) { >>>> + dev_dbg(csi2dc->dev, >>>> + "no remote mbus configuration available\n"); >>>> + goto csi2dc_get_mbus_config_defaults; >>>> + } >>>> + >>>> + if (ret) { >>>> + dev_err(csi2dc->dev, >>>> + "failed to get remote mbus configuration\n"); >>>> + goto csi2dc_get_mbus_config_defaults; >>>> + } >>>> + >>>> + if (mbus_config.flags & V4L2_MBUS_CSI2_CHANNEL_0) >>>> + csi2dc->vc = 0; >>>> + else if (mbus_config.flags & V4L2_MBUS_CSI2_CHANNEL_1) >>>> + csi2dc->vc = 1; >>>> + else if (mbus_config.flags & V4L2_MBUS_CSI2_CHANNEL_2) >>>> + csi2dc->vc = 2; >>>> + else if (mbus_config.flags & V4L2_MBUS_CSI2_CHANNEL_3) >>>> + csi2dc->vc = 3; >>>> + >>>> + dev_dbg(csi2dc->dev, "subdev sending on channel %d\n", csi2dc->vc); >>>> + >>>> + csi2dc->clk_gated = mbus_config.flags & >>>> + V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK; >>>> + >>>> + dev_dbg(csi2dc->dev, "mbus_config: %s clock\n", >>>> + csi2dc->clk_gated ? "gated" : "free running"); >>>> + >>>> + return 0; >>>> + >>>> +csi2dc_get_mbus_config_defaults: >>>> + csi2dc->vc = 0; /* Virtual ID 0 by default */ >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static void csi2dc_vp_update(struct csi2dc_device *csi2dc) >>>> +{ >>>> + u32 vp, gcfg; >>>> + >>>> + if (!csi2dc->video_pipe) { >>>> + dev_err(csi2dc->dev, "video pipeline unavailable\n"); >>>> + return; >>>> + } >>>> + >>>> + if (csi2dc->parallel_mode) { >>>> + /* In parallel mode, GPIO parallel interface must be selected */ >>>> + gcfg = csi2dc_readl(csi2dc, CSI2DC_GCFG); >>>> + gcfg |= CSI2DC_GCFG_GPIOSEL; >>>> + csi2dc_writel(csi2dc, CSI2DC_GCFG, gcfg); >>>> + return; >>>> + } >>>> + >>>> + /* serial video pipeline */ >>>> + >>>> + csi2dc_writel(csi2dc, CSI2DC_GCFG, >>>> + (SAMA7G5_HLC & CSI2DC_GCFG_HLC_MASK) | >>>> + (csi2dc->clk_gated ? 0 : CSI2DC_GCFG_MIPIFRN)); >>>> + >>>> + csi2dc_writel(csi2dc, CSI2DC_VPCOL, >>>> + CSI2DC_VPCOL_COL(0xfff) & CSI2DC_VPCOL_COL_MASK); >>>> + csi2dc_writel(csi2dc, CSI2DC_VPROW, >>>> + CSI2DC_VPROW_ROW(0xfff) & CSI2DC_VPROW_ROW_MASK); >>>> + >>>> + vp = CSI2DC_VPCFG_DT(csi2dc->cur_fmt->dt) & CSI2DC_VPCFG_DT_MASK; >>>> + vp |= CSI2DC_VPCFG_VC(csi2dc->vc) & CSI2DC_VPCFG_VC_MASK; >>>> + vp &= ~CSI2DC_VPCFG_DE; >>>> + vp |= CSI2DC_VPCFG_DM(CSI2DC_VPCFG_DM_DECODER8TO12); >>>> + vp &= ~CSI2DC_VPCFG_DP2; >>>> + vp &= ~CSI2DC_VPCFG_RMS; >>>> + vp |= CSI2DC_VPCFG_PA; >>>> + >>>> + csi2dc_writel(csi2dc, CSI2DC_VPCFG, vp); >>>> + csi2dc_writel(csi2dc, CSI2DC_VPE, CSI2DC_VPE_ENABLE); >>>> + csi2dc_writel(csi2dc, CSI2DC_PU, CSI2DC_PU_VP); >>>> +} >>>> + >>>> +static int csi2dc_s_stream(struct v4l2_subdev *csi2dc_sd, int enable) >>>> +{ >>>> + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); >>>> + int ret; >>>> + >>>> + if (enable) { >>>> + ret = pm_runtime_resume_and_get(csi2dc->dev); >>>> + if (ret < 0) >>>> + return ret; >>>> + >>>> + csi2dc_get_mbus_config(csi2dc); >>>> + >>>> + csi2dc_vp_update(csi2dc); >>>> + >>>> + return v4l2_subdev_call(csi2dc->input_sd, video, s_stream, >>>> + true); >>>> + } >>>> + /* stop streaming scenario */ >>>> + ret = v4l2_subdev_call(csi2dc->input_sd, video, s_stream, false); >>>> + >>>> + pm_runtime_put_sync(csi2dc->dev); >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int csi2dc_init_cfg(struct v4l2_subdev *csi2dc_sd, >>>> + struct v4l2_subdev_state *sd_state) >>>> +{ >>>> + struct v4l2_mbus_framefmt *v4l2_try_fmt = >>>> + v4l2_subdev_get_try_format(csi2dc_sd, sd_state, 0); >>>> + >>>> + v4l2_try_fmt->height = 480; >>>> + v4l2_try_fmt->width = 640; >>>> + v4l2_try_fmt->code = csi2dc_formats[0].mbus_code; >>>> + v4l2_try_fmt->colorspace = V4L2_COLORSPACE_SRGB; >>>> + v4l2_try_fmt->field = V4L2_FIELD_NONE; >>>> + v4l2_try_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>>> + v4l2_try_fmt->quantization = V4L2_QUANTIZATION_DEFAULT; >>>> + v4l2_try_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static const struct v4l2_subdev_pad_ops csi2dc_pad_ops = { >>>> + .enum_mbus_code = csi2dc_enum_mbus_code, >>>> + .set_fmt = csi2dc_set_fmt, >>>> + .get_fmt = csi2dc_get_fmt, >>>> + .init_cfg = csi2dc_init_cfg, >>>> +}; >>>> + >>>> +static const struct v4l2_subdev_video_ops csi2dc_video_ops = { >>>> + .s_stream = csi2dc_s_stream, >>>> +}; >>>> + >>>> +static const struct v4l2_subdev_ops csi2dc_subdev_ops = { >>>> + .pad = &csi2dc_pad_ops, >>>> + .video = &csi2dc_video_ops, >>>> +}; >>>> + >>>> +static int csi2dc_async_bound(struct v4l2_async_notifier *notifier, >>>> + struct v4l2_subdev *subdev, >>>> + struct v4l2_async_subdev *asd) >>>> +{ >>>> + struct csi2dc_device *csi2dc = container_of(notifier, >>>> + struct csi2dc_device, notifier); >>>> + int pad; >>>> + int ret; >>>> + >>>> + csi2dc->input_sd = subdev; >>>> + >>>> + pad = media_entity_get_fwnode_pad(&subdev->entity, asd->match.fwnode, >>>> + MEDIA_PAD_FL_SOURCE); >>>> + if (pad < 0) { >>>> + dev_err(csi2dc->dev, "Failed to find pad for %s\n", >>>> + subdev->name); >>>> + return pad; >>>> + } >>>> + >>>> + csi2dc->remote_pad = pad; >>>> + >>>> + ret = media_create_pad_link(&csi2dc->input_sd->entity, >>>> + csi2dc->remote_pad, >>>> + &csi2dc->csi2dc_sd.entity, 0, >>>> + MEDIA_LNK_FL_ENABLED); >>>> + if (ret) { >>>> + dev_err(csi2dc->dev, >>>> + "Failed to create pad link: %s to %s\n", >>>> + csi2dc->input_sd->entity.name, >>>> + csi2dc->csi2dc_sd.entity.name); >>>> + return ret; >>>> + } >>>> + >>>> + dev_dbg(csi2dc->dev, "link with %s pad: %d\n", >>>> + csi2dc->input_sd->name, csi2dc->remote_pad); >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static const struct v4l2_async_notifier_operations csi2dc_async_ops = { >>>> + .bound = csi2dc_async_bound, >>>> +}; >>>> + >>>> +static int csi2dc_prepare_notifier(struct csi2dc_device *csi2dc, >>>> + struct fwnode_handle *input_fwnode) >>>> +{ >>>> + struct v4l2_async_subdev *asd; >>>> + int ret = 0; >>>> + >>>> + v4l2_async_nf_init(&csi2dc->notifier); >>>> + >>>> + asd = v4l2_async_nf_add_fwnode_remote(&csi2dc->notifier, >>>> + input_fwnode, >>>> + struct v4l2_async_subdev); >>>> + >>>> + fwnode_handle_put(input_fwnode); >>>> + >>>> + if (IS_ERR(asd)) { >>>> + ret = PTR_ERR(asd); >>>> + dev_err(csi2dc->dev, >>>> + "failed to add async notifier for node %pOF: %d\n", >>>> + to_of_node(input_fwnode), ret); >>>> + v4l2_async_nf_cleanup(&csi2dc->notifier); >>>> + return ret; >>>> + } >>>> + >>>> + csi2dc->notifier.ops = &csi2dc_async_ops; >>>> + >>>> + ret = v4l2_async_subdev_nf_register(&csi2dc->csi2dc_sd, >>>> + &csi2dc->notifier); >>> >>> Drop this blank line >>> >>>> + >>>> + if (ret) { >>>> + dev_err(csi2dc->dev, "fail to register async notifier: %d\n", >>>> + ret); >>>> + v4l2_async_nf_cleanup(&csi2dc->notifier); >>>> + } >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int csi2dc_of_parse(struct csi2dc_device *csi2dc, >>>> + struct device_node *of_node) >>>> +{ >>>> + struct fwnode_handle *input_fwnode, *output_fwnode; >>>> + struct v4l2_fwnode_endpoint input_endpoint = { 0 }, >>>> + output_endpoint = { 0 }; >>>> + int ret; >>>> + >>>> + input_fwnode = fwnode_graph_get_next_endpoint(of_fwnode_handle(of_node), >>>> + NULL); >>>> + >>>> + if (!input_fwnode) { >>>> + dev_err(csi2dc->dev, >>>> + "missing port node at %pOF, input node is mandatory.\n", >>>> + of_node); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + ret = v4l2_fwnode_endpoint_parse(input_fwnode, &input_endpoint); >>>> + >>> >>> Drop this blank line >>> >>>> + if (ret) { >>>> + dev_err(csi2dc->dev, "endpoint not defined at %pOF\n", of_node); >>>> + goto csi2dc_of_parse_err; >>>> + } >>>> + >>>> + if (input_endpoint.bus_type == V4L2_MBUS_PARALLEL || >>>> + input_endpoint.bus_type == V4L2_MBUS_BT656) { >>>> + csi2dc->parallel_mode = true; >>>> + dev_dbg(csi2dc->dev, >>>> + "subdevice connected on parallel interface\n"); >>>> + } >>>> + >>>> + if (input_endpoint.bus_type == V4L2_MBUS_CSI2_DPHY) { >>>> + csi2dc->clk_gated = input_endpoint.bus.mipi_csi2.flags & >>>> + V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK; >>>> + dev_dbg(csi2dc->dev, >>>> + "subdevice connected on serial interface\n"); >>>> + dev_dbg(csi2dc->dev, "DT: %s clock\n", >>>> + csi2dc->clk_gated ? "gated" : "free running"); >>>> + } >>>> + >>>> + output_fwnode = fwnode_graph_get_next_endpoint >>>> + (of_fwnode_handle(of_node), input_fwnode); >>>> + >>>> + if (output_fwnode) >>>> + ret = v4l2_fwnode_endpoint_parse(output_fwnode, >>>> + &output_endpoint); >>>> + >>>> + fwnode_handle_put(output_fwnode); >>>> + >>>> + if (!output_fwnode || ret) { >>>> + dev_info(csi2dc->dev, >>>> + "missing output node at %pOF, data pipe available only.\n", >>>> + of_node); >>>> + } else { >>>> + if (output_endpoint.bus_type != V4L2_MBUS_PARALLEL && >>>> + output_endpoint.bus_type != V4L2_MBUS_BT656) { >>>> + dev_err(csi2dc->dev, >>>> + "output port must be parallel/bt656.\n"); >>>> + ret = -EINVAL; >>>> + goto csi2dc_of_parse_err; >>>> + } >>>> + >>>> + csi2dc->video_pipe = true; >>>> + >>>> + dev_dbg(csi2dc->dev, >>>> + "block %pOF [%d.%d]->[%d.%d] video pipeline\n", >>>> + of_node, input_endpoint.base.port, >>>> + input_endpoint.base.id, output_endpoint.base.port, >>>> + output_endpoint.base.id); >>>> + } >>>> + >>>> + /* prepare async notifier for subdevice completion */ >>>> + return csi2dc_prepare_notifier(csi2dc, input_fwnode); >>>> + >>>> +csi2dc_of_parse_err: >>>> + fwnode_handle_put(input_fwnode); >>>> + return ret; >>>> +} >>>> + >>>> +static void csi2dc_default_format(struct csi2dc_device *csi2dc) >>>> +{ >>>> + csi2dc->cur_fmt = &csi2dc_formats[0]; >>>> + >>>> + csi2dc->format.height = 480; >>>> + csi2dc->format.width = 640; >>>> + csi2dc->format.code = csi2dc_formats[0].mbus_code; >>>> + csi2dc->format.colorspace = V4L2_COLORSPACE_SRGB; >>>> + csi2dc->format.field = V4L2_FIELD_NONE; >>>> + csi2dc->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>>> + csi2dc->format.quantization = V4L2_QUANTIZATION_DEFAULT; >>>> + csi2dc->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; >>>> +} >>>> + >>>> +static int csi2dc_probe(struct platform_device *pdev) >>>> +{ >>>> + struct device *dev = &pdev->dev; >>>> + struct csi2dc_device *csi2dc; >>>> + int ret = 0; >>>> + u32 ver; >>>> + >>>> + csi2dc = devm_kzalloc(dev, sizeof(*csi2dc), GFP_KERNEL); >>>> + if (!csi2dc) >>>> + return -ENOMEM; >>>> + >>>> + csi2dc->dev = dev; >>>> + >>>> + csi2dc->base = devm_platform_ioremap_resource(pdev, 0); >>>> + if (IS_ERR(csi2dc->base)) { >>>> + dev_err(dev, "base address not set\n"); >>>> + return PTR_ERR(csi2dc->base); >>>> + } >>>> + >>>> + csi2dc->pclk = devm_clk_get(dev, "pclk"); >>>> + if (IS_ERR(csi2dc->pclk)) { >>>> + ret = PTR_ERR(csi2dc->pclk); >>>> + dev_err(dev, "failed to get pclk: %d\n", ret); >>>> + return ret; >>>> + } >>>> + >>>> + csi2dc->scck = devm_clk_get(dev, "scck"); >>>> + if (IS_ERR(csi2dc->scck)) { >>>> + ret = PTR_ERR(csi2dc->scck); >>>> + dev_err(dev, "failed to get scck: %d\n", ret); >>>> + return ret; >>>> + } >>>> + >>>> + v4l2_subdev_init(&csi2dc->csi2dc_sd, &csi2dc_subdev_ops); >>>> + >>>> + csi2dc->csi2dc_sd.owner = THIS_MODULE; >>>> + csi2dc->csi2dc_sd.dev = dev; >>>> + snprintf(csi2dc->csi2dc_sd.name, sizeof(csi2dc->csi2dc_sd.name), >>>> + "csi2dc"); >>>> + >>>> + csi2dc->csi2dc_sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; >>>> + csi2dc->csi2dc_sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE; >>>> + >>>> + platform_set_drvdata(pdev, csi2dc); >>>> + >>>> + ret = csi2dc_of_parse(csi2dc, dev->of_node); >>>> + if (ret) >>>> + goto csi2dc_probe_cleanup_entity; >>>> + >>>> + csi2dc->pads[CSI2DC_PAD_SINK].flags = MEDIA_PAD_FL_SINK; >>>> + if (csi2dc->video_pipe) >>>> + csi2dc->pads[CSI2DC_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE; >>>> + >>>> + ret = media_entity_pads_init(&csi2dc->csi2dc_sd.entity, >>>> + csi2dc->video_pipe ? CSI2DC_PADS_NUM : 1, >>>> + csi2dc->pads); >>>> + if (ret < 0) { >>>> + dev_err(dev, "media entity init failed\n"); >>>> + goto csi2dc_probe_cleanup_notifier; >>>> + } >>>> + >>>> + csi2dc_default_format(csi2dc); >>>> + >>>> + /* turn power on to validate capabilities */ >>>> + ret = csi2dc_power(csi2dc, true); >>>> + if (ret < 0) >>>> + goto csi2dc_probe_cleanup_notifier; >>>> + >>>> + pm_runtime_set_active(dev); >>>> + pm_runtime_enable(dev); >>>> + ver = csi2dc_readl(csi2dc, CSI2DC_VERSION); >>> >>> Shouldn't you verify that the version is the expected one, or that, at >>> least the device can be accessed ? >> >> If the read from the register worked, the device can be accessed. If the >> device won't respond to this, we will get a bus fault or a freeze. >> And printing the version number can help all the time by inspecting the >> bootlog to see if something valid was printed. >> Version changes from product to product but seeing a bootlog later will >> make us easily understand which product was it, and if the version was >> read correctly >> >>> >>>> + pm_request_idle(dev); >>> >>> I'm not well versed when it comes to runtime_pm but I can read that >>> >>> Documentation/power/runtime_pm.rst- `int pm_request_idle(struct device *dev);` >>> Documentation/power/runtime_pm.rst- - submit a request to execute the subsystem-level idle callback for the >>> Documentation/power/runtime_pm.rst- device (the request is represented by a work item in pm_wq); returns 0 on >>> Documentation/power/runtime_pm.rst- success or error code if the request has not been queued up >>> >>> And looking below you have >>> >>> +static const struct dev_pm_ops csi2dc_dev_pm_ops = { >>> + SET_RUNTIME_PM_OPS(csi2dc_runtime_suspend, csi2dc_runtime_resume, NULL) >>> +}; >>> >>> Which expands to >>> >>> include/linux/pm.h: SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \ >>> >>> so it seems to me you've no idle func defined, hence you above call >>> has no effect so you're device stays powered on after your above call >>> to >>> >>> ret = csi2dc_power(csi2dc, true); >>> >>> Also trying to suspend using runtime_pm while having powered on by >>> calling the resume function directly would leave the pm status >>> unbalanced ? >>> >>> I would raher call pm_runtime_resume() and >>> pm_runtime_suspend() or call the power routine directly, and later >>> enable pm_runtime >>> >>> csi2dc_power(true) >>> readl(); >>> csi2fd_power(false); >>> >>> pm_runtime_set_active(); >>> pm_runtime_enable(); >>> >>> v4l2_async_register(); >>> >>> Again, not well versed in runtime_pm, so don't assume the above makes >>> sense :) >> >> I am not versed in runtime_pm either, I will try to remove this >> 'pm_request_idle' to see what happens at the first >> pm_runtime_resume_and_get, and print messages in power on/power off to >> see how and when they are called. >> > Hi Jacopo, I added traces in my power on / power off functions to see who calls them. Here is the log : power on ------------[ cut here ]------------ WARNING: CPU: 0 PID: 1 at drivers/media/platform/atmel/microchip-csi2dc.c:331 csi2dc_power+0xd0/0x134 Modules linked in: CPU: 0 PID: 1 Comm: swapper Not tainted 5.16.0-rc4+ #62 Hardware name: Microchip SAMA7 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (__warn+0xd0/0x128) [] (__warn) from [] (warn_slowpath_fmt+0x78/0xac) [] (warn_slowpath_fmt) from [] (csi2dc_power+0xd0/0x134) [] (csi2dc_power) from [] (csi2dc_probe+0x158/0x23c) [] (csi2dc_probe) from [] (platform_probe+0x5c/0xb8) [] (platform_probe) from [] (really_probe.part.0+0x9c/0x32c) [] (really_probe.part.0) from [] (__driver_probe_device+0xa8/0x13c) [] (__driver_probe_device) from [] (driver_probe_device+0x34/0x108) [] (driver_probe_device) from [] (__driver_attach+0xb4/0x180) [] (__driver_attach) from [] (bus_for_each_dev+0x74/0xc0) [] (bus_for_each_dev) from [] (bus_add_driver+0x15c/0x1e0) [] (bus_add_driver) from [] (driver_register+0x8c/0x124) [] (driver_register) from [] (do_one_initcall+0x54/0x1d0) [] (do_one_initcall) from [] (kernel_init_freeable+0x19c/0x200) [] (kernel_init_freeable) from [] (kernel_init+0x18/0x128) [] (kernel_init) from [] (ret_from_fork+0x14/0x34) Exception stack(0xc4033fb0 to 0xc4033ff8) 3fa0: 00000000 00000000 00000000 00000000 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end trace a5faddd6c4e11ca2 ]--- microchip-csi2dc e1404000.csi2dc: Microchip CSI2DC version 145 power off ------------[ cut here ]------------ WARNING: CPU: 0 PID: 28 at drivers/media/platform/atmel/microchip-csi2dc.c:335 csi2dc_runtime_suspend+0x34/0x6c Modules linked in: CPU: 0 PID: 28 Comm: kworker/0:9 Tainted: G W 5.16.0-rc4+ #62 Hardware name: Microchip SAMA7 Workqueue: pm pm_runtime_work [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (__warn+0xd0/0x128) [] (__warn) from [] (warn_slowpath_fmt+0x78/0xac) [] (warn_slowpath_fmt) from [] (csi2dc_runtime_suspend+0x34/0x6c) [] (csi2dc_runtime_suspend) from [] (__rpm_callback+0x30/0xe4) [] (__rpm_callback) from [] (rpm_callback+0x60/0x64) [] (rpm_callback) from [] (rpm_suspend+0xd0/0x4bc) [] (rpm_suspend) from [] (pm_runtime_work+0x90/0x98) [] (pm_runtime_work) from [] (process_one_work+0x198/0x410) [] (process_one_work) from [] (worker_thread+0x7c/0x570) [] (worker_thread) from [] (kthread+0x154/0x174) [] (kthread) from [] (ret_from_fork+0x14/0x34) Exception stack(0xc4327fb0 to 0xc4327ff8) 7fa0: 00000000 00000000 00000000 00000000 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end trace a5faddd6c4e11ca3 ]--- > So it looks like after probing is finished, the runtime PM system switches the device back to suspend mode (as probably there are no 'get' operations on it ). I removed the pm_request_idle as it looks you are right, it does nothing. When trying to capture a frame, this happens: # cam -c1 -C1 [0:01:14.779664000] [229] INFO IPAManager ipa_manager.cpp:138 libcamera is not installed. Adding '//src/ipa' to the IPA search path [0:01:14.784039600] [229] WARN IPAManager ipa_manager.cpp:149 No IPA found in '/usr/lib/libcamera' [0:01:14.784219400] [229] INFO Camera camera_manager.cpp:293 libcamera v0.0.0+60036-2021.11-rc1-182-gd72a7997-dirty (2021-11-15T11:27:09+02:00) [0:01:14.800801800] [230] WARN CameraSensor camera_sensor.cpp:197 'imx219 1-0010': Recommended V4L2 control 0x009a0922 not supported [0:01:14.800952200] [230] WARN CameraSensor camera_sensor.cpp:249 'imx219 1-0010': The sensor kernel driver needs to be fixed [0:01:14.801018400] [230] WARN CameraSensor camera_sensor.cpp:251 'imx219 1-0010': See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information [0:01:14.803197200] [230] WARN CameraSensor camera_sensor.cpp:414 'imx219 1-0010': Failed to retrieve the camera location [0:01:14.805444000] [230] WARN V4L2 v4l2_pixelformat.cpp:287 Unsupported V4L2 pixel format Y16 [0:01:14.805541200] [230] WARN V4L2 v4l2_pixelformat.cpp:287 Unsupported V4L2 pixel format AR12 [0:01:14.805612200] [230] WARN V4L2 v4l2_pixelformat.cpp:287 Unsupported V4L2 pixel format AR15 Using camera /base/soc/flexcom@e2818000/i2c@600/camera@10 as cam0 [0:01:14.806657000] [229] INFO Camera camera.cpp:945 configuring streams: (0) 3264x2464-R8 power on ------------[ cut here ]------------ WARNING: CPU: 0 PID: 230 at drivers/media/platform/atmel/microchip-csi2dc.c:331 csi2dc_power+0xd0/0x134 Modules linked in: imx219 dw_csi dw_dphy CPU: 0 PID: 230 Comm: cam Tainted: G W 5.16.0-rc4+ #63 Hardware name: Microchip SAMA7 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (__warn+0xd0/0x128) [] (__warn) from [] (warn_slowpath_fmt+0x78/0xac) [] (warn_slowpath_fmt) from [] (csi2dc_power+0xd0/0x134) [] (csi2dc_power) from [] (__rpm_callback+0x30/0xe4) [] (__rpm_callback) from [] (rpm_callback+0x60/0x64) [] (rpm_callback) from [] (rpm_resume+0x3b4/0x550) [] (rpm_resume) from [] (__pm_runtime_resume+0x34/0x3c) [] (__pm_runtime_resume) from [] (csi2dc_s_stream+0xb8/0x330) [] (csi2dc_s_stream) from [] (isc_start_streaming+0x3a4/0x498) [] (isc_start_streaming) from [] (vb2_start_streaming+0x98/0x1a8) [] (vb2_start_streaming) from [] (vb2_core_qbuf+0x440/0x5d4) [] (vb2_core_qbuf) from [] (vb2_qbuf+0x80/0xdc) [] (vb2_qbuf) from [] (__video_do_ioctl+0x234/0x490) [] (__video_do_ioctl) from [] (video_usercopy+0x1b4/0x550) [] (video_usercopy) from [] (sys_ioctl+0x224/0xa0c) [] (sys_ioctl) from [] (ret_fast_syscall+0x0/0x50) Exception stack(0xc46e7fa8 to 0xc46e7ff0) 7fa0: b5d07088 b5d113c8 00000011 c044560f b66da594 b6e14000 7fc0: b5d07088 b5d113c8 b66da594 00000036 b66da5d8 fffffff8 b66da6cc b66da8e0 7fe0: b6fb63a8 b66da57c b6f70d88 b6ba9fec ---[ end trace 7204e1871cae96e7 ]--- cam0dw-csi e1400000.csi2host: number of lanes: 2 phy phy-e1400040.dphy.0: PHY Stop state not reached 0 75.378082power off ------------[ cut here ]----e------- 4WARNING: CPU: 0 PID: 230 at drivers/media/platform/atmel/microchip-csi2dc.c:335 csi2dc_runtime_suspend+0x34/0x6c Modules linked in: imx219 dw_csi dw_dphy CPU: 0 PID: 230 Comm: cam Tainted: G W 5.16.0-rc4+ #63 Hardware name: Microchip SAMA7 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (__warn+0xd0/0x128) [] (__warn) from [] (warn_slowpath_fmt+0x78/0xac) [] (warn_slowpath_fmt) from [] (csi2dc_runtime_suspend+0x34/0x6c) [] (csi2dc_runtime_suspend) from [] (__rpm_callback+0x30/0xe4) [] (__rpm_callback) from [] (rpm_callback+0x60/0x64) [] (rpm_callback) from [] (rpm_suspend+0xd0/0x4bc) [] (rpm_suspend) from [] (__pm_runtime_idle+0x3c/0x4c) [] (__pm_runtime_idle) from [] (csi2dc_s_stream+0x84/0x330) [] (csi2dc_s_stream) from [] (isc_stop_streaming+0x134/0x174) [] (isc_stop_streaming) from [] (__vb2_queue_cancel+0x28/0x2a4) [] (__vb2_queue_cancel) from [] (vb2_core_streamoff+0x18/0xac) [] (vb2_core_streamoff) from [] (__video_do_ioctl+0x234/0x490) [] (__video_do_ioctl) from [] (video_usercopy+0x1b4/0x550) [] (video_usercopy) from [] (sys_ioctl+0x224/0xa0c) [] (sys_ioctl) from [] (ret_fast_syscall+0x0/0x50) Exception stack(0xc46e7fa8 to 0xc46e7ff0) 7fa0: b5d07088 b6fb5d58 00000011 40045613 b5d071bc 00000001 7fc0: b5d07088 b6fb5d58 b5d01e20 00000036 b5d008b8 00000000 b5d01e20 0054ec00 7fe0: b6fb63a8 b66da7b4 b6f70d88 b6ba9fec ---[ end trace 7204e1871cae96e8 ]--- # # Which is expected, on start streaming, the power on is called, and on stop streaming the power off. So it looks like the driver does pm_runtime_set_active, meaning the power state is active (which is something that I do by calling first csi2dc_power(true)), and after probing is done, it's automatically sent to suspend mode. So if I do csi2dc_power(false) before finish probing, there will be a problem because it will be called again by the subsystem, leading to issues with double clock unpreparing... etc. So you are fine with this sequence? ..init this.. ..init that.. csi2dc_power(true) readl(); pm_runtime_set_active(); pm_runtime_enable(); ..finish probing.. Thanks again, Eugen >> >>> >>>> + >>>> + /* >>>> + * we must register the subdev after PM runtime has been requested, >>>> + * otherwise we might bound immediately and request pm_runtime_resume >>>> + * before runtime_enable. >>>> + */ >>>> + ret = v4l2_async_register_subdev(&csi2dc->csi2dc_sd); >>>> + if (ret) { >>>> + dev_err(csi2dc->dev, "failed to register the subdevice\n"); >>>> + goto csi2dc_probe_cleanup_notifier; >>>> + } >>>> + >>>> + dev_info(dev, "Microchip CSI2DC version %x\n", ver); >>>> + >>>> + return 0; >>>> + >>>> +csi2dc_probe_cleanup_notifier: >>>> + v4l2_async_nf_cleanup(&csi2dc->notifier); >>>> +csi2dc_probe_cleanup_entity: >>>> + media_entity_cleanup(&csi2dc->csi2dc_sd.entity); >>> >>> I wasn't honestly expecting this >>> >>> /** >>> * media_entity_cleanup() - free resources associated with an entity >>> * >>> * @entity: entity where the pads belong >>> * >>> * This function must be called during the cleanup phase after unregistering >>> * the entity (currently, it does nothing). >>> */ >>> #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER) >>> static inline void media_entity_cleanup(struct media_entity *entity) {} >>> >>> :) >> >> I saw it as well. But I thought that who knows, in the future it might >> do something, so it's better to have it in the driver rather than not >> have it at all. >> >> >> Thanks again for reviewing, >> Eugen >> >>> >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int csi2dc_remove(struct platform_device *pdev) >>>> +{ >>>> + struct csi2dc_device *csi2dc = platform_get_drvdata(pdev); >>>> + >>>> + pm_runtime_disable(&pdev->dev); >>>> + >>>> + v4l2_async_unregister_subdev(&csi2dc->csi2dc_sd); >>>> + v4l2_async_nf_unregister(&csi2dc->notifier); >>>> + v4l2_async_nf_cleanup(&csi2dc->notifier); >>>> + media_entity_cleanup(&csi2dc->csi2dc_sd.entity); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int __maybe_unused csi2dc_runtime_suspend(struct device *dev) >>>> +{ >>>> + struct csi2dc_device *csi2dc = dev_get_drvdata(dev); >>>> + >>>> + return csi2dc_power(csi2dc, false); >>>> +} >>>> + >>>> +static int __maybe_unused csi2dc_runtime_resume(struct device *dev) >>>> +{ >>>> + struct csi2dc_device *csi2dc = dev_get_drvdata(dev); >>>> + >>>> + return csi2dc_power(csi2dc, true); >>>> +} >>>> + >>>> +static const struct dev_pm_ops csi2dc_dev_pm_ops = { >>>> + SET_RUNTIME_PM_OPS(csi2dc_runtime_suspend, csi2dc_runtime_resume, NULL) >>>> +}; >>>> + >>>> +static const struct of_device_id csi2dc_of_match[] = { >>>> + { .compatible = "microchip,sama7g5-csi2dc" }, >>>> + { } >>>> +}; >>>> + >>>> +MODULE_DEVICE_TABLE(of, csi2dc_of_match); >>>> + >>>> +static struct platform_driver csi2dc_driver = { >>>> + .probe = csi2dc_probe, >>>> + .remove = csi2dc_remove, >>>> + .driver = { >>>> + .name = "microchip-csi2dc", >>>> + .pm = &csi2dc_dev_pm_ops, >>>> + .of_match_table = of_match_ptr(csi2dc_of_match), >>>> + }, >>>> +}; >>>> + >>>> +module_platform_driver(csi2dc_driver); >>>> + >>>> +MODULE_AUTHOR("Eugen Hristev "); >>>> +MODULE_DESCRIPTION("Microchip CSI2 Demux Controller driver"); >>>> +MODULE_LICENSE("GPL v2"); >>>> -- >>>> 2.25.1 >>>> >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel