Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Jose Abreu <Jose.Abreu@synopsys.com>
To:     Ong Boon Leong <boon.leong.ong@intel.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
CC:     Giuseppe Cavallaro <peppe.cavallaro@st.com>,
        Alexandre Torgue <alexandre.torgue@st.com>,
        "David S . Miller" <davem@davemloft.net>,
        "Maxime Coquelin" <mcoquelin.stm32@gmail.com>,
        Tan Tee Min <tee.min.tan@intel.com>,
        Voon Weifeng <weifeng.voon@intel.com>,
        "linux-stm32@st-md-mailman.stormreply.com" 
        <linux-stm32@st-md-mailman.stormreply.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH net 1/7] net: stmmac: fix error in updating rx tail
 pointer to last free entry
Thread-Topic: [PATCH net 1/7] net: stmmac: fix error in updating rx tail
 pointer to last free entry
Thread-Index: AQHVyfow7Yt+TlL/YEW2hhh7BAxzpqfoYdLQ
Date:   Mon, 13 Jan 2020 10:29:26 +0000
Message-ID: <BN8PR12MB32661345472470F495EFAC0DD3350@BN8PR12MB3266.namprd12.prod.outlook.com>
References: <1578967276-55956-1-git-send-email-boon.leong.ong@intel.com>
 <1578967276-55956-2-git-send-email-boon.leong.ong@intel.com>
In-Reply-To: <1578967276-55956-2-git-send-email-boon.leong.ong@intel.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: synopsys.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 683ed4e5-9ff6-424c-70c2-08d79813771d
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Jan 2020 10:29:26.6033
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: c33c9f88-1eb7-4099-9700-16013fd9e8aa
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ugTI5NAl7jF87GxkMIAFWDGRjO0JuXjZEiO1hoVfONfzLwoYP8EBGP6YPw+onFZV1v8MX0DWqdoobPPD0EgowA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR12MB3155
X-OriginatorOrg: synopsys.com
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Ong Boon Leong <boon.leong.ong@intel.com>
Date: Jan/14/2020, 02:01:10 (UTC+00:00)

> DMA_CH(#i)_RxDesc_Tail_Pointer points to an offset from the base and
> indicates the location of the last valid descriptor.
>=20
> The change introduced by "net: stmmac: Update RX Tail Pointer to last
> free entry" incorrectly updates the RxDesc_Tail_Pointer and causess
> Rx operation to freeze in corner case. The issue is explained as
> follow:-
>=20
> Say, cur_rx=3D1 and dirty_rx=3D0, then we have dirty=3D1 and entry=3D0 be=
fore
> the while (dirty-- > 0) loop of stmmac_rx_refill() is entered. When
> the while loop is 1st entered, Rx buffer[entry=3D0] is refilled and after
> entry++, then, entry=3D1. Now, the while loop condition check "dirty-- > =
0"
> and the while loop bails out because dirty=3D0. Up to this point, the
> driver code works correctly.
>=20
> However, the current implementation sets the Rx Tail Pointer to the
> location pointed by dirty_rx, just updated to the value of entry(=3D1).
> This is incorrect because the last Rx buffer that is refileld with empty
> buffer is with entry=3D0. In another words, the current logics always set=
s
> Rx Tail Pointer to the next Rx buffer to be refilled (too early).
>=20
> So, we fix this by tracking the index of the most recently refilled Rx
> buffer by using "last_refill" and use "last_refill" to update the Rx Tail
> Pointer instead of using "entry" which points to the next dirty_rx to be
> refilled in future.

I'm not sure about this ...

RX Tail points to last valid descriptor but it doesn't point to the base=20
address of that one, it points to the end address.

Let's say we have a ring buffer with just 1 descriptor. With your new=20
logic then: RX base =3D=3D RX tail (=3D=3D RX base), so the IP will not see=
 any=20
descriptor. But with old logic: RX base =3D=3D (RX base + 1), which causes=
=20
the IP to correctly see the descriptor.

Can you provide more information on the Rx operation freeze you=20
mentioned ? Can it be another issue ?

---
Thanks,
Jose Miguel Abreu