linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cpufreq: tests: Providing cpufreq regression test
@ 2014-07-18 10:23 Lukasz Majewski
  2014-07-18 11:28 ` Sachin Kamat
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Lukasz Majewski @ 2014-07-18 10:23 UTC (permalink / raw)
  To: linux-arm-kernel

This commit adds first regression test "cpufreq_freq_test.sh" for the
cpufreq subsystem.

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
---
 drivers/cpufreq/tests/README               |  23 +++++
 drivers/cpufreq/tests/cpufreq_freq_test.sh | 149 +++++++++++++++++++++++++++++
 2 files changed, 172 insertions(+)
 create mode 100644 drivers/cpufreq/tests/README
 create mode 100755 drivers/cpufreq/tests/cpufreq_freq_test.sh

diff --git a/drivers/cpufreq/tests/README b/drivers/cpufreq/tests/README
new file mode 100644
index 0000000..66638d2
--- /dev/null
+++ b/drivers/cpufreq/tests/README
@@ -0,0 +1,23 @@
+This file contains list of cpufreq's available regression tests with a short
+usage description.
+
+1. cpufreq_freq_test.sh
+
+Description:
+------------
+This test is supposed to test if cpufreq attributes exported by sysfs are
+exposing a correct values.
+
+It can work with or without boost enabled and helps spotting errors related to
+cpufreq and common clock framework.
+
+Used attributes:
+----------------
+- "scaling_available_frequencies"
+- "cpuinfo_cur_freq"
+- "scaling_governor"
+
+Target devices:
+---------------
+
+All devices which exports mentioned above sysfs attributes.
\ No newline at end of file
diff --git a/drivers/cpufreq/tests/cpufreq_freq_test.sh b/drivers/cpufreq/tests/cpufreq_freq_test.sh
new file mode 100755
index 0000000..53156ca
--- /dev/null
+++ b/drivers/cpufreq/tests/cpufreq_freq_test.sh
@@ -0,0 +1,149 @@
+#!/bin/bash
+#
+# This file provides a simple mean to test if all declared freqs at
+# "scaling_available_frequencies" can be set and if "cpuinfo_cur_freq"
+# returns this value.
+#
+# Usage: ./cpufreq_freq_test.sh
+# Requisite: Compile in "performance" governor
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, you can access it online at
+# http://www.gnu.org/licenses/gpl-2.0.html.
+#
+# Copyright (C) Samsung Electronics, 2014
+#
+# Author: Lukasz Majewski <l.majewski@samsung.com>
+
+set +x
+
+COLOUR_RED="\33[31m"
+COLOUR_BLUE="\33[34m"
+COLOUR_GREEN="\33[32m"
+COLOUR_DEFAULT="\33[0m"
+
+T_PATCH=/sys/devices/system/cpu/cpu0/cpufreq
+BOOST_PATCH=/sys/devices/system/cpu/cpufreq
+
+if [ ! -d "$T_PATCH" ]; then
+    printf "   $COLOUR_RED No patch to CPUFREQ $COLOUR_DEFAULT\n"
+    exit 1
+fi
+
+ERRORS=0
+
+OLD_GOV=`cat $T_PATCH/scaling_governor`
+echo "CURRENT GOVERNOR: $OLD_GOV"
+echo "SET GOVERNOR: performance"
+echo "performance" > $T_PATCH/scaling_governor
+
+function test_freqs1 {
+    FREQS=`cat $1`
+    for I in $FREQS; do
+	cpufreq_set_freq $I
+	if [ "$2" ]; then
+	    printf "$COLOUR_BLUE BOOST $COLOUR_DEFAULT" $I
+	fi
+	cpufreq_test_freq $I
+    done
+}
+
+function test_freqs2 {
+    FREQ=`cat $1`
+    FREQS_ARRAY=($FREQ)
+
+    for freq in ${FREQS_ARRAY[@]}
+    do
+	echo "REFERENCE FREQ: $freq"
+	for f in ${FREQS_ARRAY[@]}
+	do
+	    cpufreq_set_freq $freq
+	    echo -n "----> "
+	    cpufreq_set_freq $f
+	    cpufreq_test_freq $f
+	done
+    done
+}
+
+function restore {
+    if [ -f $BOOST_PATCH/boost ]; then
+	cpufreq_boost_state $BOOST_STATE
+    fi
+
+    echo "SET GOVERNOR: $OLD_GOV"
+    echo $OLD_GOV > $T_PATCH/scaling_governor
+}
+
+function die {
+    printf "   $COLOUR_RED FAILED $COLOUR_DEFAULT\n"
+    restore_gov
+    exit 1
+}
+
+function cpufreq_test_freq {
+    gzip < /dev/urandom > /dev/null &
+    pid=$!
+    sleep 0.1
+    CURR_FREQ=`cat $T_PATCH/cpuinfo_cur_freq`
+    if [ $1 -eq $CURR_FREQ ]; then
+	printf "\t$COLOUR_GREEN OK $COLOUR_DEFAULT\n"
+    else
+	printf "$COLOUR_RED CURRENT $CURR_FREQ $COLOUR_DEFAULT\n"
+	ERRORS=`expr $ERRORS + 1`
+	#die
+    fi
+    kill -9 $pid
+    wait $! 2>/dev/null
+}
+
+function cpufreq_set_freq {
+    echo $1 > $T_PATCH/scaling_max_freq || die $?
+    printf "FREQ:$COLOUR_GREEN %s $COLOUR_DEFAULT" $1
+}
+
+function cpufreq_boost_state {
+   echo $1 > $BOOST_PATCH/boost
+}
+
+function cpufreq_boost_status {
+   cat $BOOST_PATCH/boost
+}
+
+if [ -f $BOOST_PATCH/boost ]; then
+    echo "######################################"
+    echo "TEST BOOST OPERATION"
+    echo "######################################"
+
+    BOOST_STATE=$(cpufreq_boost_status)
+    if [ $BOOST_STATE -eq 0 ]; then
+	cpufreq_boost_state 1
+    fi
+    test_freqs1 $T_PATCH/scaling_boost_frequencies 1
+fi
+
+echo "######################################"
+echo "TEST AVAILABLE FREQS"
+echo "######################################"
+test_freqs1 $T_PATCH/scaling_available_frequencies
+
+echo "######################################"
+echo "TEST FREQS SWITCHING"
+echo "######################################"
+test_freqs2 $T_PATCH/scaling_available_frequencies
+
+echo "######################################"
+echo "ERRORS: $ERRORS"
+echo "######################################"
+
+restore
+exit 0
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] cpufreq: tests: Providing cpufreq regression test
  2014-07-18 10:23 [PATCH] cpufreq: tests: Providing cpufreq regression test Lukasz Majewski
@ 2014-07-18 11:28 ` Sachin Kamat
  2014-07-18 11:59   ` Lukasz Majewski
  2014-07-18 11:42 ` Rafael J. Wysocki
  2014-07-21  7:02 ` [PATCH v2] " Lukasz Majewski
  2 siblings, 1 reply; 18+ messages in thread
From: Sachin Kamat @ 2014-07-18 11:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Lukasz,

I tested this script on 4210 based Origen board.
This is the output:
./cpufreq_freq_test.sh
CURRENT GOVERNOR: performance
SET GOVERNOR: performance
######################################
TEST AVAILABLE FREQS
######################################
FREQ: 1200000 sleep: invalid number '0.1'
[    5.918347] random: gzip urandom read with 61 bits of entropy available
         OK
FREQ: 1000000 sleep: invalid number '0.1'
         OK
FREQ: 800000 sleep: invalid number '0.1'
         OK
FREQ: 500000 sleep: invalid number '0.1'
         OK
FREQ: 200000 sleep: invalid number '0.1'
         OK
######################################
TEST FREQS SWITCHING
######################################
REFERENCE FREQ: 1200000
FREQ: 1200000 ----> FREQ: 1200000 sleep: invalid number '0.1'
         OK
FREQ: 1200000 ----> FREQ: 1000000 sleep: invalid number '0.1'
         OK
FREQ: 1200000 ----> FREQ: 800000 sleep: invalid number '0.1'
         OK
FREQ: 1200000 ----> FREQ: 500000 sleep: invalid number '0.1'
         OK
FREQ: 1200000 ----> FREQ: 200000 sleep: invalid number '0.1'
         OK
REFERENCE FREQ: 1000000
FREQ: 1000000 ----> FREQ: 1200000 sleep: invalid number '0.1'
         OK
FREQ: 1000000 ----> FREQ: 1000000 sleep: invalid number '0.1'
         OK
FREQ: 1000000 ----> FREQ: 800000 sleep: invalid number '0.1'
         OK
FREQ: 1000000 ----> FREQ: 500000 sleep: invalid number '0.1'
         OK
FREQ: 1000000 ----> FREQ: 200000 sleep: invalid number '0.1'
         OK
REFERENCE FREQ: 800000
FREQ: 800000 ----> FREQ: 1200000 sleep: invalid number '0.1'
         OK
FREQ: 800000 ----> FREQ: 1000000 sleep: invalid number '0.1'
         OK
FREQ: 800000 ----> FREQ: 800000 sleep: invalid number '0.1'
         OK
FREQ: 800000 ----> FREQ: 500000 sleep: invalid number '0.1'
         OK
FREQ: 800000 ----> FREQ: 200000 sleep: invalid number '0.1'
         OK
REFERENCE FREQ: 500000
FREQ: 500000 ----> FREQ: 1200000 sleep: invalid number '0.1'
         OK
FREQ: 500000 ----> FREQ: 1000000 sleep: invalid number '0.1'
         OK
FREQ: 500000 ----> FREQ: 800000 sleep: invalid number '0.1'
         OK
FREQ: 500000 ----> FREQ: 500000 sleep: invalid number '0.1'
         OK
FREQ: 500000 ----> FREQ: 200000 sleep: invalid number '0.1'
         OK
REFERENCE FREQ: 200000
FREQ: 200000 ----> FREQ: 1200000 sleep: invalid number '0.1'
         OK
FREQ: 200000 ----> FREQ: 1000000 sleep: invalid number '0.1'
         OK
FREQ: 200000 ----> FREQ: 800000 sleep: invalid number '0.1'
         OK
FREQ: 200000 ----> FREQ: 500000 sleep: invalid number '0.1'
         OK
FREQ: 200000 ----> FREQ: 200000 sleep: invalid number '0.1'
         OK
######################################
ERRORS: 0
######################################

Though it says 0 errors, what does the "invalid number..." signify?


On Fri, Jul 18, 2014 at 3:53 PM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> This commit adds first regression test "cpufreq_freq_test.sh" for the
> cpufreq subsystem.
>
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> ---
>  drivers/cpufreq/tests/README               |  23 +++++
>  drivers/cpufreq/tests/cpufreq_freq_test.sh | 149 +++++++++++++++++++++++++++++
>  2 files changed, 172 insertions(+)
>  create mode 100644 drivers/cpufreq/tests/README
>  create mode 100755 drivers/cpufreq/tests/cpufreq_freq_test.sh
>
> diff --git a/drivers/cpufreq/tests/README b/drivers/cpufreq/tests/README
> new file mode 100644
> index 0000000..66638d2
> --- /dev/null
> +++ b/drivers/cpufreq/tests/README
> @@ -0,0 +1,23 @@
> +This file contains list of cpufreq's available regression tests with a short
> +usage description.
> +
> +1. cpufreq_freq_test.sh
> +
> +Description:
> +------------
> +This test is supposed to test if cpufreq attributes exported by sysfs are

s/test/script would be better

> +exposing a correct values.

s/ exposing a correct values / exposing correct values

> +
> +It can work with or without boost enabled and helps spotting errors related to

s/ helps spotting / helps in spotting

<snip>

> +
> +set +x
> +
> +COLOUR_RED="\33[31m"
> +COLOUR_BLUE="\33[34m"
> +COLOUR_GREEN="\33[32m"
> +COLOUR_DEFAULT="\33[0m"
> +
> +T_PATCH=/sys/devices/system/cpu/cpu0/cpufreq

Shouldn't this be called PATH instead of PATCH?

> +BOOST_PATCH=/sys/devices/system/cpu/cpufreq

ditto and rest of the places in the document.

-- 
Regards,
Sachin.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] cpufreq: tests: Providing cpufreq regression test
  2014-07-18 10:23 [PATCH] cpufreq: tests: Providing cpufreq regression test Lukasz Majewski
  2014-07-18 11:28 ` Sachin Kamat
