Re: [loongson2-PATCH] modification of the cpufreq module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Gang

>
>
> This patch updates some aspects of the current implementation of
> cpufreq driver for Loongson2.
>
> 1) A default cpu_wait handler is installed such that the cpu will be
> alive at the lowest possible power level when a cpu_wait call is made;
>
> 2) The number of frequency levels is reduced to 3, and the lowest
> frequency is capped as a half of the full cpu speed. The "nowait" option
> is removed.
>
> Thanks!

Herein, You need to add a more line:

Signed-off-by: Gang Liang <randomizedthinking@xxxxxxxxx>

You can generate it automatically with:

$ git commit -s

or

$ git commit --amend -s

If you have configured your name and email address with:

$ git config --global user.name "Gang Liang"
$ git config --global user.email "randomizedthinking@xxxxxxxxx"

>
> ---
>  arch/mips/include/asm/mach-loongson/loongson.h |    4 +-
>  arch/mips/kernel/cpu-probe.c                   |   21 +++++++++
>  arch/mips/kernel/cpufreq/loongson2_clock.c     |   52 +++--------------------
>  arch/mips/kernel/cpufreq/loongson2_cpufreq.c   |   54 +++++++++---------------
>  4 files changed, 50 insertions(+), 81 deletions(-)
>
> diff --git a/arch/mips/include/asm/mach-loongson/loongson.h
> b/arch/mips/include/asm/mach-loongson/loongson.h
> index 53d0bef..33164b9 100644
> --- a/arch/mips/include/asm/mach-loongson/loongson.h
> +++ b/arch/mips/include/asm/mach-loongson/loongson.h
> @@ -242,8 +242,8 @@ extern int mach_i8259_irq(void);
>
>  #ifdef CONFIG_CPU_SUPPORTS_CPUFREQ
>  #include <linux/cpufreq.h>
> -extern void loongson2_cpu_wait(void);
> -extern struct cpufreq_frequency_table loongson2_clockmod_table[];
> +/* extern void loongson2_cpu_wait(void); */
> +/* extern struct cpufreq_frequency_table loongson2_clockmod_table[]; */

Why not remove them directly?

>
>  /* Chip Config */
>  #define LOONGSON_CHIPCFG0              LOONGSON_REG(LOONGSON_REGBASE + 0x80)
> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> index be5bb16..5b3072c 100644
> --- a/arch/mips/kernel/cpu-probe.c
> +++ b/arch/mips/kernel/cpu-probe.c
> @@ -25,6 +25,9 @@
>  #include <asm/system.h>
>  #include <asm/watch.h>
>  #include <asm/spram.h>
> +
> +#include <loongson.h>
> +

This may fail when building on non-Loongson platform.

Perhaps we need to add:

#ifdef CONFIG_CPU_LOONGSON2
#include <loongson.h>
#endif

>  /*
>  * Not all of the MIPS CPUs have the "wait" instruction available. Moreover,
>  * the implementation of the "wait" feature differs between CPU families. This
> @@ -51,6 +54,21 @@ static void r39xx_wait(void)
>
>  extern void r4k_wait(void);
>
> +DEFINE_SPINLOCK(loongson2_wait_lock);

Perhaps we'd better use the RAW spinlock here:

DEFINE_RAW_SPINLOCK(loongson2_wait_lock);

and the operation should be raw_spin_lock_irqsave/restore...

> +static void loongson2_cpu_wait(void)
> +{
> +    u32 cpu_freq;
> +    unsigned long flags;
> +

The indent should be a TAB(the same to the others), you can try to
format it with:

indent -linux /path/to/file ...

and please check it with scripts/checkpatch.pl before sending your
next revision.

$ ./scripts/checkpatch.pl --strict /path/to/your_patch

and fix all of the errors and the warnings if possible.

> +    /* enter the lowest power mode available while still alive */
> +    /* future work: check cpu freq -- do nothing if no change */
> +    /* otherwise, change the frequency and propagate the clock rate */

For Comments, you'd better use something like this:

/*
 * enter ....
 */

and if you are using vim, you can format it with :gqap.

> +    spin_lock_irqsave(&loongson2_wait_lock, flags);
> +    cpu_freq = LOONGSON_CHIPCFG0;
> +       LOONGSON_CHIPCFG0 = (cpu_freq & ~0x7) | 1;

The indent problem here too.

> +    spin_unlock_irqrestore(&loongson2_wait_lock, flags);
> +}
> +
>  /*
>  * This variant is preferable as it allows testing need_resched and going to
>  * sleep depending on the outcome atomically.  Unfortunately the "It is
> @@ -212,6 +230,9 @@ void __init check_wait(void)
>                if ((c->processor_id & 0x00ff) >= 0x40)
>                        cpu_wait = r4k_wait;
>                break;
> +    case CPU_LOONGSON2:
> +        cpu_wait = loongson2_cpu_wait;
> +        break;

indent problem ;)

>        default:
>                break;
>        }

[...]

Will take a look at the left parts later.

Regards,
Wu Zhangjin
--
To unsubscribe from this list: send the line "unsubscribe cpufreq" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Devel]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Forum]     [Linux SCSI]

  Powered by Linux