On Wed, Jun 18, 2014 at 12:45:46PM -0500, Dan Murphy wrote:

+static int tas2552_i2c_read(struct tas2552_data *tas_data, int reg) +{

• int val;
• if (WARN_ON(!tas_data->tas2552_client))

• return -EINVAL;
  

• /* If powered off, return the cached value */
• mutex_lock(&tas_data->mutex);
• if (tas_data->power_state) {

• val = i2c_smbus_read_byte_data(tas_data->tas2552_client, reg);
  

Don't open code your I/O, use regmap like any other CODEC driver. This will remove a very large proportion of the code in the driver which replicates features of the regmap framework.

+static int tas2552_startup(struct snd_pcm_substream *substream,

• 	   struct snd_soc_dai *dai)
  

+{

• struct snd_soc_codec *codec = dai->codec;
• struct tas2552_data *tas2552 = snd_soc_codec_get_drvdata(codec);
• tas2552_sw_shutdown(tas2552, 1);
• tas2552_power(tas2552, 1);
• /* Turn on Class D amplifier */
• tas2552_i2c_write(tas2552, TAS2552_CFG_2, 0x80);
• tas2552_sw_shutdown(tas2552, 0);

I'd expect this to be done using DAPM.

+static const struct regmap_config tas2552_regmap_config = {

• .reg_bits = 8,
• .val_bits = 8,
• .max_register = TAS2552_MAX_REG,
• .reg_defaults = NULL,
• .num_reg_defaults = 0,

No need to set things to zero or NULL in static variables, that's the default.

• dev = &client->dev;
• data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
• if (data == NULL) {

• dev_err(dev, "Can not allocate memory\n");
  

No need to print an error, OOM messages are already quite noisy.