Skip to content

Commit 1c769c3

Browse files
Chenhongrenfabiobaltieri
authored andcommitted
usb: it82xx2: fix issue with the unexpected setting of the ready bit
In the IT82xx2 chip, the ready bit is automatically cleared by hardware. When setting other bits, there is a chance that the ready bit hasn't been cleared yet, leading to unexpected USB transactions. This change ensures that the ready bit is always set to '0' when setting the endpoint control value. Signed-off-by: Ren Chen <[email protected]>
1 parent d410231 commit 1c769c3

File tree

1 file changed

+58
-40
lines changed

1 file changed

+58
-40
lines changed

drivers/usb/device/usb_dc_it82xx2.c

Lines changed: 58 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,12 @@ enum it82xx2_transaction_types {
8181
#define DC_CONNECT_TO_HOST BIT(6) /* internal pull-up */
8282

8383
/* ENDPOINT[3..0]_CONTROL_REG */
84-
#define ENDPOINT_EN BIT(0)
85-
#define EP_SEND_STALL BIT(3)
84+
#define ENDPOINT_ENABLE_BIT BIT(0)
85+
#define ENDPOINT_READY_BIT BIT(1)
86+
#define ENDPOINT_OUTDATA_SEQ_BIT BIT(2)
87+
#define ENDPOINT_SEND_STALL_BIT BIT(3)
88+
#define ENDPOINT_ISO_ENABLE_BIT BIT(4)
89+
#define ENDPOINT_DIRECTION_BIT BIT(5)
8690

8791
enum it82xx2_ep_status {
8892
EP_INIT = 0,
@@ -417,14 +421,11 @@ static int it82xx2_usb_extend_ep_ctrl(uint8_t ep, enum it82xx2_ep_ctrl ctrl, boo
417421
}
418422
break;
419423
case EP_READY_ENABLE:
420-
unsigned int key;
421424
int idx = ((ep_idx - 4) % 3) + 1;
422425

423-
key = irq_lock();
424426
(enable) ? (ext_ctrl[idx].epn_ext_ctrl2 |= BIT((ep_idx - 4) / 3))
425427
: (ext_ctrl[idx].epn_ext_ctrl2 &= ~BIT((ep_idx - 4) / 3));
426428
ep_regs[ep_fifo].ep_ctrl.fields.ready_bit = enable;
427-
irq_unlock(key);
428429
break;
429430
default:
430431
LOG_ERR("Unknown control type 0x%x", ctrl);
@@ -438,64 +439,88 @@ static int it82xx2_usb_ep_ctrl(uint8_t ep, enum it82xx2_ep_ctrl ctrl, bool enabl
438439
{
439440
struct usb_it82xx2_regs *const usb_regs = it82xx2_get_usb_regs();
440441
struct it82xx2_usb_ep_regs *ep_regs = usb_regs->usb_ep_regs;
442+
uint8_t ep_ctrl_value;
441443
uint8_t ep_idx = USB_EP_GET_IDX(ep);
442444

443445
if (IT8XXX2_IS_EXTEND_ENDPOINT(ep_idx)) {
444446
return -EINVAL;
445447
}
446448

449+
ep_ctrl_value = ep_regs[ep_idx].ep_ctrl.value & ~ENDPOINT_READY_BIT;
450+
447451
switch (ctrl) {
448452
case EP_IN_DIRECTION_SET:
449-
ep_regs[ep_idx].ep_ctrl.fields.direction_bit = enable;
453+
if (enable) {
454+
ep_ctrl_value |= ENDPOINT_DIRECTION_BIT;
455+
} else {
456+
ep_ctrl_value &= ~ENDPOINT_DIRECTION_BIT;
457+
}
450458
break;
451459
case EP_STALL_SEND:
452-
ep_regs[ep_idx].ep_ctrl.fields.send_stall_bit = enable;
460+
if (enable) {
461+
ep_ctrl_value |= ENDPOINT_SEND_STALL_BIT;
462+
} else {
463+
ep_ctrl_value &= ~ENDPOINT_SEND_STALL_BIT;
464+
}
453465
break;
454466
case EP_STALL_CHECK:
455467
return ep_regs[ep_idx].ep_ctrl.fields.send_stall_bit;
456468
case EP_IOS_ENABLE:
457-
ep_regs[ep_idx].ep_ctrl.fields.iso_enable_bit = enable;
469+
if (enable) {
470+
ep_ctrl_value |= ENDPOINT_ISO_ENABLE_BIT;
471+
} else {
472+
ep_ctrl_value &= ~ENDPOINT_ISO_ENABLE_BIT;
473+
}
458474
break;
459475
case EP_ENABLE:
460-
ep_regs[ep_idx].ep_ctrl.fields.enable_bit = enable;
476+
if (enable) {
477+
ep_ctrl_value |= ENDPOINT_ENABLE_BIT;
478+
} else {
479+
ep_ctrl_value &= ~ENDPOINT_ENABLE_BIT;
480+
}
461481
break;
462482
case EP_READY_ENABLE:
463-
unsigned int key;
464-
465-
key = irq_lock();
466-
ep_regs[ep_idx].ep_ctrl.fields.ready_bit = enable;
467-
irq_unlock(key);
483+
if (enable) {
484+
ep_ctrl_value |= ENDPOINT_READY_BIT;
485+
} else {
486+
ep_ctrl_value &= ~ENDPOINT_READY_BIT;
487+
}
468488
break;
469489
case EP_DATA_SEQ_1:
470-
ep_regs[ep_idx].ep_ctrl.fields.outdata_sequence_bit = enable;
490+
if (enable) {
491+
ep_ctrl_value |= ENDPOINT_OUTDATA_SEQ_BIT;
492+
} else {
493+
ep_ctrl_value &= ~ENDPOINT_OUTDATA_SEQ_BIT;
494+
}
471495
break;
472496
case EP_DATA_SEQ_TOGGLE:
473497
if (!enable) {
474498
break;
475499
}
476-
if (ep_regs[ep_idx].ep_ctrl.fields.outdata_sequence_bit) {
477-
ep_regs[ep_idx].ep_ctrl.fields.outdata_sequence_bit = 0;
478-
} else {
479-
ep_regs[ep_idx].ep_ctrl.fields.outdata_sequence_bit = 1;
480-
}
500+
ep_ctrl_value ^= ENDPOINT_OUTDATA_SEQ_BIT;
481501
break;
482502
default:
483503
LOG_ERR("Unknown control type 0x%x", ctrl);
484504
return -EINVAL;
485505
}
506+
507+
ep_regs[ep_idx].ep_ctrl.value = ep_ctrl_value;
486508
return 0;
487509
}
488510

489511
static int it82xx2_usb_set_ep_ctrl(uint8_t ep, enum it82xx2_ep_ctrl ctrl, bool enable)
490512
{
491513
uint8_t ep_idx = USB_EP_GET_IDX(ep);
492514
int ret = 0;
515+
unsigned int key;
493516

517+
key = irq_lock();
494518
if (IT8XXX2_IS_EXTEND_ENDPOINT(ep_idx)) {
495519
ret = it82xx2_usb_extend_ep_ctrl(ep, ctrl, enable);
496520
} else {
497521
ret = it82xx2_usb_ep_ctrl(ep, ctrl, enable);
498522
}
523+
irq_unlock(key);
499524
return ret;
500525
}
501526

@@ -548,16 +573,16 @@ static bool it82xx2_check_setup_following_out(void)
548573
ff_regs[EP0].ep_rx_fifo_dcnt_lsb == SETUP_DATA_CNT));
549574
}
550575

