Re: [PATCH v2 2/4] ASoC: qcom: q6afe: provide fallback for digital codec clock

[Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>]

Thanks Otto for the patch,

On 29/10/2023 16:57, Otto Pflüger wrote:
> When q6afe is used as a clock provider through q6afe-clocks.c, it uses
> an interface for setting clocks that is not present in older firmware
> versions. However, using Q6AFE_LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE
> as the codec MCLK in the device tree can be useful on older platforms
> too. Provide a fallback that sets this clock using the old method when
> an old firmware version is detected.
>
> MSM8916 did not need this because of a workaround that controls this
> clock directly through the GCC driver, but newer SoCs do not have this
> clock in their GCC drivers because it is meant to be controlled by the
> DSP.
>
> Signed-off-by: Otto Pflüger <otto.pflueger@xxxxxxxxx>
> ---
>   sound/soc/qcom/qdsp6/q6afe.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
>
> diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
> index 91d39f6ad0bd..f14d3b366aa4 100644
> --- a/sound/soc/qcom/qdsp6/q6afe.c
> +++ b/sound/soc/qcom/qdsp6/q6afe.c
> @@ -1111,6 +1111,32 @@ int q6afe_set_lpass_clock(struct device *dev, int clk_id, int attri,
>   	struct q6afe *afe = dev_get_drvdata(dev->parent);
>   	struct afe_clk_set cset = {0,};
>   
> +	/*
> +	 * v2 clocks specified in the device tree may not be supported by the
> +	 * firmware. If this is the digital codec core clock, fall back to the
> +	 * old method for setting it.
> +	 */
> +	if (q6core_get_adsp_version() < Q6_ADSP_VERSION_2_7) {
> +		struct q6afe_port *port;
> +		struct afe_digital_clk_cfg dcfg = {0,};
> +		int ret;
> +
> +		if (clk_id != Q6AFE_LPASS_CLK_ID_INTERNAL_DIGITAL_CODEC_CORE)
> +			return -EINVAL;
> +
> +		port = q6afe_port_get_from_id(dev, PRIMARY_MI2S_RX);
> +		if (IS_ERR(port))
> +			return PTR_ERR(port);
> +
> +		dcfg.i2s_cfg_minor_version = AFE_API_VERSION_I2S_CONFIG;
> +		dcfg.clk_val = freq;
> +		dcfg.clk_root = 5;
> +		ret = q6afe_set_digital_codec_core_clock(port, &dcfg);
> +
> +		q6afe_port_put(port);

This redirection of clks looks totally confusing and hacky.
Why can not we do this from machine driver based something like this:

if (q6core_get_adsp_version() < Q6_ADSP_VERSION_2_7)
	ret = snd_soc_dai_set_sysclk(cpu_dai, LPAIF_DIG_CLK, freq, 0);



--srini
> +		return ret;
> +	}
> +
>   	cset.clk_set_minor_version = AFE_API_VERSION_CLOCK_SET;
>   	cset.clk_id = clk_id;
>   	cset.clk_freq_in_hz = freq;