Re: [PATCH 08/39] event/octeontx: add mailbox support
From: Eads, Gage <hidden>
Date: 2017-03-23 16:46:09
Hi Jerin, I identified a few issues below. Thanks, Gage <snip>
+static inline void +mbox_send_requeust(struct mbox *m, struct octeontx_mbox_hdr *hdr, + const void *txmsg, uint16_t txsize)
"requeust" -> "request" <snip>
+
+static inline int
+mbox_wait_response(struct mbox *m, struct octeontx_mbox_hdr *hdr,
+ void *rxmsg, uint16_t rxsize)
+{
+ int res = 0, wait;
+ uint16_t len;
+ struct mbox_ram_hdr rx_hdr;
+ uint64_t *ram_mbox_hdr = (uint64_t *)m->ram_mbox_base;
+ uint8_t *ram_mbox_msg = m->ram_mbox_base + sizeof(struct
+mbox_ram_hdr);
+
+ /* Wait for response */
+ wait = MBOX_WAIT_TIME;
+ while (wait > 0) {
+ rte_delay_us(100);
+ rx_hdr.u64 = rte_read64(ram_mbox_hdr);
+ if (rx_hdr.chan_state == MBOX_CHAN_STATE_RES)
+ break;
+ wait -= 10;
+ }
'wait' is in units of milliseconds ("Mbox operation timeout in milliseconds"), so the function subtracts 10 ms after spinning for 100 us. Is that intentional?
+
+ hdr->res_code = rx_hdr.res_code;
+ m->tag_own++;
+
+ /* Tag mismatch */
+ if (m->tag_own != rx_hdr.tag) {
+ res = -EBADR;
+ goto error;
+ }
+
+ /* PF nacked the msg */
+ if (rx_hdr.res_code != MBOX_RET_SUCCESS) {
+ res = -EBADMSG;
+ goto error;
+ }
+
+ /* Timeout */
+ if (wait <= 0) {
+ res = -ETIMEDOUT;
+ goto error;
+ }Will a timeout mean rx_hdr is invalid? If so, the timeout error should be checked before checking for a PF nack or tag mismatch. <snip>
+static inline int
+mbox_send(struct mbox *m, struct octeontx_mbox_hdr *hdr, const void
*txmsg,
+ uint16_t txsize, void *rxmsg, uint16_t rxsize) {
+ int res = -EINVAL;
+
+ if (m->init_once == 0 || hdr == NULL ||
+ txsize > MAX_RAM_MBOX_LEN || rxsize >
MAX_RAM_MBOX_LEN) {
+ ssovf_log_err("Invalid init_once=%d hdr=%p txsz=%d rxsz=%d",
+ m->init_once, hdr, txsize, rxsize);
+ return res;
+ }
+
+ rte_spinlock_lock(&m->lock);
+
+ mbox_send_requeust(m, hdr, txmsg, txsize);"requeust" -> "request"