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=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 EBD83C282C0 for ; Wed, 23 Jan 2019 22:47:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C3A562184C for ; Wed, 23 Jan 2019 22:47:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727082AbfAWWrI (ORCPT ); Wed, 23 Jan 2019 17:47:08 -0500 Received: from mga03.intel.com ([134.134.136.65]:32507 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726109AbfAWWrI (ORCPT ); Wed, 23 Jan 2019 17:47:08 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 23 Jan 2019 14:47:07 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,513,1539673200"; d="scan'208";a="128368644" Received: from dwesterg-mobl.amr.corp.intel.com ([10.252.129.243]) by orsmga002.jf.intel.com with ESMTP; 23 Jan 2019 14:47:06 -0800 Message-ID: <49028e9cdf59afe3d4184b9eee36f0d2406c8bdb.camel@linux.intel.com> Subject: Re: [PATCH] net: altera_tse: fix msgdma_tx_completion on non-zero fill_level case From: Dalon L Westergreen Reply-To: dalon.westergreen@linux.intel.com To: thor.thayer@linux.intel.com, Atsushi Nemoto , netdev@vger.kernel.org Cc: Tomonori Sakita Date: Wed, 23 Jan 2019 14:47:06 -0800 In-Reply-To: References: <20190121.172926.1822036089248860935.atsushi.nemoto@sord.co.jp> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.30.4 (3.30.4-1.fc29) 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 Tue, 2019-01-22 at 11:20 -0600, Thor Thayer wrote: > On 1/21/19 2:29 AM, Atsushi Nemoto wrote: > > From: Tomonori Sakita > > > > If fill_level was not zero and status was not BUSY, > > result of "tx_prod - tx_cons - inuse" might be zero. > > Subtracting 1 unconditionally results invalid negative return value > > on this case. > > The subtraction is not needed if fill_level was not zero. > > > > Signed-off-by: Tomonori Sakita > > Signed-off-by: Atsushi Nemoto > > --- > > drivers/net/ethernet/altera/altera_msgdma.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/altera/altera_msgdma.c > > b/drivers/net/ethernet/altera/altera_msgdma.c > > index 0fb986b..3df73d3 100644 > > --- a/drivers/net/ethernet/altera/altera_msgdma.c > > +++ b/drivers/net/ethernet/altera/altera_msgdma.c > > @@ -145,7 +145,7 @@ u32 msgdma_tx_completions(struct altera_tse_private > > *priv) > > & 0xffff; > > > > if (inuse) { /* Tx FIFO is not empty */ > > - ready = priv->tx_prod - priv->tx_cons - inuse - 1; > > + ready = priv->tx_prod - priv->tx_cons - inuse; dont think my last email went through.. I am not sure about this. This register indicates the number of entries still to be processed by the dma. the -1 is intended to represent the decriptor currently being processed. If ready is priv->tx_prod - priv->tx_cons - inuse couldn't you end up processing 1 too many packets? IE: ready is 1 greater then the actual completed packets? I do agree that we should not be returning a negative value, but i dont think i agree removing the -1 is the answer. perhaps just check that ready is greater than 0? --dalon > > } else { > > /* Check for buffered last packet */ > > status = csrrd32(priv->tx_dma_csr, msgdma_csroffs(status)); > > > > I'm adding Dalon who has done a lot of work on msgdma. I'd like to get > his comments. > > Thanks, > > Thor >