Messages in this thread | | | From | Justin Lai <> | Subject | RE: [PATCH net-next v18 02/13] rtase: Implement the .ndo_open function | Date | Thu, 9 May 2024 09:59:04 +0000 |
| |
> > > 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 = 0; i < tp->func_tx_queue_num; i++) { > > > > + tp->tx_ring[i].desc = > > > > + 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. > After returning the error, I will do the corresponding error handling in rtase_open. . > > > > 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 > > 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 missed it . > ignore my comment. >
OK, thank you for your feedback.
> > > > > > + > > > > +err_free_all_allocated_irq: > > > You are allocating from i = 1, but freeing from j = 0; > > > > Hi Ratheesh, > > I have done request_irq() once before the for loop, so there should be > > no problem starting free from j=0 here. > Thanks for pointing out.
Thank you also for your review.
| |