Received-SPF: pass (google.com: domain of linux-kernel+bounces-174330-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Ratheesh Kannoth <rkannoth@marvell.com>
To: Justin Lai <justinlai0215@realtek.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+AgACfZICAAApUsA==
Date: Thu, 9 May 2024 09:09:00 +0000
Message-ID: 
 <MWHPR1801MB19187C10FEBB29BDACE499B1D3E62@MWHPR1801MB1918.namprd18.prod.outlook.com>
References: <20240508123945.201524-1-justinlai0215@realtek.com>
 <20240508123945.201524-3-justinlai0215@realtek.com>
 <20240509065747.GB1077013@maili.marvell.com>
 <9267c5002e444000bb21e8eef4d4dc07@realtek.com>
In-Reply-To: <9267c5002e444000bb21e8eef4d4dc07@realtek.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?PtRHLfTwvB6BbnD2JiVSNITVohwU5W3AzVrihHsufq88LyXn2tBEb3p03bIY?=
 =?us-ascii?Q?L+pmGKVrt84/jcUStYyV8lkRGrzNWpHzLEkI/w+fD32Q2dZQdWnrJ2PVN2Fe?=
 =?us-ascii?Q?0CY1QnkOXoOZfaly6JdwASKMvS9ShfwE2OdZ839WgFZWGRKJpfF5dn/CfyIM?=
 =?us-ascii?Q?RukLpFLro1Kc4IbkghdilizrxLF4rE/mLRGRWoWmqHpJXlfQERUpAGJJ8eqG?=
 =?us-ascii?Q?vjKoQ8Irqpjje1pMlku06BiIzQ8YLc5AwUwD2wblqB9NasCmmih+E4VVeCR+?=
 =?us-ascii?Q?pPtbaJfMmWBdpwVSlvycM6NrBxZ0COCgZcEMw4ahq1QdJKdXbce5PAmvFK5H?=
 =?us-ascii?Q?ApTp2PsMYTJhaMCNPCM+dTShLc35et8ZI7jsCq6pNGCsSl3uCkD+SmA1NwLX?=
 =?us-ascii?Q?XM1scUdFCP+BHXCvd3R552vOpK3jXyWuSEWk6GCFPava+oDF946MDgVOHMP7?=
 =?us-ascii?Q?amAN7Z7BBGRlYpT6eirpwO7+/p1G4Gfe1DV/fuTx0nEZ3FYUAUtHsh1S6JHV?=
 =?us-ascii?Q?+MgUX6BRmz/0QWxKvSBZtpQ8zD4CflhJp2bPRFmw5bgpYc6xbRTgRspeM9zq?=
 =?us-ascii?Q?7Y/Fk8FWiV1UHpoEX3WhipE0Mm77xkCfzyO+xUHF/cAtOmowRZ1/eQuzlmO+?=
 =?us-ascii?Q?EdtFlM8znn3ncBliLiiGAj+CBYNmgrf2vM09x6FhNu8W/2w6k4eTz2/lLjPt?=
 =?us-ascii?Q?qUhtZG+Vc9+7dMTECuZCpU95bmxznU4XP9FbD1v6a050wCzflfcMrIDl2f/M?=
 =?us-ascii?Q?RWitoGHSgK/xxb32xJL59beEWaaejrgxCCzoO1Wk0LtgnQgTviymXw8nbrqz?=
 =?us-ascii?Q?ld2bD66pH2poqugTZnQWDgOQwUfrwdrPO0OgnuOjBxX9HzcuUlFwertJSBeW?=
 =?us-ascii?Q?H3julcLlmM8HHG2CU8/Rxj8we5j7fBVf2nVRerR2MlDSjYmQ4WsjcL0FFgSQ?=
 =?us-ascii?Q?KzJFcSdr5XZW5PaoWDXOKJFGRfAto8VJYtnM4MHjTRk9DWk4Oz8qxnf/K4uS?=
 =?us-ascii?Q?0dFZRew2EsAXL0JzSQJVzVU75supeqN77oE9YqHZVbQajRh5aFvwREwutnJp?=
 =?us-ascii?Q?UWV2Yro+7Sn2RGLqoKwrKuBa3KSbOGub8BoSAQSyYhrhFhkXYL/gIG16WGlJ?=
 =?us-ascii?Q?O3KS9NiM5xl9atfTyJimJF87h04Tgoz/sTKF4nXyq7f9ncrdVILpRR+V10ZY?=
 =?us-ascii?Q?mJ9+vsulTPYa3L9FMRyf6OqRFRI+LjfZefUi/WL0WARIgMbbk92l84ojjKMh?=
 =?us-ascii?Q?zXj9o7w1X6DVMSW3SX3Js53CCR+nzO+c5BOV4C1FF1O7ObaeuL8CLQxQ6jGQ?=
 =?us-ascii?Q?aTVEf9pbzznX2eEMeWfU/khC2843Mg0LvIlOOkdCCkdLDGnR4FWhpArnhQi9?=
 =?us-ascii?Q?t+T8ZfDF9VtpeESjGwSMrRINRC/K/f8KsXL1HnezchBoO+nHesW1YIcSltL3?=
 =?us-ascii?Q?fS5R4eK+tfs8rAW90kM6lBmXT1FI742DQh96C4Ot38WG0v5tOljfkEHfakWh?=
 =?us-ascii?Q?GSge1wp0Yb09r1tjG9VqxNCJsSTg15vv2AbgIwLPlx1Hj6eyc/883CWC5iRi?=
 =?us-ascii?Q?dhkK/P2Lil1xCU8GW66B/1QH0/sO8glqL6MYKDJz?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1801MB1918.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7d0956c0-bed2-4982-9a5d-08dc7007aa74
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 May 2024 09:09:00.6311
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ExFkBS3J1YhqyeltqVm2DIvxYUTEp5PQO8e9EDniDoFcpZTnQ5B+zRXTV40MYSetlT9bR2VFmt/0/nGWKsnVQA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR18MB4015
X-Proofpoint-GUID: Ek9bMvwSpwIT2tfO33uC2ar32DTNXjmi
X-Proofpoint-ORIG-GUID: Ek9bMvwSpwIT2tfO33uC2ar32DTNXjmi
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-05-09_04,2024-05-08_01,2023-05-22_02

> 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 ?
>=20
> I would like to ask you, are you referring to other places where there ar=
e error
> description messages, but not here?
other functions, you are freeing allocated resources in case of failure, bu=
t here, you are returning error directly.


> > Did you mark the skb for recycle ? Hmm ... did i miss to find the code =
?
> >
> We have done this part when using the skb and before finally releasing th=
e 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 missed=
 it . ignore my comment.=20

>=20
> > > +
> > > +err_free_all_allocated_irq:
> > You are allocating from i =3D 1, but freeing from j =3D 0;
>=20
> 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.=20