* [PATCH] iio: adc: ti_am335x_adc: fix fifo overrun recovery
@ 2017-03-10 13:57 Michael Engl
2017-03-11 18:07 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Michael Engl @ 2017-03-10 13:57 UTC (permalink / raw)
To: jic23; +Cc: Michael Engl, knaack.h, lars, pmeerw, linux-iio
VGhlIHRpYWRjX2lycV9oKGludCBpcnEsIHZvaWQgKnByaXZhdGUpIGZ1bmN0aW9uIGlzIGhhbmRs
aW5nIEZJRk8NCm92ZXJydW5zIGJ5IGNsZWFyaW5nIGZsYWdzLCBkaXNhYmxpbmcgYW5kIGVuYWJs
aW5nIHRoZSBBREMgdG8NCnJlY292ZXIuDQoNCklmIHRoZSBBREMgaXMgcnVubmluZyBpbiBjb250
aW51b3VzIG1vZGUgYSBGSUZPIG92ZXJydW4gaGFwcGVucw0KcmVndWxhcmx5LiBJZiB0aGUgZGlz
YWJsaW5nIG9mIHRoZSBBREMgaGFwcGVucyBjb25jdXJyZW50bHkgd2l0aA0KYSBuZXcgY29udmVy
c2lvbi4gSXQgbWlnaHQgaGFwcGVuIHRoYXQgdGhlIGVuYWJsaW5nIG9mIHRoZSBBREMNCmlzIGln
bm9yZWQgYnkgdGhlIGhhcmR3YXJlLiBUaGlzIHN0b3BzIHRoZSBBREMgcGVybWFuZW50bHkuIE5v
DQptb3JlIGludGVycnVwdHMgYXJlIHRyaWdnZXJlZC4NCg0KQWNjb3JkaW5nIHRvIHRoZSBBTTMz
NXggUmVmZXJlbmNlIE1hbnVhbCAoU1BSVUg3M0ggT2N0b2JlciAyMDExIC0NClJldmlzZWQgQXBy
aWwgMjAxMyAtIENoYXB0ZXIgMTIuNCBhbmQgMTIuNSkgaXQgaXMgbmVjZXNzYXJ5IHRvDQpjaGVj
ayB0aGUgQURDIEZTTSBiaXRzIGluIFJFR19BRENGU00gYmVmb3JlIGVuYWJsaW5nIHRoZSBBREMN
CmFnYWluLiBCZWNhdXNlIHRoZSBkaXNhYmxpbmcgb2YgdGhlIEFEQyBpcyBkb25lIHJpZ2h0IGFm
dGVyIHRoZQ0KY3VycmVudCBjb252ZXJzaW9uIGhhcyBiZWVuIGZpbmlzaGVkLg0KDQpUbyB0cmln
Z2VyIHRoaXMgYnVnIGl0IGlzIG5lY2Vzc2FyeSB0byBydW4gdGhlIEFEQyBpbiBjb250aW51b3Vz
DQptb2RlLiBUaGUgQURDIHZhbHVlcyBvZiBhbGwgY2hhbm5lbHMgbmVlZCB0byBiZSByZWFkIGlu
IGFuIGVuZGxlc3MNCmxvb3AuIFRoZSBidWcgYXBwZWFycyB3aXRoaW4gdGhlIGZpcnN0IDYgaG91
cnMgKH41LjQgbWlsbGlvbg0KaGFuZGxlZCBGSUZPIG92ZXJydW5zKS4gVGhlIHVzZXIgc3BhY2Ug
YXBwbGljYXRpb24gd2lsbCBoYW5nIG9uDQpyZWFkaW5nIG5ldyB2YWx1ZXMgZnJvbSB0aGUgY2hh
cmFjdGVyIGRldmljZS4NCg0KU2lnbmVkLW9mZi1ieTogTWljaGFlbCBFbmdsIDxtaWNoYWVsLmVu
Z2xAd2p3LXNvbHV0aW9ucy5jb20+DQotLS0NCsKgZHJpdmVycy9paW8vYWRjL3RpX2FtMzM1eF9h
ZGMuYyB8IDEzICsrKysrKysrKysrKy0NCsKgMSBmaWxlIGNoYW5nZWQsIDEyIGluc2VydGlvbnMo
KyksIDEgZGVsZXRpb24oLSkNCg0KZGlmZiAtLWdpdCBhL2RyaXZlcnMvaWlvL2FkYy90aV9hbTMz
NXhfYWRjLmMgYi9kcml2ZXJzL2lpby9hZGMvdGlfYW0zMzV4X2FkYy5jDQppbmRleCBhZDlkZWMz
Li40MjgyY2VjIDEwMDY0NA0KLS0tIGEvZHJpdmVycy9paW8vYWRjL3RpX2FtMzM1eF9hZGMuYw0K
KysrIGIvZHJpdmVycy9paW8vYWRjL3RpX2FtMzM1eF9hZGMuYw0KQEAgLTE2OSw3ICsxNjksOSBA
QCBzdGF0aWMgaXJxcmV0dXJuX3QgdGlhZGNfaXJxX2goaW50IGlycSwgdm9pZCAqcHJpdmF0ZSkN
CsKgew0KwqAJc3RydWN0IGlpb19kZXYgKmluZGlvX2RldiA9IHByaXZhdGU7DQrCoAlzdHJ1Y3Qg
dGlhZGNfZGV2aWNlICphZGNfZGV2ID0gaWlvX3ByaXYoaW5kaW9fZGV2KTsNCi0JdW5zaWduZWQg
aW50IHN0YXR1cywgY29uZmlnOw0KKwl1bnNpZ25lZCBpbnQgc3RhdHVzLCBjb25maWcsIGFkY19m
c207DQorCXVuc2lnbmVkIHNob3J0IGNvdW50ID0gMDsNCisNCsKgCXN0YXR1cyA9IHRpYWRjX3Jl
YWRsKGFkY19kZXYsIFJFR19JUlFTVEFUVVMpOw0KwqANCsKgCS8qDQpAQCAtMTgzLDYgKzE4NSwx
NSBAQCBzdGF0aWMgaXJxcmV0dXJuX3QgdGlhZGNfaXJxX2goaW50IGlycSwgdm9pZCAqcHJpdmF0
ZSkNCsKgCQl0aWFkY193cml0ZWwoYWRjX2RldiwgUkVHX0NUUkwsIGNvbmZpZyk7DQrCoAkJdGlh
ZGNfd3JpdGVsKGFkY19kZXYsIFJFR19JUlFTVEFUVVMsIElSUUVOQl9GSUZPMU9WUlJVTg0KwqAJ
CQkJfCBJUlFFTkJfRklGTzFVTkRSRkxXIHwgSVJRRU5CX0ZJRk8xVEhSRVMpOw0KKw0KKwkJLyog
d2FpdCBmb3IgaWRsZSBzdGF0ZS4NCisJCcKgKiBBREMgbmVlZHMgdG8gZmluaXNoIHRoZSBjdXJy
ZW50IGNvbnZlcnNpb24NCisJCcKgKiBiZWZvcmUgZGlzYWJsaW5nIHRoZSBtb2R1bGUNCisJCcKg
Ki8NCisJCWRvIHsNCisJCQlhZGNfZnNtID0gdGlhZGNfcmVhZGwoYWRjX2RldiwgUkVHX0FEQ0ZT
TSk7DQorCQl9IHdoaWxlIChhZGNfZnNtICE9IDB4MTAgJiYgY291bnQrKyA8IDEwMCk7DQorDQrC
oAkJdGlhZGNfd3JpdGVsKGFkY19kZXYsIFJFR19DVFJMLCAoY29uZmlnIHwgQ05UUkxSRUdfVFND
U1NFTkIpKTsNCsKgCQlyZXR1cm4gSVJRX0hBTkRMRUQ7DQrCoAl9IGVsc2UgaWYgKHN0YXR1cyAm
IElSUUVOQl9GSUZPMVRIUkVTKSB7DQotLcKgDQoyLjcuNA0K
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: adc: ti_am335x_adc: fix fifo overrun recovery
2017-03-10 13:57 [PATCH] iio: adc: ti_am335x_adc: fix fifo overrun recovery Michael Engl
@ 2017-03-11 18:07 ` Jonathan Cameron
2017-03-11 19:32 ` Michael Engl
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2017-03-11 18:07 UTC (permalink / raw)
To: Michael Engl; +Cc: knaack.h, lars, pmeerw, linux-iio
On 10/03/17 13:57, Michael Engl wrote:
> The tiadc_irq_h(int irq, void *private) function is handling FIFO
> overruns by clearing flags, disabling and enabling the ADC to
> recover.
>
> If the ADC is running in continuous mode a FIFO overrun happens
> regularly. If the disabling of the ADC happens concurrently with
> a new conversion. It might happen that the enabling of the ADC
> is ignored by the hardware. This stops the ADC permanently. No
> more interrupts are triggered.
>
> According to the AM335x Reference Manual (SPRUH73H October 2011 -
> Revised April 2013 - Chapter 12.4 and 12.5) it is necessary to
> check the ADC FSM bits in REG_ADCFSM before enabling the ADC
> again. Because the disabling of the ADC is done right after the
> current conversion has been finished.
The relevant comment in the latest version of that manual (rev O)
is at the end of 12.3.5
"The FIFO can also generate FIFOx_OVERRUN and FIFOx_UNDERFLOW interrupts. The user can mask
these events by programming the IRQENABLE_CLR register. To clear a FIFO underflow or FIFO overrun
interrupt, the user should write a ‘1’ to the status bit in the IRQSTS register. The TSC_ADC_SS does not
recover from these conditions automatically. Therefore, the software will need to disable and then again
enable the TSC_ADC_SS. Before the user can turn the module back on, the user must read the
ADCSTAT register to check if the status of the ADC FSM is idle and the current step is the idle step."
>
> To trigger this bug it is necessary to run the ADC in continuous
> mode. The ADC values of all channels need to be read in an endless
> loop. The bug appears within the first 6 hours (~5.4 million
> handled FIFO overruns). The user space application will hang on
> reading new values from the character device.
Impressive to track this one down, but it's the sort of nasty
hardware race that we need to be very careful with when trying
to get a fix in place.
>
> Signed-off-by: Michael Engl <michael.engl@wjw-solutions.com>
> ---
> drivers/iio/adc/ti_am335x_adc.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index ad9dec3..4282cec 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -169,7 +169,9 @@ static irqreturn_t tiadc_irq_h(int irq, void *private)
> {
> struct iio_dev *indio_dev = private;
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> - unsigned int status, config;
> + unsigned int status, config, adc_fsm;
> + unsigned short count = 0;
> +
> status = tiadc_readl(adc_dev, REG_IRQSTATUS);
>
> /*
> @@ -183,6 +185,15 @@ static irqreturn_t tiadc_irq_h(int irq, void *private)
> tiadc_writel(adc_dev, REG_CTRL, config);
> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1OVRRUN
> | IRQENB_FIFO1UNDRFLW | IRQENB_FIFO1THRES);
> +
> + /* wait for idle state.
Standard multiline comment syntax preferred.
> + * ADC needs to finish the current conversion
> + * before disabling the module
> + */
> + do {
> + adc_fsm = tiadc_readl(adc_dev, REG_ADCFSM);
> + } while (adc_fsm != 0x10 && count++ < 100);
What slightly bothers me is that we are papering over the problem, yet
this is far from guaranteed to fix it (I think!) as we might get a race
as it starts the next conversion after we have checked, but before we
disable...
Note I'm only looking at this fairly superficially so please point
out if I have missed something!
We could look for a transition, which would potentially reduce the chance
of problems, but that's nasty as well and we'd need to disable interrupts
probably to avoid any potential of a clash (for a non trivial amount of
time...)
> +
> tiadc_writel(adc_dev, REG_CTRL, (config | CNTRLREG_TSCSSENB));
> return IRQ_HANDLED;
> } else if (status & IRQENB_FIFO1THRES) {
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: adc: ti_am335x_adc: fix fifo overrun recovery
2017-03-11 18:07 ` Jonathan Cameron
@ 2017-03-11 19:32 ` Michael Engl
2017-03-13 21:22 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Michael Engl @ 2017-03-11 19:32 UTC (permalink / raw)
To: jic23; +Cc: knaack.h, lars, pmeerw, linux-iio
T24gU2FtLCAyMDE3LTAzLTExIGF0IDE4OjA3ICswMDAwLCBKb25hdGhhbiBDYW1lcm9uIHdyb3Rl
Og0KPiBPbiAxMC8wMy8xNyAxMzo1NywgTWljaGFlbCBFbmdsIHdyb3RlOg0KPiA+IA0KPiA+IFRo
ZSB0aWFkY19pcnFfaChpbnQgaXJxLCB2b2lkICpwcml2YXRlKSBmdW5jdGlvbiBpcyBoYW5kbGlu
ZyBGSUZPDQo+ID4gb3ZlcnJ1bnMgYnkgY2xlYXJpbmcgZmxhZ3MsIGRpc2FibGluZyBhbmQgZW5h
YmxpbmcgdGhlIEFEQyB0bw0KPiA+IHJlY292ZXIuDQo+ID4gDQo+ID4gSWYgdGhlIEFEQyBpcyBy
dW5uaW5nIGluIGNvbnRpbnVvdXMgbW9kZSBhIEZJRk8gb3ZlcnJ1biBoYXBwZW5zDQo+ID4gcmVn
dWxhcmx5LiBJZiB0aGUgZGlzYWJsaW5nIG9mIHRoZSBBREMgaGFwcGVucyBjb25jdXJyZW50bHkg
d2l0aA0KPiA+IGEgbmV3IGNvbnZlcnNpb24uIEl0IG1pZ2h0IGhhcHBlbiB0aGF0IHRoZSBlbmFi
bGluZyBvZiB0aGUgQURDDQo+ID4gaXMgaWdub3JlZCBieSB0aGUgaGFyZHdhcmUuIFRoaXMgc3Rv
cHMgdGhlIEFEQyBwZXJtYW5lbnRseS4gTm8NCj4gPiBtb3JlIGludGVycnVwdHMgYXJlIHRyaWdn
ZXJlZC4NCj4gPiANCj4gPiBBY2NvcmRpbmcgdG8gdGhlIEFNMzM1eCBSZWZlcmVuY2UgTWFudWFs
IChTUFJVSDczSCBPY3RvYmVyIDIwMTEgLQ0KPiA+IFJldmlzZWQgQXByaWwgMjAxMyAtIENoYXB0
ZXIgMTIuNCBhbmQgMTIuNSkgaXQgaXMgbmVjZXNzYXJ5IHRvDQo+ID4gY2hlY2sgdGhlIEFEQyBG
U00gYml0cyBpbiBSRUdfQURDRlNNIGJlZm9yZSBlbmFibGluZyB0aGUgQURDDQo+ID4gYWdhaW4u
IEJlY2F1c2UgdGhlIGRpc2FibGluZyBvZiB0aGUgQURDIGlzIGRvbmUgcmlnaHQgYWZ0ZXIgdGhl
DQo+ID4gY3VycmVudCBjb252ZXJzaW9uIGhhcyBiZWVuIGZpbmlzaGVkLg0KPiBUaGUgcmVsZXZh
bnQgY29tbWVudCBpbiB0aGUgbGF0ZXN0IHZlcnNpb24gb2YgdGhhdCBtYW51YWwgKHJldiBPKQ0K
PiBpcyBhdCB0aGUgZW5kIG9mIDEyLjMuNQ0KPiAiVGhlIEZJRk8gY2FuIGFsc28gZ2VuZXJhdGUg
RklGT3hfT1ZFUlJVTiBhbmQgRklGT3hfVU5ERVJGTE9XDQo+IGludGVycnVwdHMuIFRoZSB1c2Vy
IGNhbiBtYXNrDQo+IHRoZXNlIGV2ZW50cyBieSBwcm9ncmFtbWluZyB0aGUgSVJRRU5BQkxFX0NM
UiByZWdpc3Rlci4gVG8gY2xlYXIgYQ0KPiBGSUZPIHVuZGVyZmxvdyBvciBGSUZPIG92ZXJydW4N
Cj4gaW50ZXJydXB0LCB0aGUgdXNlciBzaG91bGQgd3JpdGUgYSDigJgx4oCZIHRvIHRoZSBzdGF0
dXMgYml0IGluIHRoZQ0KPiBJUlFTVFMgcmVnaXN0ZXIuIFRoZSBUU0NfQURDX1NTIGRvZXMgbm90
DQo+IHJlY292ZXIgZnJvbSB0aGVzZSBjb25kaXRpb25zIGF1dG9tYXRpY2FsbHkuIFRoZXJlZm9y
ZSwgdGhlIHNvZnR3YXJlDQo+IHdpbGwgbmVlZCB0byBkaXNhYmxlIGFuZCB0aGVuIGFnYWluDQo+
IGVuYWJsZSB0aGUgVFNDX0FEQ19TUy4gQmVmb3JlIHRoZSB1c2VyIGNhbiB0dXJuIHRoZSBtb2R1
bGUgYmFjayBvbiwNCj4gdGhlIHVzZXIgbXVzdCByZWFkIHRoZQ0KPiBBRENTVEFUIHJlZ2lzdGVy
IHRvIGNoZWNrIGlmIHRoZSBzdGF0dXMgb2YgdGhlIEFEQyBGU00gaXMgaWRsZSBhbmQNCj4gdGhl
IGN1cnJlbnQgc3RlcCBpcyB0aGUgaWRsZSBzdGVwLiINCj4gDQo+IA0KPiANCj4gPiANCj4gPiAN
Cj4gPiBUbyB0cmlnZ2VyIHRoaXMgYnVnIGl0IGlzIG5lY2Vzc2FyeSB0byBydW4gdGhlIEFEQyBp
biBjb250aW51b3VzDQo+ID4gbW9kZS4gVGhlIEFEQyB2YWx1ZXMgb2YgYWxsIGNoYW5uZWxzIG5l
ZWQgdG8gYmUgcmVhZCBpbiBhbiBlbmRsZXNzDQo+ID4gbG9vcC4gVGhlIGJ1ZyBhcHBlYXJzIHdp
dGhpbiB0aGUgZmlyc3QgNiBob3VycyAofjUuNCBtaWxsaW9uDQo+ID4gaGFuZGxlZCBGSUZPIG92
ZXJydW5zKS4gVGhlIHVzZXIgc3BhY2UgYXBwbGljYXRpb24gd2lsbCBoYW5nIG9uDQo+ID4gcmVh
ZGluZyBuZXcgdmFsdWVzIGZyb20gdGhlIGNoYXJhY3RlciBkZXZpY2UuDQo+IEltcHJlc3NpdmUg
dG8gdHJhY2sgdGhpcyBvbmUgZG93biwgYnV0IGl0J3MgdGhlIHNvcnQgb2YgbmFzdHkNCj4gaGFy
ZHdhcmUgcmFjZSB0aGF0IHdlIG5lZWQgdG8gYmUgdmVyeSBjYXJlZnVsIHdpdGggd2hlbiB0cnlp
bmcNCj4gdG8gZ2V0IGEgZml4IGluIHBsYWNlLg0KPiA+IA0KPiA+IA0KPiA+IFNpZ25lZC1vZmYt
Ynk6IE1pY2hhZWwgRW5nbCA8bWljaGFlbC5lbmdsQHdqdy1zb2x1dGlvbnMuY29tPg0KPiA+IC0t
LQ0KPiA+IMKgZHJpdmVycy9paW8vYWRjL3RpX2FtMzM1eF9hZGMuYyB8IDEzICsrKysrKysrKysr
Ky0NCj4gPiDCoDEgZmlsZSBjaGFuZ2VkLCAxMiBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0p
DQo+ID4gDQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvaWlvL2FkYy90aV9hbTMzNXhfYWRjLmMN
Cj4gPiBiL2RyaXZlcnMvaWlvL2FkYy90aV9hbTMzNXhfYWRjLmMNCj4gPiBpbmRleCBhZDlkZWMz
Li40MjgyY2VjIDEwMDY0NA0KPiA+IC0tLSBhL2RyaXZlcnMvaWlvL2FkYy90aV9hbTMzNXhfYWRj
LmMNCj4gPiArKysgYi9kcml2ZXJzL2lpby9hZGMvdGlfYW0zMzV4X2FkYy5jDQo+ID4gQEAgLTE2
OSw3ICsxNjksOSBAQCBzdGF0aWMgaXJxcmV0dXJuX3QgdGlhZGNfaXJxX2goaW50IGlycSwgdm9p
ZA0KPiA+ICpwcml2YXRlKQ0KPiA+IMKgew0KPiA+IMKgCXN0cnVjdCBpaW9fZGV2ICppbmRpb19k
ZXYgPSBwcml2YXRlOw0KPiA+IMKgCXN0cnVjdCB0aWFkY19kZXZpY2UgKmFkY19kZXYgPSBpaW9f
cHJpdihpbmRpb19kZXYpOw0KPiA+IC0JdW5zaWduZWQgaW50IHN0YXR1cywgY29uZmlnOw0KPiA+
ICsJdW5zaWduZWQgaW50IHN0YXR1cywgY29uZmlnLCBhZGNfZnNtOw0KPiA+ICsJdW5zaWduZWQg
c2hvcnQgY291bnQgPSAwOw0KPiA+ICsNCj4gPiDCoAlzdGF0dXMgPSB0aWFkY19yZWFkbChhZGNf
ZGV2LCBSRUdfSVJRU1RBVFVTKTsNCj4gPiDCoA0KPiA+IMKgCS8qDQo+ID4gQEAgLTE4Myw2ICsx
ODUsMTUgQEAgc3RhdGljIGlycXJldHVybl90IHRpYWRjX2lycV9oKGludCBpcnEsIHZvaWQNCj4g
PiAqcHJpdmF0ZSkNCj4gPiDCoAkJdGlhZGNfd3JpdGVsKGFkY19kZXYsIFJFR19DVFJMLCBjb25m
aWcpOw0KPiA+IMKgCQl0aWFkY193cml0ZWwoYWRjX2RldiwgUkVHX0lSUVNUQVRVUywNCj4gPiBJ
UlFFTkJfRklGTzFPVlJSVU4NCj4gPiDCoAkJCQl8IElSUUVOQl9GSUZPMVVORFJGTFcgfA0KPiA+
IElSUUVOQl9GSUZPMVRIUkVTKTsNCj4gPiArDQo+ID4gKwkJLyogd2FpdCBmb3IgaWRsZSBzdGF0
ZS4NCj4gU3RhbmRhcmQgbXVsdGlsaW5lIGNvbW1lbnQgc3ludGF4IHByZWZlcnJlZC4NCj4gPiAN
Cj4gPiArCQnCoCogQURDIG5lZWRzIHRvIGZpbmlzaCB0aGUgY3VycmVudCBjb252ZXJzaW9uDQo+
ID4gKwkJwqAqIGJlZm9yZSBkaXNhYmxpbmcgdGhlIG1vZHVsZQ0KPiA+ICsJCcKgKi8NCj4gPiAr
CQlkbyB7DQo+ID4gKwkJCWFkY19mc20gPSB0aWFkY19yZWFkbChhZGNfZGV2LA0KPiA+IFJFR19B
RENGU00pOw0KPiA+ICsJCX0gd2hpbGUgKGFkY19mc20gIT0gMHgxMCAmJiBjb3VudCsrIDwgMTAw
KTsNCj4gV2hhdCBzbGlnaHRseSBib3RoZXJzIG1lIGlzIHRoYXQgd2UgYXJlIHBhcGVyaW5nIG92
ZXIgdGhlDQo+IHByb2JsZW0swqDCoHlldA0KPiB0aGlzIGlzIGZhciBmcm9tIGd1YXJhbnRlZWQg
dG8gZml4IGl0IChJIHRoaW5rISkgYXMgd2UgbWlnaHQgZ2V0IGENCj4gcmFjZQ0KPiBhcyBpdCBz
dGFydHMgdGhlIG5leHQgY29udmVyc2lvbiBhZnRlciB3ZSBoYXZlIGNoZWNrZWQsIGJ1dCBiZWZv
cmUgd2UNCj4gZGlzYWJsZS4uLg0KPiANCj4gTm90ZSBJJ20gb25seSBsb29raW5nIGF0IHRoaXMg
ZmFpcmx5IHN1cGVyZmljaWFsbHkgc28gcGxlYXNlIHBvaW50DQo+IG91dCBpZiBJIGhhdmUgbWlz
c2VkIHNvbWV0aGluZyENCg0KUHJvYmFibHkgbXkgY29tbWVudCBpcyBhIGJpdCBjb25mdXNpbmcu
IFRoZSBBREMgaGFzIGFscmVhZHkgYmVlbg0KZGlzYWJsZWQgYnkgd3JpdGluZyB0aGUgY29uZmln
IHNvbWUgbGluZXMgYWJvdmUgdGhlIHdoaWxlIGxvb3AuwqANCg0KIg0KY29uZmlnICY9IH4oQ05U
UkxSRUdfVFNDU1NFTkIpOw0KdGlhZGNfd3JpdGVsKGFkY19kZXYsIFJFR19DVFJMLCBjb25maWcp
Ow0KIg0KQnV0IGFzIHN0YXRlZCBpbiB0aGUgUmVmZXJlbmNlIE1hbnVhbCB0aGUgQURDIG5lZWRz
IHNvbWUgdGltZSB0byBmaW5pc2gNCnRoZSBjdXJyZW50IGNvbnZlcnNpb24gdG8gZmluYWxseSBk
aXNhYmxlIHRoZSBBREMuwqANCg0KRGVzY3JpcHRpb24gb2YgIkVuYWJsZSIgQml0IGluIHRoZSBD
VFJMIFJlZ2lzdGVyICh0YWJsZSAxMi0xNCk6DQoiVFNDX0FEQ19TUyBtb2R1bGUgZW5hYmxlIGJp
dC4NCkFmdGVyIHByb2dyYW1taW5nIGFsbCB0aGUgc3RlcHMgYW5kIGNvbmZpZ3VyYXRpb24gcmVn
aXN0ZXJzLCB3cml0ZSBhDQoxdG8gdGhpcyBiaXQgdG8gdHVybiBvbiBUU0NfQURDX1NTLg0KV3Jp
dGluZyBhIDAgd2lsbCBkaXNhYmxlIHRoZSBtb2R1bGUgKGFmdGVyIHRoZSBjdXJyZW50IGNvbnZl
cnNpb24pLiINCg0KVGhhdCBtZWFucyBvbmNlIHRoZSBhZGNfZnNtIGlzIGVxdWFsIHRvIDB4MTAg
KGlkbGUgc3RhdGUpIHRoZSBBREMgc3RheXMNCmRpc2FibGVkIGFzIGxvbmcgYXMgd2UgZW5hYmxl
IHRoZSBBREMgYWdhaW4uIFdoaWNoIGlzIGRvbmUgYnkgdGhlDQpvcmlnaW5hbCBjb2RlIG9uZSBs
aW5lIGJlbG93IG9mIHRoaXMgcGF0Y2guDQoNCj4gDQo+IFdlIGNvdWxkIGxvb2sgZm9yIGEgdHJh
bnNpdGlvbiwgd2hpY2ggd291bGQgcG90ZW50aWFsbHkgcmVkdWNlIHRoZQ0KPiBjaGFuY2UNCj4g
b2YgcHJvYmxlbXMsIGJ1dCB0aGF0J3MgbmFzdHkgYXMgd2VsbCBhbmQgd2UnZCBuZWVkIHRvIGRp
c2FibGUNCj4gaW50ZXJydXB0cw0KPiBwcm9iYWJseSB0byBhdm9pZCBhbnkgcG90ZW50aWFsIG9m
IGEgY2xhc2ggKGZvciBhIG5vbiB0cml2aWFsIGFtb3VudA0KPiBvZg0KPiB0aW1lLi4uKQ0KPiA+
IA0KPiA+ICsNCj4gPiDCoAkJdGlhZGNfd3JpdGVsKGFkY19kZXYsIFJFR19DVFJMLCAoY29uZmln
IHwNCj4gPiBDTlRSTFJFR19UU0NTU0VOQikpOw0KPiA+IMKgCQlyZXR1cm4gSVJRX0hBTkRMRUQ7
DQo+ID4gwqAJfSBlbHNlIGlmIChzdGF0dXMgJiBJUlFFTkJfRklGTzFUSFJFUykgew==
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: adc: ti_am335x_adc: fix fifo overrun recovery
2017-03-11 19:32 ` Michael Engl
@ 2017-03-13 21:22 ` Jonathan Cameron
2017-03-15 18:59 ` Michael Engl
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2017-03-13 21:22 UTC (permalink / raw)
To: Michael Engl; +Cc: knaack.h, lars, pmeerw, linux-iio
On 11/03/17 19:32, Michael Engl wrote:
> On Sam, 2017-03-11 at 18:07 +0000, Jonathan Cameron wrote:
>> On 10/03/17 13:57, Michael Engl wrote:
>>>
>>> The tiadc_irq_h(int irq, void *private) function is handling FIFO
>>> overruns by clearing flags, disabling and enabling the ADC to
>>> recover.
>>>
>>> If the ADC is running in continuous mode a FIFO overrun happens
>>> regularly. If the disabling of the ADC happens concurrently with
>>> a new conversion. It might happen that the enabling of the ADC
>>> is ignored by the hardware. This stops the ADC permanently. No
>>> more interrupts are triggered.
>>>
>>> According to the AM335x Reference Manual (SPRUH73H October 2011 -
>>> Revised April 2013 - Chapter 12.4 and 12.5) it is necessary to
>>> check the ADC FSM bits in REG_ADCFSM before enabling the ADC
>>> again. Because the disabling of the ADC is done right after the
>>> current conversion has been finished.
>> The relevant comment in the latest version of that manual (rev O)
>> is at the end of 12.3.5
>> "The FIFO can also generate FIFOx_OVERRUN and FIFOx_UNDERFLOW
>> interrupts. The user can mask
>> these events by programming the IRQENABLE_CLR register. To clear a
>> FIFO underflow or FIFO overrun
>> interrupt, the user should write a ‘1’ to the status bit in the
>> IRQSTS register. The TSC_ADC_SS does not
>> recover from these conditions automatically. Therefore, the software
>> will need to disable and then again
>> enable the TSC_ADC_SS. Before the user can turn the module back on,
>> the user must read the
>> ADCSTAT register to check if the status of the ADC FSM is idle and
>> the current step is the idle step."
>>
>>
>>
>>>
>>>
>>> To trigger this bug it is necessary to run the ADC in continuous
>>> mode. The ADC values of all channels need to be read in an endless
>>> loop. The bug appears within the first 6 hours (~5.4 million
>>> handled FIFO overruns). The user space application will hang on
>>> reading new values from the character device.
>> Impressive to track this one down, but it's the sort of nasty
>> hardware race that we need to be very careful with when trying
>> to get a fix in place.
>>>
>>>
>>> Signed-off-by: Michael Engl <michael.engl@wjw-solutions.com>
>>> ---
>>> drivers/iio/adc/ti_am335x_adc.c | 13 ++++++++++++-
>>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iio/adc/ti_am335x_adc.c
>>> b/drivers/iio/adc/ti_am335x_adc.c
>>> index ad9dec3..4282cec 100644
>>> --- a/drivers/iio/adc/ti_am335x_adc.c
>>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>>> @@ -169,7 +169,9 @@ static irqreturn_t tiadc_irq_h(int irq, void
>>> *private)
>>> {
>>> struct iio_dev *indio_dev = private;
>>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>>> - unsigned int status, config;
>>> + unsigned int status, config, adc_fsm;
>>> + unsigned short count = 0;
>>> +
>>> status = tiadc_readl(adc_dev, REG_IRQSTATUS);
>>>
>>> /*
>>> @@ -183,6 +185,15 @@ static irqreturn_t tiadc_irq_h(int irq, void
>>> *private)
>>> tiadc_writel(adc_dev, REG_CTRL, config);
>>> tiadc_writel(adc_dev, REG_IRQSTATUS,
>>> IRQENB_FIFO1OVRRUN
>>> | IRQENB_FIFO1UNDRFLW |
>>> IRQENB_FIFO1THRES);
>>> +
>>> + /* wait for idle state.
>> Standard multiline comment syntax preferred.
>>>
>>> + * ADC needs to finish the current conversion
>>> + * before disabling the module
>>> + */
>>> + do {
>>> + adc_fsm = tiadc_readl(adc_dev,
>>> REG_ADCFSM);
>>> + } while (adc_fsm != 0x10 && count++ < 100);
>> What slightly bothers me is that we are papering over the
>> problem, yet
>> this is far from guaranteed to fix it (I think!) as we might get a
>> race
>> as it starts the next conversion after we have checked, but before we
>> disable...
>>
>> Note I'm only looking at this fairly superficially so please point
>> out if I have missed something!
>
> Probably my comment is a bit confusing. The ADC has already been
> disabled by writing the config some lines above the while loop.
>
> "
> config &= ~(CNTRLREG_TSCSSENB);
> tiadc_writel(adc_dev, REG_CTRL, config);
> "
> But as stated in the Reference Manual the ADC needs some time to finish
> the current conversion to finally disable the ADC.
>
> Description of "Enable" Bit in the CTRL Register (table 12-14):
> "TSC_ADC_SS module enable bit.
> After programming all the steps and configuration registers, write a
> 1to this bit to turn on TSC_ADC_SS.
> Writing a 0 will disable the module (after the current conversion)."
>
> That means once the adc_fsm is equal to 0x10 (idle state) the ADC stays
> disabled as long as we enable the ADC again. Which is done by the
> original code one line below of this patch.
Cool. Thanks for the clarification. I got lost in the datahsheet ;)
Would you mind figuring out a fixes tag for this one as I want it to
go for stable?
Thanks,
Jonathan
>
>>
>> We could look for a transition, which would potentially reduce the
>> chance
>> of problems, but that's nasty as well and we'd need to disable
>> interrupts
>> probably to avoid any potential of a clash (for a non trivial amount
>> of
>> time...)
>>>
>>> +
>>> tiadc_writel(adc_dev, REG_CTRL, (config |
>>> CNTRLREG_TSCSSENB));
>>> return IRQ_HANDLED;
>>> } else if (status & IRQENB_FIFO1THRES) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: adc: ti_am335x_adc: fix fifo overrun recovery
2017-03-13 21:22 ` Jonathan Cameron
@ 2017-03-15 18:59 ` Michael Engl
2017-03-15 19:49 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Michael Engl @ 2017-03-15 18:59 UTC (permalink / raw)
To: jic23; +Cc: knaack.h, lars, pmeerw, linux-iio
T24gTW9uLCAyMDE3LTAzLTEzIGF0IDIxOjIyICswMDAwLCBKb25hdGhhbiBDYW1lcm9uIHdyb3Rl
Og0KPiBPbiAxMS8wMy8xNyAxOTozMiwgTWljaGFlbCBFbmdsIHdyb3RlOg0KPiA+IA0KPiA+IE9u
IFNhbSwgMjAxNy0wMy0xMSBhdCAxODowNyArMDAwMCwgSm9uYXRoYW4gQ2FtZXJvbiB3cm90ZToN
Cj4gPiA+IA0KPiA+ID4gT24gMTAvMDMvMTcgMTM6NTcsIE1pY2hhZWwgRW5nbCB3cm90ZToNCj4g
PiA+ID4gDQo+ID4gPiA+IA0KPiA+ID4gPiBUaGUgdGlhZGNfaXJxX2goaW50IGlycSwgdm9pZCAq
cHJpdmF0ZSkgZnVuY3Rpb24gaXMgaGFuZGxpbmcNCj4gPiA+ID4gRklGTw0KPiA+ID4gPiBvdmVy
cnVucyBieSBjbGVhcmluZyBmbGFncywgZGlzYWJsaW5nIGFuZCBlbmFibGluZyB0aGUgQURDIHRv
DQo+ID4gPiA+IHJlY292ZXIuDQo+ID4gPiA+IA0KPiA+ID4gPiBJZiB0aGUgQURDIGlzIHJ1bm5p
bmcgaW4gY29udGludW91cyBtb2RlIGEgRklGTyBvdmVycnVuIGhhcHBlbnMNCj4gPiA+ID4gcmVn
dWxhcmx5LiBJZiB0aGUgZGlzYWJsaW5nIG9mIHRoZSBBREMgaGFwcGVucyBjb25jdXJyZW50bHkN
Cj4gPiA+ID4gd2l0aA0KPiA+ID4gPiBhIG5ldyBjb252ZXJzaW9uLiBJdCBtaWdodCBoYXBwZW4g
dGhhdCB0aGUgZW5hYmxpbmcgb2YgdGhlIEFEQw0KPiA+ID4gPiBpcyBpZ25vcmVkIGJ5IHRoZSBo
YXJkd2FyZS4gVGhpcyBzdG9wcyB0aGUgQURDIHBlcm1hbmVudGx5LiBObw0KPiA+ID4gPiBtb3Jl
IGludGVycnVwdHMgYXJlIHRyaWdnZXJlZC4NCj4gPiA+ID4gDQo+ID4gPiA+IEFjY29yZGluZyB0
byB0aGUgQU0zMzV4IFJlZmVyZW5jZSBNYW51YWwgKFNQUlVINzNIIE9jdG9iZXIgMjAxMQ0KPiA+
ID4gPiAtDQo+ID4gPiA+IFJldmlzZWQgQXByaWwgMjAxMyAtIENoYXB0ZXIgMTIuNCBhbmQgMTIu
NSkgaXQgaXMgbmVjZXNzYXJ5IHRvDQo+ID4gPiA+IGNoZWNrIHRoZSBBREMgRlNNIGJpdHMgaW4g
UkVHX0FEQ0ZTTSBiZWZvcmUgZW5hYmxpbmcgdGhlIEFEQw0KPiA+ID4gPiBhZ2Fpbi4gQmVjYXVz
ZSB0aGUgZGlzYWJsaW5nIG9mIHRoZSBBREMgaXMgZG9uZSByaWdodCBhZnRlciB0aGUNCj4gPiA+
ID4gY3VycmVudCBjb252ZXJzaW9uIGhhcyBiZWVuIGZpbmlzaGVkLg0KPiA+ID4gVGhlIHJlbGV2
YW50IGNvbW1lbnQgaW4gdGhlIGxhdGVzdCB2ZXJzaW9uIG9mIHRoYXQgbWFudWFsIChyZXYgTykN
Cj4gPiA+IGlzIGF0IHRoZSBlbmQgb2YgMTIuMy41DQo+ID4gPiAiVGhlIEZJRk8gY2FuIGFsc28g
Z2VuZXJhdGUgRklGT3hfT1ZFUlJVTiBhbmQgRklGT3hfVU5ERVJGTE9XDQo+ID4gPiBpbnRlcnJ1
cHRzLiBUaGUgdXNlciBjYW4gbWFzaw0KPiA+ID4gdGhlc2UgZXZlbnRzIGJ5IHByb2dyYW1taW5n
IHRoZSBJUlFFTkFCTEVfQ0xSIHJlZ2lzdGVyLiBUbyBjbGVhcg0KPiA+ID4gYQ0KPiA+ID4gRklG
TyB1bmRlcmZsb3cgb3IgRklGTyBvdmVycnVuDQo+ID4gPiBpbnRlcnJ1cHQsIHRoZSB1c2VyIHNo
b3VsZCB3cml0ZSBhIOKAmDHigJkgdG8gdGhlIHN0YXR1cyBiaXQgaW4gdGhlDQo+ID4gPiBJUlFT
VFMgcmVnaXN0ZXIuIFRoZSBUU0NfQURDX1NTIGRvZXMgbm90DQo+ID4gPiByZWNvdmVyIGZyb20g
dGhlc2UgY29uZGl0aW9ucyBhdXRvbWF0aWNhbGx5LiBUaGVyZWZvcmUsIHRoZQ0KPiA+ID4gc29m
dHdhcmUNCj4gPiA+IHdpbGwgbmVlZCB0byBkaXNhYmxlIGFuZCB0aGVuIGFnYWluDQo+ID4gPiBl
bmFibGUgdGhlIFRTQ19BRENfU1MuIEJlZm9yZSB0aGUgdXNlciBjYW4gdHVybiB0aGUgbW9kdWxl
IGJhY2sNCj4gPiA+IG9uLA0KPiA+ID4gdGhlIHVzZXIgbXVzdCByZWFkIHRoZQ0KPiA+ID4gQURD
U1RBVCByZWdpc3RlciB0byBjaGVjayBpZiB0aGUgc3RhdHVzIG9mIHRoZSBBREMgRlNNIGlzIGlk
bGUNCj4gPiA+IGFuZA0KPiA+ID4gdGhlIGN1cnJlbnQgc3RlcCBpcyB0aGUgaWRsZSBzdGVwLiIN
Cj4gPiA+IA0KPiA+ID4gDQo+ID4gPiANCj4gPiA+ID4gDQo+ID4gPiA+IA0KPiA+ID4gPiANCj4g
PiA+ID4gVG8gdHJpZ2dlciB0aGlzIGJ1ZyBpdCBpcyBuZWNlc3NhcnkgdG8gcnVuIHRoZSBBREMg
aW4NCj4gPiA+ID4gY29udGludW91cw0KPiA+ID4gPiBtb2RlLiBUaGUgQURDIHZhbHVlcyBvZiBh
bGwgY2hhbm5lbHMgbmVlZCB0byBiZSByZWFkIGluIGFuDQo+ID4gPiA+IGVuZGxlc3MNCj4gPiA+
ID4gbG9vcC4gVGhlIGJ1ZyBhcHBlYXJzIHdpdGhpbiB0aGUgZmlyc3QgNiBob3VycyAofjUuNCBt
aWxsaW9uDQo+ID4gPiA+IGhhbmRsZWQgRklGTyBvdmVycnVucykuIFRoZSB1c2VyIHNwYWNlIGFw
cGxpY2F0aW9uIHdpbGwgaGFuZyBvbg0KPiA+ID4gPiByZWFkaW5nIG5ldyB2YWx1ZXMgZnJvbSB0
aGUgY2hhcmFjdGVyIGRldmljZS4NCj4gPiA+IEltcHJlc3NpdmUgdG8gdHJhY2sgdGhpcyBvbmUg
ZG93biwgYnV0IGl0J3MgdGhlIHNvcnQgb2YgbmFzdHkNCj4gPiA+IGhhcmR3YXJlIHJhY2UgdGhh
dCB3ZSBuZWVkIHRvIGJlIHZlcnkgY2FyZWZ1bCB3aXRoIHdoZW4gdHJ5aW5nDQo+ID4gPiB0byBn
ZXQgYSBmaXggaW4gcGxhY2UuDQo+ID4gPiA+IA0KPiA+ID4gPiANCj4gPiA+ID4gDQo+ID4gPiA+
IFNpZ25lZC1vZmYtYnk6IE1pY2hhZWwgRW5nbCA8bWljaGFlbC5lbmdsQHdqdy1zb2x1dGlvbnMu
Y29tPg0KPiA+ID4gPiAtLS0NCj4gPiA+ID4gwqBkcml2ZXJzL2lpby9hZGMvdGlfYW0zMzV4X2Fk
Yy5jIHwgMTMgKysrKysrKysrKysrLQ0KPiA+ID4gPiDCoDEgZmlsZSBjaGFuZ2VkLCAxMiBpbnNl
cnRpb25zKCspLCAxIGRlbGV0aW9uKC0pDQo+ID4gPiA+IA0KPiA+ID4gPiBkaWZmIC0tZ2l0IGEv
ZHJpdmVycy9paW8vYWRjL3RpX2FtMzM1eF9hZGMuYw0KPiA+ID4gPiBiL2RyaXZlcnMvaWlvL2Fk
Yy90aV9hbTMzNXhfYWRjLmMNCj4gPiA+ID4gaW5kZXggYWQ5ZGVjMy4uNDI4MmNlYyAxMDA2NDQN
Cj4gPiA+ID4gLS0tIGEvZHJpdmVycy9paW8vYWRjL3RpX2FtMzM1eF9hZGMuYw0KPiA+ID4gPiAr
KysgYi9kcml2ZXJzL2lpby9hZGMvdGlfYW0zMzV4X2FkYy5jDQo+ID4gPiA+IEBAIC0xNjksNyAr
MTY5LDkgQEAgc3RhdGljIGlycXJldHVybl90IHRpYWRjX2lycV9oKGludCBpcnEsDQo+ID4gPiA+
IHZvaWQNCj4gPiA+ID4gKnByaXZhdGUpDQo+ID4gPiA+IMKgew0KPiA+ID4gPiDCoAlzdHJ1Y3Qg
aWlvX2RldiAqaW5kaW9fZGV2ID0gcHJpdmF0ZTsNCj4gPiA+ID4gwqAJc3RydWN0IHRpYWRjX2Rl
dmljZSAqYWRjX2RldiA9IGlpb19wcml2KGluZGlvX2Rldik7DQo+ID4gPiA+IC0JdW5zaWduZWQg
aW50IHN0YXR1cywgY29uZmlnOw0KPiA+ID4gPiArCXVuc2lnbmVkIGludCBzdGF0dXMsIGNvbmZp
ZywgYWRjX2ZzbTsNCj4gPiA+ID4gKwl1bnNpZ25lZCBzaG9ydCBjb3VudCA9IDA7DQo+ID4gPiA+
ICsNCj4gPiA+ID4gwqAJc3RhdHVzID0gdGlhZGNfcmVhZGwoYWRjX2RldiwgUkVHX0lSUVNUQVRV
Uyk7DQo+ID4gPiA+IMKgDQo+ID4gPiA+IMKgCS8qDQo+ID4gPiA+IEBAIC0xODMsNiArMTg1LDE1
IEBAIHN0YXRpYyBpcnFyZXR1cm5fdCB0aWFkY19pcnFfaChpbnQgaXJxLA0KPiA+ID4gPiB2b2lk
DQo+ID4gPiA+ICpwcml2YXRlKQ0KPiA+ID4gPiDCoAkJdGlhZGNfd3JpdGVsKGFkY19kZXYsIFJF
R19DVFJMLCBjb25maWcpOw0KPiA+ID4gPiDCoAkJdGlhZGNfd3JpdGVsKGFkY19kZXYsIFJFR19J
UlFTVEFUVVMsDQo+ID4gPiA+IElSUUVOQl9GSUZPMU9WUlJVTg0KPiA+ID4gPiDCoAkJCQl8IElS
UUVOQl9GSUZPMVVORFJGTFcgfA0KPiA+ID4gPiBJUlFFTkJfRklGTzFUSFJFUyk7DQo+ID4gPiA+
ICsNCj4gPiA+ID4gKwkJLyogd2FpdCBmb3IgaWRsZSBzdGF0ZS4NCj4gPiA+IFN0YW5kYXJkIG11
bHRpbGluZSBjb21tZW50IHN5bnRheCBwcmVmZXJyZWQuDQo+ID4gPiA+IA0KPiA+ID4gPiANCj4g
PiA+ID4gKwkJwqAqIEFEQyBuZWVkcyB0byBmaW5pc2ggdGhlIGN1cnJlbnQgY29udmVyc2lvbg0K
PiA+ID4gPiArCQnCoCogYmVmb3JlIGRpc2FibGluZyB0aGUgbW9kdWxlDQo+ID4gPiA+ICsJCcKg
Ki8NCj4gPiA+ID4gKwkJZG8gew0KPiA+ID4gPiArCQkJYWRjX2ZzbSA9IHRpYWRjX3JlYWRsKGFk
Y19kZXYsDQo+ID4gPiA+IFJFR19BRENGU00pOw0KPiA+ID4gPiArCQl9IHdoaWxlIChhZGNfZnNt
ICE9IDB4MTAgJiYgY291bnQrKyA8IDEwMCk7DQo+ID4gPiBXaGF0IHNsaWdodGx5IGJvdGhlcnMg
bWUgaXMgdGhhdCB3ZSBhcmUgcGFwZXJpbmcgb3ZlciB0aGUNCj4gPiA+IHByb2JsZW0swqDCoHll
dA0KPiA+ID4gdGhpcyBpcyBmYXIgZnJvbSBndWFyYW50ZWVkIHRvIGZpeCBpdCAoSSB0aGluayEp
IGFzIHdlIG1pZ2h0IGdldA0KPiA+ID4gYQ0KPiA+ID4gcmFjZQ0KPiA+ID4gYXMgaXQgc3RhcnRz
IHRoZSBuZXh0IGNvbnZlcnNpb24gYWZ0ZXIgd2UgaGF2ZSBjaGVja2VkLCBidXQNCj4gPiA+IGJl
Zm9yZSB3ZQ0KPiA+ID4gZGlzYWJsZS4uLg0KPiA+ID4gDQo+ID4gPiBOb3RlIEknbSBvbmx5IGxv
b2tpbmcgYXQgdGhpcyBmYWlybHkgc3VwZXJmaWNpYWxseSBzbyBwbGVhc2UNCj4gPiA+IHBvaW50
DQo+ID4gPiBvdXQgaWYgSSBoYXZlIG1pc3NlZCBzb21ldGhpbmchDQo+ID4gUHJvYmFibHkgbXkg
Y29tbWVudCBpcyBhIGJpdCBjb25mdXNpbmcuIFRoZSBBREMgaGFzIGFscmVhZHkgYmVlbg0KPiA+
IGRpc2FibGVkIGJ5IHdyaXRpbmcgdGhlIGNvbmZpZyBzb21lIGxpbmVzIGFib3ZlIHRoZSB3aGls
ZSBsb29wLsKgDQo+ID4gDQo+ID4gIg0KPiA+IGNvbmZpZyAmPSB+KENOVFJMUkVHX1RTQ1NTRU5C
KTsNCj4gPiB0aWFkY193cml0ZWwoYWRjX2RldiwgUkVHX0NUUkwsIGNvbmZpZyk7DQo+ID4gIg0K
PiA+IEJ1dCBhcyBzdGF0ZWQgaW4gdGhlIFJlZmVyZW5jZSBNYW51YWwgdGhlIEFEQyBuZWVkcyBz
b21lIHRpbWUgdG8NCj4gPiBmaW5pc2gNCj4gPiB0aGUgY3VycmVudCBjb252ZXJzaW9uIHRvIGZp
bmFsbHkgZGlzYWJsZSB0aGUgQURDLsKgDQo+ID4gDQo+ID4gRGVzY3JpcHRpb24gb2YgIkVuYWJs
ZSIgQml0IGluIHRoZSBDVFJMIFJlZ2lzdGVyICh0YWJsZSAxMi0xNCk6DQo+ID4gIlRTQ19BRENf
U1MgbW9kdWxlIGVuYWJsZSBiaXQuDQo+ID4gQWZ0ZXIgcHJvZ3JhbW1pbmcgYWxsIHRoZSBzdGVw
cyBhbmQgY29uZmlndXJhdGlvbiByZWdpc3RlcnMsIHdyaXRlDQo+ID4gYQ0KPiA+IDF0byB0aGlz
IGJpdCB0byB0dXJuIG9uIFRTQ19BRENfU1MuDQo+ID4gV3JpdGluZyBhIDAgd2lsbCBkaXNhYmxl
IHRoZSBtb2R1bGUgKGFmdGVyIHRoZSBjdXJyZW50DQo+ID4gY29udmVyc2lvbikuIg0KPiA+IA0K
PiA+IFRoYXQgbWVhbnMgb25jZSB0aGUgYWRjX2ZzbSBpcyBlcXVhbCB0byAweDEwIChpZGxlIHN0
YXRlKSB0aGUgQURDDQo+ID4gc3RheXMNCj4gPiBkaXNhYmxlZCBhcyBsb25nIGFzIHdlIGVuYWJs
ZSB0aGUgQURDIGFnYWluLiBXaGljaCBpcyBkb25lIGJ5IHRoZQ0KPiA+IG9yaWdpbmFsIGNvZGUg
b25lIGxpbmUgYmVsb3cgb2YgdGhpcyBwYXRjaC4NCj4gQ29vbC4gVGhhbmtzIGZvciB0aGUgY2xh
cmlmaWNhdGlvbi7CoMKgSSBnb3QgbG9zdCBpbiB0aGUgZGF0YWhzaGVldCA7KQ0KPiANCj4gV291
bGQgeW91IG1pbmQgZmlndXJpbmcgb3V0IGEgZml4ZXMgdGFnIGZvciB0aGlzIG9uZSBhcyBJIHdh
bnQgaXQgdG8NCj4gZ28gZm9yIHN0YWJsZT8NCg0KRnJvbSA5M2VkNmUxZDRjZmQzNGYzMGUwMjAx
NDA3YzkyZmY4YWYxMjlhNjg5IE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQ0KRnJvbTogTWljaGFl
bCBFbmdsIDxtaWNoYWVsLmVuZ2xAd2p3LXNvbHV0aW9ucy5jb20+DQpEYXRlOiBXZWQsIDE1IE1h
ciAyMDE3IDE5OjQ1OjM5ICswMTAwDQpTdWJqZWN0OiBbUEFUQ0hdIGlpbzogYWRjOiB0aV9hbTMz
NXg6IGZpeCBmaWZvIG92ZXJydW4gcmVjb3ZlcnkNCg0KVGhlIHRpYWRjX2lycV9oKGludCBpcnEs
IHZvaWQgKnByaXZhdGUpIGZ1bmN0aW9uIGlzIGhhbmRsaW5nIEZJRk8NCm92ZXJydW5zIGJ5IGNs
ZWFyaW5nIGZsYWdzLCBkaXNhYmxpbmcgYW5kIGVuYWJsaW5nIHRoZSBBREMgdG8NCnJlY292ZXIu
DQoNCklmIHRoZSBBREMgaXMgcnVubmluZyBpbiBjb250aW51b3VzIG1vZGUgYSBGSUZPIG92ZXJy
dW4gaGFwcGVucw0KcmVndWxhbHkuIElmIHRoZSBkaXNhYmxpbmcgb2YgdGhlIEFEQyBoYXBwZW5z
IGNvbmN1cnJlbnRseSB3aXRoDQphIG5ldyBjb252ZXJzaW9uLiBJdCBtaWdodCBoYXBwZW4gdGhh
dCB0aGUgZW5hYmxpbmcgb2YgdGhlIEFEQw0KaXMgaWdub3JlZCBieSB0aGUgaGFyZHdhcmUuIFRo
aXMgc3RvcHMgdGhlIEFEQyBwZXJtYW5lbnRseS4gTm8NCm1vcmUgaW50ZXJydXB0IGFyZSB0cmln
Z2VyZWQuDQoNCkFjY29yZGluZyB0byB0aGUgQU0zMzV4IFJlZmVyZW5jZSBNYW51YWwgKFNQUlVI
NzNIIE9jdG9iZXIgMjAxMSAtDQpSZXZpc2VkIEFwcmlsIDIwMTMgQ2hhcHRlciAxMi40IGFuZCAx
Mi41KSBpdCBpcyBuZWNlc3NhcnkgdG8NCmNoZWNrIHRoZSBBREMgRlNNIGJpdHMgaW4gUkVHX0FE
Q0ZTTSBiZWZvcmUgZW5hYmxpbmcgdGhlIEFEQw0KYWdhaW4uIEJlY2F1c2UgdGhlIGRpc2FibGlu
ZyBvZiB0aGUgQURDIGlzIGRvbmUgcmlnaHQgYWZ0ZXIgdGhlDQpjdXJyZW50IGNvbnZlcnNpb24g
aGFzIGJlZW4gZmluaXNoZWQuDQoNClRvIHRyaWdnZXIgdGhpcyBidWcgaXQgaXMgbmVjZXNzYXJ5
IHRvIHJ1biB0aGUgQURDIGluIGNvbnRpbnVvdXMNCm1vZGUuIFRoZSBBREMgdmFsdWVzIG9mIGFs
bCBjaGFubmVscyBuZWVkIHRvIGJlIHJlYWQgaW4gYW4gZW5kbGVzcw0KbG9vcC4gVGhlIGJ1ZyBh
cHBlYXJzIHdpdGhpbiB0aGUgZmlyc3QgNiBob3VycyAofjUuNCBtaWxsaW9uDQpoYW5kbGVkIEZJ
Rk8gb3ZlcnJ1bnMpLiBUaGUgdXNlciBzcGFjZSBhcHBsaWNhdGlvbiB3aWxsIGhhbmcgb24NCnJl
YWRpbmcgbmV3IHZhbHVlcyBmcm9tIHRoZSBjaGFyIGRldmljZS4NCg0KRml4ZXM6IGNhOWE1NjM4
MDVmN2EgKCJpaW86IHRpX2FtMzM1eF9hZGM6IEFkZCBjb250aW51b3VzIHNhbXBsaW5nDQpzdXBw
b3J0IikNClNpZ25lZC1vZmYtYnk6IE1pY2hhZWwgRW5nbCA8bWljaGFlbC5lbmdsQHdqdy1zb2x1
dGlvbnMuY29tPg0KLS0tDQrCoGRyaXZlcnMvaWlvL2FkYy90aV9hbTMzNXhfYWRjLmMgfCAxMyAr
KysrKysrKysrKystDQrCoDEgZmlsZSBjaGFuZ2VkLCAxMiBpbnNlcnRpb25zKCspLCAxIGRlbGV0
aW9uKC0pDQoNCmRpZmYgLS1naXQgYS9kcml2ZXJzL2lpby9hZGMvdGlfYW0zMzV4X2FkYy5jDQpi
L2RyaXZlcnMvaWlvL2FkYy90aV9hbTMzNXhfYWRjLmMNCmluZGV4IGFkOWRlYzMwYmIzMC4uNzUy
ZTkyZmZjZTk0IDEwMDY0NA0KLS0tIGEvZHJpdmVycy9paW8vYWRjL3RpX2FtMzM1eF9hZGMuYw0K
KysrIGIvZHJpdmVycy9paW8vYWRjL3RpX2FtMzM1eF9hZGMuYw0KQEAgLTE2OSw3ICsxNjksOSBA
QCBzdGF0aWMgaXJxcmV0dXJuX3QgdGlhZGNfaXJxX2goaW50IGlycSwgdm9pZA0KKnByaXZhdGUp
DQrCoHsNCsKgCXN0cnVjdCBpaW9fZGV2ICppbmRpb19kZXYgPSBwcml2YXRlOw0KwqAJc3RydWN0
IHRpYWRjX2RldmljZSAqYWRjX2RldiA9IGlpb19wcml2KGluZGlvX2Rldik7DQotCXVuc2lnbmVk
IGludCBzdGF0dXMsIGNvbmZpZzsNCisJdW5zaWduZWQgaW50IHN0YXR1cywgY29uZmlnLCBhZGNf
ZnNtOw0KKwl1bnNpZ25lZCBzaG9ydCBjb3VudCA9IDA7DQorDQrCoAlzdGF0dXMgPSB0aWFkY19y
ZWFkbChhZGNfZGV2LCBSRUdfSVJRU1RBVFVTKTsNCsKgDQrCoAkvKg0KQEAgLTE4Myw2ICsxODUs
MTUgQEAgc3RhdGljIGlycXJldHVybl90IHRpYWRjX2lycV9oKGludCBpcnEsIHZvaWQNCipwcml2
YXRlKQ0KwqAJCXRpYWRjX3dyaXRlbChhZGNfZGV2LCBSRUdfQ1RSTCwgY29uZmlnKTsNCsKgCQl0
aWFkY193cml0ZWwoYWRjX2RldiwgUkVHX0lSUVNUQVRVUywNCklSUUVOQl9GSUZPMU9WUlJVTg0K
wqAJCQkJfCBJUlFFTkJfRklGTzFVTkRSRkxXIHwNCklSUUVOQl9GSUZPMVRIUkVTKTsNCisNCisJ
CS8qIHdhaXQgZm9yIGlkbGUgc3RhdGUuDQorCQnCoMKgwqBBREMgbmVlZHMgdG8gZmluaXNoIHRo
ZSBjdXJyZW50IGNvbnZlcnNpb24NCisJCcKgwqDCoGJlZm9yZSBkaXNhYmxpbmcgdGhlIG1vZHVs
ZQ0KKwkJKi8NCisJCWRvIHsNCisJCQlhZGNfZnNtID0gdGlhZGNfcmVhZGwoYWRjX2RldiwgUkVH
X0FEQ0ZTTSk7DQorCQl9IHdoaWxlIChhZGNfZnNtICE9IDB4MTAgJiYgY291bnQrKyA8IDEwMCk7
DQorDQrCoAkJdGlhZGNfd3JpdGVsKGFkY19kZXYsIFJFR19DVFJMLCAoY29uZmlnIHwNCkNOVFJM
UkVHX1RTQ1NTRU5CKSk7DQrCoAkJcmV0dXJuIElSUV9IQU5ETEVEOw0KwqAJfSBlbHNlIGlmIChz
dGF0dXMgJiBJUlFFTkJfRklGTzFUSFJFUykgew0KLS3CoA0KMi43LjQNCg0KPiANCj4gVGhhbmtz
LA0KPiANCj4gSm9uYXRoYW4NCj4gPiANCj4gPiANCj4gPiA+IA0KPiA+ID4gDQo+ID4gPiBXZSBj
b3VsZCBsb29rIGZvciBhIHRyYW5zaXRpb24sIHdoaWNoIHdvdWxkIHBvdGVudGlhbGx5IHJlZHVj
ZQ0KPiA+ID4gdGhlDQo+ID4gPiBjaGFuY2UNCj4gPiA+IG9mIHByb2JsZW1zLCBidXQgdGhhdCdz
IG5hc3R5IGFzIHdlbGwgYW5kIHdlJ2QgbmVlZCB0byBkaXNhYmxlDQo+ID4gPiBpbnRlcnJ1cHRz
DQo+ID4gPiBwcm9iYWJseSB0byBhdm9pZCBhbnkgcG90ZW50aWFsIG9mIGEgY2xhc2ggKGZvciBh
IG5vbiB0cml2aWFsDQo+ID4gPiBhbW91bnQNCj4gPiA+IG9mDQo+ID4gPiB0aW1lLi4uKQ0KPiA+
ID4gPiANCj4gPiA+ID4gDQo+ID4gPiA+ICsNCj4gPiA+ID4gwqAJCXRpYWRjX3dyaXRlbChhZGNf
ZGV2LCBSRUdfQ1RSTCwgKGNvbmZpZyB8DQo+ID4gPiA+IENOVFJMUkVHX1RTQ1NTRU5CKSk7DQo+
ID4gPiA+IMKgCQlyZXR1cm4gSVJRX0hBTkRMRUQ7DQo+ID4gPiA+IMKgCX0gZWxzZSBpZiAoc3Rh
dHVzICYgSVJRRU5CX0ZJRk8xVEhSRVMpIHsNCg==
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: adc: ti_am335x_adc: fix fifo overrun recovery
2017-03-15 18:59 ` Michael Engl
@ 2017-03-15 19:49 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2017-03-15 19:49 UTC (permalink / raw)
To: Michael Engl; +Cc: knaack.h, lars, pmeerw, linux-iio
On 15/03/17 18:59, Michael Engl wrote:
> On Mon, 2017-03-13 at 21:22 +0000, Jonathan Cameron wrote:
>> On 11/03/17 19:32, Michael Engl wrote:
>>>
>>> On Sam, 2017-03-11 at 18:07 +0000, Jonathan Cameron wrote:
>>>>
>>>> On 10/03/17 13:57, Michael Engl wrote:
>>>>>
>>>>>
>>>>> The tiadc_irq_h(int irq, void *private) function is handling
>>>>> FIFO
>>>>> overruns by clearing flags, disabling and enabling the ADC to
>>>>> recover.
>>>>>
>>>>> If the ADC is running in continuous mode a FIFO overrun happens
>>>>> regularly. If the disabling of the ADC happens concurrently
>>>>> with
>>>>> a new conversion. It might happen that the enabling of the ADC
>>>>> is ignored by the hardware. This stops the ADC permanently. No
>>>>> more interrupts are triggered.
>>>>>
>>>>> According to the AM335x Reference Manual (SPRUH73H October 2011
>>>>> -
>>>>> Revised April 2013 - Chapter 12.4 and 12.5) it is necessary to
>>>>> check the ADC FSM bits in REG_ADCFSM before enabling the ADC
>>>>> again. Because the disabling of the ADC is done right after the
>>>>> current conversion has been finished.
>>>> The relevant comment in the latest version of that manual (rev O)
>>>> is at the end of 12.3.5
>>>> "The FIFO can also generate FIFOx_OVERRUN and FIFOx_UNDERFLOW
>>>> interrupts. The user can mask
>>>> these events by programming the IRQENABLE_CLR register. To clear
>>>> a
>>>> FIFO underflow or FIFO overrun
>>>> interrupt, the user should write a ‘1’ to the status bit in the
>>>> IRQSTS register. The TSC_ADC_SS does not
>>>> recover from these conditions automatically. Therefore, the
>>>> software
>>>> will need to disable and then again
>>>> enable the TSC_ADC_SS. Before the user can turn the module back
>>>> on,
>>>> the user must read the
>>>> ADCSTAT register to check if the status of the ADC FSM is idle
>>>> and
>>>> the current step is the idle step."
>>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>
>>>>> To trigger this bug it is necessary to run the ADC in
>>>>> continuous
>>>>> mode. The ADC values of all channels need to be read in an
>>>>> endless
>>>>> loop. The bug appears within the first 6 hours (~5.4 million
>>>>> handled FIFO overruns). The user space application will hang on
>>>>> reading new values from the character device.
>>>> Impressive to track this one down, but it's the sort of nasty
>>>> hardware race that we need to be very careful with when trying
>>>> to get a fix in place.
>>>>>
>>>>>
>>>>>
>>>>> Signed-off-by: Michael Engl <michael.engl@wjw-solutions.com>
>>>>> ---
>>>>> drivers/iio/adc/ti_am335x_adc.c | 13 ++++++++++++-
>>>>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/iio/adc/ti_am335x_adc.c
>>>>> b/drivers/iio/adc/ti_am335x_adc.c
>>>>> index ad9dec3..4282cec 100644
>>>>> --- a/drivers/iio/adc/ti_am335x_adc.c
>>>>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>>>>> @@ -169,7 +169,9 @@ static irqreturn_t tiadc_irq_h(int irq,
>>>>> void
>>>>> *private)
>>>>> {
>>>>> struct iio_dev *indio_dev = private;
>>>>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>>>>> - unsigned int status, config;
>>>>> + unsigned int status, config, adc_fsm;
>>>>> + unsigned short count = 0;
>>>>> +
>>>>> status = tiadc_readl(adc_dev, REG_IRQSTATUS);
>>>>>
>>>>> /*
>>>>> @@ -183,6 +185,15 @@ static irqreturn_t tiadc_irq_h(int irq,
>>>>> void
>>>>> *private)
>>>>> tiadc_writel(adc_dev, REG_CTRL, config);
>>>>> tiadc_writel(adc_dev, REG_IRQSTATUS,
>>>>> IRQENB_FIFO1OVRRUN
>>>>> | IRQENB_FIFO1UNDRFLW |
>>>>> IRQENB_FIFO1THRES);
>>>>> +
>>>>> + /* wait for idle state.
>>>> Standard multiline comment syntax preferred.
>>>>>
>>>>>
>>>>> + * ADC needs to finish the current conversion
>>>>> + * before disabling the module
>>>>> + */
>>>>> + do {
>>>>> + adc_fsm = tiadc_readl(adc_dev,
>>>>> REG_ADCFSM);
>>>>> + } while (adc_fsm != 0x10 && count++ < 100);
>>>> What slightly bothers me is that we are papering over the
>>>> problem, yet
>>>> this is far from guaranteed to fix it (I think!) as we might get
>>>> a
>>>> race
>>>> as it starts the next conversion after we have checked, but
>>>> before we
>>>> disable...
>>>>
>>>> Note I'm only looking at this fairly superficially so please
>>>> point
>>>> out if I have missed something!
>>> Probably my comment is a bit confusing. The ADC has already been
>>> disabled by writing the config some lines above the while loop.
>>>
>>> "
>>> config &= ~(CNTRLREG_TSCSSENB);
>>> tiadc_writel(adc_dev, REG_CTRL, config);
>>> "
>>> But as stated in the Reference Manual the ADC needs some time to
>>> finish
>>> the current conversion to finally disable the ADC.
>>>
>>> Description of "Enable" Bit in the CTRL Register (table 12-14):
>>> "TSC_ADC_SS module enable bit.
>>> After programming all the steps and configuration registers, write
>>> a
>>> 1to this bit to turn on TSC_ADC_SS.
>>> Writing a 0 will disable the module (after the current
>>> conversion)."
>>>
>>> That means once the adc_fsm is equal to 0x10 (idle state) the ADC
>>> stays
>>> disabled as long as we enable the ADC again. Which is done by the
>>> original code one line below of this patch.
>> Cool. Thanks for the clarification. I got lost in the datahsheet ;)
>>
>> Would you mind figuring out a fixes tag for this one as I want it to
>> go for stable?
>
> From 93ed6e1d4cfd34f30e0201407c92ff8af129a689 Mon Sep 17 00:00:00 2001
> From: Michael Engl <michael.engl@wjw-solutions.com>
> Date: Wed, 15 Mar 2017 19:45:39 +0100
> Subject: [PATCH] iio: adc: ti_am335x: fix fifo overrun recovery
>
> The tiadc_irq_h(int irq, void *private) function is handling FIFO
> overruns by clearing flags, disabling and enabling the ADC to
> recover.
>
> If the ADC is running in continuous mode a FIFO overrun happens
> regulaly. If the disabling of the ADC happens concurrently with
> a new conversion. It might happen that the enabling of the ADC
> is ignored by the hardware. This stops the ADC permanently. No
> more interrupt are triggered.
>
> According to the AM335x Reference Manual (SPRUH73H October 2011 -
> Revised April 2013 Chapter 12.4 and 12.5) it is necessary to
> check the ADC FSM bits in REG_ADCFSM before enabling the ADC
> again. Because the disabling of the ADC is done right after the
> current conversion has been finished.
>
> To trigger this bug it is necessary to run the ADC in continuous
> mode. The ADC values of all channels need to be read in an endless
> loop. The bug appears within the first 6 hours (~5.4 million
> handled FIFO overruns). The user space application will hang on
> reading new values from the char device.
>
> Fixes: ca9a563805f7a ("iio: ti_am335x_adc: Add continuous sampling
> support")
> Signed-off-by: Michael Engl <michael.engl@wjw-solutions.com>
As a general rule please don't paste a patch inside another thread.
It makes it really fiddly to apply. All I was after was the fixes
line!
Anyhow, thanks for tracking this down.
Applied to the fixes-togreg branch of iio.git.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ti_am335x_adc.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c
> b/drivers/iio/adc/ti_am335x_adc.c
> index ad9dec30bb30..752e92ffce94 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -169,7 +169,9 @@ static irqreturn_t tiadc_irq_h(int irq, void
> *private)
> {
> struct iio_dev *indio_dev = private;
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> - unsigned int status, config;
> + unsigned int status, config, adc_fsm;
> + unsigned short count = 0;
> +
> status = tiadc_readl(adc_dev, REG_IRQSTATUS);
>
> /*
> @@ -183,6 +185,15 @@ static irqreturn_t tiadc_irq_h(int irq, void
> *private)
> tiadc_writel(adc_dev, REG_CTRL, config);
> tiadc_writel(adc_dev, REG_IRQSTATUS,
> IRQENB_FIFO1OVRRUN
> | IRQENB_FIFO1UNDRFLW |
> IRQENB_FIFO1THRES);
> +
> + /* wait for idle state.
> + ADC needs to finish the current conversion
> + before disabling the module
> + */
> + do {
> + adc_fsm = tiadc_readl(adc_dev, REG_ADCFSM);
> + } while (adc_fsm != 0x10 && count++ < 100);
> +
> tiadc_writel(adc_dev, REG_CTRL, (config |
> CNTRLREG_TSCSSENB));
> return IRQ_HANDLED;
> } else if (status & IRQENB_FIFO1THRES) {
> --
> 2.7.4
>
>>
>> Thanks,
>>
>> Jonathan
>>>
>>>
>>>>
>>>>
>>>> We could look for a transition, which would potentially reduce
>>>> the
>>>> chance
>>>> of problems, but that's nasty as well and we'd need to disable
>>>> interrupts
>>>> probably to avoid any potential of a clash (for a non trivial
>>>> amount
>>>> of
>>>> time...)
>>>>>
>>>>>
>>>>> +
>>>>> tiadc_writel(adc_dev, REG_CTRL, (config |
>>>>> CNTRLREG_TSCSSENB));
>>>>> return IRQ_HANDLED;
>>>>> } else if (status & IRQENB_FIFO1THRES) {
> N�����r��y���b�X��ǧv�^�){.n�+����{��*"��^n�r���z�\x1a��h����&��\x1e�G���h�\x03(�階�ݢj"��\x1a�^[m�����z�ޖ���f���h���~�mml==
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-15 19:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-10 13:57 [PATCH] iio: adc: ti_am335x_adc: fix fifo overrun recovery Michael Engl
2017-03-11 18:07 ` Jonathan Cameron
2017-03-11 19:32 ` Michael Engl
2017-03-13 21:22 ` Jonathan Cameron
2017-03-15 18:59 ` Michael Engl
2017-03-15 19:49 ` Jonathan Cameron
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.