Messages in this thread | | | Date | Fri, 15 Jul 2011 21:16:50 +0900 | From | Mark Brown <> | Subject | Re: [PATCH 4/4] regulator: Convert tps65023 to use regmap API |
| |
On Fri, Jul 15, 2011 at 12:31:21PM +0200, Jean Delvare wrote:
> The driver was previously using the SMBus API and could thus work with > SMBus (non-I2C) controllers. After your change, an I2C controller is > mandatory. Are you sure this is OK for all users?
I'd be *utterly* astonished if it ever caused any issues, embedded systems like those that have regulators just don't have anything less than full I2C support, it's exceptionally rare to see any direct SMBus support in the hardware at all - the only example I can see in the tree is Blackfin and that is a proper I2C controller too, it just seems that someone overdesigned the hardware a little.
Basically, for something like a regulator it's just not an issue.
> As a general comment, this requirement will considerably limit the > interest of regmap for I2C devices (at least in its current form.) Many > systems out there only have SMBus controllers, and more importantly, > most I2C device drivers are meant to be portable across systems and > thus rely on the SMBus API. The i2c documentation encourages driver > authors to do this.
Right, so for pretty much anything except PCs the main reason for using the SMBus API is that it provides the sort of data mangling to the bus functionality that the regmap API provides. The main thing it's missing for drivers like this one is that it doesn't have a read/modify/write operation, otherwise there would be no current value in switching to the regmap API. As far as I can tell this isn't really a big deal except for those device classes like hwmon which are frequently deployed in PCs, anything else just won't care.
This should be reasonably simple to handle in the regmap API, just teach the I2C module to fall back to using the SMBus operations if the controller is limited to that. This should be done incrementally as it adds complexity, and ideally would be done by someone with some access to hardware that can use the SMBus API (I don't have any myself) so they can test.
Does that sound reasonable to you?
> > static int tps_65023_set_bits(struct tps_pmic *tps, u8 reg, u8 mask) > > { > > - int err, data;
*Please* could people delete unneeded context from mails?
| |