@ 2014-07-18 11:42 ` Rafael J. Wysocki
  2014-07-18 12:00   ` Lukasz Majewski
  2014-07-21  7:02 ` [PATCH v2] " Lukasz Majewski
  2 siblings, 1 reply; 18+ messages in thread
From: Rafael J. Wysocki @ 2014-07-18 11:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday, July 18, 2014 12:23:05 PM Lukasz Majewski wrote:
> This commit adds first regression test "cpufreq_freq_test.sh" for the
> cpufreq subsystem.

Care to add any description of how it is supposed to work and what it is
going to test?

> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> ---
>  drivers/cpufreq/tests/README               |  23 +++++
>  drivers/cpufreq/tests/cpufreq_freq_test.sh | 149 +++++++++++++++++++++++++++++
>  2 files changed, 172 insertions(+)
>  create mode 100644 drivers/cpufreq/tests/README
>  create mode 100755 drivers/cpufreq/tests/cpufreq_freq_test.sh
> 
> diff --git a/drivers/cpufreq/tests/README b/drivers/cpufreq/tests/README
> new file mode 100644
> index 0000000..66638d2
> --- /dev/null
> +++ b/drivers/cpufreq/tests/README
> @@ -0,0 +1,23 @@
> +This file contains list of cpufreq's available regression tests with a short
> +usage description.
> +
> +1. cpufreq_freq_test.sh
> +
> +Description:
> +------------
> +This test is supposed to test if cpufreq attributes exported by sysfs are
> +exposing a correct values.
> +
> +It can work with or without boost enabled and helps spotting errors related to
> +cpufreq and common clock framework.
> +
> +Used attributes:
> +----------------
> +- "scaling_available_frequencies"
> +- "cpuinfo_cur_freq"
> +- "scaling_governor"
> +
> +Target devices:
> +---------------
> +
> +All devices which exports mentioned above sysfs attributes.
> \ No newline at end of file
> diff --git a/drivers/cpufreq/tests/cpufreq_freq_test.sh b/drivers/cpufreq/tests/cpufreq_freq_test.sh
> new file mode 100755
> index 0000000..53156ca
> --- /dev/null
> +++ b/drivers/cpufreq/tests/cpufreq_freq_test.sh
> @@ -0,0 +1,149 @@
> +#!/bin/bash
> +#
> +# This file provides a simple mean to test if all declared freqs at
> +# "scaling_available_frequencies" can be set and if "cpuinfo_cur_freq"
> +# returns this value.
> +#
> +# Usage: ./cpufreq_freq_test.sh
> +# Requisite: Compile in "performance" governor
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, you can access it online at
> +# http://www.gnu.org/licenses/gpl-2.0.html.
> +#
> +# Copyright (C) Samsung Electronics, 2014
> +#
> +# Author: Lukasz Majewski <l.majewski@samsung.com>
> +
> +set +x
> +
> +COLOUR_RED="\33[31m"
> +COLOUR_BLUE="\33[34m"
> +COLOUR_GREEN="\33[32m"
> +COLOUR_DEFAULT="\33[0m"
> +
> +T_PATCH=/sys/devices/system/cpu/cpu0/cpufreq
> +BOOST_PATCH=/sys/devices/system/cpu/cpufreq
> +
> +if [ ! -d "$T_PATCH" ]; then
> +    printf "   $COLOUR_RED No patch to CPUFREQ $COLOUR_DEFAULT\n"
> +    exit 1
> +fi
> +
> +ERRORS=0
> +
> +OLD_GOV=`cat $T_PATCH/scaling_governor`
> +echo "CURRENT GOVERNOR: $OLD_GOV"
> +echo "SET GOVERNOR: performance"
> +echo "performance" > $T_PATCH/scaling_governor
> +
> +function test_freqs1 {
> +    FREQS=`cat $1`
> +    for I in $FREQS; do
> +	cpufreq_set_freq $I
> +	if [ "$2" ]; then
> +	    printf "$COLOUR_BLUE BOOST $COLOUR_DEFAULT" $I
> +	fi
> +	cpufreq_test_freq $I
> +    done
> +}
> +
> +function test_freqs2 {
> +    FREQ=`cat $1`
> +    FREQS_ARRAY=($FREQ)
> +
> +    for freq in ${FREQS_ARRAY[@]}
> +    do
> +	echo "REFERENCE FREQ: $freq"
> +	for f in ${FREQS_ARRAY[@]}
> +	do
> +	    cpufreq_set_freq $freq
> +	    echo -n "----> "
> +	    cpufreq_set_freq $f
> +	    cpufreq_test_freq $f
> +	done
> +    done
> +}
> +
> +function restore {
> +    if [ -f $BOOST_PATCH/boost ]; then
> +	cpufreq_boost_state $BOOST_STATE
> +    fi
> +
> +    echo "SET GOVERNOR: $OLD_GOV"
> +    echo $OLD_GOV > $T_PATCH/scaling_governor
> +}
> +
> +function die {
> +    printf "   $COLOUR_RED FAILED $COLOUR_DEFAULT\n"
> +    restore_gov
> +    exit 1
> +}
> +
> +function cpufreq_test_freq {
> +    gzip < /dev/urandom > /dev/null &
> +    pid=$!
> +    sleep 0.1
> +    CURR_FREQ=`cat $T_PATCH/cpuinfo_cur_freq`
> +    if [ $1 -eq $CURR_FREQ ]; then
> +	printf "\t$COLOUR_GREEN OK $COLOUR_DEFAULT\n"
> +    else
> +	printf "$COLOUR_RED CURRENT $CURR_FREQ $COLOUR_DEFAULT\n"
> +	ERRORS=`expr $ERRORS + 1`
> +	#die
> +    fi
> +    kill -9 $pid
> +    wait $! 2>/dev/null
> +}
> +
> +function cpufreq_set_freq {
> +    echo $1 > $T_PATCH/scaling_max_freq || die $?
> +    printf "FREQ:$COLOUR_GREEN %s $COLOUR_DEFAULT" $1
> +}
> +
> +function cpufreq_boost_state {
> +   echo $1 > $BOOST_PATCH/boost
> +}
> +
> +function cpufreq_boost_status {
> +   cat $BOOST_PATCH/boost
> +}
> +
> +if [ -f $BOOST_PATCH/boost ]; then
> +    echo "######################################"
> +    echo "TEST BOOST OPERATION"
> +    echo "######################################"
> +
> +    BOOST_STATE=$(cpufreq_boost_status)
> +    if [ $BOOST_STATE -eq 0 ]; then
> +	cpufreq_boost_state 1
> +    fi
> +    test_freqs1 $T_PATCH/scaling_boost_frequencies 1
> +fi
> +
> +echo "######################################"
> +echo "TEST AVAILABLE FREQS"
> +echo "######################################"
> +test_freqs1 $T_PATCH/scaling_available_frequencies
> +
> +echo "######################################"
> +echo "TEST FREQS SWITCHING"
> +echo "######################################"
> +test_freqs2 $T_PATCH/scaling_available_frequencies
> +
> +echo "######################################"
> +echo "ERRORS: $ERRORS"
> +echo "######################################"
> +
> +restore
> +exit 0
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] cpufreq: tests: Providing cpufreq regression test
  2014-07-18 11:28 ` Sachin Kamat
@ 2014-07-18 11:59   ` Lukasz Majewski
  2014-07-22  4:13     ` Sachin Kamat
  0 siblings, 1 reply; 18+ messages in thread
From: Lukasz Majewski @ 2014-07-18 11:59 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Sachin,

