On 2012-03-21 13:40, Mark Brown wrote:
On Wed, Mar 21, 2012 at 01:07:47PM +0100, Kristoffer KARLSSON wrote:
*NEVER* top post...
I believe that the assumption that all the registers are contiguous would pose a troublesome limitation in this case.
...it leads to people reading contextless things like this and having no idea what you're talking about.
You should also fix your mailer to word wrap within 80 columns - note how the quote above wraps.
Adjusted mailer. Thank you for the notice.
suggested would work just fine, we also have other controls with signed 32-bit values that span four 8-bit registers that mapped in the following manner:
bits 31-24 23-16 15-8 7-0 reg 0x59 0x5A 0x59 0x5A
This means that to write one complete 32-bit value we would actually need to write to the same 8-bit registers twice.
This makes no sense - how do the vales "span four 8-bit registers" while using only two register addresses?
Every write to register 0x5A copies the written data to the correct active internal chunk of bits of the composite (which after chip boot up is the upper bit parts of the complete 32-bit parameter) then it automatically (as part of the write operation) toggles an internal shift pointer inside the chip that makes the subsequent write to registers 0x59 and 0x5A point to the lower most bits of the full 32-bit parameter. Writing to register 0x5A the second time hence copies the written data the lower bit parts and then toggles the internal shift back to point to the upper bits again allowing for another pass of writing of the complete 32-bit value parameter.
In our hardware there is no way to determine the current toggle position of the above described internal shift pointer (or even to reset it) so it becomes very important to make sure to write exactly twice to both 0x59 and 0x5A and in that very order to make sure that the internal shift pointer always afterwards is pointing to the upper most bits of the 32-bit value allowing for configuring of a new 32-bit value.
Also I believe that the definition S1R,S2R,S4R together with S8R might in fact already be a fairly complete set with no need to add some new crop of macros for this in future. Why? Well since the value handled by ASoC-framework when setting/reading a integer control is of type long (snd_ctl_elem_value) in combination with the fact that 1 byte being the smallest register size in framework means that in a 64-bit world this would translate to a maximum of eight registers mapping the composite value of. that long value.
What happens when someone wants unsigned controls, or stereo controls, or 24 bit registers or anything else? There's way more things can vary than just the word size.
This is true. In light of this a single parametrized macro like you suggested would be preferable. This will allow for more variants like you said. I agree.
Anyhow in light of the situation above do you think we could stick to the submitted approach or do you have some other suggestion on how to define the signed 32-bit value control with registers mapped in the above described fashion?
No, sorry. This stuff just all seems really painful to use, I'd at least hope for more parameterisation.
I will make a new patch that adds only one macro with a parameter exposing a register base (ie. the starting register) in combination with a parameter for register count like you suggested. While this approach will not support the very type of register mapping discussed above it will work just fine for the several other multi register controls we also have in our hardware that really are contiguous and also supports reading back the composite signed value. For these controls this parameterized single macro would work just fine.
For the three special composite controls we have in our hardware that requires writing to the same register several times during configuration of one single parameter I propose a separate solution due to the fact that the hardware does not even support reading back those values since a read operation does not trigger the internal shift toggle like the write operations do, which means that only half the complete composite value can ever be read back. To handle these three controls I propose a separate new macro that allows for caching of the lastly written composite long value stored in that control's private value member allowing for any client reading back the lastly configured parameter while still the 8-bit chunks of the composite value can be written to the hardware in the specific sequence required for that parameter.
I will submit this additional second patch also in full so that you can have a look at it to see if you agree that it would be generic enough for addition to the asoc-core framework or if we need to keep these additional cached controls only as a specific implementation in our codec driver due to the characteristics of our hardware design which does not allow for read back of the complete composite value in all cases.
But for the first control type (composite control composed of a parameterized number of contiguous 8-bit registers) I think we agree on the implementation of a single macro for that now. I believe that it could be called SOC_SINGLE_XR8_SX then? Do you agree?