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.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel