Hi Daniel, > -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of > Daniel Vetter > Sent: Wednesday, August 26, 2015 4:18 PM > To: Yang, Libin > Cc: alsa-devel@xxxxxxxxxxxxxxxx; tiwai@xxxxxxx; intel- > gfx@xxxxxxxxxxxxxxxxxxxxx; daniel.vetter@xxxxxxxx; > jani.nikula@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v5 1/4] drm/i915: Add audio sync_audio_rate > callback > > On Tue, Aug 18, 2015 at 02:51:51PM +0800, libin.yang@xxxxxxxxx > wrote: > > From: Libin Yang <libin.yang@xxxxxxxxx> > > > > Add the sync_audio_rate callback. > > > > With the callback, audio driver can trigger > > i915 driver to set the proper N/CTS or N/M > > based on different sample rates. > > > > Signed-off-by: Libin Yang <libin.yang@xxxxxxxxx> > > --- > > include/drm/i915_component.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/include/drm/i915_component.h > b/include/drm/i915_component.h > > index c9a8b64..aabebcb 100644 > > --- a/include/drm/i915_component.h > > +++ b/include/drm/i915_component.h > > @@ -33,6 +33,7 @@ struct i915_audio_component { > > 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); > > We're missing kerneldoc for this entire struct here, which isn't great. > This needs to be fixed. Please > - pull out the ops structure so it's not inlined (kerneldoc can't handle > nested ops structures). > - please document all the callbacks with kerneldoc. In 4.3 we can have > kerneldoc in-line in structures right above each member like this > > /** > * @put_power: > * > * Longer text explaining this hook. > */ > void (*put_power)(struct device *); > > For each hook please explain at least who calls it (gfx or audio) and > where exactly it's called in the overall flow. > - Extended the overview doc section with references to the > component/ops > structure would be needed too. > > And please make sure it all looks pretty with make htmldocs. Could we start another patch session for this task because, as you know, this is a little independent on these patches? What do you think? Regards, Libin > > Thanks, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
