Re: ASoC cache code (looking toward 2.6.38 merge window)

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

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.

> > 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.

> 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.

>  config SND_SOC_WM8994
>         tristate
> +       select SND_SOC_RBTREE_CACHE

Except the machine driver can override this...