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

[Mark Brown <broonie@kernel.org>]

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

On Tue, Mar 18, 2014 at 12:41:41PM +0000, Bard Liao wrote:

Please fix your mailer to word wrap within paragraphs, this will make
your mail more legible.

> A verb command contains three parts, NID, VID, and PID.
> For example, if I want to unmute headphone, I should set:
> VID = 3'h (Set Amplifier Gain)
> NID = 22'h (headphone)
> PID = b000'h (Set left, right output unmute)

So VID+NID is an address and PID is a value?

> > - if you need more let's extend them.  Storing data in virtual registers just
> > makes things confusing and fragile.  Some older CODEC drivers did it and
> > they're harder to work with now than they should be.

> Use SOC_DAPM_SINGLE_VIRT?

Yes, that'd be a much better fit.

> If I combine NID and VID to be used as a register address, and let PID as register value,
> it will be very difficult to define(or use) a _update_btis function.
> Because I need to put all NID, VID and PID in the register address area when I issue a I2C read command.

I don't see how this follows at all - all update_bits() is doing is a
read/modify/write so if you can implement write and read then it comes
along for free.  It sounds like you need to do a cache read to get the
current PID to do a read but it's not at all obvious what that has to do
with update_bits() or how putting the value into the address avoids
having to know the current value to do a read? 

I think what you're saying above is that your read needs to have a cache
read in it to function if you use normal register values?

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

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