At Mon, 20 Dec 2010 15:27:09 +0000,
Mark Brown wrote:
>
> On Mon, Dec 20, 2010 at 03:50:54PM +0100, Takashi Iwai wrote:
>
> > It's pretty easy to do some cleanups. After 5 minutes rewrite, over
> > 300 LOC reduced. But I remember you wanted to work on it for
> > portability, e.g. endianness. The same problem seems still remaining,
>
> 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.
> > for example, snd_socc_16_16_read_i2c() has cpu_to_be16() while no
> > others have such. Any progress on this? Or should I post you patchesf
> > first?
>
> I don't think anyone's working on it, I'm not aware of it actually
> bothering anyone - the code is repetitive obviously but it's also very
> simple. I've you've got patches please go ahead and post them but bear
> in mind the thing with the memory layout.
OK, I'm going to feed a few.
> > Also, for now, the new nice cache compression support is always
> > built-in although the additional code size isn't so negligible and its
> > user is just only one. Wouldn't it be better to give new Kconfig for
> > them? I'm also no big fan for small Kconfigs, but in this case, I see
> > no better resolution.
>
> I don't see this as a particularly urgent issue, especially in the
> context of the overall ASoC code size and memory usage.
> 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.
It's no urgent issue, yeah. It's just what I feel uneasy to see such
a thing...
thanks,
Takashi
---
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 7e65b01..f3a5272 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -254,8 +254,12 @@ enum snd_soc_control_type {
enum snd_soc_compress_type {
SND_SOC_FLAT_COMPRESSION = 1,
- SND_SOC_LZO_COMPRESSION,
- SND_SOC_RBTREE_COMPRESSION
+#ifdef CONFIG_SND_SOC_LZO_CACHE
+ SND_SOC_LZO_COMPRESSION = 2,
+#endif
+#ifdef CONFIG_SND_SOC_RBTREE_CACHE
+ SND_SOC_RBTREE_COMPRESSION = 3,
+#endif
};
int snd_soc_register_platform(struct device *dev,
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 21a5465..1082c57 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -4,8 +4,6 @@
menuconfig SND_SOC
tristate "ALSA for SoC audio support"
- select LZO_COMPRESS
- select LZO_DECOMPRESS
select SND_PCM
select AC97_BUS if SND_SOC_AC97_BUS
select SND_JACK if INPUT=y || INPUT=SND
@@ -25,6 +23,14 @@ if SND_SOC
config SND_SOC_AC97_BUS
bool
+config SND_SOC_LZO_CACHE
+ bool
+ select LZO_COMPRESS
+ select LZO_DECOMPRESS
+
+config SND_SOC_RBTREE_CACHE
+ bool
+
# All the supported SoCs
source "sound/soc/atmel/Kconfig"
source "sound/soc/au1x/Kconfig"
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 0f33db2..ef9fbd3 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -305,6 +305,7 @@ config SND_SOC_WM8993
config SND_SOC_WM8994
tristate
+ select SND_SOC_RBTREE_CACHE
config SND_SOC_WM9081
tristate
diff --git a/sound/soc/soc-cache.c b/sound/soc/soc-cache.c
index 0e17b40..3c471b7 100644
--- a/sound/soc/soc-cache.c
+++ b/sound/soc/soc-cache.c
@@ -761,6 +761,11 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
}
EXPORT_SYMBOL_GPL(snd_soc_codec_set_cache_io);
+/*
+ * RB-tree cache compression
+ */
+#ifdef CONFIG_SND_SOC_LZO_CACHE
+
struct snd_soc_rbtree_node {
struct rb_node node;
unsigned int reg;
@@ -988,6 +993,13 @@ static int snd_soc_rbtree_cache_init(struct snd_soc_codec *codec)
return 0;
}
+#endif /* RBTREE cache compression */
+
+/*
+ * LZO cache compression
+ */
+#ifdef CONFIG_SND_SOC_LZO_CACHE
+
struct snd_soc_lzo_ctx {
void *wmem;
void *dst;
@@ -1400,6 +1412,8 @@ err_tofree:
return ret;
}
+#endif /* CONFIG_SND_SOC_LZO_CACHE */
+
static int snd_soc_flat_cache_sync(struct snd_soc_codec *codec)
{
int i;
@@ -1540,6 +1554,7 @@ static const struct snd_soc_cache_ops cache_types[] = {
.write = snd_soc_flat_cache_write,
.sync = snd_soc_flat_cache_sync
},
+#ifdef CONFIG_SND_SOC_LZO_CACHE
{
.id = SND_SOC_LZO_COMPRESSION,
.name = "LZO",
@@ -1549,6 +1564,8 @@ static const struct snd_soc_cache_ops cache_types[] = {
.write = snd_soc_lzo_cache_write,
.sync = snd_soc_lzo_cache_sync
},
+#endif
+#ifdef CONFIG_SND_SOC_RBTREE_CACHE
{
.id = SND_SOC_RBTREE_COMPRESSION,
.name = "rbtree",
@@ -1558,6 +1575,7 @@ static const struct snd_soc_cache_ops cache_types[] = {
.write = snd_soc_rbtree_cache_write,
.sync = snd_soc_rbtree_cache_sync
}
+#endif
};
int snd_soc_cache_init(struct snd_soc_codec *codec)
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