> Hi Lukasz,
> 
> I tested this script on 4210 based Origen board.
> This is the output:
> ./cpufreq_freq_test.sh
> CURRENT GOVERNOR: performance
> SET GOVERNOR: performance
> ######################################
> TEST AVAILABLE FREQS
> ######################################
> FREQ: 1200000 sleep: invalid number '0.1'
> [    5.918347] random: gzip urandom read with 61 bits of entropy
> available OK
> FREQ: 1000000 sleep: invalid number '0.1'
>          OK
> FREQ: 800000 sleep: invalid number '0.1'
>          OK
> FREQ: 500000 sleep: invalid number '0.1'
>          OK
> FREQ: 200000 sleep: invalid number '0.1'
>          OK
> ######################################
> TEST FREQS SWITCHING
> ######################################
> REFERENCE FREQ: 1200000
> FREQ: 1200000 ----> FREQ: 1200000 sleep: invalid number '0.1'
>          OK
> FREQ: 1200000 ----> FREQ: 1000000 sleep: invalid number '0.1'
>          OK
> FREQ: 1200000 ----> FREQ: 800000 sleep: invalid number '0.1'
>          OK
> FREQ: 1200000 ----> FREQ: 500000 sleep: invalid number '0.1'
>          OK
> FREQ: 1200000 ----> FREQ: 200000 sleep: invalid number '0.1'
>          OK
> REFERENCE FREQ: 1000000
> FREQ: 1000000 ----> FREQ: 1200000 sleep: invalid number '0.1'
>          OK
> FREQ: 1000000 ----> FREQ: 1000000 sleep: invalid number '0.1'
>          OK
> FREQ: 1000000 ----> FREQ: 800000 sleep: invalid number '0.1'
>          OK
> FREQ: 1000000 ----> FREQ: 500000 sleep: invalid number '0.1'
>          OK
> FREQ: 1000000 ----> FREQ: 200000 sleep: invalid number '0.1'
>          OK
> REFERENCE FREQ: 800000
> FREQ: 800000 ----> FREQ: 1200000 sleep: invalid number '0.1'
>          OK
> FREQ: 800000 ----> FREQ: 1000000 sleep: invalid number '0.1'
>          OK
> FREQ: 800000 ----> FREQ: 800000 sleep: invalid number '0.1'
>          OK
> FREQ: 800000 ----> FREQ: 500000 sleep: invalid number '0.1'
>          OK
> FREQ: 800000 ----> FREQ: 200000 sleep: invalid number '0.1'
>          OK
> REFERENCE FREQ: 500000
> FREQ: 500000 ----> FREQ: 1200000 sleep: invalid number '0.1'
>          OK
> FREQ: 500000 ----> FREQ: 1000000 sleep: invalid number '0.1'
>          OK
> FREQ: 500000 ----> FREQ: 800000 sleep: invalid number '0.1'
>          OK
> FREQ: 500000 ----> FREQ: 500000 sleep: invalid number '0.1'
>          OK
> FREQ: 500000 ----> FREQ: 200000 sleep: invalid number '0.1'
>          OK
> REFERENCE FREQ: 200000
> FREQ: 200000 ----> FREQ: 1200000 sleep: invalid number '0.1'
>          OK
> FREQ: 200000 ----> FREQ: 1000000 sleep: invalid number '0.1'
>          OK
> FREQ: 200000 ----> FREQ: 800000 sleep: invalid number '0.1'
>          OK
> FREQ: 200000 ----> FREQ: 500000 sleep: invalid number '0.1'
>          OK
> FREQ: 200000 ----> FREQ: 200000 sleep: invalid number '0.1'
>          OK
> ######################################
> ERRORS: 0
> ######################################
> 
> Though it says 0 errors, what does the "invalid number..." signify?

I guess that this message is caused by your default sleep
implementation.

Could you type 'sleep 0.1' and then 'sleep 1' in your console on the
target system?
Is the "invalid number" not present with the second case?


> 
> 
> On Fri, Jul 18, 2014 at 3:53 PM, Lukasz Majewski
> <l.majewski@samsung.com> wrote:
> > This commit adds first regression test "cpufreq_freq_test.sh" for
> > the cpufreq subsystem.
> >
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > ---
> >  drivers/cpufreq/tests/README               |  23 +++++
> >  drivers/cpufreq/tests/cpufreq_freq_test.sh | 149
> > +++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+)
> >  create mode 100644 drivers/cpufreq/tests/README
> >  create mode 100755 drivers/cpufreq/tests/cpufreq_freq_test.sh
> >
> > diff --git a/drivers/cpufreq/tests/README
> > b/drivers/cpufreq/tests/README new file mode 100644
> > index 0000000..66638d2
> > --- /dev/null
> > +++ b/drivers/cpufreq/tests/README
> > @@ -0,0 +1,23 @@
> > +This file contains list of cpufreq's available regression tests
> > with a short +usage description.
> > +
> > +1. cpufreq_freq_test.sh
> > +
> > +Description:
> > +------------
> > +This test is supposed to test if cpufreq attributes exported by
> > sysfs are
> 
> s/test/script would be better

Yes, you are right.

> 
> > +exposing a correct values.
> 
> s/ exposing a correct values / exposing correct values
> 
> > +
> > +It can work with or without boost enabled and helps spotting
> > errors related to
> 
> s/ helps spotting / helps in spotting
> 

Thanks for feedback.

> <snip>
> 
> > +
> > +set +x
> > +
> > +COLOUR_RED="\33[31m"
> > +COLOUR_BLUE="\33[34m"
> > +COLOUR_GREEN="\33[32m"
> > +COLOUR_DEFAULT="\33[0m"
> > +
> > +T_PATCH=/sys/devices/system/cpu/cpu0/cpufreq
> 
> Shouldn't this be called PATH instead of PATCH?

Hmm.... It really should be PATH :-).

> 
> > +BOOST_PATCH=/sys/devices/system/cpu/cpufreq
> 
> ditto and rest of the places in the document.
> 

Ok, I will correct that.

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] cpufreq: tests: Providing cpufreq regression test
  2014-07-18 11:42 ` Rafael J. Wysocki
@ 2014-07-18 12:00   ` Lukasz Majewski
  0 siblings, 0 replies; 18+ messages in thread
From: Lukasz Majewski @ 2014-07-18 12:00 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Rafael,

> On Friday, July 18, 2014 12:23:05 PM Lukasz Majewski wrote:
> > This commit adds first regression test "cpufreq_freq_test.sh" for
> > the cpufreq subsystem.
> 
> Care to add any description of how it is supposed to work and what it
> is going to test?

Ok. I will extend the description in the README file.

> 
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > ---
> >  drivers/cpufreq/tests/README               |  23 +++++
> >  drivers/cpufreq/tests/cpufreq_freq_test.sh | 149
> > +++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+)
> >  create mode 100644 drivers/cpufreq/tests/README
> >  create mode 100755 drivers/cpufreq/tests/cpufreq_freq_test.sh
> > 
> > diff --git a/drivers/cpufreq/tests/README
> > b/drivers/cpufreq/tests/README new file mode 100644
> > index 0000000..66638d2
> > --- /dev/null
> > +++ b/drivers/cpufreq/tests/README
> > @@ -0,0 +1,23 @@
> > +This file contains list of cpufreq's available regression tests
> > with a short +usage description.
> > +
> > +1. cpufreq_freq_test.sh
> > +
> > +Description:
> > +------------
> > +This test is supposed to test if cpufreq attributes exported by
> > sysfs are +exposing a correct values.
> > +
> > +It can work with or without boost enabled and helps spotting
> > errors related to +cpufreq and common clock framework.
> > +
> > +Used attributes:
> > +----------------
> > +- "scaling_available_frequencies"
> > +- "cpuinfo_cur_freq"
> > +- "scaling_governor"
> > +
> > +Target devices:
> > +---------------
> > +
> > +All devices which exports mentioned above sysfs attributes.
> > \ No newline at end of file
> > diff --git a/drivers/cpufreq/tests/cpufreq_freq_test.sh
> > b/drivers/cpufreq/tests/cpufreq_freq_test.sh new file mode 100755
> > index 0000000..53156ca
> > --- /dev/null
> > +++ b/drivers/cpufreq/tests/cpufreq_freq_test.sh
> > @@ -0,0 +1,149 @@
> > +#!/bin/bash
> > +#
> > +# This file provides a simple mean to test if all declared freqs at
> > +# "scaling_available_frequencies" can be set and if
> > "cpuinfo_cur_freq" +# returns this value.
> > +#
> > +# Usage: ./cpufreq_freq_test.sh
> > +# Requisite: Compile in "performance" governor
> > +#
> > +# This program is free software; you can redistribute it and/or
> > modify +# it under the terms of the GNU General Public License as
> > published by +# the Free Software Foundation; either version 2 of
> > the License, or +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, you can access it online at
> > +# http://www.gnu.org/licenses/gpl-2.0.html.
> > +#
> > +# Copyright (C) Samsung Electronics, 2014
> > +#
> > +# Author: Lukasz Majewski <l.majewski@samsung.com>
> > +
> > +set +x
> > +
> > +COLOUR_RED="\33[31m"
> > +COLOUR_BLUE="\33[34m"
> > +COLOUR_GREEN="\33[32m"
> > +COLOUR_DEFAULT="\33[0m"
> > +
> > +T_PATCH=/sys/devices/system/cpu/cpu0/cpufreq
> > +BOOST_PATCH=/sys/devices/system/cpu/cpufreq
> > +
> > +if [ ! -d "$T_PATCH" ]; then
> > +    printf "   $COLOUR_RED No patch to CPUFREQ $COLOUR_DEFAULT\n"
> > +    exit 1
> > +fi
> > +
> > +ERRORS=0
> > +
> > +OLD_GOV=`cat $T_PATCH/scaling_governor`
> > +echo "CURRENT GOVERNOR: $OLD_GOV"
> > +echo "SET GOVERNOR: performance"
> > +echo "performance" > $T_PATCH/scaling_governor
> > +
> > +function test_freqs1 {
> > +    FREQS=`cat $1`
> > +    for I in $FREQS; do
> > +	cpufreq_set_freq $I
> > +	if [ "$2" ]; then
> > +	    printf "$COLOUR_BLUE BOOST $COLOUR_DEFAULT" $I
> > +	fi
> > +	cpufreq_test_freq $I
> > +    done
> > +}
> > +
> > +function test_freqs2 {
> > +    FREQ=`cat $1`
> > +    FREQS_ARRAY=($FREQ)
> > +
> > +    for freq in ${FREQS_ARRAY[@]}
> > +    do
> > +	echo "REFERENCE FREQ: $freq"
> > +	for f in ${FREQS_ARRAY[@]}
> > +	do
> > +	    cpufreq_set_freq $freq
> > +	    echo -n "----> "
> > +	    cpufreq_set_freq $f
> > +	    cpufreq_test_freq $f
> > +	done
> > +    done
> > +}
> > +
> > +function restore {
> > +    if [ -f $BOOST_PATCH/boost ]; then
> > +	cpufreq_boost_state $BOOST_STATE
> > +    fi
> > +
> > +    echo "SET GOVERNOR: $OLD_GOV"
> > +    echo $OLD_GOV > $T_PATCH/scaling_governor
> > +}
> > +
> > +function die {
> > +    printf "   $COLOUR_RED FAILED $COLOUR_DEFAULT\n"
> > +    restore_gov
> > +    exit 1
> > +}
> > +
> > +function cpufreq_test_freq {
> > +    gzip < /dev/urandom > /dev/null &
> > +    pid=$!
> > +    sleep 0.1
> > +    CURR_FREQ=`cat $T_PATCH/cpuinfo_cur_freq`
> > +    if [ $1 -eq $CURR_FREQ ]; then
> > +	printf "\t$COLOUR_GREEN OK $COLOUR_DEFAULT\n"
> > +    else
> > +	printf "$COLOUR_RED CURRENT $CURR_FREQ $COLOUR_DEFAULT\n"
> > +	ERRORS=`expr $ERRORS + 1`
> > +	#die
> > +    fi
> > +    kill -9 $pid
> > +    wait $! 2>/dev/null
> > +}
> > +
> > +function cpufreq_set_freq {
> > +    echo $1 > $T_PATCH/scaling_max_freq || die $?
> > +    printf "FREQ:$COLOUR_GREEN %s $COLOUR_DEFAULT" $1
> > +}
> > +
> > +function cpufreq_boost_state {
> > +   echo $1 > $BOOST_PATCH/boost
> > +}
> > +
> > +function cpufreq_boost_status {
> > +   cat $BOOST_PATCH/boost
> > +}
> > +
> > +if [ -f $BOOST_PATCH/boost ]; then
> > +    echo "######################################"
> > +    echo "TEST BOOST OPERATION"
> > +    echo "######################################"
> > +
> > +    BOOST_STATE=$(cpufreq_boost_status)
> > +    if [ $BOOST_STATE -eq 0 ]; then
> > +	cpufreq_boost_state 1
> > +    fi
> > +    test_freqs1 $T_PATCH/scaling_boost_frequencies 1
> > +fi
> > +
> > +echo "######################################"
> > +echo "TEST AVAILABLE FREQS"
> > +echo "######################################"
> > +test_freqs1 $T_PATCH/scaling_available_frequencies
> > +
> > +echo "######################################"
> > +echo "TEST FREQS SWITCHING"
> > +echo "######################################"
> > +test_freqs2 $T_PATCH/scaling_available_frequencies
> > +
> > +echo "######################################"
> > +echo "ERRORS: $ERRORS"
> > +echo "######################################"
> > +
> > +restore
> > +exit 0
> > 
> 



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-18 10:23 [PATCH] cpufreq: tests: Providing cpufreq regression test Lukasz Majewski
  2014-07-18 11:28 ` Sachin Kamat
  2014-07-18 11:42 ` Rafael J. Wysocki
