1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
|
From eaa55efc0b559abebbcf2acea6ce267cea4c26f2 Mon Sep 17 00:00:00 2001
From: Andy Green <andy@openmoko.com>
Date: Wed, 2 Jul 2008 22:41:52 +0100
Subject: [PATCH] fix-pcf50633-disable-irq-from-suspend-until-resume.patch
Disable pcf interrupt (not for wake, just as interrupt) in
suspend, re-enable it again just before we force-call the
workqueue function at end of pcf resume, which leads to
pcf interrupt source registers getting cleared so it can
signal an interrupt normally again.
This change ends the uncontrolled appearance of pcf interrupts
during resume time which previously caused the work to attempt
to use the I2C stuff before i2c host device had itself resumed.
Now the isr work is only queued, and the isr work function called,
definitively after pcf resume completes.
In suspend time, the work function may have been queued some
time before and be pending, and it could still show up at a
bad time. Therefore if the work function sees that it is
coming since the start of pcf50633 suspend function, it
aborts without attempting to read the pcf interrupt regs,
leaving them for resume to take care of.
USB current limit and no battery work functions are also made
aware of suspend state and act accordingly.
Lastly I noticed that in early resume, i2c_get_clientdata(&pcf->client)
returns NULL, presumably because i2c device is still suspended. This
could easily make trouble for async events like interrupt work,
since pcf pointer is the client data. Disabling appearance of the
work until after pcf50633 resume will also avoid that.
Signed-off-by: Andy Green <andy@openmoko.com>
---
drivers/i2c/chips/pcf50633.c | 93 +++++++++++++++++++++++++++++-------------
1 files changed, 65 insertions(+), 28 deletions(-)
diff --git a/drivers/i2c/chips/pcf50633.c b/drivers/i2c/chips/pcf50633.c
index c014384..120fdbc 100644
--- a/drivers/i2c/chips/pcf50633.c
+++ b/drivers/i2c/chips/pcf50633.c
@@ -632,6 +632,11 @@ static void pcf50633_work_usbcurlim(struct work_struct *work)
mutex_lock(&pcf->working_lock_usb_curlimit);
+ /* just can't cope with it if we are suspending, don't reschedule */
+ if ((pcf->suspend_state == PCF50633_SS_STARTING_SUSPEND) ||
+ (pcf->suspend_state == PCF50633_SS_COMPLETED_SUSPEND))
+ goto bail;
+
dev_info(&pcf->client.dev, "pcf50633_work_usbcurlim\n");
if (!pcf->probe_completed)
@@ -663,7 +668,8 @@ bail:
reschedule:
dev_info(&pcf->client.dev, "pcf50633_work_usbcurlim rescheduling\n");
if (!schedule_work(&pcf->work_usb_curlimit))
- dev_err(&pcf->client.dev, "work item may be lost\n");
+ dev_err(&pcf->client.dev, "curlim reschedule work "
+ "already queued\n");
mutex_unlock(&pcf->working_lock_usb_curlimit);
/* don't spew, delaying whatever else is happening */
@@ -700,7 +706,7 @@ int pcf50633_notify_usb_current_limit_change(struct pcf50633_data *pcf,
pcf->pending_curlimit = ma;
if (!schedule_work(&pcf->work_usb_curlimit))
- dev_err(&pcf->client.dev, "work item may be lost\n");
+ dev_err(&pcf->client.dev, "curlim work item already queued\n");
return 0;
}
@@ -727,6 +733,9 @@ static void pcf50633_work_nobat(struct work_struct *work)
while (1) {
msleep(1000);
+ if (pcf->suspend_state != PCF50633_SS_RUNNING)
+ continue;
+
/* there's a battery in there now? */
if (reg_read(pcf, PCF50633_REG_MBCS3) & 0x40) {
@@ -761,6 +770,10 @@ static void pcf50633_work(struct work_struct *work)
mutex_lock(&pcf->working_lock);
pcf->working = 1;
+ /* sanity */
+ if (!&pcf->client.dev)
+ goto bail;
+
/*
* if we are presently suspending, we are not in a position to deal
* with pcf50633 interrupts at all.
@@ -784,42 +797,55 @@ static void pcf50633_work(struct work_struct *work)
* reloaded with their pre-suspend states yet. Therefore we will
* defer our service if we are called like that until our resume has
* completed.
+ *
+ * This shouldn't happen any more because we disable servicing this
+ * interrupt in suspend and don't re-enable it until resume is
+ * completed.
*/
- if (pcf->have_been_suspended && (pcf->have_been_suspended < 3))
+ if (pcf->suspend_state &&
+ (pcf->suspend_state != PCF50633_SS_COMPLETED_RESUME))
+ goto reschedule;
+
+ /* this is the case early in resume! Sanity check! */
+ if (i2c_get_clientdata(&pcf->client) == NULL)
goto reschedule;
/*
- * datasheet says we have to read the five IRQ
- * status regs in one transaction
- */
- ret = i2c_smbus_read_i2c_block_data(&pcf->client, PCF50633_REG_INT1, 5,
- pcfirq);
- if (ret != 5) {
+ * datasheet says we have to read the five IRQ
+ * status regs in one transaction
+ */
+ ret = i2c_smbus_read_i2c_block_data(&pcf->client,
+ PCF50633_REG_INT1,
+ sizeof(pcfirq),
+ pcfirq);
+ if (ret != sizeof(pcfirq)) {
dev_info(&pcf->client.dev,
- "Oh crap PMU IRQ register read failed -- "
- "retrying later %d\n", ret);
+ "Oh crap PMU IRQ register read failed -- "
+ "retrying later %d\n", ret);
/*
- * it shouldn't fail, we no longer attempt to use I2C while
- * it can be suspended. But we don't have much option but to
- * retry if if it ever did fail, because if we don't service
- * the interrupt to clear it, we will never see another PMU
- * interrupt edge.
+ * it shouldn't fail, we no longer attempt to use
+ * I2C while it can be suspended. But we don't have
+ * much option but to retry if if it ever did fail,
+ * because if we don't service the interrupt to clear
+ * it, we will never see another PMU interrupt edge.
*/
goto reschedule;
}
- /* hey did we just resume? */
-
- if (pcf->have_been_suspended) {
- /* resume is officially over now then */
- pcf->have_been_suspended = 0;
+ /* hey did we just resume? (because we don't get here unless we are
+ * running normally or the first call after resumption)
+ */
+ if (pcf->suspend_state != PCF50633_SS_RUNNING) {
/*
- * grab a copy of resume interrupt reasons
- * from pcf50633 POV
- */
+ * grab a copy of resume interrupt reasons
+ * from pcf50633 POV
+ */
memcpy(pcf->pcfirq_resume, pcfirq, sizeof(pcf->pcfirq_resume));
+
+ /* pcf50633 resume is really really over now then */
+ pcf->suspend_state = PCF50633_SS_RUNNING;
}
if (!pcf->coldplug_done) {
@@ -1169,6 +1195,7 @@ static void pcf50633_work(struct work_struct *work)
DEBUGPC("\n");
+bail:
pcf->working = 0;
input_sync(pcf->input_dev);
put_device(&pcf->client.dev);
@@ -1202,7 +1229,7 @@ static irqreturn_t pcf50633_irq(int irq, void *_pcf)
get_device(&pcf->client.dev);
if (!schedule_work(&pcf->work) && !pcf->working)
- dev_err(&pcf->client.dev, "work item may be lost\n");
+ dev_err(&pcf->client.dev, "pcf irq work already queued\n");
return IRQ_HANDLED;
}
@@ -2310,7 +2337,7 @@ int pcf50633_report_resumers(struct pcf50633_data *pcf, char *buf)
char *end = buf;
int n;
- for (n = 0; n < 40; n++)
+ for (n = 0; n < ARRAY_SIZE(int_names); n++)
if (int_names[n]) {
if (pcf->pcfirq_resume[n >> 3] & (1 >> (n & 7)))
end += sprintf(end, " * %s\n", int_names[n]);
@@ -2359,6 +2386,15 @@ static int pcf50633_suspend(struct device *dev, pm_message_t state)
mutex_lock(&pcf->lock);
+ pcf->suspend_state = PCF50633_SS_STARTING_SUSPEND;
+
+ /* we are not going to service any further interrupts until we
+ * resume. If the IRQ workqueue is still pending in the background,
+ * it will bail when it sees we set suspend state above
+ */
+
+ disable_irq(pcf->irq);
+
/* Save all registers that don't "survive" standby state */
pcf->standby_regs.ooctim2 = __reg_read(pcf, PCF50633_REG_OOCTIM2);
@@ -2502,6 +2538,8 @@ static int pcf50633_resume(struct device *dev)
pcf->suspend_state = PCF50633_SS_COMPLETED_RESUME;
+ enable_irq(pcf->irq);
+
mutex_unlock(&pcf->lock);
/* gratuitous call to PCF work function, in the case that the PCF
@@ -2511,8 +2549,7 @@ static int pcf50633_resume(struct device *dev)
*/
get_device(&pcf->client.dev);
- if (!schedule_work(&pcf->work) && !pcf->working)
- dev_err(&pcf->client.dev, "resume work item may be lost\n");
+ pcf50633_work(&pcf->work);
callback_all_resume_dependencies(&pcf->resume_dependency);
--
1.5.6.5
|