Messages in this thread | | | Date | Thu, 10 Oct 2013 15:08:43 +0100 | From | Mark Brown <> | Subject | Re: [PATCH V1] new API regmap_multi_write() |
| |
On Thu, Oct 10, 2013 at 12:45:03PM +0000, Opensource [Anthony Olech] wrote:
> > Why all on the same page? That doesn't seem helpful for things trying to > > build on top of this. We currently manage to split even block writes up over > > page boundaries.
> As far as I could see, the splitting of a block write that spans a page boundary > requires a read-modify-write of a "page" register. That therefore breaks the > primary raison d'etre for the new API, namely that it is atomic on the I2C bus > with respect to multiple I2C bus masters.
I would expect this to be handled by inserting a page update into the sequence (it could presumably go into the block write unless the hardware were being a bit perverse) or by just splitting the transfers at each page change.
> Cutting the transfer at a page boundary and inserting a write to a "page" > register would work very well for most of our PM MFDs because there is > no requirement to do a read modify write. Indeed I had thought that it should > be the responsibility of the device driver to insert any necessary "page" register > writes into a transfer that spans page boundaries. The driver knows where the > boundaries are so it should be easy.
This only works if it is chip specific code that is doing the update. If there is generic code doing an update (either core Linux framework code or something for an IP that appears on multiple chips) then it's not going to know that without jumping through hoops which are going to apply to all users that trigger this so may as well just be handled in the core.
Of course a driver is free to not issue updates which cross page boundaries (or to group the writes so that they get split up into the minimum set of page boundary changes) but it doesn't seem helpful to require that the caller understands any paging the device has.
> > This really doesn't feel like an idiomatic abstraction - it's a bit cumbersome to > > have the pair of arrays and try to line them up especially in native register > > format, normally we do this with an array reg_default. This would also mean > > that generic code like patches and cache syncing could pick up on the same > > functionality.
> You seem to be suggesting that the API could be used by drivers of devices that > do not support in hardware the MULTIWRITE capability. If that is the case then
Yes, and for example all SPI devices have essentially this functionality as standard since it's possible to issue an uninterrupted batch of transfers to a device with no special hardware support.
> driver need an config "multi_write_supported" field for initialization.
It's I2C specific too.
> What should the next step in progressing this be?
Like I say I'd suggest getting an API that allows drivers to send a batch of writes to the framework done first (which should be fairly simple - a first pass should probably be something like
int regmap_multi_reg_write(struct regmap *map, struct reg_default *regs, int num_regs) { for (i = 0; i < num_regs; i++) regmap_reg_write(map, regs[i].reg, regs[i].val); }
with error handling and stuff) and then loop round on how to implement the actual functionality to get I2C and SPI to do the right thing on the wire if the device supports it. It's slightly different for each, obviously you only care about I2C which is fine - I'm not saying you'd need to actually do SPI yourself.
For I2C type stuff a flag to say if the device can do this and then something to render the data appropriately and send it ought to do the trick, it should be fairly straightforward. I'm more concerned about the external API than the implementation, it's much easier to refactor the implementation if there's a problem than it is to change the external API. [unhandled content-type:application/pgp-signature] | |