> Hi, > > "Du, Changbin" <changbin...@intel.com> writes: > >> > >> >> > + dwc->regset = NULL; > >> >> > >> >> setting regset to NULL is unnecessary. We only call > dwc3_debugfs_exit() > >> >> when removing the driver. > >> >> > >> >> -- > >> >> Balbi > >> > I'd like keep this line even it is unnecessary, because It is a good > >> > habit to > >> > Avoid wild pointers. Just like the dwc->root = NULL. > >> > >> there won't be any wild pointers here, we'll free struct dwc3 *dwc itself. > >> > >> -- > >> Balbi > > I agree the dwc will be freed in current code. But the 'free' logical is out > > of the debugfs code. They should be treat as some logical independent. > Per > > this point, I still think set pointer to null is not bad. For example, if > > dwc3 > core > > code invoke dwc3_debugfs_exit twice by mistake(just an example case, > not > > really), then no crash/impact for the second call. > > the second call should crash because it's clearly wrong ;-) If dwc3 ever > calls dwc3_debugfs_exit() twice, it really deserves to crash. It's > something so wrong that we want the verbosity and urgency of a kernel > oops to make sure we fix it ASAP. > > If, however, we set it to null, it might be years before we notice > anything's wrong. > > -- > Balbi
Hmm, I agree from this point. I will combine this patch with other two patches (due to their dependency). And I'd like remove the 'dwc->root=NULL' as well, Is it ok for you? Thx, Du, Changbin -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html