[v3,3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
diff mbox

Message ID 1415694844-11230-4-git-send-email-vigneshr@ti.com
State New, archived
Headers show

Commit Message

Vignesh Raghavendra Nov. 11, 2014, 8:34 a.m. UTC
In one shot mode, sequencer automatically disables all enabled steps at
the end of each cycle. (both ADC steps and TSC steps) Hence these steps
need not be saved in reg_se_cache for clearing these steps at a later
stage.
Also, when ADC wakes up Sequencer should not be busy executing any of the
config steps except for the charge step. Previously charge step was 1 ADC
clock cycle and hence it was ignored.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/mfd/ti_am335x_tscadc.c       | 7 +++++--
 include/linux/mfd/ti_am335x_tscadc.h | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Lee Jones Nov. 11, 2014, 12:26 p.m. UTC | #1
On Tue, 11 Nov 2014, Vignesh R wrote:

> In one shot mode, sequencer automatically disables all enabled steps at
> the end of each cycle. (both ADC steps and TSC steps) Hence these steps
> need not be saved in reg_se_cache for clearing these steps at a later
> stage.
> Also, when ADC wakes up Sequencer should not be busy executing any of the
> config steps except for the charge step. Previously charge step was 1 ADC
> clock cycle and hence it was ignored.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---

You need to provide a changelog here when submitting new versions.

>  drivers/mfd/ti_am335x_tscadc.c       | 7 +++++--
>  include/linux/mfd/ti_am335x_tscadc.h | 1 +
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index d877e777cce6..110038859a8d 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -86,8 +86,12 @@ static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
>  		spin_lock_irq(&tsadc->reg_lock);
>  		finish_wait(&tsadc->reg_se_wait, &wait);
>  
> +		/*
> +		 * Sequencer should either be idle or
> +		 * busy applying the charge step.
> +		 */
>  		reg = tscadc_readl(tsadc, REG_ADCFSM);
> -		WARN_ON(reg & SEQ_STATUS);
> +		WARN_ON((reg & SEQ_STATUS) && !(reg & CHARGE_STEP));
>  		tsadc->adc_waiting = false;
>  	}
>  	tsadc->adc_in_use = true;
> @@ -96,7 +100,6 @@ static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
>  void am335x_tsc_se_set_once(struct ti_tscadc_dev *tsadc, u32 val)
>  {
>  	spin_lock_irq(&tsadc->reg_lock);
> -	tsadc->reg_se_cache |= val;

You didn't answer my question about this?

>  	am335x_tscadc_need_adc(tsadc);
>  
>  	tscadc_writel(tsadc, REG_SE, val);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index c99be5dc0f5c..fcce182e4a35 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -128,6 +128,7 @@
>  
>  /* Sequencer Status */
>  #define SEQ_STATUS BIT(5)
> +#define CHARGE_STEP		0x11
>  
>  #define ADC_CLK			3000000
>  #define TOTAL_STEPS		16
Vignesh Raghavendra Nov. 11, 2014, 3:26 p.m. UTC | #2
DQpPbiBUdWUsIDExIE5vdiAyMDE0LCBWaWduZXNoIFIgd3JvdGU6DQoNCj4gSW4gb25lIHNob3Qg
bW9kZSwgc2VxdWVuY2VyIGF1dG9tYXRpY2FsbHkgZGlzYWJsZXMgYWxsIGVuYWJsZWQgc3RlcHMg
DQo+IGF0IHRoZSBlbmQgb2YgZWFjaCBjeWNsZS4gKGJvdGggQURDIHN0ZXBzIGFuZCBUU0Mgc3Rl
cHMpIEhlbmNlIHRoZXNlIA0KPiBzdGVwcyBuZWVkIG5vdCBiZSBzYXZlZCBpbiByZWdfc2VfY2Fj
aGUgZm9yIGNsZWFyaW5nIHRoZXNlIHN0ZXBzIGF0IGEgDQo+IGxhdGVyIHN0YWdlLg0KPiBBbHNv
LCB3aGVuIEFEQyB3YWtlcyB1cCBTZXF1ZW5jZXIgc2hvdWxkIG5vdCBiZSBidXN5IGV4ZWN1dGlu
ZyBhbnkgb2YgDQo+IHRoZSBjb25maWcgc3RlcHMgZXhjZXB0IGZvciB0aGUgY2hhcmdlIHN0ZXAu
IFByZXZpb3VzbHkgY2hhcmdlIHN0ZXAgDQo+IHdhcyAxIEFEQyBjbG9jayBjeWNsZSBhbmQgaGVu
Y2UgaXQgd2FzIGlnbm9yZWQuDQo+IA0KPiBTaWduZWQtb2ZmLWJ5OiBWaWduZXNoIFIgPHZpZ25l
c2hyQHRpLmNvbT4NCj4gLS0tDQoNCllvdSBuZWVkIHRvIHByb3ZpZGUgYSBjaGFuZ2Vsb2cgaGVy
ZSB3aGVuIHN1Ym1pdHRpbmcgbmV3IHZlcnNpb25zLg0KDQpJIGhhdmUgaW5jbHVkZWQgY2hhbmdl
bG9nIGluIHRoZSBjb3ZlciBwYWdlLiBXaWxsIGluY2x1ZGUgdGhlbSBpbiANCmluZGl2aWR1YWwg
cGF0Y2hlcyBuZXh0IHRpbWUuDQoNCj4gIGRyaXZlcnMvbWZkL3RpX2FtMzM1eF90c2NhZGMuYyAg
ICAgICB8IDcgKysrKystLQ0KPiAgaW5jbHVkZS9saW51eC9tZmQvdGlfYW0zMzV4X3RzY2FkYy5o
IHwgMSArDQo+ICAyIGZpbGVzIGNoYW5nZWQsIDYgaW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMo
LSkNCj4gDQo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL21mZC90aV9hbTMzNXhfdHNjYWRjLmMgDQo+
IGIvZHJpdmVycy9tZmQvdGlfYW0zMzV4X3RzY2FkYy5jIGluZGV4IGQ4NzdlNzc3Y2NlNi4uMTEw
MDM4ODU5YThkIA0KPiAxMDA2NDQNCj4gLS0tIGEvZHJpdmVycy9tZmQvdGlfYW0zMzV4X3RzY2Fk
Yy5jDQo+ICsrKyBiL2RyaXZlcnMvbWZkL3RpX2FtMzM1eF90c2NhZGMuYw0KPiBAQCAtODYsOCAr
ODYsMTIgQEAgc3RhdGljIHZvaWQgYW0zMzV4X3RzY2FkY19uZWVkX2FkYyhzdHJ1Y3QgdGlfdHNj
YWRjX2RldiAqdHNhZGMpDQo+ICAJCXNwaW5fbG9ja19pcnEoJnRzYWRjLT5yZWdfbG9jayk7DQo+
ICAJCWZpbmlzaF93YWl0KCZ0c2FkYy0+cmVnX3NlX3dhaXQsICZ3YWl0KTsNCj4gIA0KPiArCQkv
Kg0KPiArCQkgKiBTZXF1ZW5jZXIgc2hvdWxkIGVpdGhlciBiZSBpZGxlIG9yDQo+ICsJCSAqIGJ1
c3kgYXBwbHlpbmcgdGhlIGNoYXJnZSBzdGVwLg0KPiArCQkgKi8NCj4gIAkJcmVnID0gdHNjYWRj
X3JlYWRsKHRzYWRjLCBSRUdfQURDRlNNKTsNCj4gLQkJV0FSTl9PTihyZWcgJiBTRVFfU1RBVFVT
KTsNCj4gKwkJV0FSTl9PTigocmVnICYgU0VRX1NUQVRVUykgJiYgIShyZWcgJiBDSEFSR0VfU1RF
UCkpOw0KPiAgCQl0c2FkYy0+YWRjX3dhaXRpbmcgPSBmYWxzZTsNCj4gIAl9DQo+ICAJdHNhZGMt
PmFkY19pbl91c2UgPSB0cnVlOw0KPiBAQCAtOTYsNyArMTAwLDYgQEAgc3RhdGljIHZvaWQgYW0z
MzV4X3RzY2FkY19uZWVkX2FkYyhzdHJ1Y3QgDQo+IHRpX3RzY2FkY19kZXYgKnRzYWRjKSAgdm9p
ZCBhbTMzNXhfdHNjX3NlX3NldF9vbmNlKHN0cnVjdCANCj4gdGlfdHNjYWRjX2RldiAqdHNhZGMs
IHUzMiB2YWwpICB7DQo+ICAJc3Bpbl9sb2NrX2lycSgmdHNhZGMtPnJlZ19sb2NrKTsNCj4gLQl0
c2FkYy0+cmVnX3NlX2NhY2hlIHw9IHZhbDsNCg0KWW91IGRpZG4ndCBhbnN3ZXIgbXkgcXVlc3Rp
b24gYWJvdXQgdGhpcz8NCkkgZGlkIHJlcGx5IHRvIHRoZSBxdWVzdGlvbiBpbiB0aGUgcHJldmlv
dXMgdGhyZWFkLiAgDQoNClByZXZpb3VzbHksIFRTQyBkaWQgbm90IHJlbGlhYmx5IHJlLWVuYWJs
ZSBpdHMgc3RlcHMgYXMgdGhlIFRTQyBpcnEgaGFuZGxlciByZWNlaXZlZCANCmZhbHNlIHBlbiB1
cCBldmVudHMuIEhlbmNlLCBpbiBvcmRlciB0byB1c2UgVFNDIGFmdGVyIEFEQyBvcGVyYXRpb24s
IGl0IHdhcyBuZWNlc3NhcnkgdG8NCiBzYXZlIGFuZCByZS1lbmFibGUgVFNDIHN0ZXBzIChiYXNp
Y2FsbHksIHRvIGtlZXAgVFNDIHN0ZXBzIGVuYWJsZWQgYWx3YXlzKS4NClRoZSBjaGFuZ2Ugd2Fz
IG1vcmUgb2YgYSB3b3JrYXJvdW5kIHRvIG92ZXJjb21lIGxpbWl0YXRpb24gb2YgVFNDIGlycSBo
YW5kbGVyLiBXaXRoIA0KdGhpcyBzZXJpZXMgb2YgcGF0Y2hlcywgVFNDIGlycSBoYW5kbGVyIGlz
IHZlcnkgcmVsaWFibGUgYW5kIHRoZSB3b3JrYXJvdW5kIGlzIG5vIGxvbmdlciByZXF1aXJlZC4N
Cg0KPiAgCWFtMzM1eF90c2NhZGNfbmVlZF9hZGModHNhZGMpOw0KPiAgDQo+ICAJdHNjYWRjX3dy
aXRlbCh0c2FkYywgUkVHX1NFLCB2YWwpOw0KPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9saW51eC9t
ZmQvdGlfYW0zMzV4X3RzY2FkYy5oIA0KPiBiL2luY2x1ZGUvbGludXgvbWZkL3RpX2FtMzM1eF90
c2NhZGMuaA0KPiBpbmRleCBjOTliZTVkYzBmNWMuLmZjY2UxODJlNGEzNSAxMDA2NDQNCj4gLS0t
IGEvaW5jbHVkZS9saW51eC9tZmQvdGlfYW0zMzV4X3RzY2FkYy5oDQo+ICsrKyBiL2luY2x1ZGUv
bGludXgvbWZkL3RpX2FtMzM1eF90c2NhZGMuaA0KPiBAQCAtMTI4LDYgKzEyOCw3IEBADQo+ICAN
Cj4gIC8qIFNlcXVlbmNlciBTdGF0dXMgKi8NCj4gICNkZWZpbmUgU0VRX1NUQVRVUyBCSVQoNSkN
Cj4gKyNkZWZpbmUgQ0hBUkdFX1NURVAJCTB4MTENCj4gIA0KPiAgI2RlZmluZSBBRENfQ0xLCQkJ
MzAwMDAwMA0KPiAgI2RlZmluZSBUT1RBTF9TVEVQUwkJMTYNCg0KLS0NCkxlZSBKb25lcw0KTGlu
YXJvIFNUTWljcm9lbGVjdHJvbmljcyBMYW5kaW5nIFRlYW0gTGVhZCBMaW5hcm8ub3JnIOKUgiBP
cGVuIHNvdXJjZSBzb2Z0d2FyZSBmb3IgQVJNIFNvQ3MgRm9sbG93IExpbmFybzogRmFjZWJvb2sg
fCBUd2l0dGVyIHwgQmxvZw0K
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lee Jones Nov. 13, 2014, 10:25 a.m. UTC | #3
On Tue, 11 Nov 2014, R, Vignesh wrote:
> On Tue, 11 Nov 2014, Vignesh R wrote:
> > In one shot mode, sequencer automatically disables all enabled steps 
> > at the end of each cycle. (both ADC steps and TSC steps) Hence these 
> > steps need not be saved in reg_se_cache for clearing these steps at a 
> > later stage.
> > Also, when ADC wakes up Sequencer should not be busy executing any of 
> > the config steps except for the charge step. Previously charge step 
> > was 1 ADC clock cycle and hence it was ignored.
> > 
> > Signed-off-by: Vignesh R <vigneshr@ti.com>
> > ---

[...]

> > -	tsadc->reg_se_cache |= val;
> 
> You didn't answer my question about this?
> I did reply to the question in the previous thread.  
> 
> Previously, TSC did not reliably re-enable its steps as the TSC irq handler received 
> false pen up events. Hence, in order to use TSC after ADC operation, it was necessary to
>  save and re-enable TSC steps (basically, to keep TSC steps enabled always).
> The change was more of a workaround to overcome limitation of TSC irq handler. With 
> this series of patches, TSC irq handler is very reliable and the workaround is no longer required.

Okay, thanks for the explanation.

By the way, your mailer doesn't appear to quote previous messages.  Is
there any way to fix that?

> >  	am335x_tscadc_need_adc(tsadc);
> >  
> >  	tscadc_writel(tsadc, REG_SE, val);
> > diff --git a/include/linux/mfd/ti_am335x_tscadc.h 
> > b/include/linux/mfd/ti_am335x_tscadc.h
> > index c99be5dc0f5c..fcce182e4a35 100644
> > --- a/include/linux/mfd/ti_am335x_tscadc.h
> > +++ b/include/linux/mfd/ti_am335x_tscadc.h
> > @@ -128,6 +128,7 @@
> >  
> >  /* Sequencer Status */
> >  #define SEQ_STATUS BIT(5)
> > +#define CHARGE_STEP		0x11
> >  
> >  #define ADC_CLK			3000000
> >  #define TOTAL_STEPS		16
>

Patch
diff mbox

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index d877e777cce6..110038859a8d 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -86,8 +86,12 @@  static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
 		spin_lock_irq(&tsadc->reg_lock);
 		finish_wait(&tsadc->reg_se_wait, &wait);
 
+		/*
+		 * Sequencer should either be idle or
+		 * busy applying the charge step.
+		 */
 		reg = tscadc_readl(tsadc, REG_ADCFSM);
-		WARN_ON(reg & SEQ_STATUS);
+		WARN_ON((reg & SEQ_STATUS) && !(reg & CHARGE_STEP));
 		tsadc->adc_waiting = false;
 	}
 	tsadc->adc_in_use = true;
@@ -96,7 +100,6 @@  static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
 void am335x_tsc_se_set_once(struct ti_tscadc_dev *tsadc, u32 val)
 {
 	spin_lock_irq(&tsadc->reg_lock);
-	tsadc->reg_se_cache |= val;
 	am335x_tscadc_need_adc(tsadc);
 
 	tscadc_writel(tsadc, REG_SE, val);
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index c99be5dc0f5c..fcce182e4a35 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -128,6 +128,7 @@ 
 
 /* Sequencer Status */
 #define SEQ_STATUS BIT(5)
+#define CHARGE_STEP		0x11
 
 #define ADC_CLK			3000000
 #define TOTAL_STEPS		16