Re: [[PATCH 0/9] ASoC: Intel: bytcr_rt5651: Cleanup (input mapping quirks)

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

 



Hi,

On 27-06-18 06:40, Pierre-Louis Bossart wrote:
On 6/26/18 2:47 AM, Hans de Goede wrote:
Hi,

On 25-06-18 22:01, Pierre-Louis Bossart wrote:
On 6/24/18 9:06 AM, Hans de Goede wrote:
Hi Mark,

This series is mainly a cleanup series. In the beginning of the rt5651
machine driver some wrong assumptions were made, such as the headset
mic being attached to IN2 (it is on IN3 on the 7 machines I have access
to and on all otherwise known machines).

We use a rt5651 reference board for this codec connector to the MinnowBoard. I am pretty sure for that board IN2 is used for the headset. Please give me a couple of days to double-check.

According to this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/sound/soc/intel/boards/bytcr_rt5651.c?id=60e3b52e9354550c28090237b083b20bbabed598

It is connected to IN3 on the Minnowboard Max with Realtek rt5651 eval board.

And you yourself added a quirk for it being on IN3 for the MinnowBoard Turbot:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/sound/soc/intel/boards/bytcr_rt5651.c?id=416f2b51119b8cdd899b226e4cf683d000797a8b

Note I later renamed the IN3_MAP to IN1_HS_IN3 to make clear that it
is for internal mic on IN1, hs-mis on IN3, as can be seen in:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/sound/soc/intel/boards/bytcr_rt5651.c?id=60e3b52e9354550c28090237b083b20bbabed598
(which is the commit adding both the quirk and updating the Minnowboard Max
  DMI entry to use this quirk).

Yes, you are right that headset is connected on IN3 but I wasn't completely crazy with IN2 - it's used for the line in, I just confused connectors...

The cleanups look ok and well documented, I just find that the simplification of patch 7/9 goes one bridge too far. If we ever get a quirk where the headset is not on IN3 we'll have limited ways of finding the right UCM files. Keeping hs-in3 in the filename is not big deal - your call really.

Not having the hs-in3 in the longname matches what we are doing for
the rt5640, where the headset mic also always is on the same input.

For consistency I think it is good to drop it, so lets keep this patch
set as is.

FWIW, all
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>

Thanks.

Regards,

Hans



And also adding a quirk for a machine with the intmic on IN2, which
later got fixed with a new quirk for machines with 2 internal mics
on both IN1 and IN2 and moving the one machine with the IN2 quirk
over to the new IN1_IN2 quirk, leaving the IN2 quirk as an orphan
quirk for non existing hardware.

Then I made a similar mistake adding the IN2_HS_IN3 quirk, while I
should have used the IN1_HS_IN3 (which itself only exists because
the original IN1 quirk has the headset input mapping wrong),

TL;DR: it is a bit of a mess due to a number of wrong assumptions
about how the inputs where actually routed in the past.

This series cleans this all up in small commits / one bit a time and
refers the original commit messages in its commit messages.

Note patch 10/11 is not a cleanup patch, but is more or less the
reason I took a second look at all the quirks.

Regards,

Hans

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


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

_______________________________________________
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