On Tue, Mar 11, 2014 at 03:11:36PM +0800, bardliao@realtek.com wrote:
From: Bard Liao bardliao@realtek.com
This patch adds the ALC286 codec driver.
ALC286 is a dual mode codec, which can run as HD-A or I2S mode. It is controlled by HD-A verb commands via I2C protocol.
Some or all of this documentation needs to end up in the code, we need to be able to understand and maintain the code going forwards and while the commit message is going to be kept it's still useful if the code can be followed. Right now that's extremely hard.
The following is the I/O difference between ALC286 and general I2S codecs.
- A HD-A verb command contains three parts, NID, VID, and PID. And an I2S command contains only two parts: address and data.
It'd probably help to explain waht these are.
- Not only the register address is written, but the read command also includes the entire write command.
- rt286 uses different registers for read and write the same bits.
As a result, standard regmap is difficult to be used on ALC286. We don't request a standard I/O by snd_soc_codec_set_cache_io anymore. Now we have ,reg_write and .reg_read functions for ALC286's I/O. And we don't use cache due to item 3 above. Some dummy registers (address <= 0xff) are defined for dapm routing. Thhe dummy registers are cache only.
Don't use dummy registers, DAPM already has virtual controls of various kinds - if you need more let's extend them. Storing data in virtual registers just makes things confusing and fragile. Some older CODEC drivers did it and they're harder to work with now than they should be.
Due to item 2 above, HD-A verb commands are put into the address part of regmap. When we issue HD-A verb write commands, the data part of regmap is zero.
I'm having a really hard time understanding how this follows. We do have some other devices that do things like have multi-level register addresses with registers collected in pages. By analogy here what I'd expect to see is something which combines the various pieces of addressing information into a bitfields within a single number that can be used as a register address.
Or perhaps this just isn't anything like a register map at all? Whatever is going on something that ignores the value part of the regmap interfaces doesn't seem like it is a very good fit.