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 23:35, Hans de Goede wrote:
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.

So one other solution which comes to mind here is to move the
snd_soc_card_jack_new() call into the codec driver's
rt5651_apply_properties() function (conditional on a jack src being
set in the properties).

This would make the machine driver code look like this:

	props[cnt++] = PROPERTY_ENTRY_...
	props[cnt++] = PROPERTY_ENTRY_...
	props[cnt++] = PROPERTY_ENTRY_...
	props[cnt++] = PROPERTY_ENTRY_...

	ret = device_add_properties(codec->dev, props);
	if (ret)
		return ret;

	ret = rt5651_apply_properties(codec);
	if (ret)
		return ret;

Which makes the ordering really clear without needing any
comments.

This will also make the jack-detect device-properties work with
devicetree platforms without any changes to their platform code,
which currently is not the case since the non Intel platform-code
does not call snd_soc_component_set_jack().

Thinking more about this I believe that doing the
snd_soc_card_jack_new() call inside the codec driver based on
device-properties is a better solution then doing it in the
machine driver.

Please let me know if you agree then I will rework the remaining
patches accordingly.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux