Re: [alsa-devel] [PATCH 2/2] ASoC: fsi: add master clock control functions

On Tue, Oct 30, 2012 at 07:59:45PM -0700, Kuninori Morimoto wrote:

• own = clk_get(dev, NULL);
• if (IS_ERR(own))

• return -EINVAL;
  

Any reason not to use devm_clk_get()?

• 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.

+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?