On Mon, Jan 19, 2015 at 10:31:22AM +0100, Paul Osmialowski wrote: > On Fri, 16 Jan 2015, Mark Brown wrote: > >What I'm saying is that I want to understand this change from a point of > >view that isn't tied to I2C - at the regmap level what is this doing, > From the regmap point of view, it allows its functions to have a chance to > prepare transfer medium for (synchronous) transfer (no matter what bus > handles it) before it actually start to happen (then unprepare it when it's > done) and crucially before any lock is obtained in functions like > regmap_write(), regmap_read() or regmap_update_bits. OK, so that's what should go in the changelog (along with an explanation of why this preparation is required at all) - but I still don't see the async bit of this I'm afraid. > Maybe adding a pair of callbacks (map->reg_write_sync_prepared(), > map->reg_read_sync_prepared()) would make situation clearer. No, I don't think so - it'd just complicate the callers. > >I2C is a bus that has some properties which you're saying needs some > >changes, what are those properties and those changes? > I'm not saying I2C as a bus requires changes. What I'm saying is that I2C > API can be extended to allow more detailed control on what happens with the > transfer. My point here is that your explanation is in terms of I2C specifics and not really at a generic regmap level. > >Can you be more specific please? If something needs preparing it seems > >like it'd need preparing over an async transaction just as much as over > >a synchronous one. > Even with those preparation and unpreparation stages, this transfer is still > synchronous. For example, it starts when regmap_read() starts and ends when > regmap_read() ends. Nothing is queued or deferred. Namely, when > max_gen_clk_unprepare() function calls regmap_update_bits() it expects that > when regmap_update_bits() returned, no outstanding transfer are happening > nor waiting to proceed. Everything must be completed before returning to > max_gen_clk_unprepare(). That doesn't address my question - all you're saying is that in a synchronous call path things are synchronous which is fine but obviously regmap supports async I/O too. > >Not in this pattern where the caller needs to check too. > I don't persist on that. Apparently, you're the author of this file, though > regmap_init() function was later expanded by other guys. They never assigned > bus callback function pointers directly to map operation callbacks. It is > possible to replace 'map->reg_prepare_sync_io = regmap_bus_prepare_sync_io' > with 'map->reg_prepare_sync_io = map->bus->prepare_sync_io' - this will > compile and this will work properly. But IMHO it wouldn't match with what > the others did. If you look at the other callbacks they're doing other things beyond simply forwarding the functions on. That's the problem here, the functions just add a layer of indirection and nothing else.
Attachment:
signature.asc
Description: Digital signature
