Quoting James Hogan (2013-08-16 01:26:38)
On 16/08/13 06:30, Mike Turquette wrote:
Quoting Zhangfei Gao (2013-08-15 21:03:28)
MUX could choose parent accordingly when set_rate. Add clk_rate_fops to debug whether mux choose correct parent
Example: cat clk_summary clock enable_cnt prepare_cnt rate
osc24mhz 3 3 24000000 bpll 1 1 4294967295 bpll_fout3 0 0 200000000 sfc_mux 0 0 200000000 sfc 0 0 200000000
cat osc24mhz/bpll/bpll_fout3/sfc_mux/sfc/clk_rate 200000000
echo 24000000 > osc24mhz/bpll/bpll_fout3/sfc_mux/sfc/clk_rate cat clk_summary clock enable_cnt prepare_cnt rate
osc24mhz 3 3 24000000 sfc_mux 0 0 24000000 sfc 0 0 24000000 bpll 1 1 4294967295 bpll_fout3 0 0 200000000
cat osc24mhz/sfc_mux/sfc/clk_rate 24000000
Signed-off-by: Zhangfei Gao zhangfei.gao@linaro.org
NACK. This patch just makes it possible to call clk_set_rate on every clock from userspace. I don't support that operation, it just leads to tons of horrible hacks where closed userspace crapplications manage hardware directly.
Also I'm not sure what this patch has to do with the description in the changelog. How does setting the rate of every clock help debug mux clocks? You can already see a mux clocks parent and rate from the existing debugs info.
Hi Mike,
I can this patch having value for debugging/testing any set_rate code (e.g. a PLL), not just remuxing. I think it's much more convenient to be able to just echo a few different numbers (or even a whole range from a script) into a debugfs file and see the result than to have to put temporary code in the kernel for everything you want to test. If it results in better tested clock code it might not be so bad.
Regarding misuse, it is in debugfs and requires the common clock debugfs interface to be turned on.
Lots of Android smartphones ship with debugfs enabled and I'm sure that those phones will have the clk debug stuff enabled too.
You could go one step further and make it read only by default (so one must also chmod it - assuming debugfs supports that),
I still do not think this is a strong enough deterrent. Assuming what I said above is true about enabling clk debugfs options in production devices, all that this requires is root access on a device (which isn't so hard to get) and people can damage their device by running a PLL at 4GHz or something.
or #ifdef DEBUG it so that one must explicitly put a #define DEBUG in the file, and it'd still be useful (though perhaps less obvious unless you're already looking through the code). What do you think?
I'm OK with this. This basically means that to use this feature you need to recompile the kernel, which rules out most attack vectors. And if a mfg left this enabled in a production device then they deserve to have some ruined products.
Zhangfei,
Can you respin this patch and protect these new debugfs entries with an '#ifdef DEBUG'?
Thanks, Mike
Cheers James