Date: Fri, 15 Sep 2017 18:23:52 +0800
From: Leo Yan <leo.yan@linaro.org>
To: Kim Phillips <kim.phillips@arm.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>,
        Jonathan Corbet <corbet@lwn.net>, linux-arm-kernel@lists.infradead.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] doc: coresight: correct usage for '/dev/cpu_dma_latency'
Message-ID: <20170915102352.GA19208@leoy-linaro>
References: <1503472998-9009-1-git-send-email-leo.yan@linaro.org>
 <20170823110528.ba272128cde46f40c71241cc@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170823110528.ba272128cde46f40c71241cc@arm.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1922
Lines: 49

Hi Kim,

On Wed, Aug 23, 2017 at 11:05:28AM -0500, Kim Phillips wrote:
> On Wed, 23 Aug 2017 15:23:18 +0800
> Leo Yan <leo.yan@linaro.org> wrote:
> 
> > Cc: Kim Phillips <kim.phillips@arm.com>
> > Reported-by: Kim Phillips <kim.phillips@arm.com>
> 
> Thanks; typically only the latter is needed.
> 
> >  Set latency request to /dev/cpu_dma_latency to disable all CPUs specific idle
> > -states (if latency = 0uS then disable all idle states):
> > -# echo "what_ever_latency_you_need_in_uS" > /dev/cpu_dma_latency
> > +states (if latency = 0uS then CPU Idle governor selects idle state0, so this
> > +means 'WFI' is still enabled but other deeper states have be disabled, this
> > +can avoid power off CPUs):
> > +# exec 3<> /dev/cpu_dma_latency; echo "what_ever_latency_you_need_in_uS" >&3
> 
> not a fan of the "what_ever_latency_you_need_in_uS" (including and
> especially the quotes which can create ambiguity in the user's mind):
> just put a cut-n-pasteable example, clarifying the typically-default
> value you chose, and its unit, in the text.
> 
> More to the point, how did you test this?  Are you sure that that the
> value being echoed isn't being interpreted as a binary number?

I checked for this with manually adding log in kernel function, if we
use 'echo' command and input string with 4 chars length then the
parameter cannot be parsed correctly by copy_from_user(); if input
string with other length, the string will be parsed with hexadecimal
format.

> kernel/power/qos.c:pm_qos_power_read() looks to be looking for a 32-bit
> binary integer, but I'm not sure if that's where it gets read.
> 
> Certainly, this 2013 article uses a C example to write a binary integer:
> 
> https://access.redhat.com/articles/65410
> 
> Please double-check, thanks,

Thanks for the reminding. C code with 's32' type can pass correct
value into kernel.

Have sent new version patch, please help review.

Thanks,
Leo Yan