@ 2014-07-21  7:02 ` Lukasz Majewski
  2014-07-23  5:06   ` Viresh Kumar
  2014-07-23 23:58   ` Rafael J. Wysocki
  2 siblings, 2 replies; 18+ messages in thread
From: Lukasz Majewski @ 2014-07-21  7:02 UTC (permalink / raw)
  To: linux-arm-kernel

This commit adds first regression test "cpufreq_freq_test.sh" for the
cpufreq subsystem.

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>

---
Changes for v2:
- Replace *_PATCH with *_PATH for variables names
- Corrected mistakes in the README file
- Providing detailed explanation of the patch in the README file
---
 drivers/cpufreq/tests/README               |  33 +++++++
 drivers/cpufreq/tests/cpufreq_freq_test.sh | 149 +++++++++++++++++++++++++++++
 2 files changed, 182 insertions(+)
 create mode 100644 drivers/cpufreq/tests/README
 create mode 100755 drivers/cpufreq/tests/cpufreq_freq_test.sh

diff --git a/drivers/cpufreq/tests/README b/drivers/cpufreq/tests/README
new file mode 100644
index 0000000..3e9cd80
--- /dev/null
+++ b/drivers/cpufreq/tests/README
@@ -0,0 +1,33 @@
+This file contains list of cpufreq's available regression tests with a short
+usage description.
+
+1. cpufreq_freq_test.sh
+
+Description:
+------------
+This script is supposed to test if cpufreq attributes exported by sysfs are
+exposing correct values.
+
+To achieve this goal it saves the current governor and changes it to
+"performance". Afterwards, it reads the "scaling_available_frequencies"
+property. With the list of supported frequencies it is able to enforce each of
+them by writing to "scaling_max_freq" attribute. To make the test more reliable
+a superfluous load with gzip is created to be sure that we are running with
+highest possible frequency. This high load is regulated with the 'sleep'
+duration. After this time the "cpufreq_cur_freq" is read and compared with the
+original value. As the last step the original governor is restored.
+
+This script can work with or without BOOST enabled and helps in spotting errors
+related to cpufreq and common clock framework.
+
+Used attributes:
+----------------
+- "scaling_available_frequencies"
+- "cpuinfo_cur_freq"
+- "scaling_governor"
+- "scaling_max_freq"
+
+Target devices:
+---------------
+
+All devices which exports mentioned above sysfs attributes.
\ No newline at end of file
diff --git a/drivers/cpufreq/tests/cpufreq_freq_test.sh b/drivers/cpufreq/tests/cpufreq_freq_test.sh
new file mode 100755
index 0000000..c25f05c
--- /dev/null
+++ b/drivers/cpufreq/tests/cpufreq_freq_test.sh
@@ -0,0 +1,149 @@
+#!/bin/bash
+#
+# This file provides a simple mean to test if all declared freqs at
+# "scaling_available_frequencies" can be set and if "cpuinfo_cur_freq"
+# returns this value.
+#
+# Usage: ./cpufreq_freq_test.sh
+# Requisite: Compiled in "performance" governor
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, you can access it online at
+# http://www.gnu.org/licenses/gpl-2.0.html.
+#
+# Copyright (C) Samsung Electronics, 2014
+#
+# Author: Lukasz Majewski <l.majewski@samsung.com>
+
+set +x
+
+COLOUR_RED="\33[31m"
+COLOUR_BLUE="\33[34m"
+COLOUR_GREEN="\33[32m"
+COLOUR_DEFAULT="\33[0m"
+
+T_PATH=/sys/devices/system/cpu/cpu0/cpufreq
+BOOST_PATH=/sys/devices/system/cpu/cpufreq
+
+if [ ! -d "$T_PATH" ]; then
+    printf "   $COLOUR_RED No path to CPUFREQ $COLOUR_DEFAULT\n"
+    exit 1
+fi
+
+ERRORS=0
+
+OLD_GOV=`cat $T_PATH/scaling_governor`
+echo "CURRENT GOVERNOR: $OLD_GOV"
+echo "SET GOVERNOR: performance"
+echo "performance" > $T_PATH/scaling_governor
+
+function test_freqs1 {
+    FREQS=`cat $1`
+    for I in $FREQS; do
+	cpufreq_set_freq $I
+	if [ "$2" ]; then
+	    printf "$COLOUR_BLUE BOOST $COLOUR_DEFAULT" $I
+	fi
+	cpufreq_test_freq $I
+    done
+}
+
+function test_freqs2 {
+    FREQ=`cat $1`
+    FREQS_ARRAY=($FREQ)
+
+    for freq in ${FREQS_ARRAY[@]}
+    do
+	echo "REFERENCE FREQ: $freq"
+	for f in ${FREQS_ARRAY[@]}
+	do
+	    cpufreq_set_freq $freq
+	    echo -n "----> "
+	    cpufreq_set_freq $f
+	    cpufreq_test_freq $f
+	done
+    done
+}
+
+function restore {
+    if [ -f $BOOST_PATH/boost ]; then
+	cpufreq_boost_state $BOOST_STATE
+    fi
+
+    echo "SET GOVERNOR: $OLD_GOV"
+    echo $OLD_GOV > $T_PATH/scaling_governor
+}
+
+function die {
+    printf "   $COLOUR_RED FAILED $COLOUR_DEFAULT\n"
+    restore_gov
+    exit 1
+}
+
+function cpufreq_test_freq {
+    gzip < /dev/urandom > /dev/null &
+    pid=$!
+    sleep 0.1
+    CURR_FREQ=`cat $T_PATH/cpuinfo_cur_freq`
+    if [ $1 -eq $CURR_FREQ ]; then
+	printf "\t$COLOUR_GREEN OK $COLOUR_DEFAULT\n"
+    else
+	printf "$COLOUR_RED CURRENT $CURR_FREQ $COLOUR_DEFAULT\n"
+	ERRORS=`expr $ERRORS + 1`
+	#die
+    fi
+    kill -9 $pid
+    wait $! 2>/dev/null
+}
+
+function cpufreq_set_freq {
+    echo $1 > $T_PATH/scaling_max_freq || die $?
+    printf "FREQ:$COLOUR_GREEN %s $COLOUR_DEFAULT" $1
+}
+
+function cpufreq_boost_state {
+   echo $1 > $BOOST_PATH/boost
+}
+
+function cpufreq_boost_status {
+   cat $BOOST_PATH/boost
+}
+
+if [ -f $BOOST_PATH/boost ]; then
+    echo "######################################"
+    echo "TEST BOOST OPERATION"
+    echo "######################################"
+
+    BOOST_STATE=$(cpufreq_boost_status)
+    if [ $BOOST_STATE -eq 0 ]; then
+	cpufreq_boost_state 1
+    fi
+    test_freqs1 $T_PATH/scaling_boost_frequencies 1
+fi
+
+echo "######################################"
+echo "TEST AVAILABLE FREQS"
+echo "######################################"
+test_freqs1 $T_PATH/scaling_available_frequencies
+
+echo "######################################"
+echo "TEST FREQS SWITCHING"
+echo "######################################"
+test_freqs2 $T_PATH/scaling_available_frequencies
+
+echo "######################################"
+echo "ERRORS: $ERRORS"
+echo "######################################"
+
+restore
+exit 0
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] cpufreq: tests: Providing cpufreq regression test
  2014-07-18 11:59   ` Lukasz Majewski
