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=-3.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, 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 1E3BBC433DF for ; Wed, 27 May 2020 08:46:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E5A0D2078C for ; Wed, 27 May 2020 08:46:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=bgdev-pl.20150623.gappssmtp.com header.i=@bgdev-pl.20150623.gappssmtp.com header.b="GxKyCH66" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388211AbgE0IqY (ORCPT ); Wed, 27 May 2020 04:46:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47666 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388075AbgE0IqV (ORCPT ); Wed, 27 May 2020 04:46:21 -0400 Received: from mail-io1-xd43.google.com (mail-io1-xd43.google.com [IPv6:2607:f8b0:4864:20::d43]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0AD0DC03E97D for ; Wed, 27 May 2020 01:46:20 -0700 (PDT) Received: by mail-io1-xd43.google.com with SMTP id c8so4692787iob.6 for ; Wed, 27 May 2020 01:46:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bgdev-pl.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=cpwITzrftWrJAY0LVaSeu6jYTmkJe1PVewzQjLawjdA=; b=GxKyCH668dBc5VipkdT8YFjPTCx5H6Ette6THzOjjs/DRDGNcXIMDQi5ZP5v3Sf9Oa mwVrP6RSqnYeL10DiHJM1SOkKIcn6brwgQRHX0b0oKqUeDUGj/rjNQW55Xq53NQfseRD vXhZTU3mB8ZA8VEHKczO26DPAktbj8hdettUmRE8i6vyRvcYoYFGvApZ8ukhZ5+bQK7D 9L8+5xCW38IQadhYPoyI/PQs166Rw+w8+8SWRYvSIyYFnkV+77MOB9qUtGq2ER6cdPV/ +/sr0UAOp1UfaGH7K0FiwaDoGqoJN+EI3wjFXv9n4Zv/oSigkjbvRq3z7WEgWAq+aZOX 0EsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=cpwITzrftWrJAY0LVaSeu6jYTmkJe1PVewzQjLawjdA=; b=olkLiX6Hk7KnuHXQcZx0FuNknQOlBqOKNyzo9XQSOhR3QgnoQ7xd1Eo98oYZBddWF9 4UKfU7n2eeOdWSnpTivWcSJA5MiejqJe1D6QYQsKEsK1k/BzTi+94BPKuv6Qro+WhvGV cjJcVlvTUaYpZwkF+4hTAyB3CQlm5OlbAZ20f+pLacVAVq13KYyePIupyAI38iH8r3Ne yq6wY3MFGytoOXNxyUvtKpCnLTCS5xvu6+jzHRg/NV1vJ4k7iLWFw91U2pEzrr8UrcJG LgIOz39XcysDqb8sOj134obv/WgksItAOftxhilQVP2/feOCsGgx1vOknpiXhrLuCvjf Ax4Q== X-Gm-Message-State: AOAM531hONXEf0vIUYNHyG6tJN0dgGo7/GxnXlM3xePSI52QMWfc1IGp mNRhsKHHH/TWmRgajKx8ilKago3x7RnMZ4hveNXP/Q== X-Google-Smtp-Source: ABdhPJwMXQdy6+RxEU74Dd9WQ5Tly2o2sH55kV9ZSrhh4NH0gl/D2WFHak2VKletNu2Ql3a2wNrDFJNkEIRMls/7rZo= X-Received: by 2002:a02:3e06:: with SMTP id s6mr4481666jas.57.1590569179357; Wed, 27 May 2020 01:46:19 -0700 (PDT) MIME-Version: 1.0 References: <20200522120700.838-1-brgl@bgdev.pl> <20200522120700.838-7-brgl@bgdev.pl> <20200527073150.GA3384158@ubuntu-s3-xlarge-x86> In-Reply-To: <20200527073150.GA3384158@ubuntu-s3-xlarge-x86> From: Bartosz Golaszewski Date: Wed, 27 May 2020 10:46:08 +0200 Message-ID: Subject: Re: [PATCH v5 06/11] net: ethernet: mtk-star-emac: new driver To: Nathan Chancellor Cc: Rob Herring , "David S . Miller" , Matthias Brugger , John Crispin , Sean Wang , Mark Lee , Jakub Kicinski , Arnd Bergmann , Fabien Parent , Heiner Kallweit , Edwin Peer , devicetree , Linux Kernel Mailing List , netdev , Linux ARM , "moderated list:ARM/Mediatek SoC..." , Stephane Le Provost , Pedro Tsai , Andrew Perepech , Bartosz Golaszewski , clang-built-linux@googlegroups.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Sender: devicetree-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org =C5=9Br., 27 maj 2020 o 09:31 Nathan Chancellor napisa=C5=82(a): > > On Fri, May 22, 2020 at 02:06:55PM +0200, Bartosz Golaszewski wrote: > > > > > diff --git a/drivers/net/ethernet/mediatek/mtk_star_emac.c b/drivers/ne= t/ethernet/mediatek/mtk_star_emac.c > > new file mode 100644 > > index 000000000000..789c77af501f > > --- /dev/null > > +++ b/drivers/net/ethernet/mediatek/mtk_star_emac.c > > @@ -0,0 +1,1678 @@ > > > > I've searched netdev and I cannot find any reports from others but this > function introduces a clang warning: > > drivers/net/ethernet/mediatek/mtk_star_emac.c:1296:6: warning: variable '= new_dma_addr' is used uninitialized whenever 'if' condition is true [-Wsome= times-uninitialized] > if (!new_skb) { > ^~~~~~~~ > drivers/net/ethernet/mediatek/mtk_star_emac.c:1321:23: note: uninitialize= d use occurs here > desc_data.dma_addr =3D new_dma_addr; > ^~~~~~~~~~~~ > drivers/net/ethernet/mediatek/mtk_star_emac.c:1296:2: note: remove the 'i= f' if its condition is always false > if (!new_skb) { > ^~~~~~~~~~~~~~~ > drivers/net/ethernet/mediatek/mtk_star_emac.c:1285:6: warning: variable '= new_dma_addr' is used uninitialized whenever 'if' condition is true [-Wsome= times-uninitialized] > if ((desc_data.flags & MTK_STAR_DESC_BIT_RX_CRCE) || > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/ethernet/mediatek/mtk_star_emac.c:1321:23: note: uninitialize= d use occurs here > desc_data.dma_addr =3D new_dma_addr; > ^~~~~~~~~~~~ > drivers/net/ethernet/mediatek/mtk_star_emac.c:1285:2: note: remove the 'i= f' if its condition is always false > if ((desc_data.flags & MTK_STAR_DESC_BIT_RX_CRCE) || > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/ethernet/mediatek/mtk_star_emac.c:1285:6: warning: variable '= new_dma_addr' is used uninitialized whenever '||' condition is true [-Wsome= times-uninitialized] > if ((desc_data.flags & MTK_STAR_DESC_BIT_RX_CRCE) || > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/ethernet/mediatek/mtk_star_emac.c:1321:23: note: uninitialize= d use occurs here > desc_data.dma_addr =3D new_dma_addr; > ^~~~~~~~~~~~ > drivers/net/ethernet/mediatek/mtk_star_emac.c:1285:6: note: remove the '|= |' if its condition is always false > if ((desc_data.flags & MTK_STAR_DESC_BIT_RX_CRCE) || > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/ethernet/mediatek/mtk_star_emac.c:1274:25: note: initialize t= he variable 'new_dma_addr' to silence this warning > dma_addr_t new_dma_addr; > ^ > =3D 0 > 3 warnings generated. > > > +static int mtk_star_receive_packet(struct mtk_star_priv *priv) > > +{ > > + struct mtk_star_ring *ring =3D &priv->rx_ring; > > + struct device *dev =3D mtk_star_get_dev(priv); > > + struct mtk_star_ring_desc_data desc_data; > > + struct net_device *ndev =3D priv->ndev; > > + struct sk_buff *curr_skb, *new_skb; > > + dma_addr_t new_dma_addr; > > Uninitialized here > > > + int ret; > > + > > + spin_lock(&priv->lock); > > + ret =3D mtk_star_ring_pop_tail(ring, &desc_data); > > + spin_unlock(&priv->lock); > > + if (ret) > > + return -1; > > + > > + curr_skb =3D desc_data.skb; > > + > > + if ((desc_data.flags & MTK_STAR_DESC_BIT_RX_CRCE) || > > + (desc_data.flags & MTK_STAR_DESC_BIT_RX_OSIZE)) { > > + /* Error packet -> drop and reuse skb. */ > > + new_skb =3D curr_skb; > > + goto push_new_skb; > > this goto > > > + } > > + > > + /* Prepare new skb before receiving the current one. Reuse the cu= rrent > > + * skb if we fail at any point. > > + */ > > + new_skb =3D mtk_star_alloc_skb(ndev); > > + if (!new_skb) { > > + ndev->stats.rx_dropped++; > > + new_skb =3D curr_skb; > > + goto push_new_skb; > > and this goto > > > + } > > + > > + new_dma_addr =3D mtk_star_dma_map_rx(priv, new_skb); > > + if (dma_mapping_error(dev, new_dma_addr)) { > > + ndev->stats.rx_dropped++; > > + dev_kfree_skb(new_skb); > > + new_skb =3D curr_skb; > > + netdev_err(ndev, "DMA mapping error of RX descriptor\n"); > > + goto push_new_skb; > > + } > > + > > + /* We can't fail anymore at this point: it's safe to unmap the sk= b. */ > > + mtk_star_dma_unmap_rx(priv, &desc_data); > > + > > + skb_put(desc_data.skb, desc_data.len); > > + desc_data.skb->ip_summed =3D CHECKSUM_NONE; > > + desc_data.skb->protocol =3D eth_type_trans(desc_data.skb, ndev); > > + desc_data.skb->dev =3D ndev; > > + netif_receive_skb(desc_data.skb); > > + > > +push_new_skb: > > + desc_data.dma_addr =3D new_dma_addr; > > assign it uninitialized here. > > > + desc_data.len =3D skb_tailroom(new_skb); > > + desc_data.skb =3D new_skb; > > + > > + spin_lock(&priv->lock); > > + mtk_star_ring_push_head_rx(ring, &desc_data); > > + spin_unlock(&priv->lock); > > + > > + return 0; > > +} > > I don't know if there should be a new label that excludes that > assignment for those particular gotos or if new_dma_addr should > be initialized to something at the top. Please take a look at > addressing this when you get a chance. > > Cheers, > Nathan Hi Nathan, Thanks for reporting this! I have a fix ready and will send it shortly. Bartosz