Skip to content

Commit 2e0fdc2

Browse files
aentingerMaxPayne86
aentinger
authored and
MaxPayne86
committed
Ensure that enqueueing and sending is an atomic operation, failing to do lock this leads to race-conditions where buffers are left in an unsynchronised state.
1 parent ba0ae18 commit 2e0fdc2

File tree

11 files changed

+87
-109
lines changed

11 files changed

+87
-109
lines changed

‎x8h7.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ typedef struct {
1717

1818
typedef void (*x8h7_hook_t)(void *priv, x8h7_pkt_t *pkt);
1919

20-
int x8h7_pkt_enq(uint8_t peripheral, uint8_t opcode, uint16_t size, void *data);
21-
int x8h7_pkt_send(void);
20+
int x8h7_pkt_send_sync(uint8_t peripheral, uint8_t opcode, uint16_t size, void *data);
2221
int x8h7_hook_set(uint8_t idx, x8h7_hook_t hook, void *priv);
2322
int x8h7_dbg_set(void (*hook)(void*, uint8_t*, uint16_t), void *priv);
2423
#endif /* __X8H7_H */

‎x8h7_adc.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,7 @@ static int x8h7_adc_pkt_get(struct x8h7_adc *adc, unsigned int ch)
8888

8989
static int x8h7_adc_read_chan(struct x8h7_adc *adc, unsigned int ch)
9090
{
91-
x8h7_pkt_enq(X8H7_ADC_PERIPH, ch + 1, 0, NULL);
92-
x8h7_pkt_send();
91+
x8h7_pkt_send_sync(X8H7_ADC_PERIPH, ch + 1, 0, NULL);
9392
if (x8h7_adc_pkt_get(adc, ch) < 0)
9493
return -ETIMEDOUT;
9594

‎x8h7_can.c

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,7 @@ static int x8h7_can_hw_setup(struct x8h7_can_priv *priv)
243243
x8h7_msg.field.time_segment_2,
244244
x8h7_msg.field.sync_jump_width);
245245

246-
x8h7_pkt_enq(priv->periph, X8H7_CAN_OC_INIT, sizeof(x8h7_msg.buf), x8h7_msg.buf);
247-
x8h7_pkt_send();
246+
x8h7_pkt_send_sync(priv->periph, X8H7_CAN_OC_INIT, sizeof(x8h7_msg.buf), x8h7_msg.buf);
248247

249248
return 0;
250249
}
@@ -253,8 +252,7 @@ static int x8h7_can_hw_setup(struct x8h7_can_priv *priv)
253252
*/
254253
static int x8h7_can_hw_stop(struct x8h7_can_priv *priv)
255254
{
256-
x8h7_pkt_enq(priv->periph, X8H7_CAN_OC_DEINIT, 0, NULL);
257-
x8h7_pkt_send();
255+
x8h7_pkt_send_sync(priv->periph, X8H7_CAN_OC_DEINIT, 0, NULL);
258256

259257
return 0;
260258
}
@@ -461,28 +459,28 @@ static void x8h7_can_tx_work_handler(struct work_struct *ws)
461459
int len;
462460
#endif
463461