@ 2014-07-22  4:13     ` Sachin Kamat
  0 siblings, 0 replies; 18+ messages in thread
From: Sachin Kamat @ 2014-07-22  4:13 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Lukasz,

On Fri, Jul 18, 2014 at 5:29 PM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Hi Sachin,
>
>> Hi Lukasz,
>>
>> I tested this script on 4210 based Origen board.
>> This is the output:
>> ./cpufreq_freq_test.sh
>> CURRENT GOVERNOR: performance
>> SET GOVERNOR: performance
>> ######################################
>> TEST AVAILABLE FREQS
>> ######################################
>> FREQ: 1200000 sleep: invalid number '0.1'
>> [    5.918347] random: gzip urandom read with 61 bits of entropy
>> available OK
>> FREQ: 1000000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 800000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 500000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 200000 sleep: invalid number '0.1'
>>          OK
>> ######################################
>> TEST FREQS SWITCHING
>> ######################################
>> REFERENCE FREQ: 1200000
>> FREQ: 1200000 ----> FREQ: 1200000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 1200000 ----> FREQ: 1000000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 1200000 ----> FREQ: 800000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 1200000 ----> FREQ: 500000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 1200000 ----> FREQ: 200000 sleep: invalid number '0.1'
>>          OK
>> REFERENCE FREQ: 1000000
>> FREQ: 1000000 ----> FREQ: 1200000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 1000000 ----> FREQ: 1000000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 1000000 ----> FREQ: 800000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 1000000 ----> FREQ: 500000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 1000000 ----> FREQ: 200000 sleep: invalid number '0.1'
>>          OK
>> REFERENCE FREQ: 800000
>> FREQ: 800000 ----> FREQ: 1200000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 800000 ----> FREQ: 1000000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 800000 ----> FREQ: 800000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 800000 ----> FREQ: 500000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 800000 ----> FREQ: 200000 sleep: invalid number '0.1'
>>          OK
>> REFERENCE FREQ: 500000
>> FREQ: 500000 ----> FREQ: 1200000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 500000 ----> FREQ: 1000000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 500000 ----> FREQ: 800000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 500000 ----> FREQ: 500000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 500000 ----> FREQ: 200000 sleep: invalid number '0.1'
>>          OK
>> REFERENCE FREQ: 200000
>> FREQ: 200000 ----> FREQ: 1200000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 200000 ----> FREQ: 1000000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 200000 ----> FREQ: 800000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 200000 ----> FREQ: 500000 sleep: invalid number '0.1'
>>          OK
>> FREQ: 200000 ----> FREQ: 200000 sleep: invalid number '0.1'
>>          OK
>> ######################################
>> ERRORS: 0
>> ######################################
>>
>> Though it says 0 errors, what does the "invalid number..." signify?
>
> I guess that this message is caused by your default sleep
> implementation.
>
> Could you type 'sleep 0.1' and then 'sleep 1' in your console on the
> target system?
> Is the "invalid number" not present with the second case?

Only with first case (sleep 0.1) I get the "invalid number" message.
sleep 1 seems to be OK.


-- 
Regards,
Sachin.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-21  7:02 ` [PATCH v2] " Lukasz Majewski
@ 2014-07-23  5:06   ` Viresh Kumar
  2014-07-23  7:38     ` Lukasz Majewski
  2014-07-23 23:58   ` Rafael J. Wysocki
  1 sibling, 1 reply; 18+ messages in thread
From: Viresh Kumar @ 2014-07-23  5:06 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Lukasz,

I haven't replied yet as I wanted to see what the general feed of Rafael
is going to be :)

As this is something new and wasn't sure if we really want this..

On 21 July 2014 12:32, Lukasz Majewski <l.majewski@samsung.com> wrote:
> This commit adds first regression test "cpufreq_freq_test.sh" for the
> cpufreq subsystem.

That's not enough, Tell us why we should continue reading this mail..

> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
>
> ---
> Changes for v2:
> - Replace *_PATCH with *_PATH for variables names
> - Corrected mistakes in the README file
> - Providing detailed explanation of the patch in the README file
> ---
>  drivers/cpufreq/tests/README               |  33 +++++++
>  drivers/cpufreq/tests/cpufreq_freq_test.sh | 149 +++++++++++++++++++++++++++++

Probably a better place would be tools/power/cpufreq/

@Rafael?

>  2 files changed, 182 insertions(+)
>  create mode 100644 drivers/cpufreq/tests/README
>  create mode 100755 drivers/cpufreq/tests/cpufreq_freq_test.sh
>
> diff --git a/drivers/cpufreq/tests/README b/drivers/cpufreq/tests/README
> new file mode 100644
> index 0000000..3e9cd80
> --- /dev/null
> +++ b/drivers/cpufreq/tests/README
> @@ -0,0 +1,33 @@
> +This file contains list of cpufreq's available regression tests with a short
> +usage description.
> +
> +1. cpufreq_freq_test.sh
> +
> +Description:
> +------------
> +This script is supposed to test if cpufreq attributes exported by sysfs are
> +exposing correct values.
> +
> +To achieve this goal it saves the current governor and changes it to
> +"performance". Afterwards, it reads the "scaling_available_frequencies"
> +property. With the list of supported frequencies it is able to enforce each of
> +them by writing to "scaling_max_freq" attribute. To make the test more reliable
> +a superfluous load with gzip is created to be sure that we are running with
> +highest possible frequency. This high load is regulated with the 'sleep'
> +duration. After this time the "cpufreq_cur_freq" is read and compared with the
> +original value. As the last step the original governor is restored.

I couldn't make out the purpose of this test and why we need it. How do
we ensure that "cpufreq attributes exported by sysfs are exposing
correct values"?

And actually what do we mean by this statement even? What kind of errors can
be there in exposing these values.

I want to understand the purpose of this script very clearly first and then only
will look at the details.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-23  5:06   ` Viresh Kumar
@ 2014-07-23  7:38     ` Lukasz Majewski
  2014-07-23  8:49       ` Viresh Kumar
  0 siblings, 1 reply; 18+ messages in thread
From: Lukasz Majewski @ 2014-07-23  7:38 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Viresh,

> Hi Lukasz,
> 
> I haven't replied yet as I wanted to see what the general feed of
> Rafael is going to be :)
> 
> As this is something new and wasn't sure if we really want this..

Do you want to say that we have enough tests and we don't need more ?

I always thought that we shall have as much regression tests as
possible.


> 
> On 21 July 2014 12:32, Lukasz Majewski <l.majewski@samsung.com> wrote:
> > This commit adds first regression test "cpufreq_freq_test.sh" for
> > the cpufreq subsystem.
> 
> That's not enough, Tell us why we should continue reading this mail..

Hmm... If "regression" and "test" don't catch the attention of a
diligent maintainer, then I cannot do much more to encourage him to
read the whole e-mail :-)

I can imagine that maintainers are very busy, therefore I've prepared
README file with detailed description of the script operation.

> 
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> >
> > ---
> > Changes for v2:
> > - Replace *_PATCH with *_PATH for variables names
> > - Corrected mistakes in the README file
> > - Providing detailed explanation of the patch in the README file
> > ---
> >  drivers/cpufreq/tests/README               |  33 +++++++
> >  drivers/cpufreq/tests/cpufreq_freq_test.sh | 149
> > +++++++++++++++++++++++++++++
> 
> Probably a better place would be tools/power/cpufreq/
> 
> @Rafael?

Rafael, I'm open for suggestions.

> 
> >  2 files changed, 182 insertions(+)
> >  create mode 100644 drivers/cpufreq/tests/README
> >  create mode 100755 drivers/cpufreq/tests/cpufreq_freq_test.sh
> >
> > diff --git a/drivers/cpufreq/tests/README
> > b/drivers/cpufreq/tests/README new file mode 100644
> > index 0000000..3e9cd80
> > --- /dev/null
> > +++ b/drivers/cpufreq/tests/README
> > @@ -0,0 +1,33 @@
> > +This file contains list of cpufreq's available regression tests
> > with a short +usage description.
> > +
> > +1. cpufreq_freq_test.sh
> > +
> > +Description:
> > +------------
> > +This script is supposed to test if cpufreq attributes exported by
> > sysfs are +exposing correct values.
> > +
> > +To achieve this goal it saves the current governor and changes it
> > to +"performance". Afterwards, it reads the
> > "scaling_available_frequencies" +property. With the list of
> > supported frequencies it is able to enforce each of +them by
> > writing to "scaling_max_freq" attribute. To make the test more
> > reliable +a superfluous load with gzip is created to be sure that
> > we are running with +highest possible frequency. This high load is
> > regulated with the 'sleep' +duration. After this time the
> > "cpufreq_cur_freq" is read and compared with the +original value.
> > As the last step the original governor is restored.
> 
> I couldn't make out the purpose of this test and why we need it. How
> do we ensure that "cpufreq attributes exported by sysfs are exposing
> correct values"?

First of all the cpufreq attributes are part of the subsystem API.
There are systems which actually depend on them, so we would be better
off to test if they work as intended.

Secondly, the test takes those values and then with use of other
attribute enforce the value, which is then read via cat'ing
cpufreq_cur_freq. If any of the attributes is wrong then we will spot
the error immediately.

> 
> And actually what do we mean by this statement even? What kind of
> errors can be there in exposing these values.

Errors with cpufreq and CCF cooperation - especially when some parts of
cpufreq code uses direct write to MUX, DIV or PLL SoC registers.

Also, one can check if permutations of changing all available
frequencies are working properly.

This script allowed me to find at least two bugs at Exynos cpufreq
subsystem in the past. And those fixes are already applied to mainline.

> 
> I want to understand the purpose of this script very clearly first
> and then only will look at the details.

There is a clean shell script code to go through it, the README file
with detailed description of the script purpose and operation.

What else shall I write?


> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-23  7:38     ` Lukasz Majewski
@ 2014-07-23  8:49       ` Viresh Kumar
  2014-07-23 10:10         ` Lukasz Majewski
                           ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Viresh Kumar @ 2014-07-23  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 23 July 2014 13:08, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Do you want to say that we have enough tests and we don't need more ?

No. We don't have any tests at all :)

> I always thought that we shall have as much regression tests as
> possible.

Yeah, tests are welcomed but the question is where should they get added.
Don't know if its common to add tests directly to kernel.

And also if the test is really good, not discouraging your work.

>> On 21 July 2014 12:32, Lukasz Majewski <l.majewski@samsung.com> wrote:
>> > This commit adds first regression test "cpufreq_freq_test.sh" for
>> > the cpufreq subsystem.
>>
>> That's not enough, Tell us why we should continue reading this mail..
>
> Hmm... If "regression" and "test" don't catch the attention of a
> diligent maintainer, then I cannot do much more to encourage him to
> read the whole e-mail :-)

What I meant to say was, your subject and body must be good enough
to answer most of the things. You don't have to tell much about the
implementation but other things should be pretty clear from logs.

Your current logs are quite short for something that's not a normal practice.

> I can imagine that maintainers are very busy, therefore I've prepared
> README file with detailed description of the script operation.

Yeah, a README is welcomed and would be useful for users as well..

>> I couldn't make out the purpose of this test and why we need it. How
>> do we ensure that "cpufreq attributes exported by sysfs are exposing
>> correct values"?
>
> First of all the cpufreq attributes are part of the subsystem API.
> There are systems which actually depend on them, so we would be better
> off to test if they work as intended.
>
> Secondly, the test takes those values and then with use of other
> attribute enforce the value, which is then read via cat'ing
> cpufreq_cur_freq. If any of the attributes is wrong then we will spot
> the error immediately.

Shouldn't you use userspace governor then instead of performance?
And then we don't need the gzip stuff at all. We can just set it to the
right freq and get current freq to see if it matches?

And now that we are starting to get tests added into the kernel (will still
wait to see what Rafael has to advice), we better think of the way these
are going to get added. Probably a single script with parameters like
what to test?

>> And actually what do we mean by this statement even? What kind of
>> errors can be there in exposing these values.
>
> Errors with cpufreq and CCF cooperation - especially when some parts of
> cpufreq code uses direct write to MUX, DIV or PLL SoC registers.
>
> Also, one can check if permutations of changing all available
> frequencies are working properly.

Yeah, that would be fine. Probably need to think more about scripts name.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-23  8:49       ` Viresh Kumar
@ 2014-07-23 10:10         ` Lukasz Majewski
  2014-07-23 10:48           ` Viresh Kumar
  2014-07-24 10:05           ` Javi Merino
  2014-07-23 11:58         ` Rafael J. Wysocki
  2014-07-23 15:02         ` Andrew Lunn
  2 siblings, 2 replies; 18+ messages in thread
From: Lukasz Majewski @ 2014-07-23 10:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Viresh,

> On 23 July 2014 13:08, Lukasz Majewski <l.majewski@samsung.com> wrote:
> > Do you want to say that we have enough tests and we don't need
> > more ?
> 
> No. We don't have any tests at all :)

Then we should encourage as many developers as possible to share their
private tests with us.

> 
> > I always thought that we shall have as much regression tests as
> > possible.
> 
> Yeah, tests are welcomed but the question is where should they get
> added. Don't know if its common to add tests directly to kernel.

There was a similar discussion with device tree and finally it was
included in the mainline repository.

> 
> And also if the test is really good, not discouraging your work.
> 
> >> On 21 July 2014 12:32, Lukasz Majewski <l.majewski@samsung.com>
> >> wrote:
> >> > This commit adds first regression test "cpufreq_freq_test.sh" for
> >> > the cpufreq subsystem.
> >>
> >> That's not enough, Tell us why we should continue reading this
> >> mail..
> >
> > Hmm... If "regression" and "test" don't catch the attention of a
> > diligent maintainer, then I cannot do much more to encourage him to
> > read the whole e-mail :-)
> 
> What I meant to say was, your subject and body must be good enough
> to answer most of the things. You don't have to tell much about the
> implementation but other things should be pretty clear from logs.
> 
> Your current logs are quite short for something that's not a normal
> practice.

It is hard for me to agree on this issue.

> 
> > I can imagine that maintainers are very busy, therefore I've
> > prepared README file with detailed description of the script
> > operation.
> 
> Yeah, a README is welcomed and would be useful for users as well..
> 
> >> I couldn't make out the purpose of this test and why we need it.
> >> How do we ensure that "cpufreq attributes exported by sysfs are
> >> exposing correct values"?
> >
> > First of all the cpufreq attributes are part of the subsystem API.
> > There are systems which actually depend on them, so we would be
> > better off to test if they work as intended.
> >
> > Secondly, the test takes those values and then with use of other
> > attribute enforce the value, which is then read via cat'ing
> > cpufreq_cur_freq. If any of the attributes is wrong then we will
> > spot the error immediately.
> 
> Shouldn't you use userspace governor then instead of performance?

Performance assures that we will have the right frequency set.

However, there can be a similar patch to use userspace governor and
various load to fail if ondemand's frequency flipping is detected.

> And then we don't need the gzip stuff at all. We can just set it to
> the right freq and get current freq to see if it matches?

Sometimes "interresting" things show up when you have 100% CPU load and
you try to switch frequency.
In my opinion usage of gzip makes the test more difficult to pass.

> 
> And now that we are starting to get tests added into the kernel (will
> still wait to see what Rafael has to advice),

Ok. Lets wait for Rafael's opinion.

> we better think of the
> way these are going to get added. Probably a single script with
> parameters like what to test?

It is one possible solution, where another one is to run the all scripts
in the directory.

I'm curious about Rafael's opinion.

> 
> >> And actually what do we mean by this statement even? What kind of
> >> errors can be there in exposing these values.
> >
> > Errors with cpufreq and CCF cooperation - especially when some
> > parts of cpufreq code uses direct write to MUX, DIV or PLL SoC
> > registers.
> >
> > Also, one can check if permutations of changing all available
> > frequencies are working properly.
> 
> Yeah, that would be fine. Probably need to think more about scripts
> name.

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-23 10:10         ` Lukasz Majewski
@ 2014-07-23 10:48           ` Viresh Kumar
  2014-07-24 10:05           ` Javi Merino
  1 sibling, 0 replies; 18+ messages in thread
From: Viresh Kumar @ 2014-07-23 10:48 UTC (permalink / raw)
  To: linux-arm-kernel

On 23 July 2014 15:40, Lukasz Majewski <l.majewski@samsung.com> wrote:

>> Shouldn't you use userspace governor then instead of performance?
>
> Performance assures that we will have the right frequency set.

Why wouldn't userspace assure that?

> However, there can be a similar patch to use userspace governor and
> various load to fail if ondemand's frequency flipping is detected.

That's why I want to get to the motive behind this patch.
AFAIU, we are checking if its fine to switch to available frequencies
or not and if yes, do we actually switch to those. Right?

For, this testcase we just need a single test and I still don't see why
performance is better than userspace?

>> And then we don't need the gzip stuff at all. We can just set it to
>> the right freq and get current freq to see if it matches?
>
> Sometimes "interresting" things show up when you have 100% CPU load and
> you try to switch frequency.

That's a different test then. And that's how it should be presented.
So, probably another option to the script, which isn't forced on people.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-23  8:49       ` Viresh Kumar
  2014-07-23 10:10         ` Lukasz Majewski
@ 2014-07-23 11:58         ` Rafael J. Wysocki
  2014-07-23 15:02         ` Andrew Lunn
  2 siblings, 0 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2014-07-23 11:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday, July 23, 2014 02:19:54 PM Viresh Kumar wrote:
> On 23 July 2014 13:08, Lukasz Majewski <l.majewski@samsung.com> wrote:
> > Do you want to say that we have enough tests and we don't need more ?
> 
> No. We don't have any tests at all :)
> 
> > I always thought that we shall have as much regression tests as
> > possible.
> 
> Yeah, tests are welcomed but the question is where should they get added.
> Don't know if its common to add tests directly to kernel.

Yes, it is.

> And also if the test is really good, not discouraging your work.
> 
> >> On 21 July 2014 12:32, Lukasz Majewski <l.majewski@samsung.com> wrote:
> >> > This commit adds first regression test "cpufreq_freq_test.sh" for
> >> > the cpufreq subsystem.
> >>
> >> That's not enough, Tell us why we should continue reading this mail..
> >
> > Hmm... If "regression" and "test" don't catch the attention of a
> > diligent maintainer, then I cannot do much more to encourage him to
> > read the whole e-mail :-)
> 
> What I meant to say was, your subject and body must be good enough
> to answer most of the things. You don't have to tell much about the
> implementation but other things should be pretty clear from logs.
> 
> Your current logs are quite short for something that's not a normal practice.
> 
> > I can imagine that maintainers are very busy, therefore I've prepared
> > README file with detailed description of the script operation.
> 
> Yeah, a README is welcomed and would be useful for users as well..
> 
> >> I couldn't make out the purpose of this test and why we need it. How
> >> do we ensure that "cpufreq attributes exported by sysfs are exposing
> >> correct values"?
> >
> > First of all the cpufreq attributes are part of the subsystem API.
> > There are systems which actually depend on them, so we would be better
> > off to test if they work as intended.
> >
> > Secondly, the test takes those values and then with use of other
> > attribute enforce the value, which is then read via cat'ing
> > cpufreq_cur_freq. If any of the attributes is wrong then we will spot
> > the error immediately.
> 
> Shouldn't you use userspace governor then instead of performance?
> And then we don't need the gzip stuff at all. We can just set it to the
> right freq and get current freq to see if it matches?
> 
> And now that we are starting to get tests added into the kernel (will still
> wait to see what Rafael has to advice), we better think of the way these
> are going to get added. Probably a single script with parameters like
> what to test?

I've had a look at the Lukasz' patch in the first iteration and I'm going to
look at it again shortly.

At this point I can only say that it should be clear to the user of the
script what is tested, as well as what "success" and what "failure" mean.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-23  8:49       ` Viresh Kumar
  2014-07-23 10:10         ` Lukasz Majewski
  2014-07-23 11:58         ` Rafael J. Wysocki
