At Mon, 20 Dec 2010 16:34:36 +0000, Mark Brown wrote:
On Mon, Dec 20, 2010 at 04:56:03PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
It wasn't portability, it was specifically the fact that some drivers want to do block operations which means they depend very strongly on the exact memory layout of the cache (obviously this only works for uncompressed caches).
But this is exactly what hinders the portability :) Which part of soc_4_12_spi_read/write specifies that this must be aligned to be BE while spi_7_9 to be LE? Why soc_16_16_i2c converts to BE internally while soc_16_8_i2c doesn't? I mean, such a difference isn't visible in the API level but hard-coded in the implementation although the API appears as if it's portable.
They should all be specifying something, quite what they should be specifying varies - it just comes down to whatever the standard wire format that the chip vendors have come up with. Mostly if it's not specified it should be little endian.
It's not really been an issue as pretty much all the CPUs have the same endianness.
Yeah, I know it's working now. But it's hidden inside and merely called portable ;)
There's more potential users than currently have it enabled, it's just that the WM8994 driver is the only driver that turns it on by default right now (several other CODEC drivers should but need testing just to confirm that it's OK). It's only a few kilobytes and Kconfig really isn't good at supporting this sort of selection, I'd worry about the usability issues.
Well, it drags LZO COMPRESS and DECOMPRESS unconditionally, which I feel uneasy. Usually the consumer of such lib stuff has a Kconfig, especially the feature can be optional (and in this case it is).
The necessary change is simple like below. The code that requires a compressed cache just needs to select SND_SOC_LZO_CACHE or whatever.
That's one way of doing it, but it does hurt usability.
That's a drawback, sure.
It's no urgent issue, yeah. It's just what I feel uneasy to see such a thing...
If someone's worried about memory usage in ASoC they'd be much better going after the strings than this TBH.
+#ifdef CONFIG_SND_SOC_LZO_CACHE
- SND_SOC_LZO_COMPRESSION = 2,
+#endif +#ifdef CONFIG_SND_SOC_RBTREE_CACHE
- SND_SOC_RBTREE_COMPRESSION = 3,
+#endif
If we were going to make this user selectable we should keep the defines and then fall back to a flat cache if they're turned off otherwise you end up having too much faff with Kconfig interdependencies.
My intention of the chunk above was to give a build error for unsupported cache compressions. But fallback sounds also reasonable, too.
config SND_SOC_WM8994 tristate
select SND_SOC_RBTREE_CACHEExcept the machine driver can override this...
Hmm, I thought the codec is always selected not depended? Can it be overridden?
thanks,
Takashi