464-
DBG_PRINT("\n");
465-
466462
while (tx_obj_buf_num_elems(&priv->tx_obj_buf) > 0 && h7_tx_fifo_fill_level(priv) < 1)
467463
{
468464
union x8h7_can_frame_message x8h7_can_msg;
469465
tx_obj_buf_pop(&priv->tx_obj_buf, &x8h7_can_msg);
470-
x8h7_pkt_enq(priv->periph,
471-
X8H7_CAN_OC_SEND,
472-
X8H7_CAN_HEADER_SIZE + x8h7_can_msg.field.len, /* Send 4-Byte ID, 1-Byte Length and the required number of data bytes. */
473-
x8h7_can_msg.buf);
466+
474467
#ifdef DEBUG
475468
i = 0; len = 0;
476469
for (i = 0; (i < x8h7_can_msg.field.len) && (len < sizeof(data_str)); i++)
477470
len += snprintf(data_str + len, sizeof(data_str) - len, " %02X", x8h7_can_msg.field.data[i]);
478-
DBG_PRINT("Enqueue CAN frame to H7: id = %08X, len = %d, data = [%s ]\n", x8h7_can_msg.field.id, x8h7_can_msg.field.len, data_str);
471+
DBG_PRINT("Send CAN frame to H7: id = %08X, len = %d, data = [%s ]\n", x8h7_can_msg.field.id, x8h7_can_msg.field.len, data_str);
479472
#endif
480473

481474
spin_lock(&priv->tx_obj_buf.lock);
482475
priv->req_cnt++;
483476
spin_unlock(&priv->tx_obj_buf.lock);
477+
478+
x8h7_pkt_send_sync(priv->periph,
479+
X8H7_CAN_OC_SEND,
480+
X8H7_CAN_HEADER_SIZE + x8h7_can_msg.field.len, /* Send 4-Byte ID, 1-Byte Length and the required number of data bytes. */
481+
x8h7_can_msg.buf);
484482
}
485-
x8h7_pkt_send();
483+
// x8h7_pkt_send();
486484
}
487485

488486
/**
@@ -506,8 +504,7 @@ static int x8h7_can_hw_do_set_bittiming(struct net_device *net)
506504
x8h7_msg.field.time_segment_2,
507505
x8h7_msg.field.sync_jump_width);
508506

509-
x8h7_pkt_enq(priv->periph, X8H7_CAN_OC_BITTIM, sizeof(x8h7_msg.buf), x8h7_msg.buf);
510-
x8h7_pkt_send();
507+
x8h7_pkt_send_sync(priv->periph, X8H7_CAN_OC_BITTIM, sizeof(x8h7_msg.buf), x8h7_msg.buf);
511508

512509
return 0;
513510
}
@@ -567,8 +564,7 @@ static int x8h7_can_hw_config_filter(struct x8h7_can_priv *priv,
567564

568565
DBG_PRINT("SEND idx %X, id %X, mask %X\n", idx, id, mask);
569566

570-
x8h7_pkt_enq(priv->periph, X8H7_CAN_OC_FLT, sizeof(x8h7_msg.buf), x8h7_msg.buf);
571-
x8h7_pkt_send();
567+
x8h7_pkt_send_sync(priv->periph, X8H7_CAN_OC_FLT, sizeof(x8h7_msg.buf), x8h7_msg.buf);
572568

573569
return 0;
574570
}

‎x8h7_drv.c

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,6 @@ int x8h7_pkt_enq(uint8_t peripheral, uint8_t opcode, uint16_t size, void *data)
177177
x8h7_subpkt_t *pkt;
178178
uint8_t *ptr;
179179

180-
mutex_lock(&spidev->lock);
181-
182180
ptr = spidev->x8h7_txb;
183181
hdr = (x8h7_pkthdr_t*)ptr;
184182

@@ -199,15 +197,39 @@ int x8h7_pkt_enq(uint8_t peripheral, uint8_t opcode, uint16_t size, void *data)
199197
hdr->size += sizeof(x8h7_subpkt_t) + size;
200198
hdr->checksum = hdr->size ^ 0x5555;
201199
spidev->x8h7_txl = hdr->size;
202-
mutex_unlock(&spidev->lock);
203200
return 0;
204201
}
205202

203+
return -ENOMEM;
204+
}
205+
206+
static int x8h7_pkt_send(void);
207+
/**
208+
*/
209+
int x8h7_pkt_send_sync(uint8_t peripheral, uint8_t opcode, uint16_t size, void *data)
210+
{
211+
struct spidev_data *spidev = x8h7_spidev;
212+
int ret;
213+
214+
mutex_lock(&spidev->lock);
215+
ret = x8h7_pkt_enq(peripheral, opcode, size, data);
216+
if (ret < 0) {
217+
printk("x8h7_pkt_enq failed with %d", ret);
218+
mutex_unlock(&spidev->lock);
219+
return ret;
220+
}
221+
ret = x8h7_pkt_send();
222+
if (ret < 0) {
223+
printk("x8h7_pkt_send failed with %d", ret);
224+
mutex_unlock(&spidev->lock);
225+
return ret;
226+
}
206227
mutex_unlock(&spidev->lock);
207228

208-
return -1;
229+
return ret;
209230
}
210-
EXPORT_SYMBOL_GPL(x8h7_pkt_enq);
231+
EXPORT_SYMBOL_GPL(x8h7_pkt_send_sync);
232+
211233

