On Sat, Oct 10, 2009 at 11:47:10AM +0800, Barry Song wrote:
On Thu, Oct 8, 2009 at 6:01 PM, Mark Brown
That's the idea - add new functions for any new register formats.
we do those based on the idea that it is a waste all codecs need to use almost same ways to handle register access. If we use soc-cache to
Not just that, the goal is to make it easier to make changes to the way register accesses are managed by avoiding the need to go through each and every single driver and update it. The most obvious thing is adding support for powering off the CODEC rather than just bringing it down to standby.
handle register access issues and run as abstract layer for all codecs, why not rename it to soc-reg or soc-bus? It seems cache is only a little part of the responsibility of this module.
soc-reg (or probably register) would do equally well, like I say longer term the goal is to support doing interesting things with the cache. I don't know that it's worth renaming at this point.
It's better that functions like xx_7_9_read xx_7_9_write xx_8_8_read xx_8_8_write should not become API because we never know what will be the format for codecs. We should have a xx_n_m_read/xx_n_m_write or just a xx_read/xx_write, with n and m as parameters?
They're not APIs, they're only visible within the file. Drivers using the cache only see the one function they use to initialise the cache with everything else in there hidden from them. They're not specified as part of the interface for the read and write functions because they'd be a lot of noise for callers - they don't generally change at runtime and are something that should really be abstracted away from the generic code anyway.
A codec using snd_soc_7_9_spi_write/snd_soc_8_16_read_i2c should select SPI and I2C in fact. Otherwise, why does it use these functions
The CODECs can't do this since they are selected themselves and Kconfig ignores all dependencies from things that are selected. In any case, a given board is only going to need one or the other of the control interfaces so the CODEC driver needs to leave this up to the machine drivers anyway.
The register cache code is the wrong level to be making decisions about things like this, the CODEC drivers and the register cache code are themselves not usable without a machine driver - this is utility code which should just do whatever it is asked by its users rather than forcing decisons on them.
as codec->hw_read()/codec->hw_write() but not enable SPI and I2C? It seems that defining snd_soc_7_9_spi_write and so on as NULL when SPI is not selected is really useless, just let us pass the compiling to get a module which can't work at all!
It's no use from the point of view of getting a working driver, yes, but then to get something useful you really need the machine driver to ensure that the correct controller driver is being built - simple support for the bus is not enough, we also need the controller. As discussed earlier in this thread for some of the Blackfin machines it's not even possible to do that since we can't tell which controller driver needs to be used since the machine driver can be used with many boards.
Another thing to bear in mind here is that there are people who do things like build the kernel with random .configs and we want to avoid breaking their builds due to poor luck in the configuration they generate while still getting the build coverage that results. We also need to be able to support compilation with either I2C or SPI but not both in order to avoid forcing boards to include support for a bus that may not be used anywhere on the board at all. That means that both buses need to be individually optional at the CODEC level.