[alsa-devel] [PATCH v2 0/2] Add audio support for Microsoft Surface 3 tablet
This series adds audio support for the Surface 3 tablet. This tablet identifes an rt5645 audio codec using the ACPI identifer 10EC5640, which is already associated with the rt5640, thus a quirk is required to have the correct drivers loaded. Additionally, in order for jack-detection and the tablet's internal microphone to work, the Surface 3 must be configured in the audio codec driver similarly to other devices. Resolves: https://bugzilla.kernel.org/show_bug.cgi?id=98001 Changes in v2: Remove alsa-lib changes from series Stephen Just (2): ASoC: rt5645: Add support for Surface 3 tablet ASoC: Intel: Atom: add quirk for CHT w/ RT5645 sound/soc/codecs/rt5645.c | 7 +++++++ sound/soc/intel/atom/sst/sst_acpi.c | 29 ++++++++++++++++++++++++++++- sound/soc/intel/boards/cht_bsw_rt5645.c | 1 + 3 files changed, 36 insertions(+), 1 deletion(-) -- 2.7.4
* Add a dmi match for Surface 3 * Add ACPI match "10EC5640" The Surface 3 tablet identifies the codec in ACPI as 10EC5640, but other devices use this identifier to match the rt5640 codec. However, the probe code for each driver will filter out mismatches. Cc: Mark Brown <broonie@kernel.org> Cc: Vinod Koul <vinod.koul@intel.com> Signed-off-by: Stephen Just <stephenjust@gmail.com> --- sound/soc/codecs/rt5645.c | 7 +++++++ 1 file changed, 7 insertions(+) v1 -> v2: No change diff --git a/sound/soc/codecs/rt5645.c b/sound/soc/codecs/rt5645.c index 3c6594d..761ca88 100644 --- a/sound/soc/codecs/rt5645.c +++ b/sound/soc/codecs/rt5645.c @@ -3531,6 +3531,7 @@ MODULE_DEVICE_TABLE(i2c, rt5645_i2c_id); static const struct acpi_device_id rt5645_acpi_match[] = { { "10EC5645", 0 }, { "10EC5650", 0 }, + { "10EC5640", 0 }, {}, }; MODULE_DEVICE_TABLE(acpi, rt5645_acpi_match); @@ -3561,6 +3562,12 @@ static const struct dmi_system_id dmi_platform_intel_braswell[] = { DMI_MATCH(DMI_PRODUCT_NAME, "Setzer"), }, }, + { + .ident = "Microsoft Surface 3", + .matches = { + DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"), + }, + }, { } }; -- 2.7.4
On Mon, Jul 04, 2016 at 10:53:47AM -0600, Stephen Just wrote:
* Add a dmi match for Surface 3 * Add ACPI match "10EC5640"
The Surface 3 tablet identifies the codec in ACPI as 10EC5640, but other devices use this identifier to match the rt5640 codec. However, the probe code for each driver will filter out mismatches.
Cc: Mark Brown <broonie@kernel.org> Cc: Vinod Koul <vinod.koul@intel.com> Signed-off-by: Stephen Just <stephenjust@gmail.com> --- sound/soc/codecs/rt5645.c | 7 +++++++ 1 file changed, 7 insertions(+)
v1 -> v2: No change
That is wrong!
diff --git a/sound/soc/codecs/rt5645.c b/sound/soc/codecs/rt5645.c index 3c6594d..761ca88 100644 --- a/sound/soc/codecs/rt5645.c +++ b/sound/soc/codecs/rt5645.c @@ -3531,6 +3531,7 @@ MODULE_DEVICE_TABLE(i2c, rt5645_i2c_id); static const struct acpi_device_id rt5645_acpi_match[] = { { "10EC5645", 0 }, { "10EC5650", 0 }, + { "10EC5640", 0 },
AT least here you have a change, but earlier rev was the one I posted in BZ. It is NOT nice to take other peoples change and post them as your patches.
{}, }; MODULE_DEVICE_TABLE(acpi, rt5645_acpi_match); @@ -3561,6 +3562,12 @@ static const struct dmi_system_id dmi_platform_intel_braswell[] = { DMI_MATCH(DMI_PRODUCT_NAME, "Setzer"), }, }, + { + .ident = "Microsoft Surface 3", + .matches = { + DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"), + }, + }, { } };
-- 2.7.4
-- ~Vinod
On Mon, 04 Jul 2016 18:53:47 +0200, Stephen Just wrote:
* Add a dmi match for Surface 3 * Add ACPI match "10EC5640"
The Surface 3 tablet identifies the codec in ACPI as 10EC5640, but other devices use this identifier to match the rt5640 codec. However, the probe code for each driver will filter out mismatches.
Cc: Mark Brown <broonie@kernel.org> Cc: Vinod Koul <vinod.koul@intel.com> Signed-off-by: Stephen Just <stephenjust@gmail.com> --- sound/soc/codecs/rt5645.c | 7 +++++++ 1 file changed, 7 insertions(+)
v1 -> v2: No change
diff --git a/sound/soc/codecs/rt5645.c b/sound/soc/codecs/rt5645.c index 3c6594d..761ca88 100644 --- a/sound/soc/codecs/rt5645.c +++ b/sound/soc/codecs/rt5645.c @@ -3531,6 +3531,7 @@ MODULE_DEVICE_TABLE(i2c, rt5645_i2c_id); static const struct acpi_device_id rt5645_acpi_match[] = { { "10EC5645", 0 }, { "10EC5650", 0 }, + { "10EC5640", 0 },
Better to put in the right order. Takashi
The Surface 3 uses an RT5645 audio codec, but the ACPI identifier used is that for the RT5640. Use a DMI match to override the driver_data provided by ACPI to workaround ID conflict. Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Cc: Mark Brown <broonie@kernel.org> Cc: Vinod Koul <vinod.koul@intel.com> Signed-off-by: Stephen Just <stephenjust@gmail.com> --- sound/soc/intel/atom/sst/sst_acpi.c | 29 ++++++++++++++++++++++++++++- sound/soc/intel/boards/cht_bsw_rt5645.c | 1 + 2 files changed, 29 insertions(+), 1 deletion(-) v1 -> v2: No change diff --git a/sound/soc/intel/atom/sst/sst_acpi.c b/sound/soc/intel/atom/sst/sst_acpi.c index 3bc4b63..ef40608 100644 --- a/sound/soc/intel/atom/sst/sst_acpi.c +++ b/sound/soc/intel/atom/sst/sst_acpi.c @@ -19,6 +19,7 @@ */ #include <linux/module.h> +#include <linux/dmi.h> #include <linux/fs.h> #include <linux/interrupt.h> #include <linux/slab.h> @@ -131,6 +132,8 @@ static struct sst_platform_info chv_platform_data = { .platform = "sst-mfld-platform", }; +static const struct dmi_system_id dmi_platform_quirk_table[]; + static int sst_platform_get_resources(struct intel_sst_drv *ctx) { struct resource *rsrc; @@ -224,6 +227,7 @@ static int sst_acpi_probe(struct platform_device *pdev) struct platform_device *mdev; struct platform_device *plat_dev; struct sst_platform_info *pdata; + const struct dmi_system_id *dmi_match; unsigned int dev_id; id = acpi_match_device(dev->driver->acpi_match_table, dev); @@ -231,7 +235,13 @@ static int sst_acpi_probe(struct platform_device *pdev) return -ENODEV; dev_dbg(dev, "for %s", id->id); - mach = (struct sst_acpi_mach *)id->driver_data; + /* check for quirk when getting driver_data */ + dmi_match = dmi_first_match(dmi_platform_quirk_table); + if (dmi_match) + mach = (struct sst_acpi_mach *)dmi_match->driver_data; + else + mach = (struct sst_acpi_mach *)id->driver_data; + mach = sst_acpi_find_machine(mach); if (mach == NULL) { dev_err(dev, "No matching machine driver found\n"); @@ -345,7 +355,24 @@ static struct sst_acpi_mach sst_acpi_chv[] = { /* some CHT-T platforms rely on RT5640, use Baytrail machine driver */ {"10EC5640", "bytcr_rt5640", "intel/fw_sst_22a8.bin", "bytcr_rt5640", NULL, &chv_platform_data }, + {}, +}; +static struct sst_acpi_mach sst_acpi_chv_quirk[] = { + /* some CHT-T platforms rely on RT5645 but use the ID from RT5640 */ + {"10EC5640", "cht-bsw-rt5645", "intel/fw_sst_22a8.bin", "cht-bsw", NULL, + &chv_platform_data }, + {}, +}; + +static const struct dmi_system_id dmi_platform_quirk_table[] = { + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"), + DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"), + }, + .driver_data = (unsigned long *) &sst_acpi_chv_quirk, + }, {}, }; diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index d7ef292..bf99efb 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -340,6 +340,7 @@ static struct snd_soc_card snd_soc_card_chtrt5650 = { }; static struct cht_acpi_card snd_soc_cards[] = { + {"10EC5640", CODEC_TYPE_RT5645, &snd_soc_card_chtrt5645}, {"10EC5645", CODEC_TYPE_RT5645, &snd_soc_card_chtrt5645}, {"10EC5650", CODEC_TYPE_RT5650, &snd_soc_card_chtrt5650}, }; -- 2.7.4
On Mon, Jul 04, 2016 at 10:53:48AM -0600, Stephen Just wrote:
The Surface 3 uses an RT5645 audio codec, but the ACPI identifier used is that for the RT5640. Use a DMI match to override the driver_data provided by ACPI to workaround ID conflict.
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Cc: Mark Brown <broonie@kernel.org> Cc: Vinod Koul <vinod.koul@intel.com> Signed-off-by: Stephen Just <stephenjust@gmail.com> --- sound/soc/intel/atom/sst/sst_acpi.c | 29 ++++++++++++++++++++++++++++- sound/soc/intel/boards/cht_bsw_rt5645.c | 1 + 2 files changed, 29 insertions(+), 1 deletion(-)
v1 -> v2: No change
diff --git a/sound/soc/intel/atom/sst/sst_acpi.c b/sound/soc/intel/atom/sst/sst_acpi.c index 3bc4b63..ef40608 100644 --- a/sound/soc/intel/atom/sst/sst_acpi.c +++ b/sound/soc/intel/atom/sst/sst_acpi.c @@ -19,6 +19,7 @@ */
#include <linux/module.h> +#include <linux/dmi.h> #include <linux/fs.h> #include <linux/interrupt.h> #include <linux/slab.h> @@ -131,6 +132,8 @@ static struct sst_platform_info chv_platform_data = { .platform = "sst-mfld-platform", };
+static const struct dmi_system_id dmi_platform_quirk_table[]; + static int sst_platform_get_resources(struct intel_sst_drv *ctx) { struct resource *rsrc; @@ -224,6 +227,7 @@ static int sst_acpi_probe(struct platform_device *pdev) struct platform_device *mdev; struct platform_device *plat_dev; struct sst_platform_info *pdata; + const struct dmi_system_id *dmi_match; unsigned int dev_id;
id = acpi_match_device(dev->driver->acpi_match_table, dev); @@ -231,7 +235,13 @@ static int sst_acpi_probe(struct platform_device *pdev) return -ENODEV; dev_dbg(dev, "for %s", id->id);
- mach = (struct sst_acpi_mach *)id->driver_data; + /* check for quirk when getting driver_data */ + dmi_match = dmi_first_match(dmi_platform_quirk_table); + if (dmi_match) + mach = (struct sst_acpi_mach *)dmi_match->driver_data; + else + mach = (struct sst_acpi_mach *)id->driver_data; + mach = sst_acpi_find_machine(mach); if (mach == NULL) { dev_err(dev, "No matching machine driver found\n"); @@ -345,7 +355,24 @@ static struct sst_acpi_mach sst_acpi_chv[] = { /* some CHT-T platforms rely on RT5640, use Baytrail machine driver */ {"10EC5640", "bytcr_rt5640", "intel/fw_sst_22a8.bin", "bytcr_rt5640", NULL, &chv_platform_data }, + {}, +};
+static struct sst_acpi_mach sst_acpi_chv_quirk[] = { + /* some CHT-T platforms rely on RT5645 but use the ID from RT5640 */ + {"10EC5640", "cht-bsw-rt5645", "intel/fw_sst_22a8.bin", "cht-bsw", NULL, + &chv_platform_data }, + {}, +}; + +static const struct dmi_system_id dmi_platform_quirk_table[] = { + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"), + DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"), + }, + .driver_data = (unsigned long *) &sst_acpi_chv_quirk, + }, {}, };
This seeems a bit better than the one I posted on BZ, so I will take it. But I would like to hold off a few days for upstreaming this as there are few other devices with similar configuration and we need solutions for those as well.. I would hate to redo the patches already applied. Also you should add BZ id in the changelog. -- ~Vinod
On Mon, 04 Jul 2016 18:53:48 +0200, Stephen Just wrote:
The Surface 3 uses an RT5645 audio codec, but the ACPI identifier used is that for the RT5640. Use a DMI match to override the driver_data provided by ACPI to workaround ID conflict.
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Cc: Mark Brown <broonie@kernel.org> Cc: Vinod Koul <vinod.koul@intel.com> Signed-off-by: Stephen Just <stephenjust@gmail.com> --- sound/soc/intel/atom/sst/sst_acpi.c | 29 ++++++++++++++++++++++++++++- sound/soc/intel/boards/cht_bsw_rt5645.c | 1 + 2 files changed, 29 insertions(+), 1 deletion(-)
v1 -> v2: No change
diff --git a/sound/soc/intel/atom/sst/sst_acpi.c b/sound/soc/intel/atom/sst/sst_acpi.c index 3bc4b63..ef40608 100644 --- a/sound/soc/intel/atom/sst/sst_acpi.c +++ b/sound/soc/intel/atom/sst/sst_acpi.c @@ -19,6 +19,7 @@ */
#include <linux/module.h> +#include <linux/dmi.h> #include <linux/fs.h> #include <linux/interrupt.h> #include <linux/slab.h> @@ -131,6 +132,8 @@ static struct sst_platform_info chv_platform_data = { .platform = "sst-mfld-platform", };
+static const struct dmi_system_id dmi_platform_quirk_table[]; + static int sst_platform_get_resources(struct intel_sst_drv *ctx) { struct resource *rsrc; @@ -224,6 +227,7 @@ static int sst_acpi_probe(struct platform_device *pdev) struct platform_device *mdev; struct platform_device *plat_dev; struct sst_platform_info *pdata; + const struct dmi_system_id *dmi_match; unsigned int dev_id;
id = acpi_match_device(dev->driver->acpi_match_table, dev); @@ -231,7 +235,13 @@ static int sst_acpi_probe(struct platform_device *pdev) return -ENODEV; dev_dbg(dev, "for %s", id->id);
- mach = (struct sst_acpi_mach *)id->driver_data; + /* check for quirk when getting driver_data */ + dmi_match = dmi_first_match(dmi_platform_quirk_table); + if (dmi_match) + mach = (struct sst_acpi_mach *)dmi_match->driver_data;
dmi_system_id.driver_data is void*, so you need no cast here (unlike acpi_device_id below).
+ else + mach = (struct sst_acpi_mach *)id->driver_data; + mach = sst_acpi_find_machine(mach); if (mach == NULL) { dev_err(dev, "No matching machine driver found\n"); @@ -345,7 +355,24 @@ static struct sst_acpi_mach sst_acpi_chv[] = { /* some CHT-T platforms rely on RT5640, use Baytrail machine driver */ {"10EC5640", "bytcr_rt5640", "intel/fw_sst_22a8.bin", "bytcr_rt5640", NULL, &chv_platform_data }, + {}, +};
+static struct sst_acpi_mach sst_acpi_chv_quirk[] = { + /* some CHT-T platforms rely on RT5645 but use the ID from RT5640 */ + {"10EC5640", "cht-bsw-rt5645", "intel/fw_sst_22a8.bin", "cht-bsw", NULL, + &chv_platform_data }, + {}, +}; + +static const struct dmi_system_id dmi_platform_quirk_table[] = { + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"), + DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"), + }, + .driver_data = (unsigned long *) &sst_acpi_chv_quirk,
Why cast to unsigned long pointer...? Takashi
participants (3)
-
Stephen Just -
Takashi Iwai -
Vinod Koul