On 14 October 2014 12:29, Kelvin Cheung <keguang.zhang@xxxxxxxxx> wrote: > > 2014-10-10 12:40 GMT+08:00 Viresh Kumar <viresh.kumar@xxxxxxxxxx>: >> > +#include <cpufreq.h> >> > +#include <loongson1.h> >> >> Okay, it looks like I have forgotten some of the C basics :) >> But wouldn't the above two lines search for this file in <include/*>, unless >> you have compiled it with something like -I include/linux ?? >> And even then I don't think loongson1.h is present there .. >> >> What am I missing ? > > > The two header files are located in arch/mips/include/asm/mach-loongson1 Okay, but I didn't knew it works this way as well. I thought preprocessor will search in include/ and arch/mips/include/ directories only and so you need to do this: #include <asm/mach-loongson1/cpufreq.h> Do you know why it works directly in your case? Probably for readability above might be better ? >> > +static int ls1x_cpufreq_notifier(struct notifier_block *nb, >> > + unsigned long val, void *data) >> > +{ >> > + if (val == CPUFREQ_POSTCHANGE) >> > + current_cpu_data.udelay_val = loops_per_jiffy; >> > + >> > + return NOTIFY_OK; >> > +} >> >> Why don't you do this at a single place in mips core instead of every >> mips cpufreq driver ? > > > Most of MIPS CPUs use performance counter as the system timer, which is built-in to the CPU core. They can't do cpufreq due to lack of external timer. Therefore, the above section is not a common code. Not sure if I understood your reply Or you understood mine :) Let me try again to clarify the question. On freq change you need to update 'current_cpu_data.udelay_val', this is done in both loongson drivers. What I was asking you is to register the notifier in some core code, so that this isn't required to be done in all cpufreq drivers. NOTE: The notifier routine will only be called if cpufreq is enabled for the platform.. >> > +static int ls1x_cpufreq_probe(struct platform_device *pdev) >> > +{ >> > + struct plat_ls1x_cpufreq *pdata = pdev->dev.platform_data; >> > + struct clk *clk; >> > + int ret; >> > + >> >> Try this code here: >> >> pdata = NULL; >> >> > + if (!pdata && !pdata->clk_name && !pdata->osc_clk_name) { >> >> And tell me what happens here then. :) > > > I will add an error message here. Did you try what I asked you to? I wasn't talking about the message.. Though you better add it as well.. What I have asked you will result in kernel crash ..