* Question about max frequency in ad2s90 @ 2018-11-06 1:11 Matheus Tavares Bernardino 2018-11-06 12:34 ` Ardelean, Alexandru 0 siblings, 1 reply; 6+ messages in thread From: Matheus Tavares Bernardino @ 2018-11-06 1:11 UTC (permalink / raw) To: linux-iio, graf.yang, jic23, Michael Hennerich; +Cc: kernel-usp, Victor Colombo Hello everyone, While working on the ad2s90 driver, I had a question about the maximum frequency set. Any feedback will be much appreciated. The ad2s90 driver sets its maximum speed to 830000hz (or 0.83Mhz), but the datasheet [1] specifies a maximum of 2Mhz. Above the max_speed_hz setup, there's a commentary saying "need 600ns between CS and the first falling edge of SCLK", which is also said in the datasheet. Is max_speed_hz set to 0.83Mhz, not 2Mhz, due to the required minimum delay of 600ns, somehow? If not: 1) is 0.83Mhz really correct? and 2) is the minimum delay of 600ns being respected? Thanks, Matheus. [1] https://www.analog.com/media/en/technical-documentation/data-sheets/AD2S90.pdf ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Question about max frequency in ad2s90 2018-11-06 1:11 Question about max frequency in ad2s90 Matheus Tavares Bernardino @ 2018-11-06 12:34 ` Ardelean, Alexandru 2018-11-06 15:12 ` Matheus Tavares Bernardino 2018-11-07 18:05 ` Matheus Tavares Bernardino 0 siblings, 2 replies; 6+ messages in thread From: Ardelean, Alexandru @ 2018-11-06 12:34 UTC (permalink / raw) To: jic23, linux-iio, Hennerich, Michael, matheus.bernardino, graf.yang Cc: victorcolombo, kernel-usp T24gTW9uLCAyMDE4LTExLTA1IGF0IDIzOjExIC0wMjAwLCBNYXRoZXVzIFRhdmFyZXMgQmVybmFy ZGlubyB3cm90ZToNCj4gSGVsbG8gZXZlcnlvbmUsDQo+IA0KDQpIZXksDQoNCj4gV2hpbGUgd29y a2luZyBvbiB0aGUgYWQyczkwIGRyaXZlciwgSSBoYWQgYSBxdWVzdGlvbiBhYm91dCB0aGUgbWF4 aW11bQ0KPiBmcmVxdWVuY3kgc2V0LiBBbnkgZmVlZGJhY2sgd2lsbCBiZSBtdWNoIGFwcHJlY2lh dGVkLg0KPiANCg0KSnVzdCBhIG5vdGUvcmVtaW5kZXIgZm9yIHlvdTogdGhhdCBkcml2ZXIgbG9v a3MgcHJldHR5IG9sZCBhbmQgW3NpbmNlIGl0J3MNCnN0YWdpbmcgYXMgd2VsbF0geW91IHNob3Vs ZCBhbHNvIGNvbnNpZGVyIHRoYXQgaXQncyBsaWtlbHkgdGhhdCBzb21lIFtnb29kXQ0KcHJhY3Rp Y2VzIHdlcmUgbm90IHlldCBleGlzdGluZyBhdCB0aGUgdGltZSBbYWJvdXQgaG93IHRvIGRvIHRo aW5nc10uDQoNCj4gVGhlIGFkMnM5MCBkcml2ZXIgc2V0cyBpdHMgbWF4aW11bSBzcGVlZCB0byA4 MzAwMDBoeiAob3IgMC44M01oeiksIGJ1dA0KPiB0aGUgZGF0YXNoZWV0IFsxXSBzcGVjaWZpZXMg YSBtYXhpbXVtIG9mIDJNaHouIEFib3ZlIHRoZSBtYXhfc3BlZWRfaHoNCj4gc2V0dXAsIHRoZXJl J3MgYSBjb21tZW50YXJ5IHNheWluZyAgIm5lZWQgNjAwbnMgYmV0d2VlbiBDUyBhbmQgdGhlDQo+ IGZpcnN0IGZhbGxpbmcgZWRnZSBvZiBTQ0xLIiwgd2hpY2ggaXMgYWxzbyBzYWlkIGluIHRoZSBk YXRhc2hlZXQuDQo+IA0KPiBJcyBtYXhfc3BlZWRfaHogc2V0IHRvIDAuODNNaHosIG5vdCAyTWh6 LCBkdWUgdG8gdGhlIHJlcXVpcmVkIG1pbmltdW0NCj4gZGVsYXkgb2YgNjAwbnMsIHNvbWVob3c/ IElmIG5vdDoNCj4gICAxKSBpcyAwLjgzTWh6IHJlYWxseSBjb3JyZWN0PyBhbmQNCj4gICAyKSBp cyB0aGUgbWluaW11bSBkZWxheSBvZiA2MDBucyBiZWluZyByZXNwZWN0ZWQ/DQo+IA0KDQpUaGUg MC44MyBNaHogbGltaXRhdGlvbiBsb29rcyB2ZXJ5IG11Y2ggbGlrZSBpdCB3YXMgYWRkZWQgdG8g c2F0aXNmeSB0aGUNCjYwMCBucyBsaW1pdGF0aW9uLiBJIHF1aWNrbHkgZGlkIHRoZSBjb21wdXRh dGlvbiBhbmQgaXQgcm91Z2hseSBlbmRzIHVwDQpiZWluZyAwLjgzIE1oeiBmb3IgNjAwIG5zLiAo aS5lLiAoMSAvIDAsMDAwNjAwKSAvIDIgPSBoYWxmIGEgcGVyaW9kICkNCg0KVGhhdCBsaW1pdGF0 aW9uIChmb3IgdGhlIGRlbGF5IENTIGFuZCB0aGUgZmlyc3QgZGF0YSkgaXMgaW1wb3NlZCBieSB0 aGUNCmRhdGFzaGVldCA7IHdoaWxlIHRoZSBjaGlwIGNvdWxkIG9wZXJhdGUgYXQgYSBsb3dlciBk ZWxheSAoYW5kIHN1YnNlcXVlbnRseQ0KaGlnaGVyIGZyZXF1ZW5jeSB1cCB0byAyIE1oeiksIGl0 J3Mgc2FmZXIgdG8ga2VlcCBpdCBzb21laG93IGltcGxlbWVudGVkLg0KDQpJIHRvb2sgYSBsb29r IGluIHRoZSBTUEkgY29kZSBhbmQgaXQgZG9lc24ndCBsb29rIGxpa2UgaXQgaW1wbGVtZW50cyBz b21lDQpzb3J0IG9mIGRlbGF5IGJldHdlZW4gQ1MgYW5kIGZpcnN0IGRhdGEgYmVpbmcgcmVhZC4g SSBtYXkgaGF2ZSBtaXNzZWQgc3R1ZmYNCnRob3VnaC4NCg0KSW4gYW55IGNhc2UsIEkgZG8gc3Vn Z2VzdCByZW1vdmluZyBhbGwgdGhvc2UgNCBsaW5lcyBmb3IgdGhlIFNQSSBzZXR1cA0KY29kZS4N CmkuZS4NCg0KICAgICAgICAvKiBuZWVkIDYwMG5zIGJldHdlZW4gQ1MgYW5kIHRoZSBmaXJzdCBm YWxsaW5nIGVkZ2Ugb2YgU0NMSyAqLw0KICAgICAgICBzcGktPm1heF9zcGVlZF9oeiA9IDgzMDAw MDsNCiAgICAgICAgc3BpLT5tb2RlID0gU1BJX01PREVfMzsNCiAgICAgICAgc3BpX3NldHVwKHNw aSk7DQoNClRoaXMgc2hvdWxkIGJlIGhhbmRsZWQgdmlhIGRldmljZS10cmVlLg0KDQpJbnN0ZWFk LCB3aGF0IGNvdWxkIGJlIGRvbmUgaW4gdGhlIGFkMnM5MF9wcm9iZSgpIGZ1bmN0aW9uLCBpcyB0 byBhZGQNCihlYXJseSBpbiB0aGUgY29kZSkgYSBjaGVjayBmb3IgdGhlIG1heCBmcmVxdWVuY3ku DQoNClNvbWV0aGluZyBsaWtlOg0KDQojZGVmaW5lIEFEMlM5MF9NQVhfU1BJX0ZSRVFfSFogIDIw MDAwMDANCg0KICAgICAgIGlmIChzcGktPm1heF9zcGVlZF9oeiA+IEFEMlM5MF9NQVhfU1BJX0ZS RVFfSFopIHsNCiAgICAgICAgICAgICAgICBkZXZfZXJyKCZzcGktPmRldiwgIlNQSSBDTEssICVk IEh6IGV4Y2VlZHMgMiBNSHpcbiIsDQogICAgICAgICAgICAgICAgICAgICAgICBzcGktPm1heF9z cGVlZF9oeik7DQogICAgICAgICAgICAgICAgcmV0dXJuIC1FSU5WQUw7DQogICAgICAgIH0NCg0K T3IsIHlvdSBjb3VsZCBtYWtlIEFEMlM5MF9NQVhfU1BJX0ZSRVFfSFogPSA4MzAwMDAgDQoNCk9y LCB5b3UgY291bGQganVzdCBwcmludCBhIHdhcm5pbmcgZm9yID4gODMwMDAwLg0KU29tZXRoaW5n IGxpa2U6DQoNCiNkZWZpbmUgQUQyUzkwX01BWF9TUElfU0FGRV9GUkVRX0haICA4MzAwMDANCg0K ICAgICAgIGlmIChzcGktPm1heF9zcGVlZF9oeiA+IEFEMlM5MF9NQVhfU1BJX1NBRkVfRlJFUV9I WikNCiAgICAgICAgICAgICAgICBkZXZfd2Fybigmc3BpLT5kZXYsICJTUEkgQ0xLLCAlZCBIeiA+ IDAuODMgTUh6LCBkZXZpY2UgbWF5DQpub3Qgb3BlcmF0ZSBjb3JyZWN0bHlcbiIsDQogICAgICAg ICAgICAgICAgICAgICAgICBzcGktPm1heF9zcGVlZF9oeik7DQoNCg0KSSdtIG5vdCBzdXJlIHdo YXQgd291bGQgYmUgcmVjb21tZW5kZWQgaGVyZS4gTWF5YmUgc29tZW9uZSBlbHNlIGhhcyBhDQpz dHJvbmdlciBvcGluaW9uIGFib3V0IHRoaXMuDQoNClRoYW5rcw0KQWxleA0KDQo+IFRoYW5rcywN Cj4gTWF0aGV1cy4NCj4gDQo+IFsxXSANCj4gaHR0cHM6Ly93d3cuYW5hbG9nLmNvbS9tZWRpYS9l bi90ZWNobmljYWwtZG9jdW1lbnRhdGlvbi9kYXRhLXNoZWV0cy9BRDJTOTAucGRmDQo+IA0K ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Question about max frequency in ad2s90 2018-11-06 12:34 ` Ardelean, Alexandru @ 2018-11-06 15:12 ` Matheus Tavares Bernardino 2018-11-07 8:31 ` Ardelean, Alexandru 2018-11-07 18:05 ` Matheus Tavares Bernardino 1 sibling, 1 reply; 6+ messages in thread From: Matheus Tavares Bernardino @ 2018-11-06 15:12 UTC (permalink / raw) To: alexandru.Ardelean Cc: jic23, linux-iio, Michael Hennerich, graf.yang, Victor Colombo, kernel-usp On Tue, Nov 6, 2018 at 10:34 AM Ardelean, Alexandru <alexandru.Ardelean@analog.com> wrote: > > On Mon, 2018-11-05 at 23:11 -0200, Matheus Tavares Bernardino wrote: > > Hello everyone, > > > > Hey, > Hi, Alex > > While working on the ad2s90 driver, I had a question about the maximum > > frequency set. Any feedback will be much appreciated. > > > > Just a note/reminder for you: that driver looks pretty old and [since it's > staging as well] you should also consider that it's likely that some [good] > practices were not yet existing at the time [about how to do things]. > Thanks for the reminder. Victor and I are trying to find what needs to be updated/modified to move this driver out of staging. We've sent some patches and now I'm working on the device tree support, which is the last thing to be done in a list Jonathan helped us to enumerate. So I take this opportunity to ask you to, please, let us know if there's anything else you see that needs to be done before moving it out of staging. > > The ad2s90 driver sets its maximum speed to 830000hz (or 0.83Mhz), but > > the datasheet [1] specifies a maximum of 2Mhz. Above the max_speed_hz > > setup, there's a commentary saying "need 600ns between CS and the > > first falling edge of SCLK", which is also said in the datasheet. > > > > Is max_speed_hz set to 0.83Mhz, not 2Mhz, due to the required minimum > > delay of 600ns, somehow? If not: > > 1) is 0.83Mhz really correct? and > > 2) is the minimum delay of 600ns being respected? > > > > The 0.83 Mhz limitation looks very much like it was added to satisfy the > 600 ns limitation. I quickly did the computation and it roughly ends up > being 0.83 Mhz for 600 ns. (i.e. (1 / 0,000600) / 2 = half a period ) > Thanks. Now I better understood the calculation made to use 0.83Mhz. > That limitation (for the delay CS and the first data) is imposed by the > datasheet ; while the chip could operate at a lower delay (and subsequently > higher frequency up to 2 Mhz), it's safer to keep it somehow implemented. > > I took a look in the SPI code and it doesn't look like it implements some > sort of delay between CS and first data being read. I may have missed stuff > though. > Ok. So just to check if I got it right: The chip could work at 2Mhz but needs a delay of 600ns between CS and the first data bit retrieval. The driver could implement the delay and then clock at 2Mhz, but the SPI code doesn't have a way to implement this delay, so we have to stay at a lower frequency (of maximum 0.83Mhz), to satisfy the 600ns delay. Is that right? So there's nothing to do about it, right? > In any case, I do suggest removing all those 4 lines for the SPI setup > code. > i.e. > > /* need 600ns between CS and the first falling edge of SCLK */ > spi->max_speed_hz = 830000; > spi->mode = SPI_MODE_3; > spi_setup(spi); > > This should be handled via device-tree. > Right! Thanks for pointing it out. I'm going to address those changes in the next patch set I'm working on, regarding device tree support. > Instead, what could be done in the ad2s90_probe() function, is to add > (early in the code) a check for the max frequency. > > Something like: > > #define AD2S90_MAX_SPI_FREQ_HZ 2000000 > > if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) { > dev_err(&spi->dev, "SPI CLK, %d Hz exceeds 2 MHz\n", > spi->max_speed_hz); > return -EINVAL; > } > > Or, you could make AD2S90_MAX_SPI_FREQ_HZ = 830000 > > Or, you could just print a warning for > 830000. > Something like: > > #define AD2S90_MAX_SPI_SAFE_FREQ_HZ 830000 > > if (spi->max_speed_hz > AD2S90_MAX_SPI_SAFE_FREQ_HZ) > dev_warn(&spi->dev, "SPI CLK, %d Hz > 0.83 MHz, device may > not operate correctly\n", > spi->max_speed_hz); > > > I'm not sure what would be recommended here. Maybe someone else has a > stronger opinion about this. > Got it. Maybe we could join both, adding the error for freq > 2Mhz and the warning for freq > 0.83 Mhz? Anyway, more opinions on this would be nice, as you said. Thanks for all the feedback. Matheus > Thanks > Alex > > > Thanks, > > Matheus. > > > > [1] > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD2S90.pdf > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Question about max frequency in ad2s90 2018-11-06 15:12 ` Matheus Tavares Bernardino @ 2018-11-07 8:31 ` Ardelean, Alexandru 0 siblings, 0 replies; 6+ messages in thread From: Ardelean, Alexandru @ 2018-11-07 8:31 UTC (permalink / raw) To: matheus.bernardino Cc: victorcolombo, linux-iio, Hennerich, Michael, jic23, graf.yang, kernel-usp T24gVHVlLCAyMDE4LTExLTA2IGF0IDEzOjEyIC0wMjAwLCBNYXRoZXVzIFRhdmFyZXMgQmVybmFy ZGlubyB3cm90ZToNCj4gT24gVHVlLCBOb3YgNiwgMjAxOCBhdCAxMDozNCBBTSBBcmRlbGVhbiwg QWxleGFuZHJ1DQo+IDxhbGV4YW5kcnUuQXJkZWxlYW5AYW5hbG9nLmNvbT4gd3JvdGU6DQo+ID4g DQo+ID4gT24gTW9uLCAyMDE4LTExLTA1IGF0IDIzOjExIC0wMjAwLCBNYXRoZXVzIFRhdmFyZXMg QmVybmFyZGlubyB3cm90ZToNCj4gPiA+IEhlbGxvIGV2ZXJ5b25lLA0KPiA+ID4gDQo+ID4gDQo+ ID4gSGV5LA0KPiA+IA0KPiANCj4gSGksIEFsZXgNCj4gDQo+ID4gPiBXaGlsZSB3b3JraW5nIG9u IHRoZSBhZDJzOTAgZHJpdmVyLCBJIGhhZCBhIHF1ZXN0aW9uIGFib3V0IHRoZQ0KPiA+ID4gbWF4 aW11bQ0KPiA+ID4gZnJlcXVlbmN5IHNldC4gQW55IGZlZWRiYWNrIHdpbGwgYmUgbXVjaCBhcHBy ZWNpYXRlZC4NCj4gPiA+IA0KPiA+IA0KPiA+IEp1c3QgYSBub3RlL3JlbWluZGVyIGZvciB5b3U6 IHRoYXQgZHJpdmVyIGxvb2tzIHByZXR0eSBvbGQgYW5kIFtzaW5jZQ0KPiA+IGl0J3MNCj4gPiBz dGFnaW5nIGFzIHdlbGxdIHlvdSBzaG91bGQgYWxzbyBjb25zaWRlciB0aGF0IGl0J3MgbGlrZWx5 IHRoYXQgc29tZQ0KPiA+IFtnb29kXQ0KPiA+IHByYWN0aWNlcyB3ZXJlIG5vdCB5ZXQgZXhpc3Rp bmcgYXQgdGhlIHRpbWUgW2Fib3V0IGhvdyB0byBkbyB0aGluZ3NdLg0KPiA+IA0KPiANCj4gVGhh bmtzIGZvciB0aGUgcmVtaW5kZXIuIFZpY3RvciBhbmQgSSBhcmUgdHJ5aW5nIHRvIGZpbmQgd2hh dCBuZWVkcyB0bw0KPiBiZSB1cGRhdGVkL21vZGlmaWVkIHRvIG1vdmUgdGhpcyBkcml2ZXIgb3V0 IG9mIHN0YWdpbmcuIFdlJ3ZlIHNlbnQNCj4gc29tZSBwYXRjaGVzIGFuZCBub3cgSSdtIHdvcmtp bmcgb24gdGhlIGRldmljZSB0cmVlIHN1cHBvcnQsIHdoaWNoIGlzDQo+IHRoZSBsYXN0IHRoaW5n IHRvIGJlIGRvbmUgaW4gYSBsaXN0IEpvbmF0aGFuIGhlbHBlZCB1cyB0byBlbnVtZXJhdGUuDQo+ IFNvIEkgdGFrZSB0aGlzIG9wcG9ydHVuaXR5IHRvIGFzayB5b3UgdG8sIHBsZWFzZSwgbGV0IHVz IGtub3cgaWYNCj4gdGhlcmUncyBhbnl0aGluZyBlbHNlIHlvdSBzZWUgdGhhdCBuZWVkcyB0byBi ZSBkb25lIGJlZm9yZSBtb3ZpbmcgaXQNCj4gb3V0IG9mIHN0YWdpbmcuDQo+IA0KPiA+ID4gVGhl IGFkMnM5MCBkcml2ZXIgc2V0cyBpdHMgbWF4aW11bSBzcGVlZCB0byA4MzAwMDBoeiAob3IgMC44 M01oeiksDQo+ID4gPiBidXQNCj4gPiA+IHRoZSBkYXRhc2hlZXQgWzFdIHNwZWNpZmllcyBhIG1h eGltdW0gb2YgMk1oei4gQWJvdmUgdGhlIG1heF9zcGVlZF9oeg0KPiA+ID4gc2V0dXAsIHRoZXJl J3MgYSBjb21tZW50YXJ5IHNheWluZyAgIm5lZWQgNjAwbnMgYmV0d2VlbiBDUyBhbmQgdGhlDQo+ ID4gPiBmaXJzdCBmYWxsaW5nIGVkZ2Ugb2YgU0NMSyIsIHdoaWNoIGlzIGFsc28gc2FpZCBpbiB0 aGUgZGF0YXNoZWV0Lg0KPiA+ID4gDQo+ID4gPiBJcyBtYXhfc3BlZWRfaHogc2V0IHRvIDAuODNN aHosIG5vdCAyTWh6LCBkdWUgdG8gdGhlIHJlcXVpcmVkIG1pbmltdW0NCj4gPiA+IGRlbGF5IG9m IDYwMG5zLCBzb21laG93PyBJZiBub3Q6DQo+ID4gPiAgIDEpIGlzIDAuODNNaHogcmVhbGx5IGNv cnJlY3Q/IGFuZA0KPiA+ID4gICAyKSBpcyB0aGUgbWluaW11bSBkZWxheSBvZiA2MDBucyBiZWlu ZyByZXNwZWN0ZWQ/DQo+ID4gPiANCj4gPiANCj4gPiBUaGUgMC44MyBNaHogbGltaXRhdGlvbiBs b29rcyB2ZXJ5IG11Y2ggbGlrZSBpdCB3YXMgYWRkZWQgdG8gc2F0aXNmeQ0KPiA+IHRoZQ0KPiA+ IDYwMCBucyBsaW1pdGF0aW9uLiBJIHF1aWNrbHkgZGlkIHRoZSBjb21wdXRhdGlvbiBhbmQgaXQg cm91Z2hseSBlbmRzIHVwDQo+ID4gYmVpbmcgMC44MyBNaHogZm9yIDYwMCBucy4gKGkuZS4gKDEg LyAwLDAwMDYwMCkgLyAyID0gaGFsZiBhIHBlcmlvZCApDQo+ID4gDQo+IA0KPiBUaGFua3MuIE5v dyBJIGJldHRlciB1bmRlcnN0b29kIHRoZSBjYWxjdWxhdGlvbiBtYWRlIHRvIHVzZSAwLjgzTWh6 Lg0KPiANCj4gPiBUaGF0IGxpbWl0YXRpb24gKGZvciB0aGUgZGVsYXkgQ1MgYW5kIHRoZSBmaXJz dCBkYXRhKSBpcyBpbXBvc2VkIGJ5IHRoZQ0KPiA+IGRhdGFzaGVldCA7IHdoaWxlIHRoZSBjaGlw IGNvdWxkIG9wZXJhdGUgYXQgYSBsb3dlciBkZWxheSAoYW5kDQo+ID4gc3Vic2VxdWVudGx5DQo+ ID4gaGlnaGVyIGZyZXF1ZW5jeSB1cCB0byAyIE1oeiksIGl0J3Mgc2FmZXIgdG8ga2VlcCBpdCBz b21laG93DQo+ID4gaW1wbGVtZW50ZWQuDQo+ID4gDQo+ID4gSSB0b29rIGEgbG9vayBpbiB0aGUg U1BJIGNvZGUgYW5kIGl0IGRvZXNuJ3QgbG9vayBsaWtlIGl0IGltcGxlbWVudHMNCj4gPiBzb21l DQo+ID4gc29ydCBvZiBkZWxheSBiZXR3ZWVuIENTIGFuZCBmaXJzdCBkYXRhIGJlaW5nIHJlYWQu IEkgbWF5IGhhdmUgbWlzc2VkDQo+ID4gc3R1ZmYNCj4gPiB0aG91Z2guDQo+ID4gDQo+IA0KPiBP ay4gU28ganVzdCB0byBjaGVjayBpZiBJIGdvdCBpdCByaWdodDogVGhlIGNoaXAgY291bGQgd29y ayBhdCAyTWh6DQo+IGJ1dCBuZWVkcyBhIGRlbGF5IG9mIDYwMG5zIGJldHdlZW4gQ1MgYW5kIHRo ZSBmaXJzdCBkYXRhIGJpdA0KPiByZXRyaWV2YWwuIFRoZSBkcml2ZXIgY291bGQgaW1wbGVtZW50 IHRoZSBkZWxheSBhbmQgdGhlbiBjbG9jayBhdA0KPiAyTWh6LCBidXQgdGhlIFNQSSBjb2RlIGRv ZXNuJ3QgaGF2ZSBhIHdheSB0byBpbXBsZW1lbnQgdGhpcyBkZWxheSwgc28NCj4gd2UgaGF2ZSB0 byBzdGF5IGF0IGEgbG93ZXIgZnJlcXVlbmN5IChvZiBtYXhpbXVtIDAuODNNaHopLCB0byBzYXRp c2Z5DQo+IHRoZSA2MDBucyBkZWxheS4gSXMgdGhhdCByaWdodD8NCg0KWWVzLCB0aGF0J3MgY29y cmVjdC4NCg0KPiANCj4gU28gdGhlcmUncyBub3RoaW5nIHRvIGRvIGFib3V0IGl0LCByaWdodD8N Cj4gDQoNClllcywgdGhhdCdzIGFsc28gY29ycmVjdC4NClsgV2VsbCwgd2Ugd291bGQgbmVlZCB0 byB1cGRhdGUgdGhlIFNQSSBkcml2ZXIsIGJ1dCBJJ20gbm90IHN1cmUgaWYgaXQncw0Kd29ydGgg dGhlIGNoYW5nZSBqdXN0IGZvciB0aGlzIGNoaXAsIHdoaWNoIGlzIHByZXR0eSBvbGQgOyBuZXdl ciBjaGlwcw0Kc2hvdWxkbid0IGhhdmUgdGhpcyBwcm9ibGVtIF0uDQoNCj4gPiBJbiBhbnkgY2Fz ZSwgSSBkbyBzdWdnZXN0IHJlbW92aW5nIGFsbCB0aG9zZSA0IGxpbmVzIGZvciB0aGUgU1BJIHNl dHVwDQo+ID4gY29kZS4NCj4gPiBpLmUuDQo+ID4gDQo+ID4gICAgICAgICAvKiBuZWVkIDYwMG5z IGJldHdlZW4gQ1MgYW5kIHRoZSBmaXJzdCBmYWxsaW5nIGVkZ2Ugb2YgU0NMSyAqLw0KPiA+ICAg ICAgICAgc3BpLT5tYXhfc3BlZWRfaHogPSA4MzAwMDA7DQo+ID4gICAgICAgICBzcGktPm1vZGUg PSBTUElfTU9ERV8zOw0KPiA+ICAgICAgICAgc3BpX3NldHVwKHNwaSk7DQo+ID4gDQo+ID4gVGhp cyBzaG91bGQgYmUgaGFuZGxlZCB2aWEgZGV2aWNlLXRyZWUuDQo+ID4gDQo+IA0KPiBSaWdodCEg VGhhbmtzIGZvciBwb2ludGluZyBpdCBvdXQuIEknbSBnb2luZyB0byBhZGRyZXNzIHRob3NlIGNo YW5nZXMNCj4gaW4gdGhlIG5leHQgcGF0Y2ggc2V0IEknbSB3b3JraW5nIG9uLCByZWdhcmRpbmcg ZGV2aWNlIHRyZWUgc3VwcG9ydC4NCj4gDQo+ID4gSW5zdGVhZCwgd2hhdCBjb3VsZCBiZSBkb25l IGluIHRoZSBhZDJzOTBfcHJvYmUoKSBmdW5jdGlvbiwgaXMgdG8gYWRkDQo+ID4gKGVhcmx5IGlu IHRoZSBjb2RlKSBhIGNoZWNrIGZvciB0aGUgbWF4IGZyZXF1ZW5jeS4NCj4gPiANCj4gPiBTb21l dGhpbmcgbGlrZToNCj4gPiANCj4gPiAjZGVmaW5lIEFEMlM5MF9NQVhfU1BJX0ZSRVFfSFogIDIw MDAwMDANCj4gPiANCj4gPiAgICAgICAgaWYgKHNwaS0+bWF4X3NwZWVkX2h6ID4gQUQyUzkwX01B WF9TUElfRlJFUV9IWikgew0KPiA+ICAgICAgICAgICAgICAgICBkZXZfZXJyKCZzcGktPmRldiwg IlNQSSBDTEssICVkIEh6IGV4Y2VlZHMgMiBNSHpcbiIsDQo+ID4gICAgICAgICAgICAgICAgICAg ICAgICAgc3BpLT5tYXhfc3BlZWRfaHopOw0KPiA+ICAgICAgICAgICAgICAgICByZXR1cm4gLUVJ TlZBTDsNCj4gPiAgICAgICAgIH0NCj4gPiANCj4gPiBPciwgeW91IGNvdWxkIG1ha2UgQUQyUzkw X01BWF9TUElfRlJFUV9IWiA9IDgzMDAwMA0KPiA+IA0KPiA+IE9yLCB5b3UgY291bGQganVzdCBw cmludCBhIHdhcm5pbmcgZm9yID4gODMwMDAwLg0KPiA+IFNvbWV0aGluZyBsaWtlOg0KPiA+IA0K PiA+ICNkZWZpbmUgQUQyUzkwX01BWF9TUElfU0FGRV9GUkVRX0haICA4MzAwMDANCj4gPiANCj4g PiAgICAgICAgaWYgKHNwaS0+bWF4X3NwZWVkX2h6ID4gQUQyUzkwX01BWF9TUElfU0FGRV9GUkVR X0haKQ0KPiA+ICAgICAgICAgICAgICAgICBkZXZfd2Fybigmc3BpLT5kZXYsICJTUEkgQ0xLLCAl ZCBIeiA+IDAuODMgTUh6LCBkZXZpY2UNCj4gPiBtYXkNCj4gPiBub3Qgb3BlcmF0ZSBjb3JyZWN0 bHlcbiIsDQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgc3BpLT5tYXhfc3BlZWRfaHopOw0K PiA+IA0KPiA+IA0KPiA+IEknbSBub3Qgc3VyZSB3aGF0IHdvdWxkIGJlIHJlY29tbWVuZGVkIGhl cmUuIE1heWJlIHNvbWVvbmUgZWxzZSBoYXMgYQ0KPiA+IHN0cm9uZ2VyIG9waW5pb24gYWJvdXQg dGhpcy4NCj4gPiANCj4gDQo+IEdvdCBpdC4gTWF5YmUgd2UgY291bGQgam9pbiBib3RoLCBhZGRp bmcgdGhlIGVycm9yIGZvciBmcmVxID4gMk1oeiBhbmQNCj4gdGhlIHdhcm5pbmcgZm9yIGZyZXEg PiAwLjgzIE1oej8gQW55d2F5LCBtb3JlIG9waW5pb25zIG9uIHRoaXMgd291bGQNCj4gYmUgbmlj ZSwgYXMgeW91IHNhaWQuDQo+IA0KPiBUaGFua3MgZm9yIGFsbCB0aGUgZmVlZGJhY2suDQo+IE1h dGhldXMNCj4gDQo+ID4gVGhhbmtzDQo+ID4gQWxleA0KPiA+IA0KPiA+ID4gVGhhbmtzLA0KPiA+ ID4gTWF0aGV1cy4NCj4gPiA+IA0KPiA+ID4gWzFdDQo+ID4gPiANCmh0dHBzOi8vd3d3LmFuYWxv Zy5jb20vbWVkaWEvZW4vdGVjaG5pY2FsLWRvY3VtZW50YXRpb24vZGF0YS1zaGVldHMvQUQyUzkw LnBkZg0KPiA+ID4gDQo+IA0KPiANCg== ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Question about max frequency in ad2s90 2018-11-06 12:34 ` Ardelean, Alexandru 2018-11-06 15:12 ` Matheus Tavares Bernardino @ 2018-11-07 18:05 ` Matheus Tavares Bernardino 2018-11-08 7:23 ` Ardelean, Alexandru 1 sibling, 1 reply; 6+ messages in thread From: Matheus Tavares Bernardino @ 2018-11-07 18:05 UTC (permalink / raw) To: alexandru.Ardelean Cc: jic23, linux-iio, Michael Hennerich, graf.yang, Victor Colombo, kernel-usp On Tue, Nov 6, 2018 at 10:34 AM Ardelean, Alexandru <alexandru.Ardelean@analog.com> wrote: > > On Mon, 2018-11-05 at 23:11 -0200, Matheus Tavares Bernardino wrote: > > Hello everyone, > > > > Hey, > > > While working on the ad2s90 driver, I had a question about the maximum > > frequency set. Any feedback will be much appreciated. > > > > Just a note/reminder for you: that driver looks pretty old and [since it's > staging as well] you should also consider that it's likely that some [good] > practices were not yet existing at the time [about how to do things]. > > > The ad2s90 driver sets its maximum speed to 830000hz (or 0.83Mhz), but > > the datasheet [1] specifies a maximum of 2Mhz. Above the max_speed_hz > > setup, there's a commentary saying "need 600ns between CS and the > > first falling edge of SCLK", which is also said in the datasheet. > > > > Is max_speed_hz set to 0.83Mhz, not 2Mhz, due to the required minimum > > delay of 600ns, somehow? If not: > > 1) is 0.83Mhz really correct? and > > 2) is the minimum delay of 600ns being respected? > > > > The 0.83 Mhz limitation looks very much like it was added to satisfy the > 600 ns limitation. I quickly did the computation and it roughly ends up > being 0.83 Mhz for 600 ns. (i.e. (1 / 0,000600) / 2 = half a period ) > > That limitation (for the delay CS and the first data) is imposed by the > datasheet ; while the chip could operate at a lower delay (and subsequently > higher frequency up to 2 Mhz), it's safer to keep it somehow implemented. > > I took a look in the SPI code and it doesn't look like it implements some > sort of delay between CS and first data being read. I may have missed stuff > though. > > In any case, I do suggest removing all those 4 lines for the SPI setup > code. > i.e. > > /* need 600ns between CS and the first falling edge of SCLK */ > spi->max_speed_hz = 830000; > spi->mode = SPI_MODE_3; > spi_setup(spi); > > This should be handled via device-tree. > > Instead, what could be done in the ad2s90_probe() function, is to add > (early in the code) a check for the max frequency. > > Something like: > > #define AD2S90_MAX_SPI_FREQ_HZ 2000000 > > if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) { > dev_err(&spi->dev, "SPI CLK, %d Hz exceeds 2 MHz\n", > spi->max_speed_hz); > return -EINVAL; > } > > Or, you could make AD2S90_MAX_SPI_FREQ_HZ = 830000 > > Or, you could just print a warning for > 830000. > Something like: > > #define AD2S90_MAX_SPI_SAFE_FREQ_HZ 830000 > > if (spi->max_speed_hz > AD2S90_MAX_SPI_SAFE_FREQ_HZ) > dev_warn(&spi->dev, "SPI CLK, %d Hz > 0.83 MHz, device may > not operate correctly\n", > spi->max_speed_hz); > I'm adding these suggestions in a patch set, but I don't really know what's the right way of crediting the authorship in this case. Should I set you as the author and add my S-o-B? Thanks Matheus > > I'm not sure what would be recommended here. Maybe someone else has a > stronger opinion about this. > > Thanks > Alex > > > Thanks, > > Matheus. > > > > [1] > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD2S90.pdf > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Question about max frequency in ad2s90 2018-11-07 18:05 ` Matheus Tavares Bernardino @ 2018-11-08 7:23 ` Ardelean, Alexandru 0 siblings, 0 replies; 6+ messages in thread From: Ardelean, Alexandru @ 2018-11-08 7:23 UTC (permalink / raw) To: matheus.bernardino Cc: victorcolombo, linux-iio, Hennerich, Michael, jic23, graf.yang, kernel-usp T24gV2VkLCAyMDE4LTExLTA3IGF0IDE2OjA1IC0wMjAwLCBNYXRoZXVzIFRhdmFyZXMgQmVybmFy ZGlubyB3cm90ZToNCj4gT24gVHVlLCBOb3YgNiwgMjAxOCBhdCAxMDozNCBBTSBBcmRlbGVhbiwg QWxleGFuZHJ1DQo+IDxhbGV4YW5kcnUuQXJkZWxlYW5AYW5hbG9nLmNvbT4gd3JvdGU6DQo+ID4g DQo+ID4gT24gTW9uLCAyMDE4LTExLTA1IGF0IDIzOjExIC0wMjAwLCBNYXRoZXVzIFRhdmFyZXMg QmVybmFyZGlubyB3cm90ZToNCj4gPiA+IEhlbGxvIGV2ZXJ5b25lLA0KPiA+ID4gDQo+ID4gDQo+ ID4gSGV5LA0KPiA+IA0KPiA+ID4gV2hpbGUgd29ya2luZyBvbiB0aGUgYWQyczkwIGRyaXZlciwg SSBoYWQgYSBxdWVzdGlvbiBhYm91dCB0aGUNCj4gPiA+IG1heGltdW0NCj4gPiA+IGZyZXF1ZW5j eSBzZXQuIEFueSBmZWVkYmFjayB3aWxsIGJlIG11Y2ggYXBwcmVjaWF0ZWQuDQo+ID4gPiANCj4g PiANCj4gPiBKdXN0IGEgbm90ZS9yZW1pbmRlciBmb3IgeW91OiB0aGF0IGRyaXZlciBsb29rcyBw cmV0dHkgb2xkIGFuZCBbc2luY2UNCj4gPiBpdCdzDQo+ID4gc3RhZ2luZyBhcyB3ZWxsXSB5b3Ug c2hvdWxkIGFsc28gY29uc2lkZXIgdGhhdCBpdCdzIGxpa2VseSB0aGF0IHNvbWUNCj4gPiBbZ29v ZF0NCj4gPiBwcmFjdGljZXMgd2VyZSBub3QgeWV0IGV4aXN0aW5nIGF0IHRoZSB0aW1lIFthYm91 dCBob3cgdG8gZG8gdGhpbmdzXS4NCj4gPiANCj4gPiA+IFRoZSBhZDJzOTAgZHJpdmVyIHNldHMg aXRzIG1heGltdW0gc3BlZWQgdG8gODMwMDAwaHogKG9yIDAuODNNaHopLA0KPiA+ID4gYnV0DQo+ ID4gPiB0aGUgZGF0YXNoZWV0IFsxXSBzcGVjaWZpZXMgYSBtYXhpbXVtIG9mIDJNaHouIEFib3Zl IHRoZSBtYXhfc3BlZWRfaHoNCj4gPiA+IHNldHVwLCB0aGVyZSdzIGEgY29tbWVudGFyeSBzYXlp bmcgICJuZWVkIDYwMG5zIGJldHdlZW4gQ1MgYW5kIHRoZQ0KPiA+ID4gZmlyc3QgZmFsbGluZyBl ZGdlIG9mIFNDTEsiLCB3aGljaCBpcyBhbHNvIHNhaWQgaW4gdGhlIGRhdGFzaGVldC4NCj4gPiA+ IA0KPiA+ID4gSXMgbWF4X3NwZWVkX2h6IHNldCB0byAwLjgzTWh6LCBub3QgMk1oeiwgZHVlIHRv IHRoZSByZXF1aXJlZCBtaW5pbXVtDQo+ID4gPiBkZWxheSBvZiA2MDBucywgc29tZWhvdz8gSWYg bm90Og0KPiA+ID4gICAxKSBpcyAwLjgzTWh6IHJlYWxseSBjb3JyZWN0PyBhbmQNCj4gPiA+ICAg MikgaXMgdGhlIG1pbmltdW0gZGVsYXkgb2YgNjAwbnMgYmVpbmcgcmVzcGVjdGVkPw0KPiA+ID4g DQo+ID4gDQo+ID4gVGhlIDAuODMgTWh6IGxpbWl0YXRpb24gbG9va3MgdmVyeSBtdWNoIGxpa2Ug aXQgd2FzIGFkZGVkIHRvIHNhdGlzZnkNCj4gPiB0aGUNCj4gPiA2MDAgbnMgbGltaXRhdGlvbi4g SSBxdWlja2x5IGRpZCB0aGUgY29tcHV0YXRpb24gYW5kIGl0IHJvdWdobHkgZW5kcyB1cA0KPiA+ IGJlaW5nIDAuODMgTWh6IGZvciA2MDAgbnMuIChpLmUuICgxIC8gMCwwMDA2MDApIC8gMiA9IGhh bGYgYSBwZXJpb2QgKQ0KPiA+IA0KPiA+IFRoYXQgbGltaXRhdGlvbiAoZm9yIHRoZSBkZWxheSBD UyBhbmQgdGhlIGZpcnN0IGRhdGEpIGlzIGltcG9zZWQgYnkgdGhlDQo+ID4gZGF0YXNoZWV0IDsg d2hpbGUgdGhlIGNoaXAgY291bGQgb3BlcmF0ZSBhdCBhIGxvd2VyIGRlbGF5IChhbmQNCj4gPiBz dWJzZXF1ZW50bHkNCj4gPiBoaWdoZXIgZnJlcXVlbmN5IHVwIHRvIDIgTWh6KSwgaXQncyBzYWZl ciB0byBrZWVwIGl0IHNvbWVob3cNCj4gPiBpbXBsZW1lbnRlZC4NCj4gPiANCj4gPiBJIHRvb2sg YSBsb29rIGluIHRoZSBTUEkgY29kZSBhbmQgaXQgZG9lc24ndCBsb29rIGxpa2UgaXQgaW1wbGVt ZW50cw0KPiA+IHNvbWUNCj4gPiBzb3J0IG9mIGRlbGF5IGJldHdlZW4gQ1MgYW5kIGZpcnN0IGRh dGEgYmVpbmcgcmVhZC4gSSBtYXkgaGF2ZSBtaXNzZWQNCj4gPiBzdHVmZg0KPiA+IHRob3VnaC4N Cj4gPiANCj4gPiBJbiBhbnkgY2FzZSwgSSBkbyBzdWdnZXN0IHJlbW92aW5nIGFsbCB0aG9zZSA0 IGxpbmVzIGZvciB0aGUgU1BJIHNldHVwDQo+ID4gY29kZS4NCj4gPiBpLmUuDQo+ID4gDQo+ID4g ICAgICAgICAvKiBuZWVkIDYwMG5zIGJldHdlZW4gQ1MgYW5kIHRoZSBmaXJzdCBmYWxsaW5nIGVk Z2Ugb2YgU0NMSyAqLw0KPiA+ICAgICAgICAgc3BpLT5tYXhfc3BlZWRfaHogPSA4MzAwMDA7DQo+ ID4gICAgICAgICBzcGktPm1vZGUgPSBTUElfTU9ERV8zOw0KPiA+ICAgICAgICAgc3BpX3NldHVw KHNwaSk7DQo+ID4gDQo+ID4gVGhpcyBzaG91bGQgYmUgaGFuZGxlZCB2aWEgZGV2aWNlLXRyZWUu DQo+ID4gDQo+ID4gSW5zdGVhZCwgd2hhdCBjb3VsZCBiZSBkb25lIGluIHRoZSBhZDJzOTBfcHJv YmUoKSBmdW5jdGlvbiwgaXMgdG8gYWRkDQo+ID4gKGVhcmx5IGluIHRoZSBjb2RlKSBhIGNoZWNr IGZvciB0aGUgbWF4IGZyZXF1ZW5jeS4NCj4gPiANCj4gPiBTb21ldGhpbmcgbGlrZToNCj4gPiAN Cj4gPiAjZGVmaW5lIEFEMlM5MF9NQVhfU1BJX0ZSRVFfSFogIDIwMDAwMDANCj4gPiANCj4gPiAg ICAgICAgaWYgKHNwaS0+bWF4X3NwZWVkX2h6ID4gQUQyUzkwX01BWF9TUElfRlJFUV9IWikgew0K PiA+ICAgICAgICAgICAgICAgICBkZXZfZXJyKCZzcGktPmRldiwgIlNQSSBDTEssICVkIEh6IGV4 Y2VlZHMgMiBNSHpcbiIsDQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgc3BpLT5tYXhfc3Bl ZWRfaHopOw0KPiA+ICAgICAgICAgICAgICAgICByZXR1cm4gLUVJTlZBTDsNCj4gPiAgICAgICAg IH0NCj4gPiANCj4gPiBPciwgeW91IGNvdWxkIG1ha2UgQUQyUzkwX01BWF9TUElfRlJFUV9IWiA9 IDgzMDAwMA0KPiA+IA0KPiA+IE9yLCB5b3UgY291bGQganVzdCBwcmludCBhIHdhcm5pbmcgZm9y ID4gODMwMDAwLg0KPiA+IFNvbWV0aGluZyBsaWtlOg0KPiA+IA0KPiA+ICNkZWZpbmUgQUQyUzkw X01BWF9TUElfU0FGRV9GUkVRX0haICA4MzAwMDANCj4gPiANCj4gPiAgICAgICAgaWYgKHNwaS0+ bWF4X3NwZWVkX2h6ID4gQUQyUzkwX01BWF9TUElfU0FGRV9GUkVRX0haKQ0KPiA+ICAgICAgICAg ICAgICAgICBkZXZfd2Fybigmc3BpLT5kZXYsICJTUEkgQ0xLLCAlZCBIeiA+IDAuODMgTUh6LCBk ZXZpY2UNCj4gPiBtYXkNCj4gPiBub3Qgb3BlcmF0ZSBjb3JyZWN0bHlcbiIsDQo+ID4gICAgICAg ICAgICAgICAgICAgICAgICAgc3BpLT5tYXhfc3BlZWRfaHopOw0KPiA+IA0KPiANCj4gSSdtIGFk ZGluZyB0aGVzZSBzdWdnZXN0aW9ucyBpbiBhIHBhdGNoIHNldCwgYnV0IEkgZG9uJ3QgcmVhbGx5 IGtub3cNCj4gd2hhdCdzIHRoZSByaWdodCB3YXkgb2YgY3JlZGl0aW5nIHRoZSBhdXRob3JzaGlw IGluIHRoaXMgY2FzZS4gU2hvdWxkDQo+IEkgc2V0IHlvdSBhcyB0aGUgYXV0aG9yIGFuZCBhZGQg bXkgUy1vLUI/DQoNCkZlZWwgZnJlZSB0byBub3QgaW5jbHVkZSBtZSBpbiB0aGUgcGF0Y2gtc2V0 IGluIGFueSB3YXkgOikNClRoaXMgZW1haWwgc2hvdWxkIHNlcnZlIGFzIGEgbm90ZS9tZW50aW9u IHRoYXQgSSBhbSBmaW5lIHdpdGggdGhpcy4NCg0KV2hhdCBJIHByb3ZpZGVkIHdhcyBtb3JlIG9m IGEgQyBwc2V1ZG8tY29kZSA7IHlvdXIgUy1vLUIgaW1wbGllcyB0aGF0IHlvdQ0KZGlkIHRoZSBm aW5hbCB3b3JrIGFuZCB0ZXN0aW5nIG9mIHRoZSBwYXRjaC4NCg0KVGhhbmtzDQpBbGV4DQo+IA0K PiBUaGFua3MNCj4gTWF0aGV1cw0KPiANCj4gPiANCj4gPiBJJ20gbm90IHN1cmUgd2hhdCB3b3Vs ZCBiZSByZWNvbW1lbmRlZCBoZXJlLiBNYXliZSBzb21lb25lIGVsc2UgaGFzIGENCj4gPiBzdHJv bmdlciBvcGluaW9uIGFib3V0IHRoaXMuDQo+ID4gDQo+ID4gVGhhbmtzDQo+ID4gQWxleA0KPiA+ IA0KPiA+ID4gVGhhbmtzLA0KPiA+ID4gTWF0aGV1cy4NCj4gPiA+IA0KPiA+ID4gWzFdDQo+ID4g PiANCmh0dHBzOi8vd3d3LmFuYWxvZy5jb20vbWVkaWEvZW4vdGVjaG5pY2FsLWRvY3VtZW50YXRp b24vZGF0YS1zaGVldHMvQUQyUzkwLnBkZg0KPiA+ID4gDQo+IA0KPiANCg== ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-08 7:23 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-06 1:11 Question about max frequency in ad2s90 Matheus Tavares Bernardino 2018-11-06 12:34 ` Ardelean, Alexandru 2018-11-06 15:12 ` Matheus Tavares Bernardino 2018-11-07 8:31 ` Ardelean, Alexandru 2018-11-07 18:05 ` Matheus Tavares Bernardino 2018-11-08 7:23 ` Ardelean, Alexandru
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.