Received-SPF: pass (google.com: domain of linux-kernel+bounces-174381-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Justin Lai <justinlai0215@realtek.com>
To: Ratheesh Kannoth <rkannoth@marvell.com>
CC: "kuba@kernel.org" <kuba@kernel.org>,
        "davem@davemloft.net"
	<davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org"
	<netdev@vger.kernel.org>,
        "andrew@lunn.ch" <andrew@lunn.ch>,
        "jiri@resnulli.us" <jiri@resnulli.us>,
        "horms@kernel.org" <horms@kernel.org>,
        Ping-Ke Shih <pkshih@realtek.com>, Larry Chiu <larry.chiu@realtek.com>
Subject: RE: [PATCH net-next v18 02/13] rtase: Implement the .ndo_open function
Thread-Topic: [PATCH net-next v18 02/13] rtase: Implement the .ndo_open
 function
Thread-Index: AQHaoUT5RWh4TO7cuUeh1vkT8i8o3LGN87+AgACfZICAAApUsIAADPfg
Date: Thu, 9 May 2024 09:59:04 +0000
Message-ID: <feeecf2edbe54d999b09068718e9c8b5@realtek.com>
References: <20240508123945.201524-1-justinlai0215@realtek.com>
 <20240508123945.201524-3-justinlai0215@realtek.com>
 <20240509065747.GB1077013@maili.marvell.com>
 <9267c5002e444000bb21e8eef4d4dc07@realtek.com>
 <MWHPR1801MB19187C10FEBB29BDACE499B1D3E62@MWHPR1801MB1918.namprd18.prod.outlook.com>
In-Reply-To: <MWHPR1801MB19187C10FEBB29BDACE499B1D3E62@MWHPR1801MB1918.namprd18.prod.outlook.com>
Accept-Language: zh-TW, en-US
Content-Language: zh-TW
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0

>=20
> > From: Justin Lai <justinlai0215@realtek.com>
> > Sent: Thursday, May 9, 2024 2:29 PM
> > > > +
> > > > +     /* rx and tx descriptors needs 256 bytes alignment.
> > > > +      * dma_alloc_coherent provides more.
> > > > +      */
> > > > +     for (i =3D 0; i < tp->func_tx_queue_num; i++) {
> > > > +             tp->tx_ring[i].desc =3D
> > > > +                             dma_alloc_coherent(&pdev->dev,
> > > > +
> > > RTASE_TX_RING_DESC_SIZE,
> > > > +
> > > &tp->tx_ring[i].phy_addr,
> > > > +
> GFP_KERNEL);
> > > > +             if (!tp->tx_ring[i].desc)
> > > You have handled errors gracefully very where else. why not here ?
> >
> > I would like to ask you, are you referring to other places where there
> > are error description messages, but not here?
> other functions, you are freeing allocated resources in case of failure, =
but here,
> you are returning error directly.
>=20
After returning the error, I will do the corresponding error handling in rt=
ase_open.
.
>=20
> > > Did you mark the skb for recycle ? Hmm ... did i miss to find the cod=
e ?
> > >
> > We have done this part when using the skb and before finally releasing
> > the skb resource. Do you think it would be better to do this part of
> > the process when allocating the skb?
> i think, you added skb_for_recycle() in the following patch. Sorry I miss=
ed it .
> ignore my comment.
>=20

OK, thank you for your feedback.

> >
> > > > +
> > > > +err_free_all_allocated_irq:
> > > You are allocating from i =3D 1, but freeing from j =3D 0;
> >
> > Hi Ratheesh,
> > I have done request_irq() once before the for loop, so there should be
> > no problem starting free from j=3D0 here.
> Thanks for pointing out.

Thank you also for your review.