Hi,
On 03-01-18 19:42, Pierre-Louis Bossart wrote:
Commit 78f5605c0329 ("ASoC: rt5645: cleanup DMI matching code") did a lot of useful cleanups, but the logic is a bit flawed: if there is no DMI match on an ACPI machine, the code reads non-existent properties.
Reading non existent properties is not a problem, notice that the check only checks if one of the properties is present, the others may still be unset, which is fine, then the code reading the properties will leave everything 0 / false.
Re-add previous code that checked for the existence of those properties and a fallback mode to enable jd-mode3 if there is no pdata, device property or quirk. This will help avoid having to add new DMI quirks when only the jd-mode is needed for headset playback.
Note I do think that defaulting to jd_mode = 3 is a good idea, I just believe that the commit message wrongly give the impression that reading non-existent props is a problem.
Cc: Hans de Goede hdegoede@redhat.com Cc: Bard Liao bardliao@realtek.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/codecs/rt5645.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/rt5645.c b/sound/soc/codecs/rt5645.c index ebc4f5970f24..476020445f65 100644 --- a/sound/soc/codecs/rt5645.c +++ b/sound/soc/codecs/rt5645.c @@ -3687,6 +3687,17 @@ static const struct dmi_system_id dmi_platform_data[] = { { } };
+static bool rt5645_check_dp(struct device *dev) +{
- if (device_property_present(dev, "realtek,in2-differential") ||
device_property_present(dev, "realtek,dmic1-data-pin") ||device_property_present(dev, "realtek,dmic2-data-pin") ||device_property_present(dev, "realtek,jd-mode"))return true;- return false;
+}
- static int rt5645_parse_dt(struct rt5645_priv *rt5645, struct device *dev) { rt5645->pdata.in2_diff = device_property_read_bool(dev,
@@ -3727,8 +3738,10 @@ static int rt5645_i2c_probe(struct i2c_client *i2c,
if (pdata) rt5645->pdata = *pdata;
- else
- else if (rt5645_check_dp(&i2c->dev)) rt5645_parse_dt(rt5645, &i2c->dev);
- else
rt5645->pdata = jd_mode3_platform_data;
How about:
rt5645->pdata.jd_mode = 3;
instead and remove jd_mode3_platform_data and the 2 dmi table entries pointing to it ?
Regards,
Hans
if (quirk != -1) { rt5645->pdata.in2_diff = QUIRK_IN2_DIFF(quirk);