On 15/04/2014 14:25, Lars-Peter Clausen wrote: > On 04/15/2014 01:54 PM, Boris BREZILLON wrote: >> >> >> On 15/04/2014 12:09, Mark Brown wrote: >>> On Tue, Apr 15, 2014 at 09:36:13AM +0200, Boris BREZILLON wrote: >>>> On 14/04/2014 23:04, Mark Brown wrote: >> >>>>> The transfer type gets set once per device at init time so why not >>>>> just parameterise based on val_bytes? >> >>>> Actually, you may want to transfer 1 byte registers using the block >>>> method (if your device only support block transfers). This depends on >>>> the device being accessed and what it supports, but I'm not sure we >>>> can >>>> assume 1 byte registers will always be transferred using SMBUS byte >>>> transfers. >> >>> OK, so if this a realistic issue then it seems like it's better to >>> implement three different buses - there is not really any common code >>> between the various paths. >> >> Okay, I'll create 4 different busses (one for each access type). >> BTW, should I keep these implementations in the same source file >> (regmap-smbus.c) ? >> And, should I keep one method to register an smbus regmap or should I >> provide one method per access type and get rid of the >> regmap_smbus_transfer_type enum ? > > I don't think we should leave the decision which bus to use to the > driver. Neither should the driver have to choose whether to use smbus > or native I2C. We want to use native I2C when available, because it is > more efficient than going through the smbus emulation layer. On the > other hand we want to automatically switch to smbus when native I2C is > not available and the device can work fine with smbus. Also I'm afraid > that we'll otherwise soon see code popping up like: > > if (of_property_read_bool(np, "use-smbus-regmap")) > regmap = regmap_init_smbus(...) > else > regmap = regmap_init_i2c(...) > > My suggestion is that in regmap_init_i2c() you check the capabilities > of the I2C adapter. If it supports native I2C you setup the regmap > with the regmap_i2c struct just as it does right now. If the adapter > does not support native I2C, check if the device can be supported by > smbus (reg_bytes == 8 && val_bytes % 8 == 0). For each type of smbus > operations have one regmap_bus struct, and if you can fallback to > smbus choose the bus depending on the config's val_bytes. >
What if the device only support SMBus block transfers, but the adapter support both regular I2C and SMBus block transfers (don't know if such a device exist :-)) ? My point is that I'm not sure the core remap-i2c code can decide whether to use SMBus or I2C transfer only from val_bytes value, but I might be wrong. Best Regards, Boris > - Lars -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
