On Monday, September 10, 2012, Shawn Guo wrote: > On Fri, Sep 07, 2012 at 09:31:56PM +0200, Rafael J. Wysocki wrote: > > > + struct cpufreq_target_data data; > > > + > > > + data.dev = cpu_dev; > > > + data.clk = cpu_clk; > > > + data.reg = cpu_reg; > > > + data.tol = voltage_tolerance; > > > + data.freq_table = freq_table; > > > + data.policy = policy; > > > + data.target_freq = target_freq; > > > + data.relation = relation; > > > > I'm not sure what you need the new data structure for. Both > > cpu0_set_target() and omap_target() have the same set of arguments, so it > > seems pointless to copy those values back and forth. > > > But the first 4 arguments are driver specific. They are existing > in cpufreq-cpu0 and omap-cpufreq drivers as global variables. I > haven't thought of a good way to consolidate all these differences. Hmm. Who's going to use the cpufreq-cpu0 driver, then? Is OMAP going to be the only user? > > > + > > > + return cpufreq_set_target(&data); > > > > What about calling that function cpufreq_common_set_target()? > > > All driver specific .set_target functions are named with a driver > specific prefix, so this shorter name seems explicit to tell the > "common", IMO. But if you insist, I can change. Well, it's not exactly common, so to speak. It only is shared between the cpu0 and OMAP drivers, so I'm not sure (see below). > > > } > > > > > > static int cpu0_cpufreq_init(struct cpufreq_policy *policy) > > > diff --git a/drivers/cpufreq/cpufreq.h b/drivers/cpufreq/cpufreq.h > > > new file mode 100644 > > > index 0000000..ae380b3 > > > --- /dev/null > > > +++ b/drivers/cpufreq/cpufreq.h > > > > I don't think this header is really necessary. > > > Put the stuff into include/linux/cpufreq.h? But these stuff do not > necessarily need to be that public. > > <snip> > > > > +int cpufreq_set_target(struct cpufreq_target_data *d) > > > > Why don't you use the original arguments of cpu0_set_target() here? > > > As explained above, cpu_dev, cpu_clk, cpu_reg and voltage_tolerance > are used in the function as global variables in cpufreq-cpu0 driver. > > And that's the primary reason why I think making this common set_target > thing is somehow a churn. OK I'm concerned about the copy-pasting, but if that copy-pasting is going to go away when you move OMAP to the new driver, then I agree that adding more complexity just to avoid it is probably not a good idea. What's the planned time frame for the OMAP conversion? Rafael -- 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
