On Thu, Oct 08, 2009 at 02:58:50PM +0300, Eduardo Valentin wrote:
> +struct tpa6130a2_platform_data {
> + int (*set_power)(int state);
> +};
Why is this a callback and not just a GPIO number? That'd seem simpler
for users.
> +int tpa6130a2_add_controls(struct snd_soc_codec *codec)
> +{
> + snd_soc_dapm_new_controls(codec, tpa6130a2_dapm_widgets,
> + ARRAY_SIZE(tpa6130a2_dapm_widgets));
> +
> + return snd_soc_add_controls(codec, tpa6130a2_controls,
> + ARRAY_SIZE(tpa6130a2_controls));
> +
> +}
> +EXPORT_SYMBOL(tpa6130a2_add_controls);
All the ASoC APIs are EXPORT_SYMBOL_GPL().
> + pdata = (struct tpa6130a2_platform_data *)client->dev.platform_data;
> + /* Set default register values */
> + data->regs[TPA6130A2_REG_CONTROL] = TPA6130A2_SWS |
> + TPA6130A2_HP_EN_R |
> + TPA6130A2_HP_EN_L;
This looks like you could implement stereo paths with individual power
control?
> + data->regs[TPA6130A2_REG_VOL_MUTE] = TPA6130A2_VOLUME(20);
The standard thing is to use hardware defaults and leave decisions like
this up to user space.
> + data->set_power = pdata->set_power;
> + if (!pdata->set_power) {
> + data->power_state = 1;
> + tpa6130a2_initialize();
> + }
> + tpa6130a2_power(1);
> +
> + /* Read version */
> + err = tpa6130a2_i2c_read(TPA6130A2_REG_VERSION) &
> + TPA6130A2_VERSION_MASK;
> + if ((err != 1) && (err != 2)) {
> + dev_err(dev, "Unexpected headphone amplifier chip version "
> + "of 0x%02x, was expecting 0x01 or 0x02\n", err);
> + err = -ENODEV;
This seems a bit excessive - is it really likely that the register map
would be changed incompatibly in a future version?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
