[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170323054319.GA7716@leoy-linaro>
Date: Thu, 23 Mar 2017 13:43:20 +0800
From: Leo Yan <leo.yan@...aro.org>
To: Sudeep Holla <sudeep.holla@....com>
Cc: Suzuki K Poulose <Suzuki.Poulose@....com>,
Mike Leach <mike.leach@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Wei Xu <xuwei5@...ilicon.com>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will.deacon@....com>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...eaurora.org>,
Mathieu Poirier <mathieu.poirier@...aro.org>,
John Stultz <john.stultz@...aro.org>,
Guodong Xu <guodong.xu@...aro.org>,
Haojian Zhuang <haojian.zhuang@...aro.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH v3 3/5] coresight: add support for debug module
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
Powered by blists - more mailing lists