212234
/**
213235
* Function to parse data coming from h7
@@ -284,8 +306,6 @@ int x8h7_spi_trx(struct spi_device *spi,
284306
uint8_t * data_ptr = 0;
285307
int i = 0, l = 0;
286308

287-
DBG_PRINT("\n");
288-
289309
l = 0;
290310
data_ptr = (uint8_t *)tx_buf;
291311
for (i = 0; (i < len) && (l < sizeof(data_str)); i++)
@@ -307,16 +327,13 @@ int x8h7_spi_trx(struct spi_device *spi,
307327
* Function to send/receive physically data over SPI,
308328
* moreover in this function we process received data
309329
* and dispatch to corresponding peripheral
310-
* @TODO: remove arg?
311330
*/
312-
static inline int x8h7_pkt_send_priv(int arg)
331+
static int x8h7_pkt_send(void)
313332
{
314333
struct spidev_data *spidev = x8h7_spidev;
315334
x8h7_pkthdr_t *hdr;
316335
int len;
317336

318-
mutex_lock(&spidev->lock);
319-
320337
DBG_PRINT("\n");
321338

322339
/* Exchange of the packet header. */
@@ -326,7 +343,6 @@ static inline int x8h7_pkt_send_priv(int arg)
326343
hdr = (x8h7_pkthdr_t*)spidev->x8h7_rxb;
327344
if ((hdr->size != 0) && ((hdr->size ^ 0x5555) != hdr->checksum)) {
328345
DBG_ERROR("Out of sync %04x %04x\n", hdr->size, hdr->checksum);
329-
mutex_unlock(&spidev->lock);
330346
return -1;
331347
}
332348

@@ -336,7 +352,6 @@ static inline int x8h7_pkt_send_priv(int arg)
336352
x8h7_spi_trx(spidev->spi,
337353
spidev->x8h7_txb + sizeof(x8h7_pkthdr_t), spidev->x8h7_rxb,
338354
sizeof(x8h7_pkthdr_t));
339-
mutex_unlock(&spidev->lock);
340355
return 0;
341356
}
342357

@@ -356,20 +371,13 @@ static inline int x8h7_pkt_send_priv(int arg)
356371
}
357372
}
358373

374+
memset(spidev->x8h7_txb, 0, X8H7_BUF_SIZE);
359375
memset(spidev->x8h7_rxb, 0, X8H7_BUF_SIZE);
376+
spidev->x8h7_txl = 0;
360377

361-
mutex_unlock(&spidev->lock);
362378
return 0;
363379
}
364380

365-
/**
366-
*/
367-
int x8h7_pkt_send(void)
368-
{
369-
return x8h7_pkt_send_priv(0);
370-
}
371-
EXPORT_SYMBOL_GPL(x8h7_pkt_send);
372-
373381
/**
374382
*/
375383
int x8h7_hook_set(uint8_t idx, x8h7_hook_t hook, void *priv)
@@ -398,8 +406,12 @@ EXPORT_SYMBOL_GPL(x8h7_dbg_set);
398406
*/
399407
static irqreturn_t x8h7_threaded_isr(int irq, void *data)
400408
{
409+
struct spidev_data *spidev = (struct spidev_data*)data;
410+
411+
mutex_lock(&spidev->lock);
401412
DBG_PRINT("Got IRQ from H7\n");
402-
x8h7_pkt_send_priv(1);
413+
x8h7_pkt_send();
414+
mutex_unlock(&spidev->lock);
403415

404416
return IRQ_HANDLED;
405417
}

‎x8h7_gpio.c

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,7 @@ static void gpio_irq_ack_work_func(struct work_struct *work)
125125
DBG_PRINT("\n");
126126
data[0] = inf->ack_irq;
127127
data[1] = 0x55;
128-
x8h7_pkt_enq(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_IACK, 2, data);
129-
x8h7_pkt_send();
128+
x8h7_pkt_send_sync(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_IACK, 2, data);
130129
return;
131130
}
132131

