Re: [alsa-devel] [PATCH v3 1/2] ASoC: cs35l35: Add support for Cirrus CS35L35 Amplifier

21 Dec 2016

      On Tue, Dec 13, 2016 at 10:59:03AM -0600, Li Xu wrote:
...
Add driver support for Cirrus Logic CS35L35 boosted
speaker amplifier
Signed-off-by: Li Xu li.xu@cirrus.com
Mostly just some minor comments.
...
+struct classh_cfg {

/*
* Class H Algorithm Control Variables

* You can either have it done

* automatically or you can adjust

* these variables for tuning

*

* if you do not enable the internal algorithm

* you will get a set of mixer controls for

* Class H tuning

*

* Section 4.3 of the datasheet

*/

/* Internal ClassH Algorithm  */

Feels redundant to have this extra comment after the large comment
before it.
...

+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
Do we need the workqueue header we don't seem to use any
workqueues?
...
+static int cs35l35_sdin_event(struct snd_soc_dapm_widget *w,

struct snd_kcontrol *kcontrol, int event)

+{

struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
struct cs35l35_private *cs35l35 = snd_soc_codec_get_drvdata(codec);
int ret = 0;

switch (event) {
case SND_SOC_DAPM_PRE_PMU:
regmap_update_bits(cs35l35->regmap, CS35L35_CLK_CTL1,

	CS35L35_MCLK_DIS_MASK, 0 << CS35L35_MCLK_DIS_SHIFT);

regmap_update_bits(cs35l35->regmap, CS35L35_PWRCTL1,

	CS35L35_DISCHG_FILT_MASK, 0 << CS35L35_DISCHG_FILT_SHIFT);

regmap_update_bits(cs35l35->regmap, CS35L35_PWRCTL1,

	CS35L35_PDN_ALL_MASK, 0);

break;

Break should be indented for kernel coding style.
...

case SND_SOC_DAPM_POST_PMD:
regmap_update_bits(cs35l35->regmap, CS35L35_PWRCTL1,

	CS35L35_PDN_ALL_MASK, 1);

regmap_update_bits(cs35l35->regmap, CS35L35_PWRCTL1,

	CS35L35_DISCHG_FILT_MASK, 1 << CS35L35_DISCHG_FILT_SHIFT);

ret = wait_for_completion_timeout(&cs35l35->pdn_done,

					msecs_to_jiffies(100));

if (ret == 0) {

	dev_err(codec->dev, "TIMEOUT PDN_DONE did not complete in 100ms\n");

	ret = -ETIMEDOUT;

}

regmap_update_bits(cs35l35->regmap, CS35L35_CLK_CTL1,

	CS35L35_MCLK_DIS_MASK, 1 << CS35L35_MCLK_DIS_SHIFT);

break;

Ditto.
...

default:
dev_err(codec->dev, "Invalid event = 0x%x\n", event);

ret = -EINVAL;

}
return ret;

+}

+static int cs35l35_main_amp_event(struct snd_soc_dapm_widget *w,

struct snd_kcontrol *kcontrol, int event)

+{

struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
struct cs35l35_private *cs35l35 = snd_soc_codec_get_drvdata(codec);
unsigned int reg[4];
int i;

switch (event) {
case SND_SOC_DAPM_PRE_PMU:
if (cs35l35->pdata.bst_pdn_fet_on)

	regmap_update_bits(cs35l35->regmap, CS35L35_PWRCTL2,

		CS35L35_PDN_BST_MASK, 0 << CS35L35_PDN_BST_FETON_SHIFT);

else

	regmap_update_bits(cs35l35->regmap, CS35L35_PWRCTL2,

		CS35L35_PDN_BST_MASK, 0 << CS35L35_PDN_BST_FETOFF_SHIFT);

	regmap_update_bits(cs35l35->regmap, CS35L35_PROTECT_CTL,

		CS35L35_AMP_MUTE_MASK, 0 << CS35L35_AMP_MUTE_SHIFT);

break;

case SND_SOC_DAPM_POST_PMU:
usleep_range(5000, 5100);

/* If PDM mode we must use VP

 * for Voltage control

 */

Does this comment need to split across multiple lines?
...
+static int cs35l35_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
+{

struct snd_soc_codec *codec = codec_dai->codec;
struct cs35l35_private *cs35l35 = snd_soc_codec_get_drvdata(codec);

switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
case SND_SOC_DAIFMT_CBM_CFM:
regmap_update_bits(cs35l35->regmap, CS35L35_CLK_CTL1,

		    CS35L35_MS_MASK, 1 << CS35L35_MS_SHIFT);

cs35l35->slave_mode = false;

break;

case SND_SOC_DAIFMT_CBS_CFS:
regmap_update_bits(cs35l35->regmap, CS35L35_CLK_CTL1,

		    CS35L35_MS_MASK, 0 << CS35L35_MS_SHIFT);

cs35l35->slave_mode = true;

break;

default:
return -EINVAL;

}

switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
case SND_SOC_DAIFMT_I2S:
cs35l35->i2s_mode = true;

cs35l35->pdm_mode = false;

break;

case SND_SOC_DAIFMT_PDM:
cs35l35->pdm_mode = true;

cs35l35->i2s_mode = false;

Feels a bit redundant to have both of these if they are only ever
a logical inversion of each other.
...
+static int cs35l35_get_clk_config(int sysclk, int srate)
+{

int i;

for (i = 0; i < ARRAY_SIZE(cs35l35_clk_ctl); i++) {
if (cs35l35_clk_ctl[i].sysclk == sysclk &&

	cs35l35_clk_ctl[i].srate == srate)

	return cs35l35_clk_ctl[i].clk_cfg;

}
return -EINVAL;

+}

+static int cs35l35_pcm_hw_params(struct snd_pcm_substream *substream,

		 struct snd_pcm_hw_params *params,

		 struct snd_soc_dai *dai)

+{

struct snd_soc_codec *codec = dai->codec;
struct cs35l35_private *cs35l35 = snd_soc_codec_get_drvdata(codec);
struct classh_cfg *classh = &cs35l35->pdata.classh_algo;
int srate = params_rate(params);
int ret = 0;
u8 sp_sclks;
int audin_format;
int errata_chk;

int clk_ctl = cs35l35_get_clk_config(cs35l35->sysclk, srate);

if (clk_ctl < 0) {
dev_err(codec->dev, "Invalid CLK:Rate %d:%d\n",

	cs35l35->sysclk, srate);

return -EINVAL;

}

It would normally be slightly better to set constraints in
startup based on the SYSCLK rather than returning an error in
hw_params. This allows user-space to negotiate a rate that is
actually supported and do any sample rate conversion required.
...

ret = regmap_update_bits(cs35l35->regmap, CS35L35_CLK_CTL2,
	  CS35L35_CLK_CTL2_MASK, clk_ctl);

if (ret != 0) {
dev_err(codec->dev, "Failed to set port config %d\n", ret);

return ret;

}

/* Rev A0 Errata
*

* When configured for the weak-drive detection path (CH_WKFET_DIS = 0)

* the Class H algorithm does not enable weak-drive operation for

* nonzero values of CH_WKFET_DELAY if SP_RATE = 01 or 10

*

*/

errata_chk = clk_ctl & CS35L35_SP_RATE_MASK;

if (classh->classh_wk_fet_disable == 0x00 &&
(errata_chk == 0x01 || errata_chk == 0x03)) {

ret = regmap_update_bits(cs35l35->regmap,

	CS35L35_CLASS_H_FET_DRIVE_CTL, CS35L35_CH_WKFET_DEL_MASK,

	0 << CS35L35_CH_WKFET_DEL_SHIFT);

if (ret != 0) {

	dev_err(codec->dev, "Failed to set fet config %d\n",

		ret);

	return ret;

}

}

+/*

You can pull more Monitor data from the SDOUT pin than going to SDIN

Just make sure your SCLK is fast enough to fill the frame

*/
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
switch (params_width(params)) {

case 8:

	audin_format = CS35L35_SDIN_DEPTH_8;

	break;

case 16:

	audin_format = CS35L35_SDIN_DEPTH_16;

	break;

case 24:

	audin_format = CS35L35_SDIN_DEPTH_24;

	break;

default:

	dev_err(codec->dev, "Unsupported Width %d\n",

		params_width(params));

}

regmap_update_bits(cs35l35->regmap,

	CS35L35_AUDIN_DEPTH_CTL, CS35L35_AUDIN_DEPTH_MASK,

	audin_format << CS35L35_AUDIN_DEPTH_SHIFT);

if (cs35l35->pdata.stereo) {

	regmap_update_bits(cs35l35->regmap,

		CS35L35_AUDIN_DEPTH_CTL, CS35L35_ADVIN_DEPTH_MASK,

		audin_format << CS35L35_ADVIN_DEPTH_SHIFT);

}

}

+/* We have to take the SCLK to derive num sclks

to configure the CLOCK_CTL3 register correctly

*/
if ((cs35l35->sclk / srate) % 4) {
dev_err(codec->dev, "Unsupported sclk/fs ratio %d:%d\n",

			cs35l35->sclk, srate);

return -EINVAL;

}

Again here it might be slightly better to constraints in startup.
...

sp_sclks = ((cs35l35->sclk / srate) / 4) - 1;

if (cs35l35->i2s_mode) {
/* Only certain ratios are supported in I2S Slave Mode */

if (cs35l35->slave_mode) {

	switch (sp_sclks) {

	case CS35L35_SP_SCLKS_32FS:

	case CS35L35_SP_SCLKS_48FS:

	case CS35L35_SP_SCLKS_64FS:

	break;

	default:

		dev_err(codec->dev, "ratio not supported\n");

		return -EINVAL;

	};

} else {

	/* Only certain ratios supported in I2S MASTER Mode */

	switch (sp_sclks) {

	case CS35L35_SP_SCLKS_32FS:

	case CS35L35_SP_SCLKS_64FS:

	break;

	default:

		dev_err(codec->dev, "ratio not supported\n");

		return -EINVAL;

	};

}

ret = regmap_update_bits(cs35l35->regmap,

	CS35L35_CLK_CTL3, CS35L35_SP_SCLKS_MASK,

	sp_sclks << CS35L35_SP_SCLKS_SHIFT);

if (ret != 0) {

	dev_err(codec->dev, "Failed to set fsclk %d\n", ret);

	return ret;

}

}
if (cs35l35->pdm_mode) {
regmap_update_bits(cs35l35->regmap, CS35L35_AMP_INP_DRV_CTL,

	CS35L35_PDM_MODE_MASK, 1 << CS35L35_PDM_MODE_SHIFT);

} else {
regmap_update_bits(cs35l35->regmap, CS35L35_AMP_INP_DRV_CTL,

	CS35L35_PDM_MODE_MASK, 0 << CS35L35_PDM_MODE_SHIFT);

}

This if could be combined with the one above since pdm_mode ==
!i2s_mode.
...

return ret;

+}

+static const struct snd_soc_dai_ops cs35l35_ops = {

.startup = cs35l35_pcm_startup,
.set_fmt = cs35l35_set_dai_fmt,
.hw_params = cs35l35_pcm_hw_params,
.set_sysclk = cs35l35_dai_set_sysclk,

+};

+static const struct snd_soc_dai_ops cs35l35_pdm_ops = {

.startup = cs35l35_pdm_startup,
.set_fmt = cs35l35_set_dai_fmt,
.hw_params = cs35l35_pcm_hw_params,

I would be tempted to rename the function to just
cs35l35_hw_params if it is shared between both PCM and PDM.
...

.set_sysclk = cs35l35_dai_set_sysclk,

+};

+static int cs35l35_codec_probe(struct snd_soc_codec *codec)
+{

struct cs35l35_private *cs35l35 = snd_soc_codec_get_drvdata(codec);
struct classh_cfg *classh = &cs35l35->pdata.classh_algo;
struct monitor_cfg *monitor_config = &cs35l35->pdata.mon_cfg;
int ret;

cs35l35->codec = codec;

/* Set Platform Data */
if (cs35l35->pdata.bst_vctl)
regmap_update_bits(cs35l35->regmap, CS35L35_BST_CVTR_V_CTL,

	CS35L35_BST_CTL_MASK, cs35l35->pdata.bst_vctl);

if (cs35l35->pdata.bst_ipk)
regmap_update_bits(cs35l35->regmap, CS35L35_BST_PEAK_I,

	CS35L35_BST_IPK_MASK,

	cs35l35->pdata.bst_ipk << CS35L35_BST_IPK_SHIFT);

I believe zero is a valid value for this field, but not the
default. Are we happy that the user can never set this value?
...

if (cs35l35->pdata.gain_zc)
regmap_update_bits(cs35l35->regmap, CS35L35_PROTECT_CTL,

	CS35L35_AMP_GAIN_ZC_MASK,

	cs35l35->pdata.gain_zc << CS35L35_AMP_GAIN_ZC_SHIFT);

if (cs35l35->pdata.aud_channel)
regmap_update_bits(cs35l35->regmap,

	CS35L35_AUDIN_RXLOC_CTL,

	CS35L35_AUD_IN_LR_MASK,

	cs35l35->pdata.aud_channel << CS35L35_AUD_IN_LR_SHIFT);

if (cs35l35->pdata.stereo) {
regmap_update_bits(cs35l35->regmap,

	CS35L35_ADVIN_RXLOC_CTL, CS35L35_ADV_IN_LR_MASK,

	cs35l35->pdata.adv_channel << CS35L35_ADV_IN_LR_SHIFT);

if (cs35l35->pdata.shared_bst)

	regmap_update_bits(cs35l35->regmap, CS35L35_CLASS_H_CTL,

		CS35L35_CH_STEREO_MASK, 1 << CS35L35_CH_STEREO_SHIFT);

ret = snd_soc_add_codec_controls(codec, cs35l35_adv_controls,

			ARRAY_SIZE(cs35l35_adv_controls));

if (ret)

	return ret;

}

if (cs35l35->pdata.sp_drv_str)
regmap_update_bits(cs35l35->regmap, CS35L35_CLK_CTL1,

	CS35L35_SP_DRV_MASK,

	cs35l35->pdata.sp_drv_str << CS35L35_SP_DRV_SHIFT);

if (classh->classh_algo_enable) {
if (classh->classh_bst_override)

	regmap_update_bits(cs35l35->regmap,

		CS35L35_CLASS_H_CTL, CS35L35_CH_BST_OVR_MASK,

		classh->classh_bst_override << CS35L35_CH_BST_OVR_SHIFT);

if (classh->classh_bst_max_limit)

	regmap_update_bits(cs35l35->regmap,

		CS35L35_CLASS_H_CTL, CS35L35_CH_BST_LIM_MASK,

		classh->classh_bst_max_limit << CS35L35_CH_BST_LIM_SHIFT);

This is a single bit, but the default bit is 1, so this code can
never change the value of the field.
...

if (classh->classh_mem_depth)

	regmap_update_bits(cs35l35->regmap,

		CS35L35_CLASS_H_CTL, CS35L35_CH_MEM_DEPTH_MASK,

		classh->classh_mem_depth << CS35L35_CH_MEM_DEPTH_SHIFT);

Again zero is a valid value, and not the default.
...

if (classh->classh_headroom)

	regmap_update_bits(cs35l35->regmap,

		CS35L35_CLASS_H_HEADRM_CTL, CS35L35_CH_HDRM_CTL_MASK,

		classh->classh_headroom << CS35L35_CH_HDRM_CTL_SHIFT);

if (classh->classh_release_rate)

	regmap_update_bits(cs35l35->regmap,

		CS35L35_CLASS_H_RELEASE_RATE, CS35L35_CH_REL_RATE_MASK,

		classh->classh_release_rate << CS35L35_CH_REL_RATE_SHIFT);

if (classh->classh_wk_fet_disable)

	regmap_update_bits(cs35l35->regmap,

		CS35L35_CLASS_H_FET_DRIVE_CTL, CS35L35_CH_WKFET_DIS_MASK,

		classh->classh_wk_fet_disable << CS35L35_CH_WKFET_DIS_SHIFT);

if (classh->classh_wk_fet_delay)

	regmap_update_bits(cs35l35->regmap,

		CS35L35_CLASS_H_FET_DRIVE_CTL, CS35L35_CH_WKFET_DEL_MASK,

		classh->classh_wk_fet_delay << CS35L35_CH_WKFET_DEL_SHIFT);

Again zero is a valid value, and not the default.
...

if (classh->classh_wk_fet_thld)

	regmap_update_bits(cs35l35->regmap,

		CS35L35_CLASS_H_FET_DRIVE_CTL, CS35L35_CH_WKFET_THLD_MASK,

		classh->classh_wk_fet_thld << CS35L35_CH_WKFET_THLD_SHIFT);

if (classh->classh_vpch_auto)

	regmap_update_bits(cs35l35->regmap,

		CS35L35_CLASS_H_VP_CTL, CS35L35_CH_VP_AUTO_MASK,

		classh->classh_vpch_auto << CS35L35_CH_VP_AUTO_SHIFT);

Again single bit with a default of 1.
...

if (classh->classh_vpch_rate)

	regmap_update_bits(cs35l35->regmap,

		CS35L35_CLASS_H_VP_CTL, CS35L35_CH_VP_RATE_MASK,

		classh->classh_vpch_rate << CS35L35_CH_VP_RATE_SHIFT);

Again zero is a valid value, and not the default.
...

if (classh->classh_vpch_man)

	regmap_update_bits(cs35l35->regmap,

		CS35L35_CLASS_H_VP_CTL, CS35L35_CH_VP_MAN_MASK,

		classh->classh_vpch_man << CS35L35_CH_VP_MAN_SHIFT);

}

<snip>
...
+static int cs35l35_i2c_probe(struct i2c_client *i2c_client,

	      const struct i2c_device_id *id)

+{

struct cs35l35_private *cs35l35;
struct cs35l35_platform_data *pdata =
dev_get_platdata(&i2c_client->dev);

int i;
int ret;
unsigned int devid = 0;
unsigned int reg;

cs35l35 = devm_kzalloc(&i2c_client->dev,
	       sizeof(struct cs35l35_private),

	       GFP_KERNEL);

if (!cs35l35) {
dev_err(&i2c_client->dev, "could not allocate codec\n");

return -ENOMEM;

}

i2c_set_clientdata(i2c_client, cs35l35);
cs35l35->regmap = devm_regmap_init_i2c(i2c_client, &cs35l35_regmap);
if (IS_ERR(cs35l35->regmap)) {
ret = PTR_ERR(cs35l35->regmap);

dev_err(&i2c_client->dev, "regmap_init() failed: %d\n", ret);

goto err;

}

for (i = 0; i < ARRAY_SIZE(cs35l35_supplies); i++)
cs35l35->supplies[i].supply = cs35l35_supplies[i];

cs35l35->num_supplies = ARRAY_SIZE(cs35l35_supplies);

ret = devm_regulator_bulk_get(&i2c_client->dev,
	cs35l35->num_supplies,

	cs35l35->supplies);

if (ret != 0) {
dev_err(&i2c_client->dev,

	"Failed to request core supplies: %d\n",

	ret);

return ret;

}

if (pdata) {
cs35l35->pdata = *pdata;

} else {
pdata = devm_kzalloc(&i2c_client->dev,

		     sizeof(struct cs35l35_platform_data),

		GFP_KERNEL);

if (!pdata) {

	dev_err(&i2c_client->dev,

		"could not allocate pdata\n");

	return -ENOMEM;

}

if (i2c_client->dev.of_node) {

	ret = cs35l35_handle_of_data(i2c_client, pdata);

	if (ret != 0)

		return ret;

}

cs35l35->pdata = *pdata;

}

ret = regulator_bulk_enable(cs35l35->num_supplies,
			cs35l35->supplies);

if (ret != 0) {
dev_err(&i2c_client->dev,

	"Failed to enable core supplies: %d\n",

	ret);

return ret;

}

/* returning NULL can be an option if in stereo mode */
cs35l35->reset_gpio = devm_gpiod_get_optional(&i2c_client->dev,
"reset", GPIOD_OUT_LOW);

if (IS_ERR(cs35l35->reset_gpio))
return PTR_ERR(cs35l35->reset_gpio);

This should be a goto err;
...

if (cs35l35->reset_gpio)
gpiod_set_value_cansleep(cs35l35->reset_gpio, 1);

gpiod_set_value_can_sleep does an internal NULL check on the GPIO
desc I would be tempted to just rely on that one.
...

init_completion(&cs35l35->pdn_done);

ret = regmap_register_patch(cs35l35->regmap, cs35l35_errata_patch,
		    ARRAY_SIZE(cs35l35_errata_patch));

if (ret < 0) {
dev_err(&i2c_client->dev, "Failed to apply errata patch\n");

return ret;

This should be a goto err;
...

}

ret = devm_request_threaded_irq(&i2c_client->dev, i2c_client->irq, NULL,
	cs35l35_irq, IRQF_ONESHOT | IRQF_TRIGGER_LOW,

	"cs35l35", cs35l35);

if (ret != 0) {
dev_err(&i2c_client->dev, "Failed to request IRQ: %d\n", ret);

goto err;

}
/* initialize codec */
ret = regmap_read(cs35l35->regmap, CS35L35_DEVID_AB, &reg);

devid = (reg & 0xFF) << 12;
ret = regmap_read(cs35l35->regmap, CS35L35_DEVID_CD, &reg);
devid |= (reg & 0xFF) << 4;
ret = regmap_read(cs35l35->regmap, CS35L35_DEVID_E, &reg);
devid |= (reg & 0xF0) >> 4;

if (devid != CS35L35_CHIP_ID) {
dev_err(&i2c_client->dev,

	"CS35L35 Device ID (%X). Expected ID %X\n",

	devid, CS35L35_CHIP_ID);

ret = -ENODEV;

goto err;

}

ret = regmap_read(cs35l35->regmap, CS35L35_REV_ID, &reg);
if (ret < 0) {
dev_err(&i2c_client->dev, "Get Revision ID failed\n");

goto err;

}

dev_info(&i2c_client->dev,
 "Cirrus Logic CS35L35 (%x), Revision: %02X\n", devid,

ret & 0xFF);

/* Set the INT Masks for critical errors */
regmap_write(cs35l35->regmap, CS35L35_INT_MASK_1, CS35L35_INT1_CRIT_MASK);
regmap_write(cs35l35->regmap, CS35L35_INT_MASK_2, CS35L35_INT2_CRIT_MASK);
regmap_write(cs35l35->regmap, CS35L35_INT_MASK_3, CS35L35_INT3_CRIT_MASK);
regmap_write(cs35l35->regmap, CS35L35_INT_MASK_4, CS35L35_INT4_CRIT_MASK);

regmap_update_bits(cs35l35->regmap, CS35L35_PWRCTL2,
CS35L35_PWR2_PDN_MASK, CS35L35_PWR2_PDN_MASK);

if (cs35l35->pdata.bst_pdn_fet_on)
regmap_update_bits(cs35l35->regmap, CS35L35_PWRCTL2,

	CS35L35_PDN_BST_MASK, 1 << CS35L35_PDN_BST_FETON_SHIFT);

else
regmap_update_bits(cs35l35->regmap, CS35L35_PWRCTL2,

	CS35L35_PDN_BST_MASK, 1 << CS35L35_PDN_BST_FETOFF_SHIFT);

regmap_update_bits(cs35l35->regmap, CS35L35_PWRCTL3,
CS35L35_PWR3_PDN_MASK, CS35L35_PWR3_PDN_MASK);

regmap_update_bits(cs35l35->regmap, CS35L35_PROTECT_CTL,
CS35L35_AMP_MUTE_MASK, 1 << CS35L35_AMP_MUTE_SHIFT);

ret =  snd_soc_register_codec(&i2c_client->dev,
	&soc_codec_dev_cs35l35, cs35l35_dai,

	ARRAY_SIZE(cs35l35_dai));

if (ret < 0) {
dev_err(&i2c_client->dev,

	"%s: Register codec failed\n", __func__);

goto err;

}

+err:

regulator_bulk_disable(cs35l35->num_supplies,
	       cs35l35->supplies);

return ret;

+}

+static int cs35l35_i2c_remove(struct i2c_client *client)
+{

snd_soc_unregister_codec(&client->dev);
kfree(i2c_get_clientdata(client));

clientdata was allocated with devm this kfree will cause a double
free.
...

return 0;

+}
Thanks,
Charles

Re: [alsa-devel] [PATCH v3 1/2] ASoC: cs35l35: Add support for Cirrus CS35L35 Amplifier

Charles Keepax

Signed-off-by: Li Xu li.xu@cirrus.com