From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753591AbdCWHev (ORCPT ); Thu, 23 Mar 2017 03:34:51 -0400 Received: from mail-wm0-f41.google.com ([74.125.82.41]:38078 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751555AbdCWHet (ORCPT ); Thu, 23 Mar 2017 03:34:49 -0400 Date: Thu, 23 Mar 2017 13:43:20 +0800 From: Leo Yan To: Sudeep Holla Cc: Suzuki K Poulose , Mike Leach , Rob Herring , Mark Rutland , Wei Xu , Catalin Marinas , Will Deacon , Michael Turquette , Stephen Boyd , Mathieu Poirier , John Stultz , Guodong Xu , Haojian Zhuang , Greg Kroah-Hartman , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org Subject: Re: [PATCH v3 3/5] coresight: add support for debug module Message-ID: <20170323054319.GA7716@leoy-linaro> References: <1488520809-31670-1-git-send-email-leo.yan@linaro.org> <1488520809-31670-4-git-send-email-leo.yan@linaro.org> <671a0b39-b635-6e0e-d3fa-967651f2e29c@arm.com> <4961636d-d77c-0f9a-7076-4db1ef456073@arm.com> <1e6d6a91-3bdc-7cfb-a96f-780c959e0316@arm.com> <7a028460-d598-e337-5e09-234beddca88b@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7a028460-d598-e337-5e09-234beddca88b@arm.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Mar 22, 2017 at 05:25:50PM +0000, Sudeep Holla wrote: > > > On 22/03/17 17:09, Suzuki K Poulose wrote: > > On 22/03/17 16:17, Sudeep Holla wrote: > > [...] > > >> > >> Point taken. So we could just specify that all necessary power > >> domains need to be on for proper functionality for this feature and > >> that it's highly platform specific instead of mixing cpu/cluster > >> idle details here. > >> > >>> The key point is that the caveat in using this driver is that > >>> the power management has to be considered on a platform specific > >>> basis before it is configured; and appropriate actions may be > >>> needed for it to work correctly. Without this then the driver > >>> could cause more issues than it debugs. A user selecting this > >>> _must_ be told about these issues > >>> > > > > So given all the possible caveats, I think we : > > > > 1) Shouldn't enable the driver by default at runtime even if it is > > built-in. > > 2) Should provide mechanisms to turn it on at boot (via > > kernel commandline) or anytime later (via sysfs), which kind of puts > > the responsibility back on the user : "You know what you are doing". > > 3) Shouldn't turn the driver on based on "nohlt" which the user > > could use it for some other purposes, without explicit intention of > > turning this driver on). > > 4) Should document the fact that, on some > > platforms, the user may have to disable CPUidle explicitly to get the > > driver working. But let us not make it the default. The user with a > > not so ideal platform could add "nohlt" and get it working. > > > > Agreed on all points and well summarized. > I would like to highlight (3) and (4) as it needs to be well understood. > > "nohlt" has a *different* meaning already, so using that in this > driver for something else is simple wrong as it affects the system in > unintended ways. And yes if user (mis)uses it to get things working, > it's fine but shouldn't be recommended way. Understand this point. I will try to use general way to constraint CPUIdle like other drivers. Thanks all for these good suggestions :) Thanks, Leo Yan