lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAhV-H5q5hv2sA7EAm1D1nmbG-VGPzc4kpTnHMDSFuFiTKEH7A@mail.gmail.com>
Date: Fri, 5 Jul 2024 15:34:04 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: Huacai Chen <chenhuacai@...ngson.cn>, "Rafael J . Wysocki" <rafael@...nel.org>, loongarch@...ts.linux.dev, 
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Xuerui Wang <kernel@...0n.name>, Jiaxun Yang <jiaxun.yang@...goat.com>, 
	Binbin Zhou <zhoubinbin@...ngson.cn>
Subject: Re: [PATCH V3 2/2] cpufreq: Add Loongson-3 CPUFreq driver support

Hi, Viresh,

On Fri, Jul 5, 2024 at 3:13 PM Viresh Kumar <viresh.kumar@...aro.org> wrote:
>
> On 05-07-24, 14:06, Huacai Chen wrote:
> > Some of LoongArch processors (Loongson-3 series) support DVFS, their
> > IOCSR.FEATURES has IOCSRF_FREQSCALE set. And they has a micro-core in
> > the package called SMC (System Management Controller), which can be
> > used to detect temperature, control fans, scale frequency and voltage,
> > etc.
> >
> > The Loongson-3 CPUFreq driver is very simple now, it communicate with
> > SMC, get DVFS info, set target frequency from CPUFreq core, and so on.
> >
> > There is a command list to interact with SMC, widely-used commands in
> > the CPUFreq driver include:
> >
> > CMD_GET_VERSION: Get SMC firmware version.
> >
> > CMD_GET_FEATURE: Get enabled SMC features.
> >
> > CMD_SET_FEATURE: Enable SMC features, such as basic DVFS, BOOST.
> >
> > CMD_GET_FREQ_LEVEL_NUM: Get the number of all frequency levels.
> >
> > CMD_GET_FREQ_BOOST_LEVEL: Get the first boost frequency level.
> >
> > CMD_GET_FREQ_LEVEL_INFO: Get the detail info of a frequency level.
> >
> > CMD_GET_FREQ_INFO: Get the current frequency.
> >
> > CMD_SET_FREQ_INFO: Set the target frequency.
> >
> > In future we will add automatic frequency scaling, which is similar to
> > Intel's HWP (HardWare P-State).
> >
> > Signed-off-by: Binbin Zhou <zhoubinbin@...ngson.cn>
> > Signed-off-by: Huacai Chen <chenhuacai@...ngson.cn>
> > ---
> >  MAINTAINERS                         |   1 +
> >  drivers/cpufreq/Kconfig             |  12 +
> >  drivers/cpufreq/Makefile            |   1 +
> >  drivers/cpufreq/loongson3_cpufreq.c | 395 ++++++++++++++++++++++++++++
> >  4 files changed, 409 insertions(+)
> >  create mode 100644 drivers/cpufreq/loongson3_cpufreq.c
>
> I have made some changes while applying, can you please test these ?
Thank you for your effort.

It seems except changing mutex_init to devm_mutex_init, all other
changes are line breaks? If so, I think additional tests are
unnecessary. :)

But now long lines (> 80 columns) are accepted by checkpatch.pl. Even
with --strict, only > 100 columns are warned.

Especially for the change in loongson3_cpufreq_cpu_exit(), there is
only 82 columns and I think that line can keep as the original state.
And if possible, I also want the devm_kzalloc() line keep as original.

Others are OK.

Huacai

