Re: [PATCH] ASoC: rt1308-sdw: get calibration params after power on

[Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>]

On 3/15/22 04:37, shumingf@xxxxxxxxxxx wrote:
> From: Shuming Fan <shumingf@xxxxxxxxxxx>
>
> It will be safe when getting the calibration params after power-on.
> All powers are ready to read the calibration params from EFUSE.
>
> Signed-off-by: Shuming Fan <shumingf@xxxxxxxxxxx>

For more context, this patch solves 'cracking sound' on speakers and was 
tested by the bug reporter on Dell XPS 17 9710 (SoundWire-based device)

BugLink: https://github.com/thesofproject/sof/issues/5436
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>

> ---
>   sound/soc/codecs/rt1308-sdw.c | 73 +++++++++++++++++++----------------
>   1 file changed, 40 insertions(+), 33 deletions(-)
>
> diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c
> index 149a76075c76..1ef836a68a56 100644
> --- a/sound/soc/codecs/rt1308-sdw.c
> +++ b/sound/soc/codecs/rt1308-sdw.c
> @@ -50,6 +50,8 @@ static bool rt1308_volatile_register(struct device *dev, unsigned int reg)
>   	case 0x3008:
>   	case 0x300a:
>   	case 0xc000:
> +	case 0xc860 ... 0xc863:
> +	case 0xc870 ... 0xc873:
>   		return true;
>   	default:
>   		return false;
> @@ -159,12 +161,45 @@ static int rt1308_read_prop(struct sdw_slave *slave)
>   	return 0;
>   }
>   
> +static void rt1308_apply_calib_params(struct rt1308_sdw_priv *rt1308)
> +{
> +	unsigned int efuse_m_btl_l, efuse_m_btl_r, tmp;
> +	unsigned int efuse_c_btl_l, efuse_c_btl_r;
> +
> +	/* read efuse to apply calibration parameters */
> +	regmap_write(rt1308->regmap, 0xc7f0, 0x04);
> +	regmap_write(rt1308->regmap, 0xc7f1, 0xfe);
> +	msleep(100);
> +	regmap_write(rt1308->regmap, 0xc7f0, 0x44);
> +	msleep(20);
> +	regmap_write(rt1308->regmap, 0xc240, 0x10);
> +
> +	regmap_read(rt1308->regmap, 0xc861, &tmp);
> +	efuse_m_btl_l = tmp;
> +	regmap_read(rt1308->regmap, 0xc860, &tmp);
> +	efuse_m_btl_l = efuse_m_btl_l | (tmp << 8);
> +	regmap_read(rt1308->regmap, 0xc863, &tmp);
> +	efuse_c_btl_l = tmp;
> +	regmap_read(rt1308->regmap, 0xc862, &tmp);
> +	efuse_c_btl_l = efuse_c_btl_l | (tmp << 8);
> +	regmap_read(rt1308->regmap, 0xc871, &tmp);
> +	efuse_m_btl_r = tmp;
> +	regmap_read(rt1308->regmap, 0xc870, &tmp);
> +	efuse_m_btl_r = efuse_m_btl_r | (tmp << 8);
> +	regmap_read(rt1308->regmap, 0xc873, &tmp);
> +	efuse_c_btl_r = tmp;
> +	regmap_read(rt1308->regmap, 0xc872, &tmp);
> +	efuse_c_btl_r = efuse_c_btl_r | (tmp << 8);
> +	dev_dbg(&rt1308->sdw_slave->dev, "%s m_btl_l=0x%x, m_btl_r=0x%x\n", __func__,
> +		efuse_m_btl_l, efuse_m_btl_r);
> +	dev_dbg(&rt1308->sdw_slave->dev, "%s c_btl_l=0x%x, c_btl_r=0x%x\n", __func__,
> +		efuse_c_btl_l, efuse_c_btl_r);
> +}
> +
>   static int rt1308_io_init(struct device *dev, struct sdw_slave *slave)
>   {
>   	struct rt1308_sdw_priv *rt1308 = dev_get_drvdata(dev);
>   	int ret = 0;
> -	unsigned int efuse_m_btl_l, efuse_m_btl_r, tmp;
> -	unsigned int efuse_c_btl_l, efuse_c_btl_r;
>   
>   	if (rt1308->hw_init)
>   		return 0;
> @@ -196,37 +231,6 @@ static int rt1308_io_init(struct device *dev, struct sdw_slave *slave)
>   	/* sw reset */
>   	regmap_write(rt1308->regmap, RT1308_SDW_RESET, 0);
>   
> -	/* read efuse */
> -	regmap_write(rt1308->regmap, 0xc360, 0x01);
> -	regmap_write(rt1308->regmap, 0xc361, 0x80);
> -	regmap_write(rt1308->regmap, 0xc7f0, 0x04);
> -	regmap_write(rt1308->regmap, 0xc7f1, 0xfe);
> -	msleep(100);
> -	regmap_write(rt1308->regmap, 0xc7f0, 0x44);
> -	msleep(20);
> -	regmap_write(rt1308->regmap, 0xc240, 0x10);
> -
> -	regmap_read(rt1308->regmap, 0xc861, &tmp);
> -	efuse_m_btl_l = tmp;
> -	regmap_read(rt1308->regmap, 0xc860, &tmp);
> -	efuse_m_btl_l = efuse_m_btl_l | (tmp << 8);
> -	regmap_read(rt1308->regmap, 0xc863, &tmp);
> -	efuse_c_btl_l = tmp;
> -	regmap_read(rt1308->regmap, 0xc862, &tmp);
> -	efuse_c_btl_l = efuse_c_btl_l | (tmp << 8);
> -	regmap_read(rt1308->regmap, 0xc871, &tmp);
> -	efuse_m_btl_r = tmp;
> -	regmap_read(rt1308->regmap, 0xc870, &tmp);
> -	efuse_m_btl_r = efuse_m_btl_r | (tmp << 8);
> -	regmap_read(rt1308->regmap, 0xc873, &tmp);
> -	efuse_c_btl_r = tmp;
> -	regmap_read(rt1308->regmap, 0xc872, &tmp);
> -	efuse_c_btl_r = efuse_c_btl_r | (tmp << 8);
> -	dev_dbg(&slave->dev, "%s m_btl_l=0x%x, m_btl_r=0x%x\n", __func__,
> -		efuse_m_btl_l, efuse_m_btl_r);
> -	dev_dbg(&slave->dev, "%s c_btl_l=0x%x, c_btl_r=0x%x\n", __func__,
> -		efuse_c_btl_l, efuse_c_btl_r);
> -
>   	/* initial settings */
>   	regmap_write(rt1308->regmap, 0xc103, 0xc0);
>   	regmap_write(rt1308->regmap, 0xc030, 0x17);
> @@ -323,6 +327,8 @@ static int rt1308_classd_event(struct snd_soc_dapm_widget *w,
>   {
>   	struct snd_soc_component *component =
>   		snd_soc_dapm_to_component(w->dapm);
> +	struct rt1308_sdw_priv *rt1308 =
> +		snd_soc_component_get_drvdata(component);
>   
>   	switch (event) {
>   	case SND_SOC_DAPM_POST_PMU:
> @@ -331,6 +337,7 @@ static int rt1308_classd_event(struct snd_soc_dapm_widget *w,
>   			RT1308_SDW_OFFSET | (RT1308_POWER_STATUS << 4),
>   			0x3,	0x3);
>   		msleep(40);
> +		rt1308_apply_calib_params(rt1308);
>   		break;
>   	case SND_SOC_DAPM_PRE_PMD:
>   		snd_soc_component_update_bits(component,