Re: [PATCH 07/28] drm: Update drm_plane_funcs kerneldoc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Dec 07, 2015 at 01:34:16PM +0100, Daniel Vetter wrote:
> On Mon, Dec 07, 2015 at 12:46:38PM +0100, Thierry Reding wrote:
> > On Fri, Dec 04, 2015 at 09:45:48AM +0100, Daniel Vetter wrote:
> > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > [...]
> > > +	/**
> > > +	 * @destroy:
> > > +	 *
> > > +	 * Clean up CRTC resources. This is only called at driver unload time
> > 
> > Perhaps drop "only" because there are more than a single potential
> > usage?
> 
> It's for driver unload only since you can't hotplug planes. The only
> hotpluggable thing in drm atm are connectors, and I've added these blurbs
> to highlight that.

The complete hunk is this:

+        * Clean up CRTC resources. This is only called at driver unload time
+        * through drm_mode_config_cleanup(), but can also be called at runtime
+        * when a CRTC is being hot-unplugged.

If you say CRTCs aren't hotpluggable then the above is very confusing.
So either it is only called at driver unload time (which is what you're
saying, since CRTCs aren't hotpluggable) or it can be called in other
circumstances, in which case "only" is wrong.

So I think either drop the last subsentence to reflect the current
capabilities of the subsystem, or make it something like the following
to clarify:

	The DRM subsystem doesn't currently support hotplugging CRTCs,
	but eventually that feature might be added, at which point this
	callback will be called when a CRTC in hot-unplugged.

But since nobody knows when this will happen it's somewhat premature, in
my opinion, to have such documentation. kerneldoc should document facts,
not the roadmap.

Thierry

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux