Re: [PATCH v3 1/7] ASoC: hda - add soc hda codec driver wrapper

[Vinod Koul <vinod.koul@intel.com>]

On Mon, May 04, 2015 at 02:12:26PM +0100, Mark Brown wrote:
> On Wed, Apr 29, 2015 at 01:24:24AM +0530, Vinod Koul wrote:
> 
> > For ASoC HDA codecs we need to provide match function based on id_table and
> > driver register/unregister wrapper functions
> 
> This changelog doesn't really leave me that much the wiser as to what
> this is intended to do...  what are we matching in what ID table and
> what are we wrapping?
> 
> > ---
> >  sound/soc/Kconfig             |    1 +
> >  sound/soc/Makefile            |    1 +
> >  sound/soc/hda/Kconfig         |    3 ++
> >  sound/soc/hda/Makefile        |    3 ++
> >  sound/soc/hda/soc-hda-codec.c |   85 +++++++++++++++++++++++++++++++++++++++++
> >  sound/soc/hda/soc-hda-codec.h |   44 +++++++++++++++++++++
> >  6 files changed, 137 insertions(+)
> 
> If this is for CODECs why is it in a new directory?
yes it is for HDA codecs which are enumerated over HDA links, but this is
the HDA bus code which is ASoC specfic. All generic HDA code which uses the
hda lib (new sound/hda) is kept in sound/soc/hda
All intel driver updates will go to sound/soc/intel/skylake
The codecs drivers will as usual show up in sound/soc/codecs/

> 
> > +const struct soc_hda_device_id * +snd_soc_hda_get_device_id( +
> > struct hdac_device *hdev, +			struct soc_hda_codec_driver
> > *drv)
> 
> Please can we have more normal indentation - put at least the first
> argument for the function on the same line as the function name.
Yes Takashi also pointed out, fixed now. The zeal to keep under 80 chars
made it look funny.

Thanks
-- 
~Vinod