Hello laforge,
I'd like you to do a code review. Please visit
https://gerrit.osmocom.org/c/osmo-e1-hardware/+/26911
to review the following change.
Change subject: icE1usb fw: New ICE1USB_INTF_GET_ERRORS control request ......................................................................
icE1usb fw: New ICE1USB_INTF_GET_ERRORS control request
This allows the host driver to obtain the current errors, as they would normally be reported over the IRQ endpoint. One important use case for this is to obtain the initial error state when the driver starts up.
Change-Id: I6301bb23234c66afe083ceb500cff8a40813e8b6 --- M firmware/ice40-riscv/icE1usb/ice1usb_proto.h M firmware/ice40-riscv/icE1usb/usb_e1.c 2 files changed, 21 insertions(+), 12 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-e1-hardware refs/changes/11/26911/1
diff --git a/firmware/ice40-riscv/icE1usb/ice1usb_proto.h b/firmware/ice40-riscv/icE1usb/ice1usb_proto.h index d0d7304..9ac2136 100644 --- a/firmware/ice40-riscv/icE1usb/ice1usb_proto.h +++ b/firmware/ice40-riscv/icE1usb/ice1usb_proto.h @@ -32,6 +32,7 @@ #define ICE1USB_INTF_GET_TX_CFG 0x03 /*!< struct ice1usb_tx_config */ #define ICE1USB_INTF_SET_RX_CFG 0x04 /*!< struct ice1usb_rx_config */ #define ICE1USB_INTF_GET_RX_CFG 0x05 /*!< struct ice1usb_rx_config */ +#define ICE1USB_INTF_GET_ERRORS 0x06 /*!< struct ice1usb_irq_err */
//enum e1usb_intf_capability { };
diff --git a/firmware/ice40-riscv/icE1usb/usb_e1.c b/firmware/ice40-riscv/icE1usb/usb_e1.c index 6d0d3cf..6fdaa91 100644 --- a/firmware/ice40-riscv/icE1usb/usb_e1.c +++ b/firmware/ice40-riscv/icE1usb/usb_e1.c @@ -95,6 +95,15 @@ } }
+static void +_fill_irq_err(struct ice1usb_irq_err *out, const struct e1_error_count *cur_err) +{ + out->crc = cur_err->crc; + out->align = cur_err->align; + out->ovfl = cur_err->ovfl; + out->unfl = cur_err->unfl; + out->flags = cur_err->flags; +}
static void _usb_e1_run(int port) @@ -112,18 +121,9 @@ if ((ep_regs->bd[0].csr & USB_BD_STATE_MSK) != USB_BD_STATE_RDY_DATA) { const struct e1_error_count *cur_err = e1_get_error_count(port); if (memcmp(cur_err, &usb_e1->last_err, sizeof(*cur_err))) { - struct ice1usb_irq errmsg = { - .type = ICE1USB_IRQ_T_ERRCNT, - .u = { - .errors = { - .crc = cur_err->crc, - .align = cur_err->align, - .ovfl = cur_err->ovfl, - .unfl = cur_err->unfl, - .flags = cur_err->flags, - } - } - }; + struct ice1usb_irq errmsg; + errmsg.type = ICE1USB_IRQ_T_ERRCNT; + _fill_irq_err(&errmsg.u.errors, cur_err); printf("E"); usb_data_write(ep_regs->bd[0].ptr, &errmsg, sizeof(errmsg)); ep_regs->bd[0].csr = USB_BD_STATE_RDY_DATA | USB_BD_LEN(sizeof(errmsg)); @@ -374,6 +374,7 @@ static enum usb_fnd_resp _e1_ctrl_req(struct usb_ctrl_req *req, struct usb_xfer *xfer) { + const struct e1_error_count *cur_err; struct usb_e1_state *usb_e1; int port;
@@ -420,6 +421,13 @@ memcpy(xfer->data, &usb_e1->rx_cfg, sizeof(struct ice1usb_rx_config)); xfer->len = sizeof(struct ice1usb_rx_config); break; + case ICE1USB_INTF_GET_ERRORS: + if (req->wLength < sizeof(struct ice1usb_irq_err)) + return USB_FND_ERROR; + cur_err = e1_get_error_count(port); + _fill_irq_err((struct ice1usb_irq_err *)xfer->data, cur_err); + xfer->len = sizeof(struct ice1usb_irq_err); + break; default: return USB_FND_ERROR; }
Hello laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-e1-hardware/+/26911
to look at the new patch set (#2).
Change subject: icE1usb fw: New ICE1USB_INTF_GET_ERRORS control request ......................................................................
icE1usb fw: New ICE1USB_INTF_GET_ERRORS control request
This allows the host driver to obtain the current errors, as they would normally be reported over the IRQ endpoint. One important use case for this is to obtain the initial error state when the driver starts up.
Change-Id: I6301bb23234c66afe083ceb500cff8a40813e8b6 --- M firmware/ice40-riscv/icE1usb/ice1usb_proto.h M firmware/ice40-riscv/icE1usb/usb_e1.c 2 files changed, 21 insertions(+), 12 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-e1-hardware refs/changes/11/26911/2
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1-hardware/+/26911 )
Change subject: icE1usb fw: New ICE1USB_INTF_GET_ERRORS control request ......................................................................
Patch Set 2: Code-Review+2
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-e1-hardware/+/26911 )
Change subject: icE1usb fw: New ICE1USB_INTF_GET_ERRORS control request ......................................................................
icE1usb fw: New ICE1USB_INTF_GET_ERRORS control request
This allows the host driver to obtain the current errors, as they would normally be reported over the IRQ endpoint. One important use case for this is to obtain the initial error state when the driver starts up.
Change-Id: I6301bb23234c66afe083ceb500cff8a40813e8b6 --- M firmware/ice40-riscv/icE1usb/ice1usb_proto.h M firmware/ice40-riscv/icE1usb/usb_e1.c 2 files changed, 21 insertions(+), 12 deletions(-)
Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved
diff --git a/firmware/ice40-riscv/icE1usb/ice1usb_proto.h b/firmware/ice40-riscv/icE1usb/ice1usb_proto.h index d0d7304..9ac2136 100644 --- a/firmware/ice40-riscv/icE1usb/ice1usb_proto.h +++ b/firmware/ice40-riscv/icE1usb/ice1usb_proto.h @@ -32,6 +32,7 @@ #define ICE1USB_INTF_GET_TX_CFG 0x03 /*!< struct ice1usb_tx_config */ #define ICE1USB_INTF_SET_RX_CFG 0x04 /*!< struct ice1usb_rx_config */ #define ICE1USB_INTF_GET_RX_CFG 0x05 /*!< struct ice1usb_rx_config */ +#define ICE1USB_INTF_GET_ERRORS 0x06 /*!< struct ice1usb_irq_err */
//enum e1usb_intf_capability { };
diff --git a/firmware/ice40-riscv/icE1usb/usb_e1.c b/firmware/ice40-riscv/icE1usb/usb_e1.c index 6d0d3cf..6fdaa91 100644 --- a/firmware/ice40-riscv/icE1usb/usb_e1.c +++ b/firmware/ice40-riscv/icE1usb/usb_e1.c @@ -95,6 +95,15 @@ } }
+static void +_fill_irq_err(struct ice1usb_irq_err *out, const struct e1_error_count *cur_err) +{ + out->crc = cur_err->crc; + out->align = cur_err->align; + out->ovfl = cur_err->ovfl; + out->unfl = cur_err->unfl; + out->flags = cur_err->flags; +}
static void _usb_e1_run(int port) @@ -112,18 +121,9 @@ if ((ep_regs->bd[0].csr & USB_BD_STATE_MSK) != USB_BD_STATE_RDY_DATA) { const struct e1_error_count *cur_err = e1_get_error_count(port); if (memcmp(cur_err, &usb_e1->last_err, sizeof(*cur_err))) { - struct ice1usb_irq errmsg = { - .type = ICE1USB_IRQ_T_ERRCNT, - .u = { - .errors = { - .crc = cur_err->crc, - .align = cur_err->align, - .ovfl = cur_err->ovfl, - .unfl = cur_err->unfl, - .flags = cur_err->flags, - } - } - }; + struct ice1usb_irq errmsg; + errmsg.type = ICE1USB_IRQ_T_ERRCNT; + _fill_irq_err(&errmsg.u.errors, cur_err); printf("E"); usb_data_write(ep_regs->bd[0].ptr, &errmsg, sizeof(errmsg)); ep_regs->bd[0].csr = USB_BD_STATE_RDY_DATA | USB_BD_LEN(sizeof(errmsg)); @@ -374,6 +374,7 @@ static enum usb_fnd_resp _e1_ctrl_req(struct usb_ctrl_req *req, struct usb_xfer *xfer) { + const struct e1_error_count *cur_err; struct usb_e1_state *usb_e1; int port;
@@ -420,6 +421,13 @@ memcpy(xfer->data, &usb_e1->rx_cfg, sizeof(struct ice1usb_rx_config)); xfer->len = sizeof(struct ice1usb_rx_config); break; + case ICE1USB_INTF_GET_ERRORS: + if (req->wLength < sizeof(struct ice1usb_irq_err)) + return USB_FND_ERROR; + cur_err = e1_get_error_count(port); + _fill_irq_err((struct ice1usb_irq_err *)xfer->data, cur_err); + xfer->len = sizeof(struct ice1usb_irq_err); + break; default: return USB_FND_ERROR; }