[alsa-devel] [PATCH v4 03/14] ASoC: hdac_hdmi: Use list to add pins and converters

[Mark Brown]

On Fri, Jan 08, 2016 at 07:24:46PM +0530, Vinod Koul wrote:
> On Fri, Jan 08, 2016 at 01:32:51PM +0000, Mark Brown wrote:
> > On Wed, Dec 09, 2015 at 09:46:10PM +0530, Subhransu S. Prusty wrote:

> > > Also it may be required to connect any converter to any pin
> > > dynamically as per different use cases (for example DP is
> > > connected to pin 6 on skylake board). So this will help in
> > > dynamically select and route.

> > This sounds like it's supposed to be supporting multiple outputs but...

....

> > ...it looks like we still only support one random pin and one random
> > convertor?  How do we know if we got the right ones?  I *think* this
> > mostly ends up doing the same thing as the previous version but if
> > that's the case why are we doing it?

> This is the start of conversion to have multiple outputs supported. This
> patch only converts to use list so that multiple pins can be used

> Actual code to add support for three skl pins is in this series as explained
> in the cover letter

This is the sort of thing that needs to be in the patch changelogs.  I
know that at some point we want to get to having multiple outputs but
right now I'm reviewing this individual patch and I need to be able to
tell if it does what it's supposed to be doing.  Please do work on the
quality of your changelogs, as I keep saying difficulty in following
what the code is supposed to be doing is a constant problem with the
Skylake patches.

> > As I've said before one of the reasons it sometimes takes a long time to
> > review these things is that there's a lot of big patch serieses getting
> > sent which aren't that well explained.

> > > -	snd_hdac_codec_write(&edev->hdac, pin_nid, 0,
> > > +	snd_hdac_codec_write(&edev->hdac, pin->nid, 0,

> > Huge portions of this patch are a simple refactoring like this, it'd be
> > good to have split those out as a separate patch so the actual content
> > is more visible.

> Yes this patch is only refactoring to use list for pins

It looks like there's some other stuff going on too.  The changelog
certainly seems to suggest there is.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20160108/3da01636/attachment.sig>