[alsa-devel] [PATCH v5 1/4] drm/i915: Add audio sync_audio_rate callback

[Yang, Libin]

Hi Daniel,

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of
> Daniel Vetter
> Sent: Wednesday, August 26, 2015 4:18 PM
> To: Yang, Libin
> Cc: alsa-devel at alsa-project.org; tiwai at suse.de; intel-
> gfx at lists.freedesktop.org; daniel.vetter at ffwll.ch;
> jani.nikula at linux.intel.com
> 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 at intel.com
> wrote:
> > From: Libin Yang <libin.yang at intel.com>
> >
> > 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 at intel.com>
> > ---
> >  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