Hi Mark,
On Thu, Jun 03, 2010 at 12:14:56PM +0100, Mark Brown wrote:
On Tue, Jun 01, 2010 at 09:32:38PM +1000, Stuart Longland wrote:
I know this isn't a proper submission but a few comments below. This looks like it'd be relatively easy to get submitted by stripping out a lot of the commented out code and custom interfaces (like the DA7210 driver).
Yeah... I left it there for now as I'm referring to it (and other drivers) as I go... gradually I'm replacing the ifdef'd code with my own.
The CODEC driver claims total control of the I2C device, and therefore makes it impossible to alter registers using i2c-tools. However, as a work-around; I have provided read/write access to the registers via sysfs... we use the AIC3204 attached to I2C bus 0; the CODEC therefore lives under:
/sys/bus/i2c/devices/0-0018
There are two files:
- regsel: Takes or reports back the 16-bit register address in hexadecimal
- regdata: Reads or writes the value of the register
ASoC already provides register read/write access via debugfs as standard, there's no need to implement this.
Ahh okay, wasn't aware of this. I shall investigate.
struct aic3204_setup_data { unsigned int gpio_func[2]; };
This would normally be platform data in a file in include/sound so it can be set by the architecture code when the device is registered.
In the latest version of the driver, I've ditched this for now. In its place I have provided a mechanism for presetting all registers to an arbitrary values which can be defined by the machine driver ... but even this is temporary.
There's a lot of configuration options available; such as filter coefficients and power modes. These should all ultimately be done using the existing standard APIs ... but for now, I've done something quick and *very* dirty.
/* TODO: PLL */ /* #define ENABLE_PLL */
And I managed to get the PLL working. :-)
#if 0 printk( KERN_INFO "%s: pg %d reg %d[%04x] => %02x\n", __func__, reg >> 8, reg & 0xff, reg, value[0] ); #endif
dev_dbg().
}
/*
- Perform a read/modify/write cycle on a register.
- This is a shorthand function, it reads the specified register, masks out the
- bits in and_mask, applies bits in or_mask, then writes out the result to the
- register.
- It returns the modified value; or a negative error code.
*/
There's a standard snd_soc_update_bits() function in ASoC.
I will have a look at that. Out of interest, is there an up-to-date guide on this information? I'm finding it difficult to find all these functions, much less understand what they do.
/*
- All input lines are connected when !0xf and disconnected with 0xf bit field,
- so we have to use specific dapm_put call for input mixer
*/
Could you explain in more detial what this is doing? I'm not immediately seeing what this is doing but I suspect it might be a value mux?
That comment will disappear, once I know what the function it refers to is updated (at the moment it's a stub). The comment is one of many left-overs from the TLV320AIC3x driver.
static const struct soc_enum aic3204_enum[] = { SOC_ENUM_SINGLE(DAC_LINE_MUX, 6, 3, aic3204_left_dac_mux),
Use individually named variables rather than a table for legibility.
Again, this is a reminant of the old driver. I do use separate variables in the latest version.
/* Turn on ADC or DAC */ if ( substream->stream == SNDRV_PCM_STREAM_PLAYBACK ) { aic3204_write(codec, AIC3204_DACS1, AIC3204_DACS1_LDAC_UP | AIC3204_DACS1_RDAC_UP | AIC3204_DACS1_LDACD_LEFT | AIC3204_DACS1_RDACD_RIGHT | AIC3204_DACS1_SOFT_DIS );
DAPM ought to be figuring this out for you...
Indeed, up the top of my TODO list is to figure out DAPM. :-)
void aic3204_set_headset_detection(struct snd_soc_codec *codec, int detect, int headset_debounce, int button_debounce) { #if 0
There's standard ASoC jack detection which this should integrate with.
I did see mention of this, and will have a look when I get closer to that point.
#if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE) /*
- AIC3204 2 wire address can be up to 4 devices with device addresses
- 0x18, 0x19, 0x1A, 0x1B
*/
/*
- If the i2c layer weren't so broken, we could pass this kind of data
- around
*/ static int aic3204_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
Use standard device model registration - the driver you were basing this on has been converted now...
I shall have a look at that too. For what it's worth, the comment about the addresses is invalid... the AIC3x family were configurable, the AIC3204 is *always* at 0x18.
I've put an updated version of the driver online... along with some explanitory notes:
http://www.longlandclan.yi.org/~stuartl/asoc/
The driver at this point plays audio fine, but won't record ... I just get semi-random noise with a odd-looking square wave pattern. (Not like clipping; more like the quantisation you'd see if using 4-bit PCM.)
I'm working on the mixer interface at present... as the ADC won't record anything useful unless the mixer is configured right. My problem though, is trying to understand what all the macros do. Is there a good reference on how to write these drivers?
Regards,