Skip to content

Commit 44f9200

Browse files
committed
Input: gscps2 - use guard notation when acquiring spinlock
Using guard notation makes the code more compact and error handling more robust by ensuring that locks are released in all code paths when control leaves critical section. Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Dmitry Torokhov <[email protected]>
1 parent c91ae81 commit 44f9200

File tree

1 file changed

+62
-52
lines changed

1 file changed

+62
-52
lines changed

drivers/input/serio/gscps2.c

Lines changed: 62 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ static void gscps2_flush(struct gscps2port *ps2port)
145145

146146
static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
147147
{
148-
unsigned long flags;
149148
char __iomem *addr = ps2port->addr;
150149

151150
if (!wait_TBE(addr)) {
@@ -156,9 +155,8 @@ static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
156155
while (gscps2_readb_status(addr) & GSC_STAT_RBNE)
157156
/* wait */;
158157

159-
spin_lock_irqsave(&ps2port->lock, flags);
160-
writeb(data, addr+GSC_XMTDATA);
161-
spin_unlock_irqrestore(&ps2port->lock, flags);
158+
scoped_guard(spinlock_irqsave, &ps2port->lock)
159+
writeb(data, addr+GSC_XMTDATA);
162160

163161
/* this is ugly, but due to timing of the port it seems to be necessary. */
164162
mdelay(6);
@@ -177,19 +175,19 @@ static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
177175

178176
static void gscps2_enable(struct gscps2port *ps2port, int enable)
179177
{
180-
unsigned long flags;
181178
u8 data;
182179

183180
/* now enable/disable the port */
184-
spin_lock_irqsave(&ps2port->lock, flags);
185-
gscps2_flush(ps2port);
186-
data = gscps2_readb_control(ps2port->addr);
187-
if (enable)
188-
data |= GSC_CTRL_ENBL;
189-
else
190-
data &= ~GSC_CTRL_ENBL;
191-
gscps2_writeb_control(data, ps2port->addr);
192-
spin_unlock_irqrestore(&ps2port->lock, flags);
181+
scoped_guard(spinlock_irqsave, &ps2port->lock) {
182+
gscps2_flush(ps2port);
183+
data = gscps2_readb_control(ps2port->addr);
184+
if (enable)
185+
data |= GSC_CTRL_ENBL;
186+
else
187+
data &= ~GSC_CTRL_ENBL;
188+
gscps2_writeb_control(data, ps2port->addr);
189+
}
190+
193191
wait_TBE(ps2port->addr);
194192
gscps2_flush(ps2port);
195193
}
@@ -203,15 +201,56 @@ static void gscps2_reset(struct gscps2port *ps2port)
203201
unsigned long flags;
204202

205203
/* reset the interface */
206-
spin_lock_irqsave(&ps2port->lock, flags);
204+
guard(spinlock_irqsave)(&ps2port->lock);
207205
gscps2_flush(ps2port);
208206
writeb(0xff, ps2port->addr + GSC_RESET);
209207
gscps2_flush(ps2port);
210-
spin_unlock_irqrestore(&ps2port->lock, flags);
211208
}
212209

213210
static LIST_HEAD(ps2port_list);
214211

212+
static void gscps2_read_data(struct gscps2port *ps2port)
213+
{
214+
u8 status;
215+
216+
do {
217+
status = gscps2_readb_status(ps2port->addr);
218+
if (!(status & GSC_STAT_RBNE))
219+
break;
220+
221+
ps2port->buffer[ps2port->append].ste = status;
222+
ps2port->buffer[ps2port->append].data =
223+
gscps2_readb_input(ps2port->addr);
224+
} while (true);
225+
}
226+
227+
static bool gscps2_report_data(struct gscps2port *ps2port)
228+
{
229+
unsigned int rxflags;
230+
u8 data, status;
231+
232+
while (ps2port->act != ps2port->append) {
233+
/*
234+
* Did new data arrived while we read existing data ?
235+
* If yes, exit now and let the new irq handler start
236+
* over again.
237+
*/
238+
if (gscps2_readb_status(ps2port->addr) & GSC_STAT_CMPINTR)
239+
return true;
240+
241+
status = ps2port->buffer[ps2port->act].str;
242+
data = ps2port->buffer[ps2port->act].data;
243+
244+
ps2port->act = (ps2port->act + 1) & BUFFER_SIZE;
245+
rxflags = ((status & GSC_STAT_TERR) ? SERIO_TIMEOUT : 0 ) |
246+
((status & GSC_STAT_PERR) ? SERIO_PARITY : 0 );
247+
248+
serio_interrupt(ps2port->port, data, rxflags);
249+
}
250+
251+
return false;
252+
}
253+
215254
/**
216255
* gscps2_interrupt() - Interruption service routine
217256
*
@@ -229,47 +268,18 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
229268
struct gscps2port *ps2port;
230269

231270
list_for_each_entry(ps2port, &ps2port_list, node) {
271+
guard(spinlock_irqsave)(&ps2port->lock);
232272

233-
unsigned long flags;
234-
spin_lock_irqsave(&ps2port->lock, flags);
235-
236-
while ( (ps2port->buffer[ps2port->append].str =
237-
gscps2_readb_status(ps2port->addr)) & GSC_STAT_RBNE ) {
238-
ps2port->buffer[ps2port->append].data =
239-
gscps2_readb_input(ps2port->addr);
240-
ps2port->append = ((ps2port->append+1) & BUFFER_SIZE);
241-
}
242-
243-
spin_unlock_irqrestore(&ps2port->lock, flags);
244-
273+
gscps2_read_data(ps2port);
245274
} /* list_for_each_entry */
246275

247276
/* all data was read from the ports - now report the data to upper layer */
248-
249277
list_for_each_entry(ps2port, &ps2port_list, node) {
250-
251-
while (ps2port->act != ps2port->append) {
252-
253-
unsigned int rxflags;
254-
u8 data, status;
255-
256-
/* Did new data arrived while we read existing data ?
257-
If yes, exit now and let the new irq handler start over again */
258-
if (gscps2_readb_status(ps2port->addr) & GSC_STAT_CMPINTR)
259-
return IRQ_HANDLED;
260-
261-
status = ps2port->buffer[ps2port->act].str;
262-
data = ps2port->buffer[ps2port->act].data;
263-
264-
ps2port->act = ((ps2port->act+1) & BUFFER_SIZE);
265-
rxflags = ((status & GSC_STAT_TERR) ? SERIO_TIMEOUT : 0 ) |
266-
((status & GSC_STAT_PERR) ? SERIO_PARITY : 0 );
267-
268-
serio_interrupt(ps2port->port, data, rxflags);
269-
270-
} /* while() */
271-
272-
} /* list_for_each_entry */
278+
if (gscps2_report_data(ps2port)) {
279+
/* More data ready - break early to restart interrupt */
280+
break;
281+
}
282+
}
273283

274284
return IRQ_HANDLED;
275285
}

0 commit comments

Comments
 (0)