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.