Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754217AbcDYKFl (ORCPT ); Mon, 25 Apr 2016 06:05:41 -0400 Received: from foss.arm.com ([217.140.101.70]:44576 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754114AbcDYKFj (ORCPT ); Mon, 25 Apr 2016 06:05:39 -0400 Date: Mon, 25 Apr 2016 11:05:32 +0100 From: Mark Rutland To: "Huang, Tao" Cc: Marc Zyngier , devicetree@vger.kernel.org, davidriley@chromium.org, heiko@sntech.de, pawel.moll@arm.com, ijc+devicetree@hellion.org.uk, catalin.marinas@arm.com, will.deacon@arm.com, dianders@chromium.org, smbarber@chromium.org, linux-rockchip@lists.infradead.org, robh+dt@kernel.org, galak@codeaurora.org, jwerner@chromium.org, linux-kernel@vger.kernel.org, Jianqun Xu , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs Message-ID: <20160425100531.GC25087@leverpostej> References: <1461122150-9042-1-git-send-email-jay.xu@rock-chips.com> <1461211092-26331-1-git-send-email-jay.xu@rock-chips.com> <20160421101930.GG6879@leverpostej> <5718AFB8.5070004@rock-chips.com> <20160421123018.096d4a75@arm.com> <571DE803.3010902@rock-chips.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <571DE803.3010902@rock-chips.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4971 Lines: 139 On Mon, Apr 25, 2016 at 05:48:51PM +0800, Huang, Tao wrote: > Hi, Marc: > On 2016年04月21日 19:30, Marc Zyngier wrote: > > On Thu, 21 Apr 2016 18:47:20 +0800 > > "Huang, Tao" wrote: > > > >> Hi, Mark: > >> On 2016年04月21日 18:19, Mark Rutland wrote: > >>> On Thu, Apr 21, 2016 at 11:58:12AM +0800, Jianqun Xu wrote: > >>>> + cpu_l0: cpu@0 { > >>>> + device_type = "cpu"; > >>>> + compatible = "arm,cortex-a53", "arm,armv8"; > >>>> + reg = <0x0 0x0>; > >>>> + enable-method = "psci"; > >>>> + #cooling-cells = <2>; /* min followed by max */ > >>>> + clocks = <&cru ARMCLKL>; > >>>> + }; > >>>> + cpu_b0: cpu@100 { > >>>> + device_type = "cpu"; > >>>> + compatible = "arm,cortex-a72", "arm,armv8"; > >>>> + reg = <0x0 0x100>; > >>>> + enable-method = "psci"; > >>>> + #cooling-cells = <2>; /* min followed by max */ > >>>> + clocks = <&cru ARMCLKB>; > >>>> + }; > >>>> + > >>>> + arm-pmu { > >>>> + compatible = "arm,armv8-pmuv3"; > >>>> + interrupts = ; > >>>> + }; > >>> This is wrong, and must go. There should be a separate node for the PMU > >>> of each microarchitecture, with the appropriate compatible string to > >>> represent that (see the juno dts). > >> You are right. The first version we wrote is: > >> pmu_a53 { > >> compatible = "arm,cortex-a53-pmu"; > >> interrupts = ; > >> interrupt-affinity = <&cpu_l0>, > >> <&cpu_l1>, > >> <&cpu_l2>, > >> <&cpu_l3>; > >> }; > >> > >> pmu_a72 { > >> compatible = "arm,cortex-a72-pmu"; > >> interrupts = ; > >> interrupt-affinity = <&cpu_b0>, > >> <&cpu_b1>; > >> }; > >> but unfortunately, the arm pmu driver do not support PPI in two cluster > >> well, > >> so we have to replace with this implementation. > >>> In this case things are messier as the same PPI number is being used > >>> across clusters. Marc (Cc'd) has been working on PPI partitions, which > >>> should allow us to support that. > >> Great! So what we can do right now? Wait this feature, and delete > >> arm-pmu node? > > I'd rather you have a look at the patches, test them with your HW, > > and comment on what doesn't work! > > > > You can find the patches over there: > > > > https://lkml.org/lkml/2016/4/11/182 > > > > and on the following branch: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git > > irq/percpu-partition > > I tested these patches. Because our kernel is based on v4.4, so I back > port most changes about > include/linux/irqdomain.h > kernel/irq/irqdomain.c > drivers/irqchip/irq-gic-v3.c > and change rk3399.dtsi base on your arm,gic-v3.txt: > > gic: interrupt-controller@fee00000 { > compatible = "arm,gic-v3"; > - #interrupt-cells = <3>; > + #interrupt-cells = <4>; > #address-cells = <2>; > #size-cells = <2>; > ... > + > + ppi-partitions { > + part0: interrupt-partition-0 { > + affinity = <&cpu_l0 &cpu_l1 &cpu_l2 &cpu_l3>; > + }; > + > + part1: interrupt-partition-1 { > + affinity = <&cpu_b0 &cpu_b1>; > + }; > + }; > > and change every interrupts from three cells to four cells, such as > saradc: saradc@ff100000 { > compatible = "rockchip,rk3399-saradc"; > reg = <0x0 0xff100000 0x0 0x100>; > - interrupts = ; > + interrupts = ; > #io-channel-cells = <1>; > clocks = <&cru SCLK_SARADC>, <&cru PCLK_SARADC>; > clock-names = "saradc", "apb_pclk"; > > and pmu define as: > pmu_a53 { > compatible = "arm,cortex-a53-pmu"; > interrupts = ; > interrupt-affinity = <&cpu_l0>, > <&cpu_l1>, > <&cpu_l2>, > <&cpu_l3>; > }; > > pmu_a72 { > compatible = "arm,cortex-a72-pmu", "arm,cortex-a57-pmu"; That Cortex-A57 PMU fallback should just go. We already have Cortex-A72 PMU support upstream, and I believe there are sufficient differences such that the Cortex-A72 PMU is not a strict superset of the Cortex-A57 PMU. > interrupts = ; > interrupt-affinity = <&cpu_b0>, > <&cpu_b1>; > }; > > It can boot. And I test with Android simpleperf stat and perf top, it works! > So these patches work on RK3399. There is still work to do in the driver, as Marc pointed out. While it may appear to work, it will be requesting percpu IRQs on wrong CPUs (e.g. see how cpu_pmu_request_irq calls cpu_pmu_enable_percpu_irq, on each CPU), and we will need to update the binding codument to cover this case. Thanks, Mark.