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"

Reply via email to