On Tue, Dec 10, 2013 at 04:13:15PM -0800, Stephen Boyd wrote: > On 12/10/13 15:50, Mark Brown wrote: > > On Tue, Dec 10, 2013 at 03:35:18PM -0800, Stephen Boyd wrote: > >> + reg += sizeof(u16); > > I'd expect this to generate out of bounds accesses and bad interactions > > on the bus if we go through the loop more than once since it appears to > > incrementing the address of reg for every register. I'm also having a > ssbi_read() just reads the same register x number of times and doesn't > do any sort of incrementing of address. My understanding was that > regmap_bulk_read() will read incrementing addresses and then call down > into this code with the sequential addresses formatted into the reg > buffer. That was the flaw. Instead we need to take reg and then That's possibly not the ideal decision for the underlying API, if nothing else it's confusing naming given what a read function would normally do. > increment reg by 1 every time through this loop. Or should we just have > use_single_rw == true? No, it doesn't - it increments the address of reg by the size of a register value each time. Using use_single_rw might make sense, or if you can't do bulk I/O at all then hooking in via reg_read() and reg_write() in the config rather than trying to parse out the buffers might be even better (you can still make helpers to set that up).
Attachment:
signature.asc
Description: Digital signature
