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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 09, 2015 at 09:46:10PM +0530, Subhransu S. Prusty wrote:

> Future platforms may have a different set of pins/converters.
> So use lists to add pins and converters based on enumeration.

> 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...

> +	/*
> +	 * Currently on board only 1 pin and 1 converter is enabled for
> +	 * simplification, more will be added eventually
> +	 * So using fixed map for dai_id:pin:cvt
> +	 */
> +	cvt = list_first_entry(&hdmi->cvt_list, struct hdac_hdmi_cvt, head);
> +	pin = list_first_entry(&hdmi->pin_list, struct hdac_hdmi_pin, head);

...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?

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.

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux