Hi Mark
Thank you for checking this patch
- own = clk_get(dev, NULL);
- if (IS_ERR(own))
return -EINVAL;Any reason not to use devm_clk_get()?
Ahh yes, Sorry. BTW, currect code calls this function many times now, (this clk is local variable now) then should I use...
1) use devm_clk_get() / devm_clk_put() pair 2) use clk_get() / clk_put() pair 3) keep these clocks under private date (struct fsi_prive) and use devm_clk_get() only _probe timing.
- if (xck) {
*xck = clk_get(NULL, fsi_is_port_a(fsi) ? "fsiack" : "fsibck");if (IS_ERR(*xck)) {dev_err(dev, "can't get fsixck clock\n");return -EINVAL;}- }
This looks wrong - I would expect the driver to be requesting the clock with some fixed name and the struct device. The platform should remap this onto the appropriate underlying clock using clkdev or something.
I see. I will fix in v2.
+static int fsi_clk_set_rate_external(struct device *dev,
struct fsi_priv *fsi,int enable)+{
- struct clk *xck, *ick;
- int ret = 0;
- ret = fsi_clk_gets(dev, fsi, &xck, &ick, NULL);
- if (ret < 0) {
dev_err(dev, "clk gets failed\n");return ret;- }
- if (enable) {
clk_disable(ick);Are we sure that calls to this function are always balanced so that calls to clk_enable() and clk_disable() are balanced?
I balanced it by local technic. But I can say it is un-understandable. Can I use counter ?
Best regards --- Kuninori Morimoto