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=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,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 39D74C282C3 for ; Thu, 24 Jan 2019 14:57:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 049DB21872 for ; Thu, 24 Jan 2019 14:57:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728288AbfAXO51 (ORCPT ); Thu, 24 Jan 2019 09:57:27 -0500 Received: from mga14.intel.com ([192.55.52.115]:9886 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728167AbfAXO51 (ORCPT ); Thu, 24 Jan 2019 09:57:27 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Jan 2019 06:57:26 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,516,1539673200"; d="scan'208";a="112335825" Received: from dwesterg-mobl.amr.corp.intel.com ([10.251.1.179]) by orsmga008.jf.intel.com with ESMTP; 24 Jan 2019 06:57:26 -0800 Message-ID: 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: Atsushi Nemoto Cc: thor.thayer@linux.intel.com, netdev@vger.kernel.org, tomonori.sakita@sord.co.jp Date: Thu, 24 Jan 2019 06:57:25 -0800 In-Reply-To: <20190124.140544.1043020473181933407.atsushi.nemoto@sord.co.jp> References: <20190121.172926.1822036089248860935.atsushi.nemoto@sord.co.jp> <49028e9cdf59afe3d4184b9eee36f0d2406c8bdb.camel@linux.intel.com> <20190124.140544.1043020473181933407.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 Thu, 2019-01-24 at 14:05 +0900, Atsushi Nemoto wrote: > On Wed, 23 Jan 2019 14:47:06 -0800, Dalon L Westergreen < > dalon.westergreen@linux.intel.com> wrote: > > > > 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? > > Thank you for review. > > I agree with you. It would be OK returning a possibly off-by-one > value unless it is not an negative value. > > Then, how about this instead? > This works for me. thanks. > --- a/drivers/net/ethernet/altera/altera_msgdma.c > +++ b/drivers/net/ethernet/altera/altera_msgdma.c > @@ -145,7 +145,8 @@ 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 = max_t(int, > + priv->tx_prod - priv->tx_cons - inuse - 1, 0); > } else { > /* Check for buffered last packet */ > status = csrrd32(priv->tx_dma_csr, msgdma_csroffs(status));