On Mon, Dec 12, 2022 at 12:13:54PM +0100, Oliver Neukum wrote: > > > On 12.12.22 11:31, Johan Hovold wrote: > > On Mon, Dec 12, 2022 at 11:19:00AM +0100, Oliver Neukum wrote: > >> On 11.12.22 13:06, Johan Hovold wrote: > >> > >>> Due to a misunderstanding, a redundant and misleading kernel doc comment > >>> for usb_set_intfdata() was recently added which claimed that the driver > >>> data pointer must not be cleared during disconnect before "all actions > >>> [are] completed", which is both imprecise and incorrect. > >> > >> OK, but is that a reason to remove all kerneldoc? Kerneldoc is generally > >> a good thing. And if a pointer is NULLed by driver core, that will need > >> to be in it. IMHO you'd better just remove the questionable part of the > >> kerneldoc. > > > > Yeah, I started off with just rewriting the kernel doc and removing the > > obviously incorrect bits, but then there is essentially nothing left of > > the documentation. > > 1. that the function exists and its purpose That should be apparent from the function name (and implementation). > 2. its parameters Apparent from the prototype. But sure, it would not show up in generated documentation (like many other functions). > most kerneldoc isn't exactly a great revelation. Nevertheless it > serves a purpose. Yeah, we have a lot of /** * set_x_to_y() - set x to y * @x: the x * @y: the y */ it seems. Not sure how much value there is in that, though. And in this case there was also no kernel doc for usb_get_intfdata() which is equally self documenting. Why add redundant docs for only one of these functions? I'd rather drop this particular documentation which was added due to a misunderstanding then go down the rabbit hole of adding mindless kernel doc to every helper we have. > > A driver does not need to care that the pointer is cleared by driver > > core after the driver is unbound. The driver is gone. > > Is that true even with respect to sysfs? Yes. The (device group) attributes are removed by driver core before ->remove() is called, otherwise you'd have an even bigger issue with the driver data itself which is typically deallocated before the pointer is cleared. Johan
