Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1211107 > unrolled thread

Re: [PATCH v5 6/8] iio: gyro: bmg160: optimize i2c transfers in trigger handler

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v5 6/8] iio: gyro: bmg160: optimize i2c transfers in  trigger handler Wolfram Sang <wsa@the-dreams.de> - 2015-08-21 12:30 +0200
    Re: [PATCH v5 6/8] iio: gyro: bmg160: optimize i2c transfers in  trigger handler "Pandruvada, Srinivas" <srinivas.pandruvada@intel.com> - 2015-08-21 18:00 +0200
      Re: [PATCH v5 6/8] iio: gyro: bmg160: optimize i2c transfers in  trigger handler Wolfram Sang <wsa@the-dreams.de> - 2015-08-22 06:10 +0200
        Re: [PATCH v5 6/8] iio: gyro: bmg160: optimize i2c transfers in  trigger handler Jonathan Cameron <jic23@kernel.org> - 2015-08-22 19:30 +0200
          Re: [PATCH v5 6/8] iio: gyro: bmg160: optimize i2c transfers in  trigger handler Wolfram Sang <wsa@the-dreams.de> - 2015-08-24 13:50 +0200

#1211107 — Re: [PATCH v5 6/8] iio: gyro: bmg160: optimize i2c transfers in trigger handler

[Multipart message — attachments visible in raw view] — view raw

On Mon, Aug 17, 2015 at 11:09:43AM +0200, Markus Pargmann wrote:
> On Sun, Aug 16, 2015 at 10:24:47AM +0100, Jonathan Cameron wrote:
> > On 12/08/15 15:31, Irina Tirdea wrote:
> > > Some i2c busses (e.g.: Synopsys DesignWare I2C adapter) need to
> > > enable/disable the bus at each i2c transfer and must wait for
> > > the enable/disable to happen before sending the data.
> > > 
> > > When reading data in the trigger handler, the bmg160 driver does
> > > one i2c transfer for each axis. This has an impact on the frequency
> > > of the gyroscope at high sample rates due to additional delays
> > > introduced by the i2c bus at each transfer.
> > > 
> > > Reading all axis values in one i2c transfer reduces the delays
> > > introduced by the i2c bus. Uses i2c_smbus_read_i2c_block_data_or_emulated
> > > that will fallback to reading each axis as a separate word in case i2c
> > > block read is not supported.
> > > 
> > > Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> > > Acked-by: Jonathan Cameron <jic23@kernel.org>
> > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Note, that in the meantime the bmg160 driver just went all regmap
> > on us (as part of adding SPI support - though that step hasn't
> > happened yet).  Hence we'll need a means of telling regmap about this
> > possibility.
> 
> Perhaps this is covered by a regmap_bulk_read()?
> 
> The series[1] I am working on implements a i2c smbus block data regmap
> bus driver. Regmap should then automatically do a block read in
> regmap_bulk_read.

Hmm, so doesn't your series make Irina's series obsolete? It addresses
the same problem only at a different layer (i2c core vs. regmap), or? It
would mean that i2c client drivers which want to support byte, word, or
block transfers should be converted to regmap. I assume most of the
potential candidates are register based devices anyhow. Then, regmap
would be the proper abstraction layer. Have I overlooked something?

Thanks,

   Wolfram

[toc] | [next] | [standalone]

#1211225

