linux-kernel - RE: [PATCH v4] cpufreq: powerpc: Add cpufreq driver for Freescale e500mc SoCs

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] [day] [month] [year] [list]

Message-ID: <D07C73A334FF604B95B3CBD2A545D07B0B12831E@039-SN2MPN1-013.039d.mgd.msft.net>
Date:	Tue, 9 Apr 2013 10:04:11 +0000
From:	Tang Yuantian-B29983 <B29983@...escale.com>
To:	Viresh Kumar <viresh.kumar@...aro.org>
CC:	"rjw@...k.pl" <rjw@...k.pl>,
	"cpufreq@...r.kernel.org" <cpufreq@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	Li Yang-R58472 <r58472@...escale.com>
Subject: RE: [PATCH v4] cpufreq: powerpc: Add cpufreq driver for Freescale
 e500mc SoCs

Thanks, you make my code look better each review.

Thanks,
Yuantian


> -----Original Message-----
> From: Viresh Kumar [mailto:viresh.kumar@...aro.org]
> Sent: 2013年4月9日 17:47
> To: Tang Yuantian-B29983
> Cc: rjw@...k.pl; cpufreq@...r.kernel.org; linux-pm@...r.kernel.org;
> linux-kernel@...r.kernel.org; linuxppc-dev@...ts.ozlabs.org; Li Yang-
> R58472
> Subject: Re: [PATCH v4] cpufreq: powerpc: Add cpufreq driver for
> Freescale e500mc SoCs
> 
> Mostly good now, V5 should be the final one.
> 
> On 9 April 2013 14:02,  <Yuantian.Tang@...escale.com> wrote:
> > diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c
> > b/drivers/cpufreq/ppc-corenet-cpufreq.c
> 
> > +static int corenet_cpufreq_target(struct cpufreq_policy *policy,
> > +               unsigned int target_freq, unsigned int relation) {
> > +       struct cpufreq_freqs freqs;
> > +       unsigned int new;
> > +       struct clk *parent;
> > +       int ret;
> > +       struct cpu_data *data = per_cpu(cpu_data, policy->cpu);
> > +
> > +       cpufreq_frequency_table_target(policy, data->table,
> > +                       target_freq, relation, &new);
> > +
> > +       if (policy->cur == data->table[new].frequency)
> > +               return 0;
> > +
> > +       freqs.old = policy->cur;
> > +       freqs.new = data->table[new].frequency;
> > +       freqs.cpu = policy->cpu;
> 
> You don't need to set freqs.cpu..
> 
> > +       mutex_lock(&cpufreq_lock);
> > +       cpufreq_notify_transition(policy, &freqs, CPUFREQ_PRECHANGE);
> > +
> > +       parent = of_clk_get(data->parent, new);
> > +       ret = clk_set_parent(data->clk, parent);
> > +       if (ret) {
> > +               freqs.new = freqs.old;
> > +               cpufreq_notify_transition(policy, &freqs,
> CPUFREQ_POSTCHANGE);
> > +               mutex_unlock(&cpufreq_lock);
> > +               return ret;
> > +       }
> > +
> > +       cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
> > +       mutex_unlock(&cpufreq_lock);
> 
> What about writing it as:
> 
> +       ret = clk_set_parent(data->clk, parent);
> +       if (ret)
> +               freqs.new = freqs.old;
> +
> +       cpufreq_notify_transition(policy, &freqs, CPUFREQ_POSTCHANGE);
> +       mutex_unlock(&cpufreq_lock);
> 
> return ret;
> 
> > +       return 0;
> > +}