From mboxrd@z Thu Jan 1 00:00:00 1970 From: Leo Yan Subject: Re: [PATCH v5 6/9] coresight: add support for CPU debug module Date: Fri, 31 Mar 2017 08:54:20 +0800 Message-ID: <20170331005420.GA6149@leoy-linaro> References: <1490466197-29163-1-git-send-email-leo.yan@linaro.org> <1490466197-29163-7-git-send-email-leo.yan@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-doc-owner@vger.kernel.org To: Sudeep Holla Cc: Jonathan Corbet , Rob Herring , Mark Rutland , Wei Xu , Catalin Marinas , Will Deacon , Andy Gross , David Brown , Michael Turquette , Stephen Boyd , Mathieu Poirier , Guodong Xu , John Stultz , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-clk@vger.kernel.org, mike.leach@linaro.org, Suzuki.Poulose@arm.com List-Id: linux-arm-msm@vger.kernel.org On Thu, Mar 30, 2017 at 04:56:52PM +0100, Sudeep Holla wrote: [...] > > +static struct pm_qos_request debug_qos_req; > > +static int idle_constraint = PM_QOS_DEFAULT_VALUE; > > +module_param(idle_constraint, int, 0600); > > +MODULE_PARM_DESC(idle_constraint, "Latency requirement in microseconds for CPU " > > + "idle states (default is -1, which means have no limiation " > > + "to CPU idle states; 0 means disabling all idle states; user " > > + "can choose other platform dependent values so can disable " > > + "specific idle states for the platform)"); > > + > > NACK for this. Why you want the policy inside the driver. You can always > do that from the user-space. I have mentioned it several times now. > What can't you do these ? > > 1. echo "what_ever_latency_you_need_in_uS" > /dev/cpu_dma_latency > 2. echo 1 > /sys/devices/system/cpu/cpu$cpu/cpuidle/state$state/disable > (for all cpus and their states) (1) is definitely simpler way to > disable deeper idle if latency = 0uS > > You can always warn user about that when it's enabled via debugfs/sysfs Thanks for suggestion, now it's clear for me. > -- > Regards, > Sudeep From mboxrd@z Thu Jan 1 00:00:00 1970 From: leo.yan@linaro.org (Leo Yan) Date: Fri, 31 Mar 2017 08:54:20 +0800 Subject: [PATCH v5 6/9] coresight: add support for CPU debug module In-Reply-To: References: <1490466197-29163-1-git-send-email-leo.yan@linaro.org> <1490466197-29163-7-git-send-email-leo.yan@linaro.org> Message-ID: <20170331005420.GA6149@leoy-linaro> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Mar 30, 2017 at 04:56:52PM +0100, Sudeep Holla wrote: [...] > > +static struct pm_qos_request debug_qos_req; > > +static int idle_constraint = PM_QOS_DEFAULT_VALUE; > > +module_param(idle_constraint, int, 0600); > > +MODULE_PARM_DESC(idle_constraint, "Latency requirement in microseconds for CPU " > > + "idle states (default is -1, which means have no limiation " > > + "to CPU idle states; 0 means disabling all idle states; user " > > + "can choose other platform dependent values so can disable " > > + "specific idle states for the platform)"); > > + > > NACK for this. Why you want the policy inside the driver. You can always > do that from the user-space. I have mentioned it several times now. > What can't you do these ? > > 1. echo "what_ever_latency_you_need_in_uS" > /dev/cpu_dma_latency > 2. echo 1 > /sys/devices/system/cpu/cpu$cpu/cpuidle/state$state/disable > (for all cpus and their states) (1) is definitely simpler way to > disable deeper idle if latency = 0uS > > You can always warn user about that when it's enabled via debugfs/sysfs Thanks for suggestion, now it's clear for me. > -- > Regards, > Sudeep