T24gRnJpLCAyMDE1LTA4LTIxIGF0IDEyOjIxICswMjAwLCBXb2xmcmFtIFNhbmcgd3JvdGU6DQo+
IE9uIE1vbiwgQXVnIDE3LCAyMDE1IGF0IDExOjA5OjQzQU0gKzAyMDAsIE1hcmt1cyBQYXJnbWFu
biB3cm90ZToNCj4gPiBPbiBTdW4sIEF1ZyAxNiwgMjAxNSBhdCAxMDoyNDo0N0FNICswMTAwLCBK
b25hdGhhbiBDYW1lcm9uIHdyb3RlOg0KPiA+ID4gT24gMTIvMDgvMTUgMTU6MzEsIElyaW5hIFRp
cmRlYSB3cm90ZToNCj4gPiA+ID4gU29tZSBpMmMgYnVzc2VzIChlLmcuOiBTeW5vcHN5cyBEZXNp
Z25XYXJlIEkyQyBhZGFwdGVyKSBuZWVkIHRvDQo+ID4gPiA+IGVuYWJsZS9kaXNhYmxlIHRoZSBi
dXMgYXQgZWFjaCBpMmMgdHJhbnNmZXIgYW5kIG11c3Qgd2FpdCBmb3INCj4gPiA+ID4gdGhlIGVu
YWJsZS9kaXNhYmxlIHRvIGhhcHBlbiBiZWZvcmUgc2VuZGluZyB0aGUgZGF0YS4NCj4gPiA+ID4g
DQo+ID4gPiA+IFdoZW4gcmVhZGluZyBkYXRhIGluIHRoZSB0cmlnZ2VyIGhhbmRsZXIsIHRoZSBi
bWcxNjAgZHJpdmVyIA0KPiA+ID4gPiBkb2VzDQo+ID4gPiA+IG9uZSBpMmMgdHJhbnNmZXIgZm9y
IGVhY2ggYXhpcy4gVGhpcyBoYXMgYW4gaW1wYWN0IG9uIHRoZSANCj4gPiA+ID4gZnJlcXVlbmN5
DQo+ID4gPiA+IG9mIHRoZSBneXJvc2NvcGUgYXQgaGlnaCBzYW1wbGUgcmF0ZXMgZHVlIHRvIGFk
ZGl0aW9uYWwgZGVsYXlzDQo+ID4gPiA+IGludHJvZHVjZWQgYnkgdGhlIGkyYyBidXMgYXQgZWFj
aCB0cmFuc2Zlci4NCj4gPiA+ID4gDQo+ID4gPiA+IFJlYWRpbmcgYWxsIGF4aXMgdmFsdWVzIGlu
IG9uZSBpMmMgdHJhbnNmZXIgcmVkdWNlcyB0aGUgZGVsYXlzDQo+ID4gPiA+IGludHJvZHVjZWQg
YnkgdGhlIGkyYyBidXMuIFVzZXMgDQo+ID4gPiA+IGkyY19zbWJ1c19yZWFkX2kyY19ibG9ja19k
YXRhX29yX2VtdWxhdGVkDQo+ID4gPiA+IHRoYXQgd2lsbCBmYWxsYmFjayB0byByZWFkaW5nIGVh
Y2ggYXhpcyBhcyBhIHNlcGFyYXRlIHdvcmQgaW4gDQo+ID4gPiA+IGNhc2UgaTJjDQo+ID4gPiA+
IGJsb2NrIHJlYWQgaXMgbm90IHN1cHBvcnRlZC4NCj4gPiA+ID4gDQo+ID4gPiA+IFNpZ25lZC1v
ZmYtYnk6IElyaW5hIFRpcmRlYSA8aXJpbmEudGlyZGVhQGludGVsLmNvbT4NCj4gPiA+ID4gQWNr
ZWQtYnk6IEpvbmF0aGFuIENhbWVyb24gPGppYzIzQGtlcm5lbC5vcmc+DQo+ID4gPiA+IEFja2Vk
LWJ5OiBTcmluaXZhcyBQYW5kcnV2YWRhIDwNCj4gPiA+ID4gc3Jpbml2YXMucGFuZHJ1dmFkYUBs
aW51eC5pbnRlbC5jb20+DQo+ID4gPiBOb3RlLCB0aGF0IGluIHRoZSBtZWFudGltZSB0aGUgYm1n
MTYwIGRyaXZlciBqdXN0IHdlbnQgYWxsIHJlZ21hcA0KPiA+ID4gb24gdXMgKGFzIHBhcnQgb2Yg
YWRkaW5nIFNQSSBzdXBwb3J0IC0gdGhvdWdoIHRoYXQgc3RlcCBoYXNuJ3QNCj4gPiA+IGhhcHBl
bmVkIHlldCkuICBIZW5jZSB3ZSdsbCBuZWVkIGEgbWVhbnMgb2YgdGVsbGluZyByZWdtYXAgYWJv
dXQgDQo+ID4gPiB0aGlzDQo+ID4gPiBwb3NzaWJpbGl0eS4NCj4gPiANCj4gPiBQZXJoYXBzIHRo
aXMgaXMgY292ZXJlZCBieSBhIHJlZ21hcF9idWxrX3JlYWQoKT8NCj4gPiANCj4gPiBUaGUgc2Vy
aWVzWzFdIEkgYW0gd29ya2luZyBvbiBpbXBsZW1lbnRzIGEgaTJjIHNtYnVzIGJsb2NrIGRhdGEg
DQo+ID4gcmVnbWFwDQo+ID4gYnVzIGRyaXZlci4gUmVnbWFwIHNob3VsZCB0aGVuIGF1dG9tYXRp
Y2FsbHkgZG8gYSBibG9jayByZWFkIGluDQo+ID4gcmVnbWFwX2J1bGtfcmVhZC4NCj4gDQo+IEht
bSwgc28gZG9lc24ndCB5b3VyIHNlcmllcyBtYWtlIElyaW5hJ3Mgc2VyaWVzIG9ic29sZXRlPyBJ
dCANCj4gYWRkcmVzc2VzDQo+IHRoZSBzYW1lIHByb2JsZW0gb25seSBhdCBhIGRpZmZlcmVudCBs
YXllciAoaTJjIGNvcmUgdnMuIHJlZ21hcCksIG9yPyANCj4gSXQNCj4gd291bGQgbWVhbiB0aGF0
IGkyYyBjbGllbnQgZHJpdmVycyB3aGljaCB3YW50IHRvIHN1cHBvcnQgYnl0ZSwgd29yZCwgDQo+
IG9yDQo+IGJsb2NrIHRyYW5zZmVycyBzaG91bGQgYmUgY29udmVydGVkIHRvIHJlZ21hcC4gSSBh
c3N1bWUgbW9zdCBvZiB0aGUNCj4gcG90ZW50aWFsIGNhbmRpZGF0ZXMgYXJlIHJlZ2lzdGVyIGJh
c2VkIGRldmljZXMgYW55aG93LiBUaGVuLCByZWdtYXANCj4gd291bGQgYmUgdGhlIHByb3BlciBh
YnN0cmFjdGlvbiBsYXllci4gSGF2ZSBJIG92ZXJsb29rZWQgc29tZXRoaW5nPw0KPiANClRoaXMg
aXMgdGhlIG9ubHkgZHJpdmVyIGNvbnZlcnRlZCB0byB1c2UgcmVnbWFwLCBiZWNhdXNlIG9mIFNQ
SSBtb2RlDQpzdXBwb3J0LiBUaGUgb3RoZXIgZHJpdmVycyB3aWxsIHN0aWxsIHVzZSBJcmluYSdz
IGNoYW5nZXMuDQoNClRoYW5rcw0KU3Jpbml2YXMNCg0KDQo+IFRoYW5rcywNCj4gDQo+ICAgIFdv
bGZyYW0NCj4g
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211397