>
> diff --git a/drivers/cpufreq/loongson3_cpufreq.c b/drivers/cpufreq/loongson3_cpufreq.c
> index a530e4a56b78..cd3efdeeddd9 100644
> --- a/drivers/cpufreq/loongson3_cpufreq.c
> +++ b/drivers/cpufreq/loongson3_cpufreq.c
> @@ -31,10 +31,10 @@ union smc_message {
>  };
>
>  /* Command return values */
> -#define CMD_OK                         0  /* No error */
> -#define CMD_ERROR                      1  /* Regular error */
> -#define CMD_NOCMD                      2  /* Command does not support */
> -#define CMD_INVAL                      3  /* Invalid Parameter */
> +#define CMD_OK                         0 /* No error */
> +#define CMD_ERROR                      1 /* Regular error */
> +#define CMD_NOCMD                      2 /* Command does not support */
> +#define CMD_INVAL                      3 /* Invalid Parameter */
>
>  /* Version commands */
>  /*
> @@ -173,7 +173,8 @@ static struct mutex cpufreq_mutex[MAX_PACKAGES];
>  static struct cpufreq_driver loongson3_cpufreq_driver;
>  static DEFINE_PER_CPU(struct loongson3_freq_data *, freq_data);
>
> -static inline int do_service_request(u32 id, u32 info, u32 cmd, u32 val, u32 extra)
> +static inline int
> +do_service_request(u32 id, u32 info, u32 cmd, u32 val, u32 extra)
>  {
>         int retries;
>         unsigned int cpu = smp_processor_id();
> @@ -226,11 +227,13 @@ static unsigned int loongson3_cpufreq_get(unsigned int cpu)
>         return ret * KILO;
>  }
>
> -static int loongson3_cpufreq_target(struct cpufreq_policy *policy, unsigned int index)
> +static int
> +loongson3_cpufreq_target(struct cpufreq_policy *policy, unsigned int index)
>  {
>         int ret;
>
> -       ret = do_service_request(cpu_data[policy->cpu].core, FREQ_INFO_TYPE_LEVEL, CMD_SET_FREQ_INFO, index, 0);
> +       ret = do_service_request(cpu_data[policy->cpu].core,
> +                       FREQ_INFO_TYPE_LEVEL, CMD_SET_FREQ_INFO, index, 0);
>
>         return (ret >= 0) ? 0 : ret;
>  }
> @@ -255,14 +258,16 @@ static int configure_freq_table(int cpu)
>         boost_level = ret;
>
>         freq_level = min(max_level, FREQ_MAX_LEVEL);
> -       data = devm_kzalloc(&pdev->dev, struct_size(data, table, freq_level + 1), GFP_KERNEL);
> +       data = devm_kzalloc(&pdev->dev, struct_size(data, table, freq_level + 1),
> +                           GFP_KERNEL);
>         if (!data)
>                 return -ENOMEM;
>
>         data->def_freq_level = boost_level - 1;
>
>         for (i = 0; i < freq_level; i++) {
> -               ret = do_service_request(cpu, FREQ_INFO_TYPE_FREQ, CMD_GET_FREQ_LEVEL_INFO, i, 0);
> +               ret = do_service_request(cpu, FREQ_INFO_TYPE_FREQ,
> +                                        CMD_GET_FREQ_LEVEL_INFO, i, 0);
>                 if (ret < 0) {
>                         devm_kfree(&pdev->dev, data);
>                         return ret;
> @@ -290,6 +295,7 @@ static int loongson3_cpufreq_cpu_init(struct cpufreq_policy *policy)
>
>         policy->cpuinfo.transition_latency = 10000;
>         policy->freq_table = per_cpu(freq_data, cpu)->table;
> +
>         policy->suspend_freq = policy->freq_table[per_cpu(freq_data, cpu)->def_freq_level].frequency;
>         cpumask_copy(policy->cpus, topology_sibling_cpumask(cpu));
>
> @@ -314,7 +320,8 @@ static int loongson3_cpufreq_cpu_exit(struct cpufreq_policy *policy)
>  {
>         int cpu = policy->cpu;
>
> -       loongson3_cpufreq_target(policy, per_cpu(freq_data, cpu)->def_freq_level);
> +       loongson3_cpufreq_target(policy, per_cpu(freq_data,
> +                                cpu)->def_freq_level);
>
>         return 0;
>  }
> @@ -348,13 +355,14 @@ static int loongson3_cpufreq_probe(struct platform_device *pdev)
>         int i, ret;
>
>         for (i = 0; i < MAX_PACKAGES; i++)
> -               mutex_init(&cpufreq_mutex[i]);
> +               devm_mutex_init(&pdev->dev, &cpufreq_mutex[i]);
>
>         ret = do_service_request(0, 0, CMD_GET_VERSION, 0, 0);
>         if (ret <= 0)
>                 return -EPERM;
>
> -       ret =  do_service_request(FEATURE_DVFS, 0, CMD_SET_FEATURE, FEATURE_DVFS_ENABLE | FEATURE_DVFS_BOOST, 0);
> +       ret = do_service_request(FEATURE_DVFS, 0, CMD_SET_FEATURE,
> +                                FEATURE_DVFS_ENABLE | FEATURE_DVFS_BOOST, 0);
>         if (ret < 0)
>                 return -EPERM;
>
>
> --
> viresh

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