[dpdk-dev] [PATCH 08/39] event/octeontx: add mailbox support
Eads, Gage
gage.eads at intel.com
Thu Mar 23 17:46:07 CET 2017
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"
More information about the dev
mailing list