Re: [PATCHv2 2/3] misc: add support for bmp18x chips to the bmp085driver

[Mark Brown]

On Tue, Mar 06, 2012 at 11:17:52PM +0100, Eric Andersson wrote:
> On 21:27 Tue 06 Mar     , Arnd Bergmann wrote:

> > I don't know much about regmap, but your description sounds like you
> > should be using it here, to simplify the differences between i2c and
> > spi in your driver instead of implementing your own abstraction
> > layer.

regmap provides I2C and SPI register I/O for most devices using those
buses (and will do more buses in future I expect) and also provides
diagnostic (things like register I/O logging and register map dumps) and
optional cache infrastructure on top of that very cheaply (so you can
easily do things like avoid the read part of read/modify/write cycles or
restore the register state on resume).  The win is partly getting the
infrastructure and partly the fact that all the code for formatting the
data for the bus is factored out from the individual driver, usually
there's a substantial code saving.

> > IMHO this would be better expressed if you make CONFIG_BMP085 a hidden
> > option that is selected when either BMP085_I2C or BMP085_SPI are enabled.
> > Otherwise you make it possible to build just the base driver but neither
> > of the front-ends, which is a bit pointless.

> This setup is inspired from other drivers such as ad525x_dpot.c and
> ad714x.c, which also have their own bus abstraction. If regmap is a more
> generic way of solving this I am all for it!

The idiom Arnd describes is the more normal one for the reasons he
mentions - see for example what most MFDs that have this problem do.

You'll need the same setup even with regmap since you'll need to
register I2C and SPI drivers.  What you save is all the register I/O
code - often the I2C and SPI specific code gets reduced to just calling
regmap_init_X().  The regmap subsystem only appeared in 3.1 so lots of
drivers predate it and could benefit from using it, we're starting to
see conversions happening in some subsystems.

Attachment: signature.asc
Description: Digital signature