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 3D33EC282C6 for ; Mon, 3 Mar 2025 18:52:48 +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:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Ph3ScwWRu2TuwRLc+ZG4TvVe/7bgmrSr+8al0+WMn/w=; b=ZgbcKkOde6K7O+Gqpj39qsMNgJ VshaVZ5yJ+OMGwG8V0PzpN+H/ypsg+0ziDb8QIre34XvKr84DjlcwZNbPTYTCtqj0qxEK12lNHpx3 3MO88MaNP3y7jPpf6FooHnGdbn1Tp9bxZX/RyrTz4S8Md3Z3Z7PdArI9Wy3yqlB4P5+A3nDzgFeLt soFMtcAocwyTUMVFxkeuHHKWRPNRNTCQgFIH2kNEY0Ga69B0V0EQJ6ekSzk484bNvakC8Jig+7DsG OKcTG3hs4J1ro/Dilms5plZh/DzaEGuwmC7s27B7ydyALqLzfOhOkswBFMbd7Tvi1xw4w9tWBc5M0 amLGWzmg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tpAu5-00000001zO8-2dx7; Mon, 03 Mar 2025 18:52:37 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1tpAra-00000001ywa-0msY for linux-arm-kernel@lists.infradead.org; Mon, 03 Mar 2025 18:50:03 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 3E9CE5C5AC7; Mon, 3 Mar 2025 18:47:44 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AA80BC4CED6; Mon, 3 Mar 2025 18:50:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741027800; bh=ay2Mn2td7LnvXWZz0y/fV3J9yKVFlbL+Ii+8sPB9FlI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=ZOyoKEdboUWUG+YMMkdL5RPjsQQjxkQgZljsCmkfUknphh5NWjVd5xuL2U5V5YcPd IycN7aSsNST2N/J6LErZhR1QBg+cSC2AxMlhHhMhlI5qge3YS08YlGALiMwSP4GGfx koSc1w6VFEM/kLaIoZAfFlHxtHrKzuxX0wstpuhDgBxe0fzvghFQVz+sZINJH7RgKz qqgvIF9/c79a16CFL7ks+slGk061OyzBAf2SapL7k3eydnwojqTphxYwrJKVV1hh75 d04bon1Z4dwQWtqHZz6+s8JAIvbP1zuwf8LjklHvnl4IZoTdDALzF0KYPiPVsHkhYy noOLQ+bZpO1cg== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1tpArW-009xKj-CA; Mon, 03 Mar 2025 18:49:58 +0000 Date: Mon, 03 Mar 2025 18:49:58 +0000 Message-ID: <86ikoqoso9.wl-maz@kernel.org> From: Marc Zyngier To: Peter Chen Cc: robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, catalin.marinas@arm.com, will@kernel.org, arnd@arndb.de, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, cix-kernel-upstream@cixtech.com, marcin@juszkiewicz.com.pl, Fugang Duan Subject: Re: [PATCH v3 6/6] arm64: dts: cix: add initial CIX P1(SKY1) dts support In-Reply-To: References: <20250227120619.1741431-1-peter.chen@cixtech.com> <20250227120619.1741431-7-peter.chen@cixtech.com> <86r03ip0kf.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.4 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: peter.chen@cixtech.com, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, catalin.marinas@arm.com, will@kernel.org, arnd@arndb.de, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, cix-kernel-upstream@cixtech.com, marcin@juszkiewicz.com.pl, fugang.duan@cixtech.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250303_105002_320361_CAF978C2 X-CRM114-Status: GOOD ( 30.89 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 03 Mar 2025 11:38:47 +0000, Peter Chen wrote: > > On 25-02-28 15:10:24, Marc Zyngier wrote: > > Hi Marc, > > Thanks for your detail review. > > > > + > > > + cpu10: cpu@a00 { > > > + compatible = "arm,cortex-a720"; > > > + enable-method = "psci"; > > > + reg = <0x0 0xa00>; > > > + device_type = "cpu"; > > > + capacity-dmips-mhz = <1024>; > > > + }; > > > + > > > + cpu11: cpu@b00 { > > > + compatible = "arm,cortex-a720"; > > > + enable-method = "psci"; > > > + reg = <0x0 0xb00>; > > > + device_type = "cpu"; > > > + capacity-dmips-mhz = <1024>; > > > + }; > > > > Given that half the A720s are advertised with lower clock speed, how > > comes they all have the same capacity? > > According to Documentation/devicetree/bindings/cpu/cpu-capacity.txt > "capacity-dmips-mhz" is u32 value representing CPU capacity expressed > in normalized DMIPS/MHz, it means CPU capability per MHz. For sky1 > SoC, both middle and big cores are A720, so their capability per MHz > are the same. Ah, fair enough. I missed that detail. > > > > + > > > + pmu-a520 { > > > + compatible = "arm,cortex-a520-pmu"; > > > + interrupts = ; > > > + }; > > > + > > > + pmu-a720 { > > > + compatible = "arm,cortex-a720-pmu"; > > > + interrupts = ; > > > + }; > > > > This is wrong. The default configuration for PPIs is to expose the > > *same* device on all CPUs. You must use PPI affinities for your PMUs. > > Please see the GICv3 binding for the details. > > We have discussed internally, we have not seen the benefits routing > different PPI interrupt to dedicated CPUs. Any use cases? This isn't about changing the PPI. It is about matching CPUs with their PMU. Here, you are saying "both PMU types are connected to all the CPUs using PPI7". That's obviously not the case. > I prefer changing pmu nodes as one generic Armv8 PMU node. Is it accepted? No, that's not acceptable. > Or must I keep both pmu for A520 and A720, and add PPI affinities to > describe hardware well? This is an established practice on all big-little systems: each PMU node has an affinity that indicates which CPUs they are connected to. For GICv3+, this is carried by the interrupt specifier. Please look at existing SoCs supported, such as rk3399, for example. > > > > > > + > > > + pmu-spe { > > > + compatible = "arm,statistical-profiling-extension-v1"; > > > + interrupts = ; > > > + }; > > > + > > > + psci { > > > + compatible = "arm,psci-1.0"; > > > + method = "smc"; > > > + }; > > > + > > > + soc@0 { > > > + compatible = "simple-bus"; > > > + ranges = <0 0 0 0 0x20 0>; > > > + dma-ranges; > > > + #address-cells = <2>; > > > + #size-cells = <2>; > > > + > > > + gic: interrupt-controller@e010000 { > > > + compatible = "arm,gic-v3"; > > > + reg = <0x0 0x0e010000 0 0x10000>, /* GICD */ > > > + <0x0 0x0e090000 0 0x300000>; /* GICR * 12 */ > > > + interrupts = ; > > > + #interrupt-cells = <3>; > > > > This will need to be bumped up to 4, and all the interrupt specifiers adjusted. > > Depends on if PPI affinities is must. Definitely a must, unless you want to completely remove all traces of the PMU, which is of course silly, but a valid alternative. [...] > > > + arm,no-tick-in-suspend; > > > > Why do you need this? Is the HW so broken that you have implemented > > the global counter in a power domain that isn't always on? > > > > Not hardware broken, just arch timer will be powered off at cpu idle > and system suspend due to power consumption reason. This is not about the timer. This is about the global counter. If your counter stops ticking when you're in idle or suspended, your system is broken and you need this property. If the timer (or more precisely the comparator) is turned off because the CPU is off, then that's the expected behaviour and you don't need this property. -- Without deviation from the norm, progress is not possible.