[Intel-gfx] [PATCH] drm/i915: add kerneldoc for i915_audio_component

[Yang, Libin]

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of
> Daniel Vetter
> Sent: Wednesday, September 02, 2015 8:18 PM
> To: Yang, Libin
> Cc: intel-gfx at lists.freedesktop.org; daniel.vetter at ffwll.ch;
> jani.nikula at linux.intel.com; ville.syrjala at linux.intel.com
> Subject: Re: [PATCH] drm/i915: add kerneldoc for
> i915_audio_component
> 
> On Wed, Sep 02, 2015 at 02:12:24PM +0800, libin.yang at intel.com
> wrote:
> > From: Libin Yang <libin.yang at intel.com>
> >
> > Add the kerneldoc for i915_audio_component in i915_component.h
> >
> > Signed-off-by: Libin Yang <libin.yang at intel.com>
> > ---
> >  include/drm/i915_component.h | 39 ++++++++++++++++++++++++--
> -------------
> >  1 file changed, 24 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/drm/i915_component.h
> b/include/drm/i915_component.h
> > index 8ad6f1b..187acc8 100644
> > --- a/include/drm/i915_component.h
> > +++ b/include/drm/i915_component.h
> > @@ -24,23 +24,32 @@
> >  #ifndef _I915_COMPONENT_H_
> >  #define _I915_COMPONENT_H_
> >
> > +/**
> > + * struct i915_audio_component_ops - callbacks defined in gfx
> driver
> > + * @owner: the module owner
> > + * @get_power: get the POWER_DOMAIN_AUDIO power well
> > + * @put_power: put the POWER_DOMAIN_AUDIO power well
> > + * @codec_wake_override: Enable/Disable generating the codec
> wake signal
> > + * @get_cdclk_freq: get the Core Display Clock in KHz
> > + * @sync_audio_rate: set n/cts based on the sample rate
> > + */
> > +struct i915_audio_component_ops {
> > +	struct module *owner;
> 
> New kerneldoc in 4.3 allows you to split structure documentation up
> into
> per-member comments. Especially with vtables I think this makes a lot
> of
> sense, since then you have enough space to document where and how
> exactly
> a given hook is called (looks, place in the overall sequence).
> 
> Also please include your stancas in the drm.tmpl docbook template,
> otherwise it won't be included in the html docs. And finally please add

OK, I will add it in drm.tmpl.

> a
> DOC: overview section which explains at a high level how i915 and
> hda-intel corporate for hdmi/dp audio.

Where the DOC should be located in?

> 
> Thanks, Daniel
> 
> > +	void (*get_power)(struct device *);
> > +	void (*put_power)(struct device *);
> > +	void (*codec_wake_override)(struct device *, bool enable);
> > +	int (*get_cdclk_freq)(struct device *);
> > +	int (*sync_audio_rate)(struct device *, int port, int rate);
> > +};
> > +
> > +/**
> > + * struct i915_audio_component - used for audio video interaction
> > + * @dev: the device from gfx driver
> > + * @ops: callback for audio driver calling
> > + */
> >  struct i915_audio_component {
> >  	struct device *dev;
> > -
> > -	const struct i915_audio_component_ops {
> > -		struct module *owner;
> > -		void (*get_power)(struct device *);
> > -		void (*put_power)(struct device *);
> > -		void (*codec_wake_override)(struct device *, bool
> enable);
> > -		int (*get_cdclk_freq)(struct device *);
> > -		/**
> > -		 * @sync_audio_rate: set n/cts based on the sample
> rate
> > -		 *
> > -		 * Called from audio driver. After audio driver sets the
> > -		 * sample rate, it will call this function to set n/cts
> > -		 */
> > -		int (*sync_audio_rate)(struct device *, int port, int
> rate);
> > -	} *ops;
> > +	const struct i915_audio_component_ops *ops;
> >  };
> >
> >  #endif /* _I915_COMPONENT_H_ */
> > --
> > 1.9.1
> >
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch