> -----Original Message----- > From: Viresh Kumar [mailto:viresh.kumar@xxxxxxxxxx] > Sent: 2013年3月29日 11:17 > To: Tang Yuantian-B29983 > Cc: rjw@xxxxxxx; cpufreq@xxxxxxxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx; > linuxppc-dev@xxxxxxxxxxxxxxxx; Li Yang-R58472 > Subject: Re: [PATCH 2/2 v2] cpufreq: Add cpufreq driver for Freescale > e500mc SoCs > > On 29 March 2013 08:21, Tang Yuantian-B29983 <B29983@xxxxxxxxxxxxx> wrote: > >> > +static int corenet_cpufreq_cpu_init(struct cpufreq_policy *policy) > { > >> > + unsigned int cpu = policy->cpu; > >> > + struct device_node *np; > >> > + int i, count; > >> > + struct clk *clk; > >> > + struct cpufreq_frequency_table *table; > >> > + struct cpu_data *data; > >> > + > >> > + np = of_get_cpu_node(cpu, NULL); > >> > + if (!np) > >> > + return -ENODEV; > >> > + > >> > + data = kzalloc(sizeof(struct cpu_data), GFP_KERNEL); > >> > >> I told you, you missed my comment earlier. > >> > >> You need to write: sizeof(*data) :( > >> > > This is new added statement, what you told last time is about the next > kcalloc()... > > I said about using sizeof() in generic, I copied below from my first mail > on this topic > > > + table = kcalloc(count + 1, > > kzalloc?? > > > + sizeof(struct cpufreq_frequency_table), > > + GFP_KERNEL); > > sizeof(*table) > > And you missed this one as you never replied to it. :) I thought it was OK here. Apparently, sizeof(*table) is better. But kcalloc is OK. > > > Are there some reasons that we can't use sizeof(struct cpu_data) > > instead of sizeof(*data)? > > Documentation/CodiingStyle: Chapter 14: Allocating memory > Thanks Regards, Yuantian > >> > + if (!table) { > >> > + pr_err("%s: no memory\n", __func__); > >> > + goto err_nomem2; > >> > + } > >> > + > >> > + for (i = cpu; i < freq_data.cpus_per_cluster + cpu; i++) > >> > + cpumask_set_cpu(i, policy->cpus); > >> > >> I can see some regression here. Suppose you have two clusters of 4 > >> cpus > >> each: (0123) and (4567).. Now at boot time above code will work > >> perfectly fine. Now you hot unplug 0,1,2,3 and then hotplug 3 in. > >> > >> Here, init would be called for cpu 3 and so you will end up saving > >> 3456 in your policy->cpus > >> > >> :) > > Good catch.. will fix. > > Thanks. ?韬{.n?????%??檩??w?{.n???珉z??^n?■???h?璀?{?夸z罐?+€?zf"?????i?????_璁?:+v??撸?