@@ -185,8 +184,7 @@ static int x8h7_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
185184
data[0] = offset;
186185
data[1] = GPIO_MODE_INPUT;
187186

188-
x8h7_pkt_enq(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_DIR, 2, data);
189-
x8h7_pkt_send();
187+
x8h7_pkt_send_sync(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_DIR, 2, data);
190188

191189
DBG_PRINT(" dir %08X\n", inf->gpio_dir);
192190
return 0;
@@ -204,8 +202,7 @@ static int x8h7_gpio_get(struct gpio_chip *chip, unsigned offset)
204202
}
205203

206204
data[0] = offset;
207-
x8h7_pkt_enq(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_RD, 1, data);
208-
x8h7_pkt_send();
205+
x8h7_pkt_send_sync(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_RD, 1, data);
209206
if (x8h7_gpio_pkt_get(inf) < 0)
210207
return -ETIMEDOUT;
211208

@@ -241,10 +238,9 @@ static int x8h7_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
241238
}
242239
data[0] = offset;
243240
data[1] = !!value;
244-
x8h7_pkt_enq(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_WR, 2, data);
241+
x8h7_pkt_send_sync(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_WR, 2, data);
245242
data[1] = GPIO_MODE_OUTPUT_PP;
246-
x8h7_pkt_enq(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_DIR, 2, data);
247-
x8h7_pkt_send();
243+
x8h7_pkt_send_sync(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_DIR, 2, data);
248244

249245
DBG_PRINT("dir %08X write %08X\n", inf->gpio_dir, inf->gpio_val);
250246
return 0;
@@ -269,8 +265,7 @@ static void x8h7_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
269265

270266
data[0] = offset;
271267
data[1] = !!value;
272-
x8h7_pkt_enq(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_WR, 2, data);
273-
x8h7_pkt_send();
268+
x8h7_pkt_send_sync(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_WR, 2, data);
274269

275270
DBG_PRINT("write %08X\n", inf->gpio_val);
276271
}
@@ -305,8 +300,7 @@ static int x8h7_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
305300
default:
306301
return -ENOTSUPP;
307302
}
308-
x8h7_pkt_enq(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_DIR, 2, data);
309-
x8h7_pkt_send();
303+
x8h7_pkt_send_sync(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_DIR, 2, data);
310304

311305
return 0;
312306
}
@@ -338,7 +332,7 @@ static void x8h7_gpio_irq_unmask(struct irq_data *d)
338332
// Send mask
339333
data[0] = irq;
340334
data[1] = inf->gpio_ien;
341-
x8h7_pkt_enq(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_IEN, 2, data);
335+
x8h7_pkt_send_sync(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_IEN, 2, data);
342336
inf->tx_cnt++;
343337

344338
DBG_PRINT("irq %ld, ien %02X\n", irq, inf->gpio_ien);
@@ -356,7 +350,7 @@ static void x8h7_gpio_irq_mask(struct irq_data *d)
356350
// Send mask
357351
data[0] = irq;
358352
data[1] = inf->gpio_ien;
359-
x8h7_pkt_enq(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_IEN, 2, data);
353+
x8h7_pkt_send_sync(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_IEN, 2, data);
360354
inf->tx_cnt++;
361355

362356
DBG_PRINT("irq %ld, ien %02X\n", irq, inf->gpio_ien);
@@ -396,7 +390,7 @@ static int x8h7_gpio_irq_set_type(struct irq_data *d, unsigned int flow_type)
396390
// Send interrupt type
397391
data[0] = irq;
398392
data[1] = inf->irq_conf;
399-
x8h7_pkt_enq(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_IRQ_TYPE, 2, data);
393+
x8h7_pkt_send_sync(X8H7_GPIO_PERIPH, X8H7_GPIO_OC_IRQ_TYPE, 2, data);
400394
inf->tx_cnt++;
401395

402396
return 0;
@@ -419,7 +413,7 @@ static void x8h7_gpio_irq_bus_sync_unlock(struct irq_data *d)
419413

