Received-SPF: pass (google.com: domain of linux-kernel+bounces-211942-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 12 Jun 2024 19:52:32 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: broonie@kernel.org, perex@perex.cz, lgirdwood@gmail.com, 
	alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, 
	krzk+dt@kernel.org, neil.armstrong@linaro.org, krzysztof.kozlowski@linaro.org
Subject: Re: [PATCH v2 2/2] ASoC: codec: lpass-rx-macro: add suppor for 2.5
 codec version
Message-ID: <mpm4pmt5ieofmpxmq5putvytyuiepbmnv5flsfqiwbtc54sb6k@jpoeaeojzzis>
References: <20240606122559.116698-1-srinivas.kandagatla@linaro.org>
 <20240606122559.116698-3-srinivas.kandagatla@linaro.org>
 <qjyuvejxvjfanhqi3xpgobqjuugh52okxiutdprprx43emee7t@gzh7go6yc77z>
 <5bf5ee5e-d24f-476f-9500-9d1b7adcfc72@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5bf5ee5e-d24f-476f-9500-9d1b7adcfc72@linaro.org>

On Wed, Jun 12, 2024 at 03:37:56PM +0100, Srinivas Kandagatla wrote:
> 
> 
> On 07/06/2024 12:03, Dmitry Baryshkov wrote:
> > On Thu, Jun 06, 2024 at 01:25:59PM +0100, srinivas.kandagatla@linaro.org wrote:
> > > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> > > 
> > > LPASS Codec v2.5 has significant changes in the rx register offsets.
> > > Due to this headset playback on SM8550, SM8650, x1e80100 and all SoCs
> > > after SM8450 have only Left working.
> > > 
> > > This patch adjusts the registers to accomdate 2.5 changes. With this
> > > fixed now L and R are functional on Headset playback.
> > > 
> > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> > > ---
> > >   sound/soc/codecs/lpass-rx-macro.c | 565 ++++++++++++++++++++++--------
> > >   1 file changed, 410 insertions(+), 155 deletions(-)
> > > 
> > > diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
> > > index f35187d69cac..bb8ede0e7076 100644
> > > --- a/sound/soc/codecs/lpass-rx-macro.c
> > > +++ b/sound/soc/codecs/lpass-rx-macro.c
> > >   static int rx_macro_probe(struct platform_device *pdev)
> > >   {
> > > +	struct reg_default *reg_defaults;
> > >   	struct device *dev = &pdev->dev;
> > >   	kernel_ulong_t flags;
> > >   	struct rx_macro *rx;
> > >   	void __iomem *base;
> > > -	int ret;
> > > +	int ret, def_count;
> > >   	flags = (kernel_ulong_t)device_get_match_data(dev);
> > > @@ -3567,6 +3793,33 @@ static int rx_macro_probe(struct platform_device *pdev)
> > >   		goto err;
> > >   	}
> > > +	rx->codec_version = lpass_macro_get_codec_version();
> > 
> > What guarantees that VA macro has been probed already? If I'm not
> > mistaken, we might easily get a default '0' here instead of a correct
> > version.
> 
> fsgen(Frame sync gen) clk is derived from VA macro, so if we are here that
> means the va macro is probed.

Is this written in stone or is it just a current way how these codecs
are connected?

> > > +	switch (rx->codec_version) {
> > > +	case LPASS_CODEC_VERSION_2_5 ... LPASS_CODEC_VERSION_2_8:
> > > +		rx->rxn_reg_offset = 0xc0;
> > > +		def_count = ARRAY_SIZE(rx_defaults) + ARRAY_SIZE(rx_2_5_defaults);
> > > +		reg_defaults = kmalloc_array(def_count, sizeof(struct reg_default), GFP_KERNEL);
> > > +		if (!reg_defaults)
> > > +			return -ENOMEM;
> > > +		memcpy(&reg_defaults[0], rx_defaults, sizeof(rx_defaults));
> > > +		memcpy(&reg_defaults[ARRAY_SIZE(rx_defaults)],
> > > +				rx_2_5_defaults, sizeof(rx_2_5_defaults));
> > > +		break;
> > > +	default:
> > > +		rx->rxn_reg_offset = 0x80;
> > > +		def_count = ARRAY_SIZE(rx_defaults) + ARRAY_SIZE(rx_pre_2_5_defaults);
> > > +		reg_defaults = kmalloc_array(def_count, sizeof(struct reg_default), GFP_KERNEL);
> > > +		if (!reg_defaults)
> > > +			return -ENOMEM;
> > > +		memcpy(&reg_defaults[0], rx_defaults, sizeof(rx_defaults));
> > > +		memcpy(&reg_defaults[ARRAY_SIZE(rx_defaults)],
> > > +				rx_pre_2_5_defaults, sizeof(rx_pre_2_5_defaults));
> > > +		break;
> > > +	}
> > > +
> > > +	rx_regmap_config.reg_defaults = reg_defaults,
> > > +	rx_regmap_config.num_reg_defaults = def_count;
> > > +
> > >   	rx->regmap = devm_regmap_init_mmio(dev, base, &rx_regmap_config);
> > >   	if (IS_ERR(rx->regmap)) {
> > >   		ret = PTR_ERR(rx->regmap);
> > > @@ -3629,6 +3882,7 @@ static int rx_macro_probe(struct platform_device *pdev)
> > >   	if (ret)
> > >   		goto err_clkout;
> > > +	kfree(reg_defaults);
> > >   	return 0;
> > >   err_clkout:
> > > @@ -3642,6 +3896,7 @@ static int rx_macro_probe(struct platform_device *pdev)
> > >   err_dcodec:
> > >   	clk_disable_unprepare(rx->macro);
> > >   err:
> > > +	kfree(reg_defaults);
> > >   	lpass_macro_pds_exit(rx->pds);
> > >   	return ret;
> > > -- 
> > > 2.21.0
> > > 
> > 

-- 
With best wishes
Dmitry