On Thu, 2020-06-25 at 19:07 -0700, Jeff Kirsher wrote: > From: Alice Michael <alice.mich...@intel.com> > > Implement mailbox setup, take down, and commands. [] > diff --git a/drivers/net/ethernet/intel/iecm/iecm_controlq.c > b/drivers/net/ethernet/intel/iecm/iecm_controlq.c [] > @@ -73,7 +142,74 @@ enum iecm_status iecm_ctlq_add(struct iecm_hw *hw, > struct iecm_ctlq_create_info *qinfo, > struct iecm_ctlq_info **cq)
Multiple functions using **cp and *cp can be error prone. > { > - /* stub */ > + enum iecm_status status = 0; > + bool is_rxq = false; > + > + if (!qinfo->len || !qinfo->buf_size || > + qinfo->len > IECM_CTLQ_MAX_RING_SIZE || > + qinfo->buf_size > IECM_CTLQ_MAX_BUF_LEN) > + return IECM_ERR_CFG; > + > + *cq = kcalloc(1, sizeof(struct iecm_ctlq_info), GFP_KERNEL); > + if (!(*cq)) > + return IECM_ERR_NO_MEMORY; pity there is an iecm_status and not just a generic -ENOMEM. Is there much value in the separate enum? [] @@ -152,7 +388,58 @@ enum iecm_status iecm_ctlq_clean_sq(struct iecm_ctlq_info *cq, > u16 *clean_count, > struct iecm_ctlq_msg *msg_status[]) > { > - /* stub */ > + enum iecm_status ret = 0; > + struct iecm_ctlq_desc *desc; > + u16 i = 0, num_to_clean; > + u16 ntc, desc_err; > + > + if (!cq || !cq->ring_size) > + return IECM_ERR_CTLQ_EMPTY; > + > + if (*clean_count == 0) > + return 0; > + else if (*clean_count > cq->ring_size) > + return IECM_ERR_PARAM; unnecessary else > + > + mutex_lock(&cq->cq_lock); > + > + ntc = cq->next_to_clean; > + > + num_to_clean = *clean_count; > + > + for (i = 0; i < num_to_clean; i++) { > + /* Fetch next descriptor and check if marked as done */ > + desc = IECM_CTLQ_DESC(cq, ntc); > + if (!(le16_to_cpu(desc->flags) & IECM_CTLQ_FLAG_DD)) > + break; > + > + desc_err = le16_to_cpu(desc->ret_val); > + if (desc_err) { > + /* strip off FW internal code */ > + desc_err &= 0xff; > + } > + > + msg_status[i] = cq->bi.tx_msg[ntc]; > + msg_status[i]->status = desc_err; > + > + cq->bi.tx_msg[ntc] = NULL; > + > + /* Zero out any stale data */ > + memset(desc, 0, sizeof(*desc)); > + > + ntc++; > + if (ntc == cq->ring_size) > + ntc = 0; > + } > + > + cq->next_to_clean = ntc; > + > + mutex_unlock(&cq->cq_lock); > + > + /* Return number of descriptors actually cleaned */ > + *clean_count = i; > + > + return ret; > } > > /** > @@ -175,7 +462,111 @@ enum iecm_status iecm_ctlq_post_rx_buffs(struct iecm_hw > *hw, > u16 *buff_count, > struct iecm_dma_mem **buffs) > { > - /* stub */ > + enum iecm_status status = 0; > + struct iecm_ctlq_desc *desc; > + u16 ntp = cq->next_to_post; > + bool buffs_avail = false; > + u16 tbp = ntp + 1; > + int i = 0; > + > + if (*buff_count > cq->ring_size) > + return IECM_ERR_PARAM; > + > + if (*buff_count > 0) > + buffs_avail = true; > + > + mutex_lock(&cq->cq_lock); > + > + if (tbp >= cq->ring_size) > + tbp = 0; > + > + if (tbp == cq->next_to_clean) > + /* Nothing to do */ > + goto post_buffs_out; > + > + /* Post buffers for as many as provided or up until the last one used */ > + while (ntp != cq->next_to_clean) { > + desc = IECM_CTLQ_DESC(cq, ntp); > + > + if (!cq->bi.rx_buff[ntp]) { if (cq->bi.rx_buff[ntp]) continue; and unindent? > + if (!buffs_avail) { > + /* If the caller hasn't given us any buffers or > + * there are none left, search the ring itself > + * for an available buffer to move to this > + * entry starting at the next entry in the ring > + */ > + tbp = ntp + 1; > + > + /* Wrap ring if necessary */ > + if (tbp >= cq->ring_size) > + tbp = 0; > + > + while (tbp != cq->next_to_clean) { > + if (cq->bi.rx_buff[tbp]) { > + cq->bi.rx_buff[ntp] = > + cq->bi.rx_buff[tbp]; > + cq->bi.rx_buff[tbp] = NULL; > + > + /* Found a buffer, no need to > + * search anymore > + */ > + break; > + } > + > + /* Wrap ring if necessary */ > + tbp++; > + if (tbp >= cq->ring_size) > + tbp = 0; > + } > + > + if (tbp == cq->next_to_clean) > + goto post_buffs_out; > + } else { > + /* Give back pointer to DMA buffer */ > + cq->bi.rx_buff[ntp] = buffs[i]; > + i++; > + > + if (i >= *buff_count) > + buffs_avail = false; > + } > + } > + > + desc->flags = > + cpu_to_le16(IECM_CTLQ_FLAG_BUF | IECM_CTLQ_FLAG_RD); > + > + /* Post buffers to descriptor */ > + desc->datalen = cpu_to_le16(cq->bi.rx_buff[ntp]->size); > + desc->params.indirect.addr_high = > + cpu_to_le32(IECM_HI_DWORD(cq->bi.rx_buff[ntp]->pa)); > + desc->params.indirect.addr_low = > + cpu_to_le32(IECM_LO_DWORD(cq->bi.rx_buff[ntp]->pa)); > + > + ntp++; > + if (ntp == cq->ring_size) > + ntp = 0; > + } [] > @@ -186,7 +555,27 @@ iecm_wait_for_event(struct iecm_adapter *adapter, > enum iecm_vport_vc_state state, > enum iecm_vport_vc_state err_check) > { > - /* stub */ > + enum iecm_status status; > + int event; > + > + event = wait_event_timeout(adapter->vchnl_wq, > + test_and_clear_bit(state, adapter->vc_state), > + msecs_to_jiffies(500)); > + if (event) { > + if (test_and_clear_bit(err_check, adapter->vc_state)) { > + dev_err(&adapter->pdev->dev, > + "VC response error %d", err_check); missing format terminating newlines > + status = IECM_ERR_CFG; > + } else { > + status = 0; > + } > + } else { > + /* Timeout occurred */ > + status = IECM_ERR_NOT_READY; > + dev_err(&adapter->pdev->dev, "VC timeout, state = %u", state); here too