ASoC: rt5670/cht_bsw_rt5672: Some bugfixes and cleanups

ASoC: rt5670/cht_bsw_rt5672: Some bugfixes and cleanups

[Hans de Goede]

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

[PATCH 1/6] ASoC: Intel: cht_bsw_rt5672: Change bus format to I2S 2 channel

[Hans de Goede]

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;
 	}
 
-- 
2.26.2

Re: [PATCH 1/6] ASoC: Intel: cht_bsw_rt5672: Change bus format to I2S 2 channel

[Pierre-Louis Bossart]

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);
>   	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;
>   	}
>   
> 

Re: [PATCH 1/6] ASoC: Intel: cht_bsw_rt5672: Change bus format to I2S 2 channel

[Mark Brown]

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

Re: [PATCH 1/6] ASoC: Intel: cht_bsw_rt5672: Change bus format to I2S 2 channel

[Sasha Levin]

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?

-- 
Thanks
Sasha

Re: [PATCH 1/6] ASoC: Intel: cht_bsw_rt5672: Change bus format to I2S 2 channel

[Pierre-Louis Bossart]

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

Re: [PATCH 1/6] ASoC: Intel: cht_bsw_rt5672: Change bus format to I2S 2 channel

[Hans de Goede]

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

Re: [PATCH 1/6] ASoC: Intel: cht_bsw_rt5672: Change bus format to I2S 2 channel

[Mark Brown]

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

Re: [PATCH 1/6] ASoC: Intel: cht_bsw_rt5672: Change bus format to I2S 2 channel

[Sasha Levin]

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?

-- 
Thanks
Sasha

[PATCH 2/6] ASoC: rt5670: Correct RT5670_LDO_SEL_MASK

[Hans de Goede]

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) */
-- 
2.26.2

[PATCH 3/6] ASoC: rt5670: Add new gpio1_is_ext_spk_en quirk and enable it on the Lenovo Miix 2 10

[Hans de Goede]

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);
-- 
2.26.2

[PATCH 4/6] ASoC: rt5670: Fix dac- and adc- vol-tlv values being off by a factor of 10

[Hans de Goede]

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 */
-- 
2.26.2

Re: [PATCH 4/6] ASoC: rt5670: Fix dac- and adc- vol-tlv values being off by a factor of 10

[Pierre-Louis Bossart]

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 */
> 

[PATCH 5/6] ASoC: rt5670: Remove struct rt5670_platform_data

[Hans de Goede]

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];
-- 
2.26.2

Re: [PATCH 5/6] ASoC: rt5670: Remove struct rt5670_platform_data

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 395 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 5/6] ASoC: rt5670: Remove struct rt5670_platform_data

[Hans de Goede]

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

Re: [PATCH 5/6] ASoC: rt5670: Remove struct rt5670_platform_data

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 480 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 6/6] ASoC: rt5670: Rename dev_gpio to gpio1_is_irq

[Hans de Goede]

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;
-- 
2.26.2

Re: ASoC: rt5670/cht_bsw_rt5672: Some bugfixes and cleanups

[Pierre-Louis Bossart]

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>

Re: ASoC: rt5670/cht_bsw_rt5672: Some bugfixes and cleanups

[Hans de Goede]

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

Re: ASoC: rt5670/cht_bsw_rt5672: Some bugfixes and cleanups

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: ASoC: rt5670/cht_bsw_rt5672: Some bugfixes and cleanups

[Hans de Goede]

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

[PATCH 1/6] ASoC: Intel: cht_bsw_rt5672: Change bus format to I2S 2 channel

[Hans de Goede]

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)

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;
 	}
 
-- 
2.26.2

[PATCH 2/6] ASoC: rt5670: Correct RT5670_LDO_SEL_MASK

[Hans de Goede]

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) */
-- 
2.26.2