Re: [PATCH v2 06/32] ASoC: rt5651: Configure jack-detect source through a device-property

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

 



Hi,

On 01-03-18 20:30, Mark Brown wrote:
On Sun, Feb 25, 2018 at 11:46:47AM +0100, Hans de Goede wrote:
Configure the jack-detect source through a device-property which can be
set by code outside of the codec driver. Rather then putting platform
specific DMI quirks inside the generic codec driver.

And move the jack-detect-source quirk for the KIANO SlimNote 14.2, which
was present inside the codec driver to the machine driver, where we can
bundle it together with the other quirks already present for this laptop.

Multiple things in a patch again :/

Yes because the property replaces the quirk, I can split the changes between
the codec and machine driver into 2 patches but then the intermediate state
will be that the jack-detection no longer works.

The property itself is fine but...

@@ -360,6 +377,13 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime)
  			dev_err(card->dev, "unable to set MCLK rate\n");
  	}
+ props[cnt++] = PROPERTY_ENTRY_U32("realtek,jack-detect-source",
+				BYT_RT5651_JDSRC(byt_rt5651_quirk));
+
+	ret = device_add_properties(codec->dev, props);
+	if (ret)
+		return ret;
+
  	ret = snd_soc_card_jack_new(runtime->card, "Headset",

I'm having a hard time geting comfortable with this; it's all a bit
fragile feeling to me - someone deciding to centralise the property
parsing (eg, if they are using a platform where platform data makes
sense or want to cross-validate with some other property on probe) could
easily break things and there's not even a comment in the CODEC driver.

It is not _that_ fragile, but I agree that it would be good to add
a comment about not moving the property-parsing to the codec driver.

I will rebase and submit a new version with a comment added.
Do you want me to split this patch into separate codec and machine
driver patches while at it?

It'll work but it doesn't seem to match expectations for how firmware
data is provided which is what's bugging me here.

The problem here is that the data is not coming from the firmware,
which is a reality we have to deal with leading to some sort of
compromise.

It at least needs a
"here be dragons" style comment in the code.

Ack.

Ideally we'd have a general way to massage the ACPI data at init time
and could have board files that are instantiated from DMI to do that
(like are needed even more for SFI) but that's a long way off.

Something like DT overlays for ACPI? With the overlays being triggered
by a DMI match? Interesting concept, but given how much discussion there
still is surrounding DT overlays I don't see this happening anytime soon.

Regards,

Hans
_______________________________________________
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