420414
/* Invoke send only if there are pending events */
421415
if(inf->tx_cnt != 0) {
422-
x8h7_pkt_send();
416+
//x8h7_pkt_send();
423417
inf->tx_cnt = 0;
424418
}
425419
mutex_unlock(&inf->lock);

‎x8h7_h7.c

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,7 @@ static ssize_t x8h7_h7_write(struct file *file,
220220
return -EFAULT;
221221
}
222222
223-
x8h7_pkt_enq(X8H7_H7_PERIPH, X8H7_H7_OC_DATA, len, data);
224-
x8h7_pkt_send();
223+
x8h7_pkt_send_sync(X8H7_H7_PERIPH, X8H7_H7_OC_DATA, len, data);
225224
226225
return len;
227226
*/
@@ -248,8 +247,7 @@ static long x8h7_h7_ioctl(struct file *file, unsigned int cmd, unsigned long arg
248247
}
249248
switch (cmd) {
250249
case X8H7_IOCTL_FW_VER:
251-
x8h7_pkt_enq(X8H7_H7_PERIPH, X8H7_H7_OC_FW_GET, 0, NULL);
252-
x8h7_pkt_send();
250+
x8h7_pkt_send_sync(X8H7_H7_PERIPH, X8H7_H7_OC_FW_GET, 0, NULL);
253251
retval = x8h7_h7_pkt_get(priv);
254252
if (retval < 0) {
255253
return -ETIMEDOUT;
@@ -287,18 +285,14 @@ static long x8h7_h7_ioctl(struct file *file, unsigned int cmd, unsigned long arg
287285
case X8H7_IOCTL_MODE_GET:
288286
retval = put_user(priv->mode, (__u32 __user *)arg);
289287
break;
290-
case X8H7_IOCTL_PKT_ENQ:
288+
case X8H7_IOCTL_PKT_SYNC_SEND:
291289
retval = copy_from_user(&pkt, (void __user *)arg, sizeof(pkt));
292290
if (retval == 0) {
293-
retval = x8h7_pkt_enq(pkt.peripheral, pkt.opcode, pkt.size, pkt.data);
294-
DBG_PRINT("x8h7_pkt_enq(%02X, %02X, %04X, ...) return %d\n",
291+
retval = x8h7_pkt_send_sync(pkt.peripheral, pkt.opcode, pkt.size, pkt.data);
292+
DBG_PRINT("x8h7_pkt_send_sync(%02X, %02X, %04X, ...) return %d\n",
295293
pkt.peripheral, pkt.opcode, pkt.size, retval);
296294
}
297295
break;
298-
case X8H7_IOCTL_PKT_SEND:
299-
retval = x8h7_pkt_send();
300-
DBG_PRINT("x8h7_pkt_send() return: %ld\n", retval);
301-
break;
302296
default:
303297
retval = -ENOTTY;
304298
break;
@@ -319,8 +313,7 @@ ssize_t x8h7_read_firmware_version(char * buf, size_t const buf_size)
319313
{
320314
struct x8h7_h7_priv * priv = x8h7_h7;
321315

322-
x8h7_pkt_enq(X8H7_H7_PERIPH, X8H7_H7_OC_FW_GET, 0, NULL);
323-
x8h7_pkt_send();
316+
x8h7_pkt_send_sync(X8H7_H7_PERIPH, X8H7_H7_OC_FW_GET, 0, NULL);
324317
if (x8h7_h7_pkt_get(priv) < 0)
325318
return -ETIMEDOUT;
326319

@@ -354,8 +347,7 @@ ssize_t x8h7_read_chip_uid(char * buf, size_t const buf_size)
354347
{
355348
struct x8h7_h7_priv * priv = x8h7_h7;
356349

357-
x8h7_pkt_enq(X8H7_H7_PERIPH, X8H7_GET_UID_REQ, 0, NULL);
358-
x8h7_pkt_send();
350+
x8h7_pkt_send_sync(X8H7_H7_PERIPH, X8H7_GET_UID_REQ, 0, NULL);
359351
if (x8h7_h7_pkt_get(priv) < 0)
360352
return -ETIMEDOUT;
361353

0 commit comments

Comments
 (0)