18 Jan
2012
18 Jan
'12
6:53 p.m.
On Wed, Jan 18, 2012 at 04:38:28PM +0100, Wolfram Sang wrote:
+static int max9768_write(struct snd_soc_codec *codec, unsigned int reg,
unsigned int value)
Use regmap for register I/O please.
- data = (reg << 6) | value;
- ret = i2c_smbus_write_byte(codec->control_data, data);
You may need to add a new register format, but that shouldn't be too much work.
- if (max9768->flags & MAX9768_FLAG_CLASSIC_PWM) {
ret = snd_soc_write(codec, MAX9768_CTRL, MAX9768_CTRL_PWM);if (ret)return ret;- }
Some documentation on what this does would be nice. Is this something that might usefully be changed at runtime?
- ret = snd_soc_add_controls(codec, max9768_volume,
ARRAY_SIZE(max9768_volume));- if (ret)
return ret;
Use the controls field in the driver. I'd also expect to see some sort of DAPM support, even if it's just mapping out the pins. Since you're manually controlling the mute it'd be nice to make the unmute be (mute && power) to save on pops and clicks from the CODEC being amplified.
err = gpio_request_one(pdata->mute_gpio, GPIOF_INIT_HIGH, "MAX9768 Mute");max9768->mute_gpio = err ?: pdata->mute_gpio;
I really don't like the ternery operator at the best of times...