@ 2014-07-23 15:02         ` Andrew Lunn
  2 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2014-07-23 15:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jul 23, 2014 at 02:19:54PM +0530, Viresh Kumar wrote:
> On 23 July 2014 13:08, Lukasz Majewski <l.majewski@samsung.com> wrote:
> > Do you want to say that we have enough tests and we don't need more ?
> 
> No. We don't have any tests at all :)

Not really true. I've found bugs triggering opps using cpufreq-bench.
http://marc.info/?l=linux-pm&m=138165517321579&w=2

and i hope you learned from that experience and run this tool when
making changes to the core.

There is an old writeup of cpufreq-bench here:
https://lwn.net/Articles/339862/

and the code itself is in the mainline tree, tools/power/cpupower/bench

	Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-21  7:02 ` [PATCH v2] " Lukasz Majewski
  2014-07-23  5:06   ` Viresh Kumar
@ 2014-07-23 23:58   ` Rafael J. Wysocki
  2014-07-24  7:04     ` Lukasz Majewski
  1 sibling, 1 reply; 18+ messages in thread
From: Rafael J. Wysocki @ 2014-07-23 23:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday, July 21, 2014 09:02:34 AM Lukasz Majewski wrote:
> This commit adds first regression test "cpufreq_freq_test.sh" for the
> cpufreq subsystem.

First of all, I'm not seeing any explanation why this script should be
shipped with the kernel.

What regressions it tests against in particular and how it does that.

Please write that down in the changelog.  It doesn't need to be very
detailed.

Second, I'm not sure this is the first such test (someone already
mentioned cpupower).

> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> 
> ---
> Changes for v2:
> - Replace *_PATCH with *_PATH for variables names
> - Corrected mistakes in the README file
> - Providing detailed explanation of the patch in the README file
> ---
>  drivers/cpufreq/tests/README               |  33 +++++++
>  drivers/cpufreq/tests/cpufreq_freq_test.sh | 149 +++++++++++++++++++++++++++++
>  2 files changed, 182 insertions(+)
>  create mode 100644 drivers/cpufreq/tests/README
>  create mode 100755 drivers/cpufreq/tests/cpufreq_freq_test.sh
> 
> diff --git a/drivers/cpufreq/tests/README b/drivers/cpufreq/tests/README

drivers/cpufreq/ is not a place for scripts.

We have scripts/ for that and you can add a "power" subdirectory in there
and put your script into it.

Alternatively, you can use the existing tools/power/ directory for that (but
then please add a subdirectory for your script).

> new file mode 100644
> index 0000000..3e9cd80
> --- /dev/null
> +++ b/drivers/cpufreq/tests/README
> @@ -0,0 +1,33 @@
> +This file contains list of cpufreq's available regression tests with a short
> +usage description.
> +
> +1. cpufreq_freq_test.sh
> +
> +Description:
> +------------
> +This script is supposed to test if cpufreq attributes exported by sysfs are
> +exposing correct values.
> +
> +To achieve this goal it saves the current governor and changes it to
> +"performance". Afterwards, it reads the "scaling_available_frequencies"
> +property. With the list of supported frequencies it is able to enforce each of
> +them by writing to "scaling_max_freq" attribute. To make the test more reliable
> +a superfluous load with gzip is created to be sure that we are running with
> +highest possible frequency. This high load is regulated with the 'sleep'
> +duration. After this time the "cpufreq_cur_freq" is read and compared with the
> +original value. As the last step the original governor is restored.
> +
> +This script can work with or without BOOST enabled and helps in spotting errors
> +related to cpufreq and common clock framework.
> +
> +Used attributes:
> +----------------
> +- "scaling_available_frequencies"
> +- "cpuinfo_cur_freq"
> +- "scaling_governor"
> +- "scaling_max_freq"
> +
> +Target devices:
> +---------------
> +
> +All devices which exports mentioned above sysfs attributes.
> \ No newline at end of file
> diff --git a/drivers/cpufreq/tests/cpufreq_freq_test.sh b/drivers/cpufreq/tests/cpufreq_freq_test.sh
> new file mode 100755
> index 0000000..c25f05c
> --- /dev/null
> +++ b/drivers/cpufreq/tests/cpufreq_freq_test.sh
> @@ -0,0 +1,149 @@
> +#!/bin/bash
> +#
> +# This file provides a simple mean to test if all declared freqs at
> +# "scaling_available_frequencies" can be set and if "cpuinfo_cur_freq"
> +# returns this value.
> +#
> +# Usage: ./cpufreq_freq_test.sh
> +# Requisite: Compiled in "performance" governor
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, you can access it online at
> +# http://www.gnu.org/licenses/gpl-2.0.html.
> +#
> +# Copyright (C) Samsung Electronics, 2014
> +#
> +# Author: Lukasz Majewski <l.majewski@samsung.com>
> +
> +set +x
> +
> +COLOUR_RED="\33[31m"
> +COLOUR_BLUE="\33[34m"
> +COLOUR_GREEN="\33[32m"
> +COLOUR_DEFAULT="\33[0m"
> +
> +T_PATH=/sys/devices/system/cpu/cpu0/cpufreq
> +BOOST_PATH=/sys/devices/system/cpu/cpufreq
> +
> +if [ ! -d "$T_PATH" ]; then
> +    printf "   $COLOUR_RED No path to CPUFREQ $COLOUR_DEFAULT\n"
> +    exit 1
> +fi
> +
> +ERRORS=0
> +
> +OLD_GOV=`cat $T_PATH/scaling_governor`
> +echo "CURRENT GOVERNOR: $OLD_GOV"
> +echo "SET GOVERNOR: performance"
> +echo "performance" > $T_PATH/scaling_governor
> +
> +function test_freqs1 {
> +    FREQS=`cat $1`
> +    for I in $FREQS; do
> +	cpufreq_set_freq $I
> +	if [ "$2" ]; then
> +	    printf "$COLOUR_BLUE BOOST $COLOUR_DEFAULT" $I
> +	fi
> +	cpufreq_test_freq $I
> +    done
> +}
> +
> +function test_freqs2 {
> +    FREQ=`cat $1`
> +    FREQS_ARRAY=($FREQ)
> +
> +    for freq in ${FREQS_ARRAY[@]}
> +    do
> +	echo "REFERENCE FREQ: $freq"
> +	for f in ${FREQS_ARRAY[@]}
> +	do
> +	    cpufreq_set_freq $freq
> +	    echo -n "----> "
> +	    cpufreq_set_freq $f
> +	    cpufreq_test_freq $f
> +	done
> +    done
> +}
> +
> +function restore {
> +    if [ -f $BOOST_PATH/boost ]; then
> +	cpufreq_boost_state $BOOST_STATE
> +    fi
> +
> +    echo "SET GOVERNOR: $OLD_GOV"
> +    echo $OLD_GOV > $T_PATH/scaling_governor
> +}
> +
> +function die {
> +    printf "   $COLOUR_RED FAILED $COLOUR_DEFAULT\n"
> +    restore_gov
> +    exit 1
> +}
> +
> +function cpufreq_test_freq {
> +    gzip < /dev/urandom > /dev/null &
> +    pid=$!
> +    sleep 0.1
> +    CURR_FREQ=`cat $T_PATH/cpuinfo_cur_freq`
> +    if [ $1 -eq $CURR_FREQ ]; then
> +	printf "\t$COLOUR_GREEN OK $COLOUR_DEFAULT\n"
> +    else
> +	printf "$COLOUR_RED CURRENT $CURR_FREQ $COLOUR_DEFAULT\n"
> +	ERRORS=`expr $ERRORS + 1`
> +	#die
> +    fi
> +    kill -9 $pid
> +    wait $! 2>/dev/null
> +}
> +
> +function cpufreq_set_freq {
> +    echo $1 > $T_PATH/scaling_max_freq || die $?
> +    printf "FREQ:$COLOUR_GREEN %s $COLOUR_DEFAULT" $1
> +}
> +
> +function cpufreq_boost_state {
> +   echo $1 > $BOOST_PATH/boost
> +}
> +
> +function cpufreq_boost_status {
> +   cat $BOOST_PATH/boost
> +}
> +
> +if [ -f $BOOST_PATH/boost ]; then
> +    echo "######################################"
> +    echo "TEST BOOST OPERATION"
> +    echo "######################################"
> +
> +    BOOST_STATE=$(cpufreq_boost_status)
> +    if [ $BOOST_STATE -eq 0 ]; then
> +	cpufreq_boost_state 1
> +    fi
> +    test_freqs1 $T_PATH/scaling_boost_frequencies 1
> +fi
> +
> +echo "######################################"
> +echo "TEST AVAILABLE FREQS"
> +echo "######################################"
> +test_freqs1 $T_PATH/scaling_available_frequencies
> +
> +echo "######################################"
> +echo "TEST FREQS SWITCHING"
> +echo "######################################"
> +test_freqs2 $T_PATH/scaling_available_frequencies
> +
> +echo "######################################"
> +echo "ERRORS: $ERRORS"
> +echo "######################################"
> +
> +restore
> +exit 0
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-23 23:58   ` Rafael J. Wysocki
@ 2014-07-24  7:04     ` Lukasz Majewski
  2014-08-06 22:50       ` Rafael J. Wysocki
  0 siblings, 1 reply; 18+ messages in thread
From: Lukasz Majewski @ 2014-07-24  7:04 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Rafael,

> On Monday, July 21, 2014 09:02:34 AM Lukasz Majewski wrote:
> > This commit adds first regression test "cpufreq_freq_test.sh" for
> > the cpufreq subsystem.
> 
> First of all, I'm not seeing any explanation why this script should be
> shipped with the kernel.

OK.

> 
> What regressions it tests against in particular and 

Do you require SHA's/commit messages of commits which were developed to
fix issues spotted with this test script?

> how it does that.

Is this information required in the commit message or can it stay in
the README file created in the same commit?

> 
> Please write that down in the changelog.  It doesn't need to be very
> detailed.
> 
> Second, I'm not sure this is the first such test (someone already
> mentioned cpupower).

It's no problem for me to add such scripts to other work.

> 
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > 
> > ---
> > Changes for v2:
> > - Replace *_PATCH with *_PATH for variables names
> > - Corrected mistakes in the README file
> > - Providing detailed explanation of the patch in the README file
> > ---
> >  drivers/cpufreq/tests/README               |  33 +++++++
> >  drivers/cpufreq/tests/cpufreq_freq_test.sh | 149
> > +++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+)
> >  create mode 100644 drivers/cpufreq/tests/README
> >  create mode 100755 drivers/cpufreq/tests/cpufreq_freq_test.sh
> > 
> > diff --git a/drivers/cpufreq/tests/README
> > b/drivers/cpufreq/tests/README
> 
> drivers/cpufreq/ is not a place for scripts.
> 
> We have scripts/ for that and you can add a "power" subdirectory in
> there and put your script into it.
> 
> Alternatively, you can use the existing tools/power/ directory for
> that (but then please add a subdirectory for your script).

Ok.

