On Thu, Feb 05, 2015 at 12:53:42PM -0800, Kenneth Westfield wrote:

+	int bitwidth, ret;

+	bitwidth = snd_pcm_format_width(format);
+	if (bitwidth < 0) {
+		dev_err(dai->dev, "%s() invalid bit width given\n", __func__);

Print the error code.

+	regval = 0;
+	regval |= LPAIF_I2SCTL_LOOPBACK_DISABLE;
+	regval |= LPAIF_I2SCTL_WSSRC_INTERNAL;

Why not just write a single assignment statement?

+	default:
+		dev_err(dai->dev, "%s() invalid bitwidth given: %u\n",
+				__func__, bitwidth);

bitwidth is a signed type but you are using an unsigned format specifier
here.

+	reg = LPAIF_I2SCTL_REG(LPAIF_I2S_PORT_MI2S);
+	mask = LPAIF_I2SCTL_SPKEN_MASK;
+	val = LPAIF_I2SCTL_SPKEN_ENABLE;
+	ret = regmap_update_bits(drvdata->lpaif_map, reg, mask, val);

None of these intermediate variables seem to be doing a lot, why not
just specify the constants directly as arguments (that's the more normal
style)?  A similar thing applies in several other places in this file.

+#ifdef CONFIG_OF
+static const struct of_device_id lpass_cpu_device_id[] = {
+	{.compatible = "qcom," DRV_NAME},
+	{}
+};
+#endif

Using DRV_NAME in the compatible like this makes it impossible to grep
for the driver which isn't helpful.  In general I prefer not to use
DRV_NAME at all (exactly how often do we change the driver name?) but in
this case it's actively harmful.