[alsa-devel] [PATCH] Add support for tlv320aic3007 to tlv320aic3x driver
The attached patch adds support for the tlv320aic3007 codec to the tlv320aic3x driver.
The tlv320aic3007 is similar to the aic31, but has an additional class-D speaker amp. The speaker amp control register overlaps with the mono output register of other codecs in this family, so we allow the board driver to pass in a model id to identify the codec being used.
randolph
On Thu, Aug 19, 2010 at 02:19:16PM +0800, Randolph Chung wrote:
The attached patch adds support for the tlv320aic3007 codec to the tlv320aic3x driver.
The tlv320aic3007 is similar to the aic31, but has an additional class-D speaker amp. The speaker amp control register overlaps with the mono output register of other codecs in this family, so we allow the board driver to pass in a model id to identify the codec being used.
Rather than passing something in the platform data you should use the I2C device ID table for the device and have the board register the device as a tlv320aic3007.
I would review the patch in more detail but you attached it as a base64 encoded attachment which means my MUA hasn't quoted it for me and won't even display it unless I explicitly open it... The main thing I noticed was that you're using an enum instead of a TLV control for the amp volume.
As I reminded you last time you always need to CC maintainers on patches. This is very helpful for ensuring that we actually see things on busy lists and is helpful for keeping track of what needs reviewing.
Rather than passing something in the platform data you should use the I2C device ID table for the device and have the board register the device as a tlv320aic3007.
Yes, that will work too - if that's the preference I will rework the patch that way.
I would review the patch in more detail but you attached it as a base64 encoded attachment which means my MUA hasn't quoted it for me and won't even display it unless I explicitly open it...
My apologies - since you reported whitespace damage in my previous post I included the patch as an attachment and apparently my mail client decided to base64 encode it. I will post followups from another MUA.
The main thing I noticed was that you're using an enum instead of a TLV control for the amp volume.
The amp volume (gain) only has 4 steps (0, 6dB, 12dB, 18dB). I can rework as a TLV if that's the preference. TI has a (non-published?) driver for this codec that used an enum so this part was copied over.
As I reminded you last time you always need to CC maintainers on patches. This is very helpful for ensuring that we actually see things on busy lists and is helpful for keeping track of what needs reviewing.
You copied my last patch to lrg (and he acked the patch), so I copied my patch to lrg. Previous patches to this driver were also signed off by him. Should I copy to somebody else?
randolph
On Thu, Aug 19, 2010 at 06:00:29PM +0800, Randolph Chung wrote:
Rather than passing something in the platform data you should use the I2C device ID table for the device and have the board register the device as a tlv320aic3007.
Yes, that will work too - if that's the preference I will rework the patch that way.
Yes, please.
I would review the patch in more detail but you attached it as a base64 encoded attachment which means my MUA hasn't quoted it for me and won't even display it unless I explicitly open it...
My apologies - since you reported whitespace damage in my previous post I included the patch as an attachment and apparently my mail client decided to base64 encode it. I will post followups from another MUA.
git send-email is usually the way forward.
The main thing I noticed was that you're using an enum instead of a TLV control for the amp volume.
The amp volume (gain) only has 4 steps (0, 6dB, 12dB, 18dB). I can rework as a TLV if that's the preference. TI has a (non-published?) driver for this codec that used an enum so this part was copied over.
The number of steps doesn't make any difference - the advantage of a proper " Volume" control with TLV information is that applications can understand and interpret the control, displaying it appropriately in UIs and so on.
Non-mainline drivers aren't a terribly reliable guide since they haven't been reviewed.
As I reminded you last time you always need to CC maintainers on patches. This is very helpful for ensuring that we actually see things on busy lists and is helpful for keeping track of what needs reviewing.
You copied my last patch to lrg (and he acked the patch), so I copied my patch to lrg. Previous patches to this driver were also signed off by him. Should I copy to somebody else?
Me. Liam and myself are comaintainers for ASoC. See MAINTAINERS (there is a script scripts/get_maintainers.pl in the kernel which can parse this automatically).
participants (2)
-
Mark Brown -
Randolph Chung