On Mon, Mar 11, 2024 at 01:04:18PM +0100, Hardik Gajjar wrote: > On Fri, Mar 08, 2024 at 05:47:37PM +0530, Krishna Kurapati PSSNV wrote: > > > > > > On 3/8/2024 5:25 PM, Hardik Gajjar wrote: > > > On Thu, Mar 07, 2024 at 11:12:07PM +0530, Krishna Kurapati PSSNV wrote: > > > > > > > > [...] > > > > > > > > I believe using gether_cleanup altogether may not be an optimal solution. > > > The creation and deletion of network interfaces should align with the behavior of each specific network driver. > > > > > > For instance, in the case of NCM, the usb0 interface is created upon the creation of a directory > > > in config/usb_gadget/gX/functions/ncm.usb0, and it is removed when the corresponding directory > > > is deleted. This follows a standard flow observed in many network drivers, where interfaces are > > > created during driver loading/probing and deleted during removal. > > > > > > > Hi Hardik, > > > > Yeah. I observed this behavior. > > > > > Typically, deleting the gadget on every disconnection is not considered a good practice, as it can > > > negatively impact the user experience when accessing the gadget. > > > > > > > Got it. I was suspecting issues might come up during fast Plug-In/ Plug-Out > > cases as setting and cleanup of network interface might take some time. > > > > > In our specific scenario, retaining the usb0 network interface has proven to enhance performance > > > and stabilize connections. Previous attempts to remove it resulted in an observed increase in time of 300ms, > > > particularly at the start of Apple CarPlay sessions. > > > > > > > Thanks for this info. Makes sense to keep it in free_inst only. But my > > initial doubt only stemmed from the fact that actions taken during bind must > > be reversed during unbind. > > > > > Furthermore, it's important to highlight that in Qualcomm products and msm kernels, the inclusion of gether_cleanup > > > in the unbind process was eventually reverted. While the specific reason for reverting the patch is unknown, > > > it suggests that the addition may not have yielded the intended or required results > > > > > > Following is the revert patch details in msm-5.4 kernel, if you want check it. > > > > > > Revert "usb: gadget: f_ncm: allocate/free net device upon driver bind/unbind" > > > > > > This reverts commit 006d8adf555a8c6d34113f564ade312d68abd3b3. > > > > > > Move back the allocation of netdevice to alloc_inst(), one-time > > > registration to bind(), deregistration and free to rm_inst(). The > > > UI update issue will be taken up with proper stakeholders. > > > > > > Change-Id: I56448b08f6796a43ec5b0dfe0dd2d42cdc0eec14 > > > > > > > Agree. This was merged first in 4.19 downstream (most probably for car play > > use case only) and then reverted in 5.4. But present 4.19 code in QC still > > keeps it in unbind path. The only reason I suspect it was reverted was to > > get back on to same page with upstream driver (atleast starting from 5.10, > > this was the reasoning to remove gether_cleanup in unbind path and put it > > back in free_inst). > > > > Thanks, > > Krishna, > > Thanks Krinshna for your comment > Come to the first comment from Greg. > > > What commit id does this fix? > > > > thanks, > > > > greg k-h > > >Hi Greg, > > >The network device parent is not being properly cleaned up during unbind since the \ > >initial commit of the driver. In that case, should I mention the First commit ID of \ > >the driver as broken commit or would you advice to say, For example "Clean up netdev \ > >parent in unbind". > > >Thanks, > >Hardik > > @Greg, > > Can you please provide guidance on how to proceed? Should I update the commit message to exclude the term 'Fix' in the title? > > Thanks, > Hardik Hi Greg, Should we move forward if there are no further comments? Thanks, Hardik
