Re: [PATCH v6] ASoC: add RT286 CODEC driver

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3787 bytes --]

On Thu, May 08, 2014 at 09:05:01AM +0200, Lars-Peter Clausen wrote:
> On 05/07/2014 09:49 PM, Mark Brown wrote:

> >My first thought would be to extend those so we have callbacks when
> >required, to be honest I'd forgotten they weren't there already.

> But that's just the normal ..._EXT() controls with custom put/get handlers.

Hrm, yes.  That's the issue - I have never really differentiated between
virtual and external enums here, the external controls are (as far as
DAPM is concerned) also virtual.  To be honest I'd forgotten that there
were external controls in DAPM, there's only one user.

> >Right, to me that's not actually a register map at all since it's
> >missing so many of the assumptions that back up register maps.

> Well there is a register map, there is just no linear address space. It's
> more hierarchical, but you can still map that hierarchical addressing scheme
> to a linear one. I think what best describes HDA is object oriented RPC. You

The non-linear addressing isn't the worst bit, I'd say the worst bit is
that the read and write formats for the data aren't consistent with each
other.  That's really the fundamental assumption here.

> have your objects, called nodes, each node has a unique identifier, called
> the node ID (NID). Then you have functions, called verbs, each verb has a
> specific ID. A node can support a certain set of verbs. And each verb has a
> verb specific payload, which is the functions signature. Most of the verbs
> are some kind of read/write register functions.

Yes, it makes sense as RPC (and hence as external controls as I have
been suggesting).

> So the address of the register in which the amplifier gain is stored is the
> NID + It's a amplifier you want to access + whether it's a input or output
> amplifier + whether it's left or right amplifier + amplifier index. You can
> map that onto a linear address space and in the regmap read/write callback
> do the mapping to the appropriate verb payload layout.

It is of course always going to be possible to map any set of settings
onto a register map, the question is if it accomplishes anything to do
so.  Once you're talking about having a mapping function like that
which understands the contents of all the registers it sounds like the
contortions to fit into a register map are more effort than is being
saved and doing nothing for comprehensibility.

> >>the same data). In a sense this is similar to devices which have registers
> >>with different sizes, we also support these with custom regmap callbacks.

> >But those are only breaking that one assumption so they fit into the
> >idea of a register map much more readily - it's really only the physical
> >I/O code that actually notices anything, all the cache and other
> >operations can carry on uninterrupted.

> This is one of the reasons why I suggest using regmap. You probably still
> want caching, you probably still be able to sync the register cache, etc. If
> you don't use regmap you'd have to implement this on your own.

So how does HDA handle this?  We can obviously keep recording settings
in the same way as we do for virtual enums, writing them out shouldn't
be so hard.   The cache code isn't going to buy us much if we have to
write things out control by control anyway, it essentially just boils
down to a fancy list walk.

If you really want to reuse regmap having a write only regmap internal
to the driver (not presented to ASoC) which just remembers the last
value written to every NID/VID combination might work and at least
avoids the ugly bits with trying to convince ASoC there are registers
since you don't need to worry about reading the data back and can just
pretend that read values match written values since we never look at
them except to write them back out.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]