Re: [RFC PATCH v2] regmap: smbus: add support for regmap over smbus

From: Lars-Peter Clausen
Date: Tue Apr 15 2014 - 15:18:30 EST


On 04/15/2014 06:46 PM, Mark Brown wrote:
On Tue, Apr 15, 2014 at 03:34:55PM +0200, Lars-Peter Clausen wrote:
On 04/15/2014 02:56 PM, Mark Brown wrote:

- Providing APIs for registering actual smbus devices as a convenience
for devices with that constraint, regardless of how that is done
behind the scenes.

- Having the I2C implementation automatically use the smbus APIs if
it can and either the controller is smbus only or it makes sense to
do so for optimisation.

I don't think it makes sense to expose smbus explicitly. We can already
describe smbus restricted devices just fine with the current regmap_config
and already support them with the current I2C regmap implementation. Using

Please note what I said about convenience - if lots of devices have
exactly the same set of constraints to set up it's possible sensible to
have a standard way of specifying them. It's going to be a bit easier
to just say it's smbus than to remember exactly what that means.

Maybe, but the only shared constraint is reg_bits = 8 (and if it is pure smbus use_single_rw = true).


native I2C to access these devices will be more efficient than going through
the smbus emulation layer. The smbus emulation layer essentially does the
same as we do in regmap, so using the smbus emulation layer through regmap
means doing the same thing twice.

Right, that's why I said it's probably only worth it if the controller
does smbus natively.

As I see it there are currently 3 cases:

1) Device is strictly smbus only and the controller supports native smbus
=> Use smbus
2) The device is smbus compatible but has extensions (e.g. support for multi
register writes) and the controller supports only smbus.
=> Use smbus
3) For every other case
=> Use native I2C.

It's not clear to me that if the controller supports smbus we shouldn't
use it; presumably it's adding some value to have written the code to
take advantage of it. That would mean another case for device is smbus
only and controller has explicit smbus support.

Potentially yes, but not necessarily in the first version of the driver. It's a bit of an exotic case, there seem to be only two I2C controller drivers in the Linux kernel that have support for both raw I2C and native SMBus and neither of them don't seem to be fairly recent (i2c-powermac.c and i2c-pasemi.c). And on the other hand most devices are SMBus compatible have extensions like being able to write/read multiple register in one transfer if raw I2C access is available, which is something we want to make use of if available.

And the other thing to consider is that a client driver currently has no way to query whether the controller driver supports native SMBus or only emulated SMBus. This is surely something that can be added to the I2C core, but this is necessarily something that we require before any SMBus support is added to regmap.

So in conclusion I think the best way forward is to create a patch that checks if native I2C is available, and if not falls back to either SMBUS_{WRITE,READ}_BYTE or SMBUS_{WRITE,READ}_WORD operations (Depending on whether val_bits is 8 or 16). Such a patch, I think, has the most effect for the least amount of work since it is rather simple and self contained and allows all devices which are SMBus compatible to be used with SMBus-only controllers. Everything else discussed in this thread (optimizations for special cases, convince helpers) can then be implemented on top of that.

- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/