Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752474AbbGOOpd (ORCPT ); Wed, 15 Jul 2015 10:45:33 -0400 Received: from pandora.arm.linux.org.uk ([78.32.30.218]:44317 "EHLO pandora.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751468AbbGOOpc (ORCPT ); Wed, 15 Jul 2015 10:45:32 -0400 Date: Wed, 15 Jul 2015 15:45:07 +0100 From: Russell King - ARM Linux To: Lorenzo Pieralisi Cc: Jisheng Zhang , Will Deacon , Mark Rutland , "daniel.lezcano@linaro.org" , Catalin Marinas , "galak@codeaurora.org" , "agross@codeaurora.org" , "davidb@codeaurora.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v3 3/3] arm: kernel: implement cpuidle_ops with psci backend Message-ID: <20150715144507.GZ7557@n2100.arm.linux.org.uk> References: <1436430685-2202-1-git-send-email-jszhang@marvell.com> <1436430685-2202-4-git-send-email-jszhang@marvell.com> <20150714103421.GU7557@n2100.arm.linux.org.uk> <20150714110302.GA334@red-moon> <20150714122904.GV7557@n2100.arm.linux.org.uk> <20150714145546.GA14556@red-moon> <20150714204138.GW7557@n2100.arm.linux.org.uk> <20150715134603.GA14862@red-moon> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150715134603.GA14862@red-moon> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3081 Lines: 68 On Wed, Jul 15, 2015 at 02:46:03PM +0100, Lorenzo Pieralisi wrote: > On Tue, Jul 14, 2015 at 09:41:38PM +0100, Russell King - ARM Linux wrote: > > Sorry, NAK, and end of discussion. There is nothing more to be said > > here. > > I beg to differ. To solve the issue that you brought up with this series, > we can create an arch function that allows to set CPUidle operations, which > would remove the need for the OF construct you did not like, this mirrors > what's done for PSCI smp operations (something similar to smp_set_ops), > does that sound a reasonable approach to you ? What's the point of that? This is _all_ that arch/arm/kernel/psci_cpuidle.c will contain after this series: +#include +#include + +#include + +static struct cpuidle_ops psci_cpuidle_ops __initdata = { + .suspend = cpu_psci_cpu_suspend, + .init = cpu_psci_cpu_init_idle, +}; +CPUIDLE_METHOD_OF_DECLARE(psci_cpuidle, "psci", &psci_cpuidle_ops); There is _nothing_ ARM specific there. All it's doing is providing an entry in the DT linker-built table for that data structure, and that data structure contains a pair of function pointers to code which _will_ be located in drivers/firmware/psci.c. I'm saying _that_ is totally pointless, that data structure and CPUIDLE_METHOD_OF_DECLARE() _should_ live with the code in drivers/firmware/psci.c. There's nothing in the above quoted code which points anything to any 32bit ARM specific stuff at all (so there's no need for a smp_set_ops() type thing), it's all about giving DT a way to specify the CPU idle operations which should be used, in this case, allowing DT to specify that the _generic_ PSCI CPU idle operations are to be used. > As for M.Rutland's series[1], and the patch that moves common PSCI code to > drivers/firmware I reiterate my point, please review it[1] and provide us > with feedback if any otherwise acknowledge the code move, which is the > basis on top of which everything else can be developed, I do not understand > why you are stopping [1] from getting into the kernel since it moves > code from arch/arm to drivers/firmware to have it in a common and shared > place between ARM and ARM64, what's the issue you have with that or put > it differently why are you NAKing it ? I'm NAKing everything related to moving the PSCI code around until someone in the PSCI maintainer pool (that's not me - that's ARM Ltd) clearly demonstrates that they know what they're talking about, and has a plan behind this reorganisation. Right now, I'm getting the impression that there is _no_ plan, or if there is, it's an absurd plan which results in data structures which should not be in arch/arm being left behind there. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/