> 
> > new file mode 100644
> > index 0000000..3e9cd80
> > --- /dev/null
> > +++ b/drivers/cpufreq/tests/README
> > @@ -0,0 +1,33 @@
> > +This file contains list of cpufreq's available regression tests
> > with a short +usage description.
> > +
> > +1. cpufreq_freq_test.sh
> > +
> > +Description:
> > +------------
> > +This script is supposed to test if cpufreq attributes exported by
> > sysfs are +exposing correct values.
> > +
> > +To achieve this goal it saves the current governor and changes it
> > to +"performance". Afterwards, it reads the
> > "scaling_available_frequencies" +property. With the list of
> > supported frequencies it is able to enforce each of +them by
> > writing to "scaling_max_freq" attribute. To make the test more
> > reliable +a superfluous load with gzip is created to be sure that
> > we are running with +highest possible frequency. This high load is
> > regulated with the 'sleep' +duration. After this time the
> > "cpufreq_cur_freq" is read and compared with the +original value.
> > As the last step the original governor is restored. + +This script
> > can work with or without BOOST enabled and helps in spotting errors
> > +related to cpufreq and common clock framework. +
> > +Used attributes:
> > +----------------
> > +- "scaling_available_frequencies"
> > +- "cpuinfo_cur_freq"
> > +- "scaling_governor"
> > +- "scaling_max_freq"
> > +
> > +Target devices:
> > +---------------
> > +
> > +All devices which exports mentioned above sysfs attributes.
> > \ No newline at end of file
> > diff --git a/drivers/cpufreq/tests/cpufreq_freq_test.sh
> > b/drivers/cpufreq/tests/cpufreq_freq_test.sh new file mode 100755
> > index 0000000..c25f05c
> > --- /dev/null
> > +++ b/drivers/cpufreq/tests/cpufreq_freq_test.sh
> > @@ -0,0 +1,149 @@
> > +#!/bin/bash
> > +#
> > +# This file provides a simple mean to test if all declared freqs at
> > +# "scaling_available_frequencies" can be set and if
> > "cpuinfo_cur_freq" +# returns this value.
> > +#
> > +# Usage: ./cpufreq_freq_test.sh
> > +# Requisite: Compiled in "performance" governor
> > +#
> > +# This program is free software; you can redistribute it and/or
> > modify +# it under the terms of the GNU General Public License as
> > published by +# the Free Software Foundation; either version 2 of
> > the License, or +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, you can access it online at
> > +# http://www.gnu.org/licenses/gpl-2.0.html.
> > +#
> > +# Copyright (C) Samsung Electronics, 2014
> > +#
> > +# Author: Lukasz Majewski <l.majewski@samsung.com>
> > +
> > +set +x
> > +
> > +COLOUR_RED="\33[31m"
> > +COLOUR_BLUE="\33[34m"
> > +COLOUR_GREEN="\33[32m"
> > +COLOUR_DEFAULT="\33[0m"
> > +
> > +T_PATH=/sys/devices/system/cpu/cpu0/cpufreq
> > +BOOST_PATH=/sys/devices/system/cpu/cpufreq
> > +
> > +if [ ! -d "$T_PATH" ]; then
> > +    printf "   $COLOUR_RED No path to CPUFREQ $COLOUR_DEFAULT\n"
> > +    exit 1
> > +fi
> > +
> > +ERRORS=0
> > +
> > +OLD_GOV=`cat $T_PATH/scaling_governor`
> > +echo "CURRENT GOVERNOR: $OLD_GOV"
> > +echo "SET GOVERNOR: performance"
> > +echo "performance" > $T_PATH/scaling_governor
> > +
> > +function test_freqs1 {
> > +    FREQS=`cat $1`
> > +    for I in $FREQS; do
> > +	cpufreq_set_freq $I
> > +	if [ "$2" ]; then
> > +	    printf "$COLOUR_BLUE BOOST $COLOUR_DEFAULT" $I
> > +	fi
> > +	cpufreq_test_freq $I
> > +    done
> > +}
> > +
> > +function test_freqs2 {
> > +    FREQ=`cat $1`
> > +    FREQS_ARRAY=($FREQ)
> > +
> > +    for freq in ${FREQS_ARRAY[@]}
> > +    do
> > +	echo "REFERENCE FREQ: $freq"
> > +	for f in ${FREQS_ARRAY[@]}
> > +	do
> > +	    cpufreq_set_freq $freq
> > +	    echo -n "----> "
> > +	    cpufreq_set_freq $f
> > +	    cpufreq_test_freq $f
> > +	done
> > +    done
> > +}
> > +
> > +function restore {
> > +    if [ -f $BOOST_PATH/boost ]; then
> > +	cpufreq_boost_state $BOOST_STATE
> > +    fi
> > +
> > +    echo "SET GOVERNOR: $OLD_GOV"
> > +    echo $OLD_GOV > $T_PATH/scaling_governor
> > +}
> > +
> > +function die {
> > +    printf "   $COLOUR_RED FAILED $COLOUR_DEFAULT\n"
> > +    restore_gov
> > +    exit 1
> > +}
> > +
> > +function cpufreq_test_freq {
> > +    gzip < /dev/urandom > /dev/null &
> > +    pid=$!
> > +    sleep 0.1
> > +    CURR_FREQ=`cat $T_PATH/cpuinfo_cur_freq`
> > +    if [ $1 -eq $CURR_FREQ ]; then
> > +	printf "\t$COLOUR_GREEN OK $COLOUR_DEFAULT\n"
> > +    else
> > +	printf "$COLOUR_RED CURRENT $CURR_FREQ $COLOUR_DEFAULT\n"
> > +	ERRORS=`expr $ERRORS + 1`
> > +	#die
> > +    fi
> > +    kill -9 $pid
> > +    wait $! 2>/dev/null
> > +}
> > +
> > +function cpufreq_set_freq {
> > +    echo $1 > $T_PATH/scaling_max_freq || die $?
> > +    printf "FREQ:$COLOUR_GREEN %s $COLOUR_DEFAULT" $1
> > +}
> > +
> > +function cpufreq_boost_state {
> > +   echo $1 > $BOOST_PATH/boost
> > +}
> > +
> > +function cpufreq_boost_status {
> > +   cat $BOOST_PATH/boost
> > +}
> > +
> > +if [ -f $BOOST_PATH/boost ]; then
> > +    echo "######################################"
> > +    echo "TEST BOOST OPERATION"
> > +    echo "######################################"
> > +
> > +    BOOST_STATE=$(cpufreq_boost_status)
> > +    if [ $BOOST_STATE -eq 0 ]; then
> > +	cpufreq_boost_state 1
> > +    fi
> > +    test_freqs1 $T_PATH/scaling_boost_frequencies 1
> > +fi
> > +
> > +echo "######################################"
> > +echo "TEST AVAILABLE FREQS"
> > +echo "######################################"
> > +test_freqs1 $T_PATH/scaling_available_frequencies
> > +
> > +echo "######################################"
> > +echo "TEST FREQS SWITCHING"
> > +echo "######################################"
> > +test_freqs2 $T_PATH/scaling_available_frequencies
> > +
> > +echo "######################################"
> > +echo "ERRORS: $ERRORS"
> > +echo "######################################"
> > +
> > +restore
> > +exit 0
> > 
> 



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-23 10:10         ` Lukasz Majewski
  2014-07-23 10:48           ` Viresh Kumar
@ 2014-07-24 10:05           ` Javi Merino
  1 sibling, 0 replies; 18+ messages in thread
From: Javi Merino @ 2014-07-24 10:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jul 23, 2014 at 11:10:42AM +0100, Lukasz Majewski wrote:
> Hi Viresh,
> 
> > On 23 July 2014 13:08, Lukasz Majewski <l.majewski@samsung.com> wrote:
> > > Do you want to say that we have enough tests and we don't need
> > > more ?
> > 
> > No. We don't have any tests at all :)
> 
> Then we should encourage as many developers as possible to share their
> private tests with us.

Linaro publishes their regression tests here:

https://git.linaro.org/tools/pm-qa.git

Description:

https://wiki.linaro.org/WorkingGroups/PowerManagement/Resources/TestSuite/PmQaSpecification#Cpufreq

I have only used the thermal ones, I can't comment on the cpufreq
tests.

Cheers,
Javi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] cpufreq: tests: Providing cpufreq regression test
  2014-07-24  7:04     ` Lukasz Majewski
@ 2014-08-06 22:50       ` Rafael J. Wysocki
  0 siblings, 0 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2014-08-06 22:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday, July 24, 2014 09:04:02 AM Lukasz Majewski wrote:
> Hi Rafael,
> 
> > On Monday, July 21, 2014 09:02:34 AM Lukasz Majewski wrote:
> > > This commit adds first regression test "cpufreq_freq_test.sh" for
> > > the cpufreq subsystem.
> > 
> > First of all, I'm not seeing any explanation why this script should be
> > shipped with the kernel.
> 
> OK.
> 
> > 
> > What regressions it tests against in particular and 
> 
> Do you require SHA's/commit messages of commits which were developed to
> fix issues spotted with this test script?

No.  I want information about what kind of bugs can be catched with the
help of this script.

> > how it does that.
> 
> Is this information required in the commit message or can it stay in
> the README file created in the same commit?

There should be *some* information in the changelog too.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2014-08-06 22:50 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-18 10:23 [PATCH] cpufreq: tests: Providing cpufreq regression test Lukasz Majewski
2014-07-18 11:28 ` Sachin Kamat
2014-07-18 11:59   ` Lukasz Majewski
2014-07-22  4:13     ` Sachin Kamat
2014-07-18 11:42 ` Rafael J. Wysocki
2014-07-18 12:00   ` Lukasz Majewski
2014-07-21  7:02 ` [PATCH v2] " Lukasz Majewski
2014-07-23  5:06   ` Viresh Kumar
2014-07-23  7:38     ` Lukasz Majewski
2014-07-23  8:49       ` Viresh Kumar
2014-07-23 10:10         ` Lukasz Majewski
2014-07-23 10:48           ` Viresh Kumar
2014-07-24 10:05           ` Javi Merino
2014-07-23 11:58         ` Rafael J. Wysocki
2014-07-23 15:02         ` Andrew Lunn
2014-07-23 23:58   ` Rafael J. Wysocki
2014-07-24  7:04     ` Lukasz Majewski
2014-08-06 22:50       ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).