ASoC: rt5670/cht_bsw_rt5672: Some bugfixes and cleanups
Hi All,
This series is mainly the result of me working on fixing the extra set of speakers in the Lenovo Miix 2 10 kbd dock not working with Linux (patches 1 and 3) while working on this I noticed some other bugs (patches 2 and 4) and I also ended up doing some generic cleanups (patches 5 and 6). For patches 1-3 or 1-4 it would be nice it they can go to a future 5.8-rc# 5-6 are mostly -next material.
Regards,
Hans
The default mode for SSP configuration is TDM 4 slot and so far we were using this for the bus format on cht-bsw-rt56732 boards.
One board, the Lenovo Miix 2 10 uses not 1 but 2 codecs connected to SSP2. The second piggy-backed, output-only codec is inside the keyboard-dock (which has extra speakers). Unlike the main rt5672 codec, we cannot configure this codec, it is hard coded to use 2 channel 24 bit I2S.
Using 4 channel TDM leads to the dock speakers codec (which listens in on the data send from the SSP to the rt5672 codec) emiting horribly distorted sound.
Since we only support 2 channels anyways, there is no need for TDM on any cht-bsw-rt5672 designs. So we can simply use I2S 2ch everywhere.
This commit fixes the Lenovo Miix 2 10 dock speakers issue by changing the bus format set in cht_codec_fixup() to I2S 2 channel.
This change has been tested on the following devices with a rt5672 codec:
Lenovo Miix 2 10 Lenovo Thinkpad 8 Lenovo Thinkpad 10 (gen 1)
Cc: stable@vger.kernel.org BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1786723 Signed-off-by: Hans de Goede hdegoede@redhat.com --- sound/soc/intel/boards/cht_bsw_rt5672.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c index 7a43c70a1378..22e432768edb 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5672.c +++ b/sound/soc/intel/boards/cht_bsw_rt5672.c @@ -253,21 +253,20 @@ static int cht_codec_fixup(struct snd_soc_pcm_runtime *rtd, params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
/* - * Default mode for SSP configuration is TDM 4 slot + * Default mode for SSP configuration is TDM 4 slot. One board/design, + * the Lenovo Miix 2 10 uses not 1 but 2 codecs connected to SSP2. The + * second piggy-backed, output-only codec is inside the keyboard-dock + * (which has extra speakers). Unlike the main rt5672 codec, we cannot + * configure this codec, it is hard coded to use 2 channel 24 bit I2S. + * Since we only support 2 channels anyways, there is no need for TDM + * on any cht-bsw-rt5672 designs. So we simply use I2S 2ch everywhere. */ - ret = snd_soc_dai_set_fmt(asoc_rtd_to_codec(rtd, 0), - SND_SOC_DAIFMT_DSP_B | - SND_SOC_DAIFMT_IB_NF | + ret = snd_soc_dai_set_fmt(asoc_rtd_to_cpu(rtd, 0), + SND_SOC_DAIFMT_I2S | + SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS); if (ret < 0) { - dev_err(rtd->dev, "can't set format to TDM %d\n", ret); - return ret; - } - - /* TDM 4 slots 24 bit, set Rx & Tx bitmask to 4 active slots */ - ret = snd_soc_dai_set_tdm_slot(asoc_rtd_to_codec(rtd, 0), 0xF, 0xF, 4, 24); - if (ret < 0) { - dev_err(rtd->dev, "can't set codec TDM slot %d\n", ret); + dev_err(rtd->dev, "can't set format to I2S, err %d\n", ret); return ret; }
On 6/28/20 10:52 AM, Hans de Goede wrote:
The default mode for SSP configuration is TDM 4 slot and so far we were using this for the bus format on cht-bsw-rt56732 boards.
One board, the Lenovo Miix 2 10 uses not 1 but 2 codecs connected to SSP2. The second piggy-backed, output-only codec is inside the keyboard-dock (which has extra speakers). Unlike the main rt5672 codec, we cannot configure this codec, it is hard coded to use 2 channel 24 bit I2S.
Using 4 channel TDM leads to the dock speakers codec (which listens in on the data send from the SSP to the rt5672 codec) emiting horribly distorted sound.
Since we only support 2 channels anyways, there is no need for TDM on any cht-bsw-rt5672 designs. So we can simply use I2S 2ch everywhere.
This commit fixes the Lenovo Miix 2 10 dock speakers issue by changing the bus format set in cht_codec_fixup() to I2S 2 channel.
This change has been tested on the following devices with a rt5672 codec:
Lenovo Miix 2 10 Lenovo Thinkpad 8 Lenovo Thinkpad 10 (gen 1)
Cc: stable@vger.kernel.org BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1786723 Signed-off-by: Hans de Goede hdegoede@redhat.com
sound/soc/intel/boards/cht_bsw_rt5672.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c index 7a43c70a1378..22e432768edb 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5672.c +++ b/sound/soc/intel/boards/cht_bsw_rt5672.c @@ -253,21 +253,20 @@ static int cht_codec_fixup(struct snd_soc_pcm_runtime *rtd, params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
/*
* Default mode for SSP configuration is TDM 4 slot
* Default mode for SSP configuration is TDM 4 slot. One board/design,* the Lenovo Miix 2 10 uses not 1 but 2 codecs connected to SSP2. The* second piggy-backed, output-only codec is inside the keyboard-dock* (which has extra speakers). Unlike the main rt5672 codec, we cannot* configure this codec, it is hard coded to use 2 channel 24 bit I2S.* Since we only support 2 channels anyways, there is no need for TDM* on any cht-bsw-rt5672 designs. So we simply use I2S 2ch everywhere.
The change looks fine, but you may want to add additional comments to explain what happens here.
The default mode for the SSP configuration is TDM 4 slot for the cpu-dai (hard-coded in DSP firmware) the default mode for the SSP configuration is I2S for the codec-dai (hard-coded in the 'SSP2-Codec" .dai_fmt masks, so far unused).
So with this change, you trade one dynamic configuration for another (was codec, not cpu). It works because of the pre-existing hard-coded values for the parts that are not set in this function.
- ret = snd_soc_dai_set_fmt(asoc_rtd_to_codec(rtd, 0),
SND_SOC_DAIFMT_DSP_B |SND_SOC_DAIFMT_IB_NF |
- ret = snd_soc_dai_set_fmt(asoc_rtd_to_cpu(rtd, 0),
SND_SOC_DAIFMT_I2S |SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS);
dev_err(rtd->dev, "can't set format to TDM %d\n", ret);return ret;- }
- /* TDM 4 slots 24 bit, set Rx & Tx bitmask to 4 active slots */
- ret = snd_soc_dai_set_tdm_slot(asoc_rtd_to_codec(rtd, 0), 0xF, 0xF, 4, 24);
- if (ret < 0) {
dev_err(rtd->dev, "can't set codec TDM slot %d\n", ret);
dev_err(rtd->dev, "can't set format to I2S, err %d\n", ret); > return ret;
On Sun, 28 Jun 2020 17:52:26 +0200, Hans de Goede wrote:
The default mode for SSP configuration is TDM 4 slot and so far we were using this for the bus format on cht-bsw-rt56732 boards.
One board, the Lenovo Miix 2 10 uses not 1 but 2 codecs connected to SSP2. The second piggy-backed, output-only codec is inside the keyboard-dock (which has extra speakers). Unlike the main rt5672 codec, we cannot configure this codec, it is hard coded to use 2 channel 24 bit I2S.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: Intel: cht_bsw_rt5672: Change bus format to I2S 2 channel commit: 0ceb8a36d023d4bb4ffca3474a452fb1dfaa0ef2 [2/4] ASoC: rt5670: Correct RT5670_LDO_SEL_MASK commit: 5cacc6f5764e94fa753b2c1f5f7f1f3f74286e82 [3/4] ASoC: rt5670: Add new gpio1_is_ext_spk_en quirk and enable it on the Lenovo Miix 2 10 commit: 85ca6b17e2bb96b19caac3b02c003d670b66de96 [4/4] ASoC: rt5670: Fix dac- and adc- vol-tlv values being off by a factor of 10 commit: 3f31f7d9b5404a10648abe536c8b408bfb4502e1
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.7.6, v5.4.49, v4.19.130, v4.14.186, v4.9.228, v4.4.228.
v5.7.6: Build OK! v5.4.49: Failed to apply! Possible dependencies: 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer") 157b006f6be46 ("ASoC: bdw-rt5677: Add a DAI link for rt5677 SPI PCM device") 17fe95d6df932 ("ASoC: Intel: boards: Add CML m/c using RT1011 and RT5682") 332719b1840b9 ("ASoC: Intel: bytcr_rt5640: Remove code duplication in byt_rt5640_codec_fixup") 35dc19ad86fdf ("ASoC: Intel: Add machine driver for da7219_max98373") 461c623270e4f ("ASoC: rt5677: Load firmware via SPI using delayed work") 4f0637eae56f0 ("ASoC: Intel: common: add ACPI matching tables for JSL") 57ad18906f242 ("ASoC: Intel: bxt-da7219-max98357a: common hdmi codec support") 59bbd703ea2ea ("ASoC: intel: sof_rt5682: common hdmi codec support") 5b425814f13f3 ("ASoC: intel: Add Broadwell rt5650 machine driver") 7d2ae58376658 ("ASoC: Intel: bxt_rt298: common hdmi codec support") 8039105987fcd ("ASoC: Intel: boards: sof_rt5682: use dependency on SOF_HDA_LINK") a0e0d135427cf ("ASoC: rt5677: Add a PCM device for streaming hotword via SPI") ba0b3a977ecf5 ("ASoC: rt5677: Set ADC clock to use PLL and enable ASRC") dfe87aa86cd92 ("ASoC: Intel: glk_rt5682_max98357a: common hdmi codec support") f40ed2e8db8d5 ("ASoC: Intel: sof_pcm512x: add support for SOF platforms with pcm512x")
v4.19.130: Failed to apply! Possible dependencies: 0b7990e38971d ("ASoC: add for_each_rtd_codec_dai() macro") 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer") 10b02b53a9986 ("ASoC: Intel: select relevant machine drivers for SOF") 35bc99aaa1a3a ("ASoC: Intel: Skylake: Add more platform granularity") 5b425814f13f3 ("ASoC: intel: Add Broadwell rt5650 machine driver") 6bae5ea949892 ("ASoC: hdac_hda: add asoc extension for legacy HDA codec drivers") 7c33b5f16915a ("ASoC: Intel: Boards: Machine driver for SKL+ w/ HDAudio codecs") 8c4e7c2ee8096 ("ASoC: Intel: Skylake: fix Kconfigs, make HDaudio codec optional") 98061fdbfccc0 ("ASoC: add for_each_card_links() macro") bca0ac1d96739 ("ASoC: Intel: Boards: Add KBL Dialog Maxim I2S machine driver") bcb1fd1fcd650 ("ASoC: add for_each_card_rtds() macro") e894efef9ac7c ("ASoC: core: add support to card rebind")
v4.14.186: Failed to apply! Possible dependencies: 0b7990e38971d ("ASoC: add for_each_rtd_codec_dai() macro") 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer") 1a11d88f499ce ("ASoC: meson: add tdm formatter base driver") 273d778ef38a8 ("ASoC: snd_soc_component_driver has endianness") 291bfb928863d ("ASoC: topology: Revert recent changes while boot errors are investigated") 45f8cb57da0d7 ("ASoC: core: Allow topology to override machine driver FE DAI link config.") 53eb4b7aaa045 ("ASoC: meson: add axg spdif output") 57d552e3ea760 ("ASoC: meson: add axg frddr driver") 5d61f0ba6524d ("ASoC: pcm: Sync delayed work before releasing resources") 69941bab7c7ae ("ASoC: snd_soc_component_driver has non_legacy_dai_naming") 6dc4fa179fb86 ("ASoC: meson: add axg fifo base driver") 7864a79f37b55 ("ASoC: meson: add axg sound card support") 7a679ea75a1bc ("ASoC: Intel: Enable tdm slots for max98927") 7ba236ce58bd7 ("ASoC: add Component level set_bias_level") 7ed4877b403c9 ("ASoC: meson: add axg toddr driver") 98061fdbfccc0 ("ASoC: add for_each_card_links() macro") 9900a4226c785 ("ASoC: remove unneeded dai->driver->ops check") a655de808cbde ("ASoC: core: Allow topology to override machine driver FE DAI link config.") bcb1fd1fcd650 ("ASoC: add for_each_card_rtds() macro") bf14adcc4ddd1 ("ASoC: Intel: cht-bsw-rt5672: allow for topology-defined codec-dai setup") c41c2a355b863 ("ASoC: meson: add tdm output driver") d60e4f1e4be5e ("ASoC: meson: add tdm interface driver") e0dac41b8c21d ("ASoC: soc-core: add snd_soc_add_component()") f11a5c27f9287 ("ASoC: core: Add name prefix for machines with topology rewrites") f523acebbb74f ("ASoC: add Component level pcm_new/pcm_free v2") fbb16563c6c2b ("ASoC: snd_soc_component_driver has pmdown_time")
v4.9.228: Failed to apply! Possible dependencies: 0b7990e38971d ("ASoC: add for_each_rtd_codec_dai() macro") 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer") 17fb175520e54 ("ASoC: Define API to find a dai link") 1a11d88f499ce ("ASoC: meson: add tdm formatter base driver") 1a653aa447256 ("ASoC: core: replace aux_comp_list to component_dev_list") 273d778ef38a8 ("ASoC: snd_soc_component_driver has endianness") 2a18483a7fb41 ("ASoC: Intel: Add Kabylake machine driver for RT5514, RT5663 and MAX98927") 44c07365e9e2c ("ASoC: add Component level set_jack") 53eb4b7aaa045 ("ASoC: meson: add axg spdif output") 57d552e3ea760 ("ASoC: meson: add axg frddr driver") 69941bab7c7ae ("ASoC: snd_soc_component_driver has non_legacy_dai_naming") 6dc4fa179fb86 ("ASoC: meson: add axg fifo base driver") 71ccef0df533c ("ASoC: add Component level set_sysclk") 759db1c4660b5 ("ASoC: Intel: boards: add card for MinnowBoardMax/Up I2S access") 7864a79f37b55 ("ASoC: meson: add axg sound card support") 7a679ea75a1bc ("ASoC: Intel: Enable tdm slots for max98927") 7ba236ce58bd7 ("ASoC: add Component level set_bias_level") 7ed4877b403c9 ("ASoC: meson: add axg toddr driver") 804e73adf5cf4 ("ASoC: rt5670: Fix GPIO headset detection regression") 82cf89de2c9c2 ("ASoC: Intel: add machine driver for BYT/CHT + DA7213") 9178feb4538e0 ("ASoC: add Component level suspend/resume") 98061fdbfccc0 ("ASoC: add for_each_card_links() macro") a655de808cbde ("ASoC: core: Allow topology to override machine driver FE DAI link config.") bcb1fd1fcd650 ("ASoC: add for_each_card_rtds() macro") bf14adcc4ddd1 ("ASoC: Intel: cht-bsw-rt5672: allow for topology-defined codec-dai setup") c41c2a355b863 ("ASoC: meson: add tdm output driver") d60e4f1e4be5e ("ASoC: meson: add tdm interface driver") d9fc40639dc1b ("ASoC: core: replace codec_dev_list to component_dev_list on Card") ec040dd5ef647 ("ASoC: Intel: Add Kabylake Realtek Maxim machine driver") ef641e5d5e6c7 ("ASoC: add Component level set_pll") fbb16563c6c2b ("ASoC: snd_soc_component_driver has pmdown_time")
v4.4.228: Failed to apply! Possible dependencies: 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer") 17fb175520e54 ("ASoC: Define API to find a dai link") 1a497983a5ae6 ("ASoC: Change the PCM runtime array to a list") 49a5ba1cd9da4 ("ASoC: soc_bind_dai_link() directly returns success for a bound DAI link") 6f2f1ff0de83a ("ASoC: Change 2nd argument of soc_bind_dai_link() to DAI link pointer") 804e73adf5cf4 ("ASoC: rt5670: Fix GPIO headset detection regression") 923c5e61ecd9b ("ASoC: Define soc_init_dai_link() to wrap link intialization.") 98061fdbfccc0 ("ASoC: add for_each_card_links() macro") bcb1fd1fcd650 ("ASoC: add for_each_card_rtds() macro") bf14adcc4ddd1 ("ASoC: Intel: cht-bsw-rt5672: allow for topology-defined codec-dai setup") f8f80361d07d5 ("ASoC: Implement DAI links in a list & define API to add/remove a link")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
On 7/1/20 2:33 PM, Sasha Levin wrote:
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.7.6, v5.4.49, v4.19.130, v4.14.186, v4.9.228, v4.4.228.
v5.7.6: Build OK! v5.4.49: Failed to apply! Possible dependencies: 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer")
This patch is probably the missing dependency, but it's quite large and invasive.
if we wanted to apply this patch to stable versions < 5.7, we should replace all occurrences of
asoc_rtd_to_cpu(rtd, 0) by rtd->cpu_dai
and
asoc_rtd_to_codec(rtd, 0) by rtd->codec_dai
Hi,
On 7/1/20 9:46 PM, Pierre-Louis Bossart wrote:
On 7/1/20 2:33 PM, Sasha Levin wrote:
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.7.6, v5.4.49, v4.19.130, v4.14.186, v4.9.228, v4.4.228.
v5.7.6: Build OK! v5.4.49: Failed to apply! Possible dependencies: 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer")
This patch is probably the missing dependency, but it's quite large and invasive.
if we wanted to apply this patch to stable versions < 5.7, we should replace all occurrences of
asoc_rtd_to_cpu(rtd, 0) by rtd->cpu_dai
and
asoc_rtd_to_codec(rtd, 0) by rtd->codec_dai
This fix affects only 1 model tablet, so I think it is fine to just add it to 5.7 and skip it for older kernels.
Regards,
Hans
On Sun, 28 Jun 2020 17:52:26 +0200, Hans de Goede wrote:
The default mode for SSP configuration is TDM 4 slot and so far we were using this for the bus format on cht-bsw-rt56732 boards.
One board, the Lenovo Miix 2 10 uses not 1 but 2 codecs connected to SSP2. The second piggy-backed, output-only codec is inside the keyboard-dock (which has extra speakers). Unlike the main rt5672 codec, we cannot configure this codec, it is hard coded to use 2 channel 24 bit I2S.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: rt5670: Remove struct rt5670_platform_data commit: c14f61a89c1335f95d9b37624ee157fb1fd424ee [2/2] ASoC: rt5670: Rename dev_gpio to gpio1_is_irq commit: 883330c11fa6dca55e30f8612398b3e0abc51dc5
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.7.6, v5.4.49, v4.19.130, v4.14.186, v4.9.228, v4.4.228.
v5.7.6: Build OK! v5.4.49: Failed to apply! Possible dependencies: 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer") 157b006f6be46 ("ASoC: bdw-rt5677: Add a DAI link for rt5677 SPI PCM device") 17fe95d6df932 ("ASoC: Intel: boards: Add CML m/c using RT1011 and RT5682") 332719b1840b9 ("ASoC: Intel: bytcr_rt5640: Remove code duplication in byt_rt5640_codec_fixup") 35dc19ad86fdf ("ASoC: Intel: Add machine driver for da7219_max98373") 461c623270e4f ("ASoC: rt5677: Load firmware via SPI using delayed work") 4f0637eae56f0 ("ASoC: Intel: common: add ACPI matching tables for JSL") 57ad18906f242 ("ASoC: Intel: bxt-da7219-max98357a: common hdmi codec support") 59bbd703ea2ea ("ASoC: intel: sof_rt5682: common hdmi codec support") 5b425814f13f3 ("ASoC: intel: Add Broadwell rt5650 machine driver") 7d2ae58376658 ("ASoC: Intel: bxt_rt298: common hdmi codec support") 8039105987fcd ("ASoC: Intel: boards: sof_rt5682: use dependency on SOF_HDA_LINK") a0e0d135427cf ("ASoC: rt5677: Add a PCM device for streaming hotword via SPI") ba0b3a977ecf5 ("ASoC: rt5677: Set ADC clock to use PLL and enable ASRC") dfe87aa86cd92 ("ASoC: Intel: glk_rt5682_max98357a: common hdmi codec support") f40ed2e8db8d5 ("ASoC: Intel: sof_pcm512x: add support for SOF platforms with pcm512x")
v4.19.130: Failed to apply! Possible dependencies: 0b7990e38971d ("ASoC: add for_each_rtd_codec_dai() macro") 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer") 10b02b53a9986 ("ASoC: Intel: select relevant machine drivers for SOF") 35bc99aaa1a3a ("ASoC: Intel: Skylake: Add more platform granularity") 5b425814f13f3 ("ASoC: intel: Add Broadwell rt5650 machine driver") 6bae5ea949892 ("ASoC: hdac_hda: add asoc extension for legacy HDA codec drivers") 7c33b5f16915a ("ASoC: Intel: Boards: Machine driver for SKL+ w/ HDAudio codecs") 8c4e7c2ee8096 ("ASoC: Intel: Skylake: fix Kconfigs, make HDaudio codec optional") 98061fdbfccc0 ("ASoC: add for_each_card_links() macro") bca0ac1d96739 ("ASoC: Intel: Boards: Add KBL Dialog Maxim I2S machine driver") bcb1fd1fcd650 ("ASoC: add for_each_card_rtds() macro") e894efef9ac7c ("ASoC: core: add support to card rebind")
v4.14.186: Failed to apply! Possible dependencies: 0b7990e38971d ("ASoC: add for_each_rtd_codec_dai() macro") 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer") 1a11d88f499ce ("ASoC: meson: add tdm formatter base driver") 273d778ef38a8 ("ASoC: snd_soc_component_driver has endianness") 291bfb928863d ("ASoC: topology: Revert recent changes while boot errors are investigated") 45f8cb57da0d7 ("ASoC: core: Allow topology to override machine driver FE DAI link config.") 53eb4b7aaa045 ("ASoC: meson: add axg spdif output") 57d552e3ea760 ("ASoC: meson: add axg frddr driver") 5d61f0ba6524d ("ASoC: pcm: Sync delayed work before releasing resources") 69941bab7c7ae ("ASoC: snd_soc_component_driver has non_legacy_dai_naming") 6dc4fa179fb86 ("ASoC: meson: add axg fifo base driver") 7864a79f37b55 ("ASoC: meson: add axg sound card support") 7a679ea75a1bc ("ASoC: Intel: Enable tdm slots for max98927") 7ba236ce58bd7 ("ASoC: add Component level set_bias_level") 7ed4877b403c9 ("ASoC: meson: add axg toddr driver") 98061fdbfccc0 ("ASoC: add for_each_card_links() macro") 9900a4226c785 ("ASoC: remove unneeded dai->driver->ops check") a655de808cbde ("ASoC: core: Allow topology to override machine driver FE DAI link config.") bcb1fd1fcd650 ("ASoC: add for_each_card_rtds() macro") bf14adcc4ddd1 ("ASoC: Intel: cht-bsw-rt5672: allow for topology-defined codec-dai setup") c41c2a355b863 ("ASoC: meson: add tdm output driver") d60e4f1e4be5e ("ASoC: meson: add tdm interface driver") e0dac41b8c21d ("ASoC: soc-core: add snd_soc_add_component()") f11a5c27f9287 ("ASoC: core: Add name prefix for machines with topology rewrites") f523acebbb74f ("ASoC: add Component level pcm_new/pcm_free v2") fbb16563c6c2b ("ASoC: snd_soc_component_driver has pmdown_time")
v4.9.228: Failed to apply! Possible dependencies: 0b7990e38971d ("ASoC: add for_each_rtd_codec_dai() macro") 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer") 17fb175520e54 ("ASoC: Define API to find a dai link") 1a11d88f499ce ("ASoC: meson: add tdm formatter base driver") 1a653aa447256 ("ASoC: core: replace aux_comp_list to component_dev_list") 273d778ef38a8 ("ASoC: snd_soc_component_driver has endianness") 2a18483a7fb41 ("ASoC: Intel: Add Kabylake machine driver for RT5514, RT5663 and MAX98927") 44c07365e9e2c ("ASoC: add Component level set_jack") 53eb4b7aaa045 ("ASoC: meson: add axg spdif output") 57d552e3ea760 ("ASoC: meson: add axg frddr driver") 69941bab7c7ae ("ASoC: snd_soc_component_driver has non_legacy_dai_naming") 6dc4fa179fb86 ("ASoC: meson: add axg fifo base driver") 71ccef0df533c ("ASoC: add Component level set_sysclk") 759db1c4660b5 ("ASoC: Intel: boards: add card for MinnowBoardMax/Up I2S access") 7864a79f37b55 ("ASoC: meson: add axg sound card support") 7a679ea75a1bc ("ASoC: Intel: Enable tdm slots for max98927") 7ba236ce58bd7 ("ASoC: add Component level set_bias_level") 7ed4877b403c9 ("ASoC: meson: add axg toddr driver") 804e73adf5cf4 ("ASoC: rt5670: Fix GPIO headset detection regression") 82cf89de2c9c2 ("ASoC: Intel: add machine driver for BYT/CHT + DA7213") 9178feb4538e0 ("ASoC: add Component level suspend/resume") 98061fdbfccc0 ("ASoC: add for_each_card_links() macro") a655de808cbde ("ASoC: core: Allow topology to override machine driver FE DAI link config.") bcb1fd1fcd650 ("ASoC: add for_each_card_rtds() macro") bf14adcc4ddd1 ("ASoC: Intel: cht-bsw-rt5672: allow for topology-defined codec-dai setup") c41c2a355b863 ("ASoC: meson: add tdm output driver") d60e4f1e4be5e ("ASoC: meson: add tdm interface driver") d9fc40639dc1b ("ASoC: core: replace codec_dev_list to component_dev_list on Card") ec040dd5ef647 ("ASoC: Intel: Add Kabylake Realtek Maxim machine driver") ef641e5d5e6c7 ("ASoC: add Component level set_pll") fbb16563c6c2b ("ASoC: snd_soc_component_driver has pmdown_time")
v4.4.228: Failed to apply! Possible dependencies: 0d1571c197a92 ("ASoC: intel: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer") 17fb175520e54 ("ASoC: Define API to find a dai link") 1a497983a5ae6 ("ASoC: Change the PCM runtime array to a list") 49a5ba1cd9da4 ("ASoC: soc_bind_dai_link() directly returns success for a bound DAI link") 6f2f1ff0de83a ("ASoC: Change 2nd argument of soc_bind_dai_link() to DAI link pointer") 804e73adf5cf4 ("ASoC: rt5670: Fix GPIO headset detection regression") 923c5e61ecd9b ("ASoC: Define soc_init_dai_link() to wrap link intialization.") 98061fdbfccc0 ("ASoC: add for_each_card_links() macro") bcb1fd1fcd650 ("ASoC: add for_each_card_rtds() macro") bf14adcc4ddd1 ("ASoC: Intel: cht-bsw-rt5672: allow for topology-defined codec-dai setup") f8f80361d07d5 ("ASoC: Implement DAI links in a list & define API to add/remove a link")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
The RT5670_PWR_ANLG1 register has 3 bits to select the LDO voltage, so the correct mask is 0x7 not 0x3.
Because of this wrong mask we were programming the ldo bits to a setting of binary 001 (0x05 & 0x03) instead of binary 101 when moving to SND_SOC_BIAS_PREPARE.
According to the datasheet 001 is a reserved value, so no idea what it did, since the driver was working fine before I guess we got lucky and it does something which is ok.
Fixes: 5e8351de740d ("ASoC: add RT5670 CODEC driver") Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede hdegoede@redhat.com --- sound/soc/codecs/rt5670.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/rt5670.h b/sound/soc/codecs/rt5670.h index a8c3e44770b8..de0203369b7c 100644 --- a/sound/soc/codecs/rt5670.h +++ b/sound/soc/codecs/rt5670.h @@ -757,7 +757,7 @@ #define RT5670_PWR_VREF2_BIT 4 #define RT5670_PWR_FV2 (0x1 << 3) #define RT5670_PWR_FV2_BIT 3 -#define RT5670_LDO_SEL_MASK (0x3) +#define RT5670_LDO_SEL_MASK (0x7) #define RT5670_LDO_SEL_SFT 0
/* Power Management for Analog 2 (0x64) */
The Lenovo Miix 2 10 has a keyboard dock with extra speakers in the dock. Rather then the ACL5672's GPIO1 pin being used as IRQ to the CPU, it is actually used to enable the amplifier for these speakers (the IRQ to the CPU comes directly from the jack-detect switch).
Add a quirk for having an ext speaker-amplifier enable pin on GPIO1 and replace the Lenovo Miix 2 10's dmi_system_id table entry's wrong GPIO_DEV quirk (which needs to be renamed to GPIO1_IS_IRQ) with the new RT5670_GPIO1_IS_EXT_SPK_EN quirk, so that we enable the external speaker-amplifier as necessary.
Also update the ident field for the dmi_system_id table entry, the Miix models are not Thinkpads.
Fixes: 67e03ff3f32f ("ASoC: codecs: rt5670: add Thinkpad Tablet 10 quirk") BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1786723 Signed-off-by: Hans de Goede hdegoede@redhat.com --- include/sound/rt5670.h | 1 + sound/soc/codecs/rt5670.c | 71 ++++++++++++++++++++++++++++++--------- 2 files changed, 57 insertions(+), 15 deletions(-)
diff --git a/include/sound/rt5670.h b/include/sound/rt5670.h index f9024c7a1600..02e1d7778354 100644 --- a/include/sound/rt5670.h +++ b/include/sound/rt5670.h @@ -12,6 +12,7 @@ struct rt5670_platform_data { int jd_mode; bool in2_diff; bool dev_gpio; + bool gpio1_is_ext_spk_en;
bool dmic_en; unsigned int dmic1_data_pin; diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index 70fee6849ab0..f21181734170 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -31,18 +31,19 @@ #include "rt5670.h" #include "rt5670-dsp.h"
-#define RT5670_DEV_GPIO BIT(0) -#define RT5670_IN2_DIFF BIT(1) -#define RT5670_DMIC_EN BIT(2) -#define RT5670_DMIC1_IN2P BIT(3) -#define RT5670_DMIC1_GPIO6 BIT(4) -#define RT5670_DMIC1_GPIO7 BIT(5) -#define RT5670_DMIC2_INR BIT(6) -#define RT5670_DMIC2_GPIO8 BIT(7) -#define RT5670_DMIC3_GPIO5 BIT(8) -#define RT5670_JD_MODE1 BIT(9) -#define RT5670_JD_MODE2 BIT(10) -#define RT5670_JD_MODE3 BIT(11) +#define RT5670_DEV_GPIO BIT(0) +#define RT5670_IN2_DIFF BIT(1) +#define RT5670_DMIC_EN BIT(2) +#define RT5670_DMIC1_IN2P BIT(3) +#define RT5670_DMIC1_GPIO6 BIT(4) +#define RT5670_DMIC1_GPIO7 BIT(5) +#define RT5670_DMIC2_INR BIT(6) +#define RT5670_DMIC2_GPIO8 BIT(7) +#define RT5670_DMIC3_GPIO5 BIT(8) +#define RT5670_JD_MODE1 BIT(9) +#define RT5670_JD_MODE2 BIT(10) +#define RT5670_JD_MODE3 BIT(11) +#define RT5670_GPIO1_IS_EXT_SPK_EN BIT(12)
static unsigned long rt5670_quirk; static unsigned int quirk_override; @@ -1447,6 +1448,33 @@ static int rt5670_hp_event(struct snd_soc_dapm_widget *w, return 0; }
+static int rt5670_spk_event(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm); + struct rt5670_priv *rt5670 = snd_soc_component_get_drvdata(component); + + if (!rt5670->pdata.gpio1_is_ext_spk_en) + return 0; + + switch (event) { + case SND_SOC_DAPM_POST_PMU: + regmap_update_bits(rt5670->regmap, RT5670_GPIO_CTRL2, + RT5670_GP1_OUT_MASK, RT5670_GP1_OUT_HI); + break; + + case SND_SOC_DAPM_PRE_PMD: + regmap_update_bits(rt5670->regmap, RT5670_GPIO_CTRL2, + RT5670_GP1_OUT_MASK, RT5670_GP1_OUT_LO); + break; + + default: + return 0; + } + + return 0; +} + static int rt5670_bst1_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol, int event) { @@ -1860,7 +1888,9 @@ static const struct snd_soc_dapm_widget rt5670_specific_dapm_widgets[] = { };
static const struct snd_soc_dapm_widget rt5672_specific_dapm_widgets[] = { - SND_SOC_DAPM_PGA("SPO Amp", SND_SOC_NOPM, 0, 0, NULL, 0), + SND_SOC_DAPM_PGA_E("SPO Amp", SND_SOC_NOPM, 0, 0, NULL, 0, + rt5670_spk_event, SND_SOC_DAPM_PRE_PMD | + SND_SOC_DAPM_POST_PMU), SND_SOC_DAPM_OUTPUT("SPOLP"), SND_SOC_DAPM_OUTPUT("SPOLN"), SND_SOC_DAPM_OUTPUT("SPORP"), @@ -2857,14 +2887,14 @@ static const struct dmi_system_id dmi_platform_intel_quirks[] = { }, { .callback = rt5670_quirk_cb, - .ident = "Lenovo Thinkpad Tablet 10", + .ident = "Lenovo Miix 2 10", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Miix 2 10"), }, .driver_data = (unsigned long *)(RT5670_DMIC_EN | RT5670_DMIC1_IN2P | - RT5670_DEV_GPIO | + RT5670_GPIO1_IS_EXT_SPK_EN | RT5670_JD_MODE2), }, { @@ -2924,6 +2954,10 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, rt5670->pdata.dev_gpio = true; dev_info(&i2c->dev, "quirk dev_gpio\n"); } + if (rt5670_quirk & RT5670_GPIO1_IS_EXT_SPK_EN) { + rt5670->pdata.gpio1_is_ext_spk_en = true; + dev_info(&i2c->dev, "quirk GPIO1 is external speaker enable\n"); + } if (rt5670_quirk & RT5670_IN2_DIFF) { rt5670->pdata.in2_diff = true; dev_info(&i2c->dev, "quirk IN2_DIFF\n"); @@ -3023,6 +3057,13 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, RT5670_GP1_PF_MASK, RT5670_GP1_PF_OUT); }
+ if (rt5670->pdata.gpio1_is_ext_spk_en) { + regmap_update_bits(rt5670->regmap, RT5670_GPIO_CTRL1, + RT5670_GP1_PIN_MASK, RT5670_GP1_PIN_GPIO1); + regmap_update_bits(rt5670->regmap, RT5670_GPIO_CTRL2, + RT5670_GP1_PF_MASK, RT5670_GP1_PF_OUT); + } + if (rt5670->pdata.jd_mode) { regmap_update_bits(rt5670->regmap, RT5670_GLB_CLK, RT5670_SCLK_SRC_MASK, RT5670_SCLK_SRC_RCCLK);
The adc_vol_tlv volume-control has a range from -17.625 dB to +30 dB, not -176.25 dB to + 300 dB. This wrong scale is esp. a problem in userspace apps which translate the dB scale to a linear scale. With the logarithmic dB scale being of by a factor of 10 we loose all precision in the lower area of the range when apps translate things to a linear scale.
E.g. the 0 dB default, which corresponds with a value of 47 of the 0 - 127 range for the control, would be shown as 0/100 in alsa-mixer.
Since the centi-dB values used in the TLV struct cannot represent the 0.375 dB step size used by these controls, change the TLV definition for them to specify a min and max value instead of min + stepsize.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- sound/soc/codecs/rt5670.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index f21181734170..dfbc0ca38ff7 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -603,9 +603,9 @@ int rt5670_set_jack_detect(struct snd_soc_component *component, EXPORT_SYMBOL_GPL(rt5670_set_jack_detect);
static const DECLARE_TLV_DB_SCALE(out_vol_tlv, -4650, 150, 0); -static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -65625, 375, 0); +static const DECLARE_TLV_DB_MINMAX(dac_vol_tlv, -6562, 0); static const DECLARE_TLV_DB_SCALE(in_vol_tlv, -3450, 150, 0); -static const DECLARE_TLV_DB_SCALE(adc_vol_tlv, -17625, 375, 0); +static const DECLARE_TLV_DB_MINMAX(adc_vol_tlv, -1762, 3000); static const DECLARE_TLV_DB_SCALE(adc_bst_tlv, 0, 1200, 0);
/* {0, +20, +24, +30, +35, +40, +44, +50, +52} dB */
On 6/28/20 10:52 AM, Hans de Goede wrote:
The adc_vol_tlv volume-control has a range from -17.625 dB to +30 dB, not -176.25 dB to + 300 dB. This wrong scale is esp. a problem in userspace
D'oh! nice catch.
apps which translate the dB scale to a linear scale. With the logarithmic dB scale being of by a factor of 10 we loose all precision in the lower area of the range when apps translate things to a linear scale.
E.g. the 0 dB default, which corresponds with a value of 47 of the 0 - 127 range for the control, would be shown as 0/100 in alsa-mixer.
Since the centi-dB values used in the TLV struct cannot represent the 0.375 dB step size used by these controls, change the TLV definition for them to specify a min and max value instead of min + stepsize.
Signed-off-by: Hans de Goede hdegoede@redhat.com
sound/soc/codecs/rt5670.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index f21181734170..dfbc0ca38ff7 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -603,9 +603,9 @@ int rt5670_set_jack_detect(struct snd_soc_component *component, EXPORT_SYMBOL_GPL(rt5670_set_jack_detect);
static const DECLARE_TLV_DB_SCALE(out_vol_tlv, -4650, 150, 0); -static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -65625, 375, 0); +static const DECLARE_TLV_DB_MINMAX(dac_vol_tlv, -6562, 0); static const DECLARE_TLV_DB_SCALE(in_vol_tlv, -3450, 150, 0); -static const DECLARE_TLV_DB_SCALE(adc_vol_tlv, -17625, 375, 0); +static const DECLARE_TLV_DB_MINMAX(adc_vol_tlv, -1762, 3000); static const DECLARE_TLV_DB_SCALE(adc_bst_tlv, 0, 1200, 0);
/* {0, +20, +24, +30, +35, +40, +44, +50, +52} dB */
platform_data is an obsolete concept, instead device_properties, set through e.g. device-tree, should be used.
struct rt5670_platform_data is only used internally by the rt5670 codec driver, so lets remove it before someone starts relying on it.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- include/sound/rt5670.h | 26 ------------------ sound/soc/codecs/rt5670.c | 55 ++++++++++++++++++--------------------- sound/soc/codecs/rt5670.h | 16 +++++++++--- 3 files changed, 38 insertions(+), 59 deletions(-) delete mode 100644 include/sound/rt5670.h
diff --git a/include/sound/rt5670.h b/include/sound/rt5670.h deleted file mode 100644 index 02e1d7778354..000000000000 --- a/include/sound/rt5670.h +++ /dev/null @@ -1,26 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -/* - * linux/sound/rt5670.h -- Platform data for RT5670 - * - * Copyright 2014 Realtek Microelectronics - */ - -#ifndef __LINUX_SND_RT5670_H -#define __LINUX_SND_RT5670_H - -struct rt5670_platform_data { - int jd_mode; - bool in2_diff; - bool dev_gpio; - bool gpio1_is_ext_spk_en; - - bool dmic_en; - unsigned int dmic1_data_pin; - /* 0 = GPIO6; 1 = IN2P; 3 = GPIO7*/ - unsigned int dmic2_data_pin; - /* 0 = GPIO8; 1 = IN3N; */ - unsigned int dmic3_data_pin; - /* 0 = GPIO9; 1 = GPIO10; 2 = GPIO5*/ -}; - -#endif diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index dfbc0ca38ff7..515ab60bfe9c 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -25,7 +25,6 @@ #include <sound/soc-dapm.h> #include <sound/initval.h> #include <sound/tlv.h> -#include <sound/rt5670.h>
#include "rl6231.h" #include "rt5670.h" @@ -518,7 +517,7 @@ static int rt5670_irq_detection(void *data) struct snd_soc_jack *jack = rt5670->jack; int val, btn_type, report = jack->status;
- if (rt5670->pdata.jd_mode == 1) /* 2 port */ + if (rt5670->jd_mode == 1) /* 2 port */ val = snd_soc_component_read32(rt5670->component, RT5670_A_JD_CTRL1) & 0x0070; else val = snd_soc_component_read32(rt5670->component, RT5670_A_JD_CTRL1) & 0x0020; @@ -1454,7 +1453,7 @@ static int rt5670_spk_event(struct snd_soc_dapm_widget *w, struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm); struct rt5670_priv *rt5670 = snd_soc_component_get_drvdata(component);
- if (!rt5670->pdata.gpio1_is_ext_spk_en) + if (!rt5670->gpio1_is_ext_spk_en) return 0;
switch (event) { @@ -2624,7 +2623,7 @@ static int rt5670_set_bias_level(struct snd_soc_component *component, RT5670_LDO_SEL_MASK, 0x3); break; case SND_SOC_BIAS_OFF: - if (rt5670->pdata.jd_mode) + if (rt5670->jd_mode) snd_soc_component_update_bits(component, RT5670_PWR_ANLG1, RT5670_PWR_VREF1 | RT5670_PWR_MB | RT5670_PWR_BG | RT5670_PWR_VREF2 | @@ -2927,7 +2926,6 @@ static const struct dmi_system_id dmi_platform_intel_quirks[] = { static int rt5670_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { - struct rt5670_platform_data *pdata = dev_get_platdata(&i2c->dev); struct rt5670_priv *rt5670; int ret; unsigned int val; @@ -2940,9 +2938,6 @@ static int rt5670_i2c_probe(struct i2c_client *i2c,
i2c_set_clientdata(i2c, rt5670);
- if (pdata) - rt5670->pdata = *pdata; - dmi_check_system(dmi_platform_intel_quirks); if (quirk_override) { dev_info(&i2c->dev, "Overriding quirk 0x%x => 0x%x\n", @@ -2951,56 +2946,56 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, }
if (rt5670_quirk & RT5670_DEV_GPIO) { - rt5670->pdata.dev_gpio = true; + rt5670->dev_gpio = true; dev_info(&i2c->dev, "quirk dev_gpio\n"); } if (rt5670_quirk & RT5670_GPIO1_IS_EXT_SPK_EN) { - rt5670->pdata.gpio1_is_ext_spk_en = true; + rt5670->gpio1_is_ext_spk_en = true; dev_info(&i2c->dev, "quirk GPIO1 is external speaker enable\n"); } if (rt5670_quirk & RT5670_IN2_DIFF) { - rt5670->pdata.in2_diff = true; + rt5670->in2_diff = true; dev_info(&i2c->dev, "quirk IN2_DIFF\n"); } if (rt5670_quirk & RT5670_DMIC_EN) { - rt5670->pdata.dmic_en = true; + rt5670->dmic_en = true; dev_info(&i2c->dev, "quirk DMIC enabled\n"); } if (rt5670_quirk & RT5670_DMIC1_IN2P) { - rt5670->pdata.dmic1_data_pin = RT5670_DMIC_DATA_IN2P; + rt5670->dmic1_data_pin = RT5670_DMIC_DATA_IN2P; dev_info(&i2c->dev, "quirk DMIC1 on IN2P pin\n"); } if (rt5670_quirk & RT5670_DMIC1_GPIO6) { - rt5670->pdata.dmic1_data_pin = RT5670_DMIC_DATA_GPIO6; + rt5670->dmic1_data_pin = RT5670_DMIC_DATA_GPIO6; dev_info(&i2c->dev, "quirk DMIC1 on GPIO6 pin\n"); } if (rt5670_quirk & RT5670_DMIC1_GPIO7) { - rt5670->pdata.dmic1_data_pin = RT5670_DMIC_DATA_GPIO7; + rt5670->dmic1_data_pin = RT5670_DMIC_DATA_GPIO7; dev_info(&i2c->dev, "quirk DMIC1 on GPIO7 pin\n"); } if (rt5670_quirk & RT5670_DMIC2_INR) { - rt5670->pdata.dmic2_data_pin = RT5670_DMIC_DATA_IN3N; + rt5670->dmic2_data_pin = RT5670_DMIC_DATA_IN3N; dev_info(&i2c->dev, "quirk DMIC2 on INR pin\n"); } if (rt5670_quirk & RT5670_DMIC2_GPIO8) { - rt5670->pdata.dmic2_data_pin = RT5670_DMIC_DATA_GPIO8; + rt5670->dmic2_data_pin = RT5670_DMIC_DATA_GPIO8; dev_info(&i2c->dev, "quirk DMIC2 on GPIO8 pin\n"); } if (rt5670_quirk & RT5670_DMIC3_GPIO5) { - rt5670->pdata.dmic3_data_pin = RT5670_DMIC_DATA_GPIO5; + rt5670->dmic3_data_pin = RT5670_DMIC_DATA_GPIO5; dev_info(&i2c->dev, "quirk DMIC3 on GPIO5 pin\n"); }
if (rt5670_quirk & RT5670_JD_MODE1) { - rt5670->pdata.jd_mode = 1; + rt5670->jd_mode = 1; dev_info(&i2c->dev, "quirk JD mode 1\n"); } if (rt5670_quirk & RT5670_JD_MODE2) { - rt5670->pdata.jd_mode = 2; + rt5670->jd_mode = 2; dev_info(&i2c->dev, "quirk JD mode 2\n"); } if (rt5670_quirk & RT5670_JD_MODE3) { - rt5670->pdata.jd_mode = 3; + rt5670->jd_mode = 3; dev_info(&i2c->dev, "quirk JD mode 3\n"); }
@@ -3041,11 +3036,11 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, regmap_update_bits(rt5670->regmap, RT5670_DIG_MISC, RT5670_MCLK_DET, RT5670_MCLK_DET);
- if (rt5670->pdata.in2_diff) + if (rt5670->in2_diff) regmap_update_bits(rt5670->regmap, RT5670_IN2, RT5670_IN_DF2, RT5670_IN_DF2);
- if (rt5670->pdata.dev_gpio) { + if (rt5670->dev_gpio) { /* for push button */ regmap_write(rt5670->regmap, RT5670_IL_CMD, 0x0000); regmap_write(rt5670->regmap, RT5670_IL_CMD2, 0x0010); @@ -3057,14 +3052,14 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, RT5670_GP1_PF_MASK, RT5670_GP1_PF_OUT); }
- if (rt5670->pdata.gpio1_is_ext_spk_en) { + if (rt5670->gpio1_is_ext_spk_en) { regmap_update_bits(rt5670->regmap, RT5670_GPIO_CTRL1, RT5670_GP1_PIN_MASK, RT5670_GP1_PIN_GPIO1); regmap_update_bits(rt5670->regmap, RT5670_GPIO_CTRL2, RT5670_GP1_PF_MASK, RT5670_GP1_PF_OUT); }
- if (rt5670->pdata.jd_mode) { + if (rt5670->jd_mode) { regmap_update_bits(rt5670->regmap, RT5670_GLB_CLK, RT5670_SCLK_SRC_MASK, RT5670_SCLK_SRC_RCCLK); rt5670->sysclk = 0; @@ -3079,7 +3074,7 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, RT5670_JD_TRI_CBJ_SEL_MASK | RT5670_JD_TRI_HPO_SEL_MASK, RT5670_JD_CBJ_JD1_1 | RT5670_JD_HPO_JD1_1); - switch (rt5670->pdata.jd_mode) { + switch (rt5670->jd_mode) { case 1: regmap_update_bits(rt5670->regmap, RT5670_A_JD_CTRL1, RT5670_JD1_MODE_MASK, @@ -3100,12 +3095,12 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, } }
- if (rt5670->pdata.dmic_en) { + if (rt5670->dmic_en) { regmap_update_bits(rt5670->regmap, RT5670_GPIO_CTRL1, RT5670_GP2_PIN_MASK, RT5670_GP2_PIN_DMIC1_SCL);
- switch (rt5670->pdata.dmic1_data_pin) { + switch (rt5670->dmic1_data_pin) { case RT5670_DMIC_DATA_IN2P: regmap_update_bits(rt5670->regmap, RT5670_DMIC_CTRL1, RT5670_DMIC_1_DP_MASK, @@ -3134,7 +3129,7 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, break; }
- switch (rt5670->pdata.dmic2_data_pin) { + switch (rt5670->dmic2_data_pin) { case RT5670_DMIC_DATA_IN3N: regmap_update_bits(rt5670->regmap, RT5670_DMIC_CTRL1, RT5670_DMIC_2_DP_MASK, @@ -3154,7 +3149,7 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, break; }
- switch (rt5670->pdata.dmic3_data_pin) { + switch (rt5670->dmic3_data_pin) { case RT5670_DMIC_DATA_GPIO5: regmap_update_bits(rt5670->regmap, RT5670_DMIC_CTRL2, RT5670_DMIC_3_DP_MASK, diff --git a/sound/soc/codecs/rt5670.h b/sound/soc/codecs/rt5670.h index de0203369b7c..657420805918 100644 --- a/sound/soc/codecs/rt5670.h +++ b/sound/soc/codecs/rt5670.h @@ -9,8 +9,6 @@ #ifndef __RT5670_H__ #define __RT5670_H__
-#include <sound/rt5670.h> - /* Info */ #define RT5670_RESET 0x00 #define RT5670_VENDOR_ID 0xfd @@ -1988,11 +1986,23 @@ int rt5670_sel_asrc_clk_src(struct snd_soc_component *component,
struct rt5670_priv { struct snd_soc_component *component; - struct rt5670_platform_data pdata; struct regmap *regmap; struct snd_soc_jack *jack; struct snd_soc_jack_gpio hp_gpio;
+ int jd_mode; + bool in2_diff; + bool dev_gpio; + bool gpio1_is_ext_spk_en; + + bool dmic_en; + unsigned int dmic1_data_pin; + /* 0 = GPIO6; 1 = IN2P; 3 = GPIO7*/ + unsigned int dmic2_data_pin; + /* 0 = GPIO8; 1 = IN3N; */ + unsigned int dmic3_data_pin; + /* 0 = GPIO9; 1 = GPIO10; 2 = GPIO5*/ + int sysclk; int sysclk_src; int lrck[RT5670_AIFS];
On Sun, Jun 28, 2020 at 05:52:30PM +0200, Hans de Goede wrote:
platform_data is an obsolete concept, instead device_properties, set through e.g. device-tree, should be used.
struct rt5670_platform_data is only used internally by the rt5670 codec driver, so lets remove it before someone starts relying on it.
This doesn't apply against current code, please check and resend.
Hi,
On 6/29/20 10:09 PM, Mark Brown wrote:
On Sun, Jun 28, 2020 at 05:52:30PM +0200, Hans de Goede wrote:
platform_data is an obsolete concept, instead device_properties, set through e.g. device-tree, should be used.
struct rt5670_platform_data is only used internally by the rt5670 codec driver, so lets remove it before someone starts relying on it.
This doesn't apply against current code, please check and resend.
Did you try to apply this to your for-5.9 branch (I know that was my own suggestion) ? I must admit I did not think this through, this patch depends on some of the changes made in patches 1-4 :|
So either you could add patches 5 and 6 to your for-5.8 branch too, or it might be best to delay these 2 till after 5.9-rc1 otherwise we are going to get a merge conflict.
Regards,
Hans
On Mon, Jun 29, 2020 at 10:24:01PM +0200, Hans de Goede wrote:
On 6/29/20 10:09 PM, Mark Brown wrote:
This doesn't apply against current code, please check and resend.
Did you try to apply this to your for-5.9 branch (I know that was my own suggestion) ? I must admit I did not think this through, this patch depends on some of the changes made in patches 1-4 :|
Yes, and on a merge of that with the 5.8 branch. It's probably a conflict with Morimoto-san's work.
Rename the not really descriptive dev_gpio quirk / setting to gpio1_is_irq, which describes what it actually does.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- sound/soc/codecs/rt5670.c | 24 ++++++++++++------------ sound/soc/codecs/rt5670.h | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index 515ab60bfe9c..a37601bfbfcc 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -30,7 +30,7 @@ #include "rt5670.h" #include "rt5670-dsp.h"
-#define RT5670_DEV_GPIO BIT(0) +#define RT5670_GPIO1_IS_IRQ BIT(0) #define RT5670_IN2_DIFF BIT(1) #define RT5670_DMIC_EN BIT(2) #define RT5670_DMIC1_IN2P BIT(3) @@ -2833,7 +2833,7 @@ static const struct dmi_system_id dmi_platform_intel_quirks[] = { }, .driver_data = (unsigned long *)(RT5670_DMIC_EN | RT5670_DMIC1_IN2P | - RT5670_DEV_GPIO | + RT5670_GPIO1_IS_IRQ | RT5670_JD_MODE1), }, { @@ -2845,7 +2845,7 @@ static const struct dmi_system_id dmi_platform_intel_quirks[] = { }, .driver_data = (unsigned long *)(RT5670_DMIC_EN | RT5670_DMIC1_IN2P | - RT5670_DEV_GPIO | + RT5670_GPIO1_IS_IRQ | RT5670_JD_MODE1), }, { @@ -2857,7 +2857,7 @@ static const struct dmi_system_id dmi_platform_intel_quirks[] = { }, .driver_data = (unsigned long *)(RT5670_DMIC_EN | RT5670_DMIC2_INR | - RT5670_DEV_GPIO | + RT5670_GPIO1_IS_IRQ | RT5670_JD_MODE1), }, { @@ -2869,7 +2869,7 @@ static const struct dmi_system_id dmi_platform_intel_quirks[] = { }, .driver_data = (unsigned long *)(RT5670_DMIC_EN | RT5670_DMIC1_IN2P | - RT5670_DEV_GPIO | + RT5670_GPIO1_IS_IRQ | RT5670_JD_MODE1), }, { @@ -2881,7 +2881,7 @@ static const struct dmi_system_id dmi_platform_intel_quirks[] = { }, .driver_data = (unsigned long *)(RT5670_DMIC_EN | RT5670_DMIC1_IN2P | - RT5670_DEV_GPIO | + RT5670_GPIO1_IS_IRQ | RT5670_JD_MODE1), }, { @@ -2905,7 +2905,7 @@ static const struct dmi_system_id dmi_platform_intel_quirks[] = { }, .driver_data = (unsigned long *)(RT5670_DMIC_EN | RT5670_DMIC2_INR | - RT5670_DEV_GPIO | + RT5670_GPIO1_IS_IRQ | RT5670_JD_MODE3), }, { @@ -2917,7 +2917,7 @@ static const struct dmi_system_id dmi_platform_intel_quirks[] = { }, .driver_data = (unsigned long *)(RT5670_DMIC_EN | RT5670_DMIC2_INR | - RT5670_DEV_GPIO | + RT5670_GPIO1_IS_IRQ | RT5670_JD_MODE3), }, {} @@ -2945,9 +2945,9 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, rt5670_quirk = quirk_override; }
- if (rt5670_quirk & RT5670_DEV_GPIO) { - rt5670->dev_gpio = true; - dev_info(&i2c->dev, "quirk dev_gpio\n"); + if (rt5670_quirk & RT5670_GPIO1_IS_IRQ) { + rt5670->gpio1_is_irq = true; + dev_info(&i2c->dev, "quirk GPIO1 is IRQ\n"); } if (rt5670_quirk & RT5670_GPIO1_IS_EXT_SPK_EN) { rt5670->gpio1_is_ext_spk_en = true; @@ -3040,7 +3040,7 @@ static int rt5670_i2c_probe(struct i2c_client *i2c, regmap_update_bits(rt5670->regmap, RT5670_IN2, RT5670_IN_DF2, RT5670_IN_DF2);
- if (rt5670->dev_gpio) { + if (rt5670->gpio1_is_irq) { /* for push button */ regmap_write(rt5670->regmap, RT5670_IL_CMD, 0x0000); regmap_write(rt5670->regmap, RT5670_IL_CMD2, 0x0010); diff --git a/sound/soc/codecs/rt5670.h b/sound/soc/codecs/rt5670.h index 657420805918..56b13fe6bd3c 100644 --- a/sound/soc/codecs/rt5670.h +++ b/sound/soc/codecs/rt5670.h @@ -1992,7 +1992,7 @@ struct rt5670_priv {
int jd_mode; bool in2_diff; - bool dev_gpio; + bool gpio1_is_irq; bool gpio1_is_ext_spk_en;
bool dmic_en;
On 6/28/20 10:52 AM, Hans de Goede wrote:
Hi All,
This series is mainly the result of me working on fixing the extra set of speakers in the Lenovo Miix 2 10 kbd dock not working with Linux (patches 1 and 3) while working on this I noticed some other bugs (patches 2 and 4) and I also ended up doing some generic cleanups (patches 5 and 6). For patches 1-3 or 1-4 it would be nice it they can go to a future 5.8-rc# 5-6 are mostly -next material.
I added a minor comment on patch 1 but is a good set of changes, thanks Hans!
Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
Hi,
On 6/29/20 5:14 PM, Pierre-Louis Bossart wrote:
On 6/28/20 10:52 AM, Hans de Goede wrote:
Hi All,
This series is mainly the result of me working on fixing the extra set of speakers in the Lenovo Miix 2 10 kbd dock not working with Linux (patches 1 and 3) while working on this I noticed some other bugs (patches 2 and 4) and I also ended up doing some generic cleanups (patches 5 and 6). For patches 1-3 or 1-4 it would be nice it they can go to a future 5.8-rc# 5-6 are mostly -next material.
I added a minor comment on patch 1 but is a good set of changes, thanks Hans!
Since Mark has already merged this, I believe it is best to keep patch 1 as is, still thank you for the clarification of what is going on.
Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
And thank you for the review; and Mark thank you for merging these.
Regards,
Hans
On Mon, Jun 29, 2020 at 10:02:23PM +0200, Hans de Goede wrote:
On 6/29/20 5:14 PM, Pierre-Louis Bossart wrote:
I added a minor comment on patch 1 but is a good set of changes, thanks Hans!
Since Mark has already merged this, I believe it is best to keep patch 1 as is, still thank you for the clarification of what is going on.
I was kind of expecting an incremental patch for that TBH.
Hi,
On 6/29/20 10:07 PM, Mark Brown wrote:
On Mon, Jun 29, 2020 at 10:02:23PM +0200, Hans de Goede wrote:
On 6/29/20 5:14 PM, Pierre-Louis Bossart wrote:
I added a minor comment on patch 1 but is a good set of changes, thanks Hans!
Since Mark has already merged this, I believe it is best to keep patch 1 as is, still thank you for the clarification of what is going on.
I was kind of expecting an incremental patch for that TBH.
As I see, ok I can do that. This byt/cht stuff is mostly a hobby project for me and I need to do some $dayjob things first. I will post an incremental patch somewhere around the weekend.
Regards,
Hans
participants (4)
-
Hans de Goede -
Mark Brown -
Pierre-Louis Bossart -
Sasha Levin