551-
static inline void it82xx2_handler_setup(uint8_t fifo_idx, uint8_t ep_ctrl)
576+
static inline void it82xx2_handler_setup(uint8_t fifo_idx)
552577
{
553578
struct usb_it82xx2_regs *const usb_regs = it82xx2_get_usb_regs();
554579
struct it82xx2_usb_ep_regs *ep_regs = usb_regs->usb_ep_regs;
555580
struct it82xx2_usb_ep_fifo_regs *ff_regs = usb_regs->fifo_regs;
556581
uint8_t ep_idx = fifo_idx;
557582

558583
/* wrong trans */
559-
if (ep_ctrl & EP_SEND_STALL) {
560-
ep_regs[fifo_idx].ep_ctrl.fields.send_stall_bit = 0;
584+
if (ep_regs[ep_idx].ep_ctrl.fields.send_stall_bit) {
585+
it82xx2_usb_set_ep_ctrl(fifo_idx, EP_STALL_SEND, false);
561586
udata0.st_state = STALL_SEND;
562587
ff_regs[fifo_idx].ep_rx_fifo_ctrl = FIFO_FORCE_EMPTY;
563588
LOG_DBG("Clear Stall Bit & RX FIFO");
@@ -596,14 +621,14 @@ static inline void it82xx2_handler_setup(uint8_t fifo_idx, uint8_t ep_ctrl)
596621
}
597622
}
598623

599-
static inline void it82xx2_handler_in(const uint8_t ep_idx, const uint8_t ep_ctrl)
624+
static inline void it82xx2_handler_in(const uint8_t ep_idx)
600625
{
601626
struct usb_it82xx2_regs *const usb_regs = it82xx2_get_usb_regs();
602627
struct it82xx2_usb_ep_regs *ep_regs = usb_regs->usb_ep_regs;
603628

604629
if (ep_idx == 0) {
605-
if (ep_ctrl & EP_SEND_STALL) {
606-
ep_regs[ep_idx].ep_ctrl.fields.send_stall_bit = 0;
630+
if (ep_regs[ep_idx].ep_ctrl.fields.send_stall_bit) {
631+
it82xx2_usb_set_ep_ctrl(ep_idx, EP_STALL_SEND, false);
607632
udata0.st_state = STALL_SEND;
608633
LOG_DBG("Clear Stall Bit");
609634
return;
@@ -657,9 +682,6 @@ static inline void it82xx2_handler_in(const uint8_t ep_idx, const uint8_t ep_ctr
657682

658683
static inline void it82xx2_handler_out(const uint8_t ep_idx)
659684
{
660-
struct usb_it82xx2_regs *const usb_regs = it82xx2_get_usb_regs();
661-
struct it82xx2_usb_ep_regs *ep_regs = usb_regs->usb_ep_regs;
662-
663685
if (ep_idx == 0) {
664686
/* ep0 wrong enter check */
665687
if (udata0.st_state >= STATUS_ST) {
@@ -688,7 +710,7 @@ static inline void it82xx2_handler_out(const uint8_t ep_idx)
688710
udata0.last_token = udata0.now_token;
689711
udata0.now_token = SETUP_TOKEN;
690712
udata0.st_state = SETUP_ST;
691-
ep_regs[ep_idx].ep_ctrl.fields.outdata_sequence_bit = 1;
713+
it82xx2_usb_set_ep_ctrl(ep_idx, EP_DATA_SEQ_1, true);
692714
udata0.ep_data[ep_idx].cb_out(ep_idx | USB_EP_DIR_OUT, USB_DC_EP_SETUP);
693715

694716
if (udata0.no_data_ctrl) {
@@ -799,12 +821,10 @@ static bool it82xx2_usb_fake_token(const uint8_t ep_idx, uint8_t *token_type)
799821
static void it82xx2_usb_dc_trans_done(void)
800822
{
801823
struct usb_it82xx2_regs *const usb_regs = it82xx2_get_usb_regs();
802-
struct it82xx2_usb_ep_regs *ep_regs = usb_regs->usb_ep_regs;
803824
struct epn_ext_ctrl_regs *epn_ext_ctrl =
804825
usb_regs->fifo_regs[EP_EXT_REGS_DX].ext_0_3.epn_ext_ctrl;
805826

806827
for (uint8_t fifo_idx = 0; fifo_idx < 4; fifo_idx++) {
807-
uint8_t ep_ctrl = ep_regs[fifo_idx].ep_ctrl.value;
808828
uint8_t ep_idx, token_type;
809829

810830
if (fifo_idx == 0) {
@@ -819,10 +839,10 @@ static void it82xx2_usb_dc_trans_done(void)
819839
if (!it82xx2_usb_fake_token(ep_idx, &token_type)) {
820840
switch (token_type) {
821841
case DC_SETUP_TRANS:
822-
it82xx2_handler_setup(fifo_idx, ep_ctrl);
842+
it82xx2_handler_setup(fifo_idx);
823843
break;
824844
case DC_IN_TRANS:
825-
it82xx2_handler_in(ep_idx, ep_ctrl);
845+
it82xx2_handler_in(ep_idx);
826846
break;
827847
case DC_OUTDATA_TRANS:
828848
it82xx2_handler_out(ep_idx);
@@ -1001,7 +1021,7 @@ int usb_dc_reset(void)
10011021
}
10021022
}
10031023

1004-
ep_regs[0].ep_ctrl.value = ENDPOINT_EN;
1024+
ep_regs[0].ep_ctrl.value = ENDPOINT_ENABLE_BIT;
10051025
usb_regs->dc_address = DC_ADDR_NULL;
10061026
udata0.addr = DC_ADDR_NULL;
10071027
usb_regs->dc_interrupt_status = DC_NAK_SENT_INT | DC_SOF_RECEIVED;
@@ -1093,9 +1113,7 @@ int usb_dc_ep_configure(const struct usb_dc_ep_cfg_data *const cfg)
10931113
it82xx2_usb_set_ep_ctrl(ep_idx, EP_IN_DIRECTION_SET, in);
10941114

10951115
if (in) {
1096-
if (IT8XXX2_IS_EXTEND_ENDPOINT(ep_idx)) {
1097-
it82xx2_usb_extend_ep_ctrl(ep_idx, EP_DATA_SEQ_1, false);
1098-
}
1116+
it82xx2_usb_set_ep_ctrl(ep_idx, EP_DATA_SEQ_1, false);
10991117
udata0.ep_data[ep_idx].ep_status = EP_CONFIG_IN;
11001118
} else {
11011119
udata0.ep_data[ep_idx].ep_status = EP_CONFIG_OUT;
@@ -1215,7 +1233,7 @@ int usb_dc_ep_set_stall(const uint8_t ep)
12151233
}
12161234

12171235
if (idx < 198) {
1218-
ep_regs[ep_idx].ep_ctrl.fields.send_stall_bit = 0;
1236+
it82xx2_usb_set_ep_ctrl(ep_idx, EP_STALL_SEND, false);
12191237
}
12201238

12211239
udata0.no_data_ctrl = false;
@@ -1432,7 +1450,7 @@ int usb_dc_ep_read(uint8_t ep, uint8_t *buf, uint32_t max_data_len,
14321450
ff_regs[0].ep_tx_fifo_ctrl = FIFO_FORCE_EMPTY;
14331451
if (buf[6] != 0 || buf[7] != 0) {
14341452
/* set status IN after data OUT */
1435-
ep_regs[0].ep_ctrl.fields.outdata_sequence_bit = 1;
1453+
it82xx2_usb_set_ep_ctrl(ep_idx, EP_DATA_SEQ_1, true);
14361454
it82xx2_usb_set_ep_ctrl(ep_idx, EP_READY_ENABLE, true);
14371455
} else {
14381456
/* no_data_ctrl status */

0 commit comments

Comments
 (0)