From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailgw02.mediatek.com (unknown [210.61.82.184]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2CBF746B5 for ; Mon, 6 Jun 2022 10:41:23 +0000 (UTC) X-UUID: d0bd998342f8456386e86a98878ab3f1-20220606 X-CID-P-RULE: Release_Ham X-CID-O-INFO: VERSION:1.1.5,REQID:50314f12-4639-419c-a74f-fd6e80fa176d,OB:0,LO B:0,IP:0,URL:0,TC:0,Content:0,EDM:0,RT:0,SF:0,FILE:0,RULE:Release_Ham,ACTI ON:release,TS:0 X-CID-META: VersionHash:2a19b09,CLOUDID:1e28c2ad-3171-4dd4-a2d9-73b846daf167,C OID:IGNORED,Recheck:0,SF:nil,TC:nil,Content:0,EDM:-3,IP:nil,URL:0,File:nil ,QS:0,BEC:nil X-UUID: d0bd998342f8456386e86a98878ab3f1-20220606 Received: from mtkexhb01.mediatek.inc [(172.21.101.102)] by mailgw02.mediatek.com (envelope-from ) (Generic MTA with TLSv1.2 ECDHE-RSA-AES256-SHA384 256/256) with ESMTP id 1547484081; Mon, 06 Jun 2022 18:41:19 +0800 Received: from mtkmbs11n1.mediatek.inc (172.21.101.186) by mtkmbs10n1.mediatek.inc (172.21.101.34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.792.15; Mon, 6 Jun 2022 18:41:17 +0800 Received: from mtksdccf07 (172.21.84.99) by mtkmbs11n1.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.2.792.3 via Frontend Transport; Mon, 6 Jun 2022 18:41:17 +0800 Message-ID: <3f02705349535c8a28451233a5f5fc152ac8fa81.camel@mediatek.com> Subject: Re: [PATCH v1 05/15] remoteproc: mediatek: Add SCP core 1 driver for dual-core scp From: Tinghan Shen To: AngeloGioacchino Del Regno , Bjorn Andersson , Mathieu Poirier , Rob Herring , "Krzysztof Kozlowski" , Matthias Brugger , Lee Jones , Benson Leung , Guenter Roeck , Daisuke Nojiri , Sebastian Reichel , "Dustin L. Howett" , Tzung-Bi Shih , "Gustavo A. R. Silva" , Prashant Malani , "Brian Norris" CC: , , , , , , , Date: Mon, 6 Jun 2022 18:41:17 +0800 In-Reply-To: <4062e8be-3ac7-c6e5-dc15-bb11bd6051fc@collabora.com> References: <20220601112201.15510-1-tinghan.shen@mediatek.com> <20220601112201.15510-6-tinghan.shen@mediatek.com> <4062e8be-3ac7-c6e5-dc15-bb11bd6051fc@collabora.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 Precedence: bulk X-Mailing-List: chrome-platform@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-MTK: N On Mon, 2022-06-06 at 12:08 +0200, AngeloGioacchino Del Regno wrote: > Il 06/06/22 11:52, Tinghan Shen ha scritto: > > On Mon, 2022-06-06 at 11:15 +0200, AngeloGioacchino Del Regno wrote: > > > Il 01/06/22 13:21, Tinghan Shen ha scritto: > > > > MT8195 SCP is a dual-core processor. The mtk_scp.c driver only controls > > > > SCP core 0. This patch adds a basic driver to control the another core. > > > > > > > > Core 1 and core 0 of the SCP are housed in the same subsys.They see > > > > registers and memory in the same way. > > > > > > > > Core 1 of the SCP features its own set of core configuration registers, > > > > interrupt controller, timers, and DMAs. The rest of the peripherals > > > > in this subsystem are shared by core 0 and core 1. > > > > > > > > As for memory, core 1 has its own cache memory, and the SCP SRAM is shared > > > > by core 0 and core 1. > > > > > > > > > > Hello Tinghan, > > > > > > checking all the patches that are introducing support for the secondary SCP core, > > > it's clear that you're practically reusing *most of* mtk_scp in mtk_scp_dual. > > > > > > I don't think that adding a new configuration option for MTK_SCP_DUALCORE (nor a > > > new file just for that) is a good idea... the code is "short enough" so you should > > > really just add support for multi-core SCP in mtk_scp.c instead. > > > > > > After doing so, I have a hunch that we'll be able to reduce the size of this > > > implementation even more, as I see literally too much common code :-) > > > > > > > Hi Angelo, > > > > Thanks for your review. > > > > This series has 2 new files, mtk_scp_dual.c and mtk_scp_subdev.c. > > Is your advice to merge both files into mtk_scp.c, > > or to merely merge mtk_scp_dual.c to mtk_scp.c? > > > > Thanks, > > TingHan > > > > > > > > I suggest to merge both into mtk_scp.c and commonize/generalize functions inside > of there as much as possible... including the removal of #if IS_ENABLED(...) > macro usages, as you can simply check that during runtime by setting a bool > variable to true when it's dual-core. > > Let's do this first step. > I'll give you a more exhaustive review on v2, when this main step is done. > > Cheers, > Angelo Hi Angelo, Ok, I'll merge these files and send next version. Thanks, TingHan