[Multipart message — attachments visible in raw view] — view raw

> > > The series[1] I am working on implements a i2c smbus block data 
> > > regmap
> > > bus driver. Regmap should then automatically do a block read in
> > > regmap_bulk_read.
> > 
> > Hmm, so doesn't your series make Irina's series obsolete? It 
> > addresses
> > the same problem only at a different layer (i2c core vs. regmap), or? 
> > It
> > would mean that i2c client drivers which want to support byte, word, 
> > or
> > block transfers should be converted to regmap. I assume most of the
> > potential candidates are register based devices anyhow. Then, regmap
> > would be the proper abstraction layer. Have I overlooked something?
> > 
> This is the only driver converted to use regmap, because of SPI mode
> support. The other drivers will still use Irina's changes.

The question is if they should. Or rather be converted to regmap. It is
an open question and I am seeking for further input.

[toc] | [prev] | [next] | [standalone]

#1211485

On 22/08/15 05:02, Wolfram Sang wrote:
> 
>>>> The series[1] I am working on implements a i2c smbus block data 
>>>> regmap
>>>> bus driver. Regmap should then automatically do a block read in
>>>> regmap_bulk_read.
>>>
>>> Hmm, so doesn't your series make Irina's series obsolete? It 
>>> addresses
>>> the same problem only at a different layer (i2c core vs. regmap), or? 
>>> It
>>> would mean that i2c client drivers which want to support byte, word, 
>>> or
>>> block transfers should be converted to regmap. I assume most of the
>>> potential candidates are register based devices anyhow. Then, regmap
>>> would be the proper abstraction layer. Have I overlooked something?
>>>
>> This is the only driver converted to use regmap, because of SPI mode
>> support. The other drivers will still use Irina's changes.
> 
> The question is if they should. Or rather be converted to regmap. It is
> an open question and I am seeking for further input.
> 
There are a fairly large number of legacy drivers where this might apply.
Do we have any real idea of how many?  I'm assuming Irina only made use
of it in ones that were of personal interest.  A quick grep suggests
10's of drivers use the block call. I'm guessing quite a few would use
it if needed for a particular board.

Do we want to insist on a much larger change (conversion to regmap)
when if this in place, a simple single functional call change will do the
job?

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

[toc] | [prev] | [next] | [standalone]

#1212095

[Multipart message — attachments visible in raw view] — view raw

> Do we want to insist on a much larger change (conversion to regmap)
> when if this in place, a simple single functional call change will do the
> job?

I'd assume that regmap conversion will happen later quite likely anyhow.
Most of those devices will have I2C/SPI dual interfaces; or people will
find out that caching registers can reduce the bus load, etc...

That being said, I'll keep Irina's patches for the next merge window.
But we should keep an eye how/if the new function is used. Kernel
growth is an issue at times...

Thanks,

   Wolfram

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web