Re: [PATCH] USB: drop misleading usb_set_intfdata() kernel doc

[Johan Hovold <johan@kernel.org>]

On Mon, Dec 12, 2022 at 02:27:46PM +0100, Oliver Neukum wrote:
> On 12.12.22 14:14, Johan Hovold wrote:
> > On Mon, Dec 12, 2022 at 12:13:54PM +0100, Oliver Neukum wrote:

> > 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?
> 
> Because knowing that one of them exists makes the other much more
> obvious.

I doubt anyone finds out about this function by reading generated kernel
documentation. You look at a driver, grep the function and look at the
single-line implementation.

Driver data is such as integral part of the driver model so it's kinda
hard to miss. Still dev_set_drvdata() also has no kernel doc either.
 
> > 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.
> 
> Yes, but those are not the alternatives.
> Removing the perfectly good part of the kerneldoc is a needless regression,
> albeit a minor one.

But it was never perfectly good. It claims that a driver "should" use it,
when it may not need to (think simple driver using devres) and talks
about driver core resetting the pointer which is irrelevant.

> > 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
> 
> So what happens if user space calls read() or write() on an existing fd?
> Sorry, but this is an issue we need to be sure about.

No need to be sorry, and I've looked at this before. kernfs handles the
serialisation.

But as I mentioned above, this is again irrelevant for the question at
hand as the driver data is freed before the pointer is set to NULL.

Johan