From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2BD14C433E0 for ; Fri, 26 Jun 2020 02:57:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 03842207D8 for ; Fri, 26 Jun 2020 02:57:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728242AbgFZC5g (ORCPT ); Thu, 25 Jun 2020 22:57:36 -0400 Received: from smtprelay0185.hostedemail.com ([216.40.44.185]:54208 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728169AbgFZC5f (ORCPT ); Thu, 25 Jun 2020 22:57:35 -0400 Received: from filter.hostedemail.com (clb03-v110.bra.tucows.net [216.40.38.60]) by smtprelay07.hostedemail.com (Postfix) with ESMTP id 047B1181D337B; Fri, 26 Jun 2020 02:57:35 +0000 (UTC) X-Session-Marker: 6A6F6540706572636865732E636F6D X-HE-Tag: roof83_5f1467e26e52 X-Filterd-Recvd-Size: 7032 Received: from XPS-9350.home (unknown [47.151.133.149]) (Authenticated sender: joe@perches.com) by omf13.hostedemail.com (Postfix) with ESMTPA; Fri, 26 Jun 2020 02:57:32 +0000 (UTC) Message-ID: Subject: Re: [net-next v3 06/15] iecm: Implement mailbox functionality From: Joe Perches To: Jeff Kirsher , davem@davemloft.net Cc: Alice Michael , netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com, Alan Brady , Phani Burra , Joshua Hay , Madhu Chittim , Pavan Kumar Linga , Donald Skidmore , Jesse Brandeburg , Sridhar Samudrala Date: Thu, 25 Jun 2020 19:57:31 -0700 In-Reply-To: <20200626020737.775377-7-jeffrey.t.kirsher@intel.com> References: <20200626020737.775377-1-jeffrey.t.kirsher@intel.com> <20200626020737.775377-7-jeffrey.t.kirsher@intel.com> Content-Type: text/plain; charset="ISO-8859-1" User-Agent: Evolution 3.36.2-0ubuntu1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, 2020-06-25 at 19:07 -0700, Jeff Kirsher wrote: > From: Alice Michael > > 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