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 :/ 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'll work but it doesn't seem to match expectations for how firmware data is provided which is what's bugging me here. It at least needs a "here be dragons" style comment in the code. 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.
Attachment:
signature.asc
Description: PGP signature