Received-SPF: pass (google.com: domain of linux-kernel+bounces-94944-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: "Ding, Shenghao" <shenghao-ding@ti.com>
To: Mark Brown <broonie@kernel.org>
CC: "andriy.shevchenko@linux.intel.com" <andriy.shevchenko@linux.intel.com>,
        "lgirdwood@gmail.com" <lgirdwood@gmail.com>,
        "perex@perex.cz"
	<perex@perex.cz>,
        "pierre-louis.bossart@linux.intel.com"
	<pierre-louis.bossart@linux.intel.com>,
        "13916275206@139.com"
	<13916275206@139.com>,
        "alsa-devel@alsa-project.org"
	<alsa-devel@alsa-project.org>,
        "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>,
        "liam.r.girdwood@intel.com"
	<liam.r.girdwood@intel.com>,
        "bard.liao@intel.com" <bard.liao@intel.com>,
        "mengdong.lin@intel.com" <mengdong.lin@intel.com>,
        "yung-chuan.liao@linux.intel.com" <yung-chuan.liao@linux.intel.com>,
        "Xu,
 Baojun" <baojun.xu@ti.com>, "Lu, Kevin" <kevin-lu@ti.com>,
        "tiwai@suse.de"
	<tiwai@suse.de>, "soyer@irl.hu" <soyer@irl.hu>,
        "Baojun.Xu@fpt.com"
	<Baojun.Xu@fpt.com>,
        "Navada Kanyana, Mukund" <navada@ti.com>
Subject: RE: [EXTERNAL] Re: [PATCH v10] The tas2783 is a smart audio amplifier
 with integrated MIPI SoundWire interface (Version 1.2.1 compliant), I2C, and
 I2S/TDM interfaces designed for portable applications. An on-chip DSP
 supports Texas Instruments SmartAmp sp
Thread-Topic: [EXTERNAL] Re: [PATCH v10] The tas2783 is a smart audio
 amplifier with integrated MIPI SoundWire interface (Version 1.2.1 compliant),
 I2C, and I2S/TDM interfaces designed for portable applications. An on-chip
 DSP supports Texas Instruments SmartAmp sp
Thread-Index: AQHabzH/b3I/bWWVFUK0e05J8vV+lrErpzBQ
Date: Thu, 7 Mar 2024 03:56:40 +0000
Message-ID: <dff90323b41a4e3a8e0e8540fbb32154@ti.com>
Accept-Language: en-US, zh-CN
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0


> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Wednesday, March 6, 2024 3:19 AM
> To: Ding, Shenghao <shenghao-ding@ti.com>
> Cc: andriy.shevchenko@linux.intel.com; lgirdwood@gmail.com;
> perex@perex.cz; pierre-louis.bossart@linux.intel.com;
> 13916275206@139.com; alsa-devel@alsa-project.org; linux-
> kernel@vger.kernel.org; liam.r.girdwood@intel.com; bard.liao@intel.com;
> mengdong.lin@intel.com; yung-chuan.liao@linux.intel.com; Xu, Baojun
> <baojun.xu@ti.com>; Lu, Kevin <kevin-lu@ti.com>; tiwai@suse.de;
> soyer@irl.hu; Baojun.Xu@fpt.com; Navada Kanyana, Mukund
> <navada@ti.com>
> Subject: [EXTERNAL] Re: [PATCH v10] The tas2783 is a smart audio amplifie=
r
> with integrated MIPI SoundWire interface (Version 1.2.1 compliant), I2C, =
and
> I2S/TDM interfaces designed for portable applications. An on-chip DSP
> supports Texas Instruments SmartAmp spe...
>=20
> On Tue, Mar 05, 2024 at 04:43:35PM +0800, Shenghao Ding wrote:
> > The ASoC component provides the majority of the functionality of the
> > device, all the audio functions.
>=20
> > +static const struct reg_default tas2783_reg_defaults[] =3D {
> > +	/* Default values for ROM mode. Activated. */
> > +	{ 0x8002, 0x1a },	/* AMP inactive. */
> > +	{ 0x8097, 0xc8 },
> > +	{ 0x80b5, 0x74 },
> > +	{ 0x8099, 0x20 },
> > +	{ 0xfe8d, 0x0d },
> > +	{ 0xfebe, 0x4a },
> > +	{ 0x8230, 0x00 },
> > +	{ 0x8231, 0x00 },
> > +	{ 0x8232, 0x00 },
> > +	{ 0x8233, 0x01 },
> > +	{ 0x8418, 0x00 },	/* Set volume to 0 dB. */
> > +	{ 0x8419, 0x00 },
> > +	{ 0x841a, 0x00 },
> > +	{ 0x841b, 0x00 },
> > +	{ 0x8428, 0x40 },	/* Unmute channel */
> > +	{ 0x8429, 0x00 },
> > +	{ 0x842a, 0x00 },
> > +	{ 0x842b, 0x00 },
> > +	{ 0x8548, 0x00 },	/* Set volume to 0 dB. */
> > +	{ 0x8549, 0x00 },
> > +	{ 0x854a, 0x00 },
> > +	{ 0x854b, 0x00 },
> > +	{ 0x8558, 0x40 },	/* Unmute channel */
> > +	{ 0x8559, 0x00 },
> > +	{ 0x855a, 0x00 },
> > +	{ 0x855b, 0x00 },
> > +	{ 0x800a, 0x3a },	/* Enable both channel */
>=20
> These comments sound like this register default table is not actually the
> physical default register values that the chip has...
>=20
> > +static const struct regmap_config tasdevice_regmap =3D {
> > +	.reg_bits =3D 32,
> > +	.val_bits =3D 8,
> > +	.readable_reg =3D tas2783_readable_register,
> > +	.volatile_reg =3D tas2783_volatile_register,
> > +	.max_register =3D 0x44ffffff,
> > +	.reg_defaults =3D tas2783_reg_defaults,
> > +	.num_reg_defaults =3D ARRAY_SIZE(tas2783_reg_defaults),
>=20
> ...but this is set as the register defaults.  This will cause problems wi=
th things
> like cache sync where we don't write values out if they're not the defaul=
ts.
> We also try to keep default settings from the silicon except in the most
> obvious cases, it avoids issues with trying to work out what to set and
> accomodate different use cases for different systems.
>=20
> If you do need to set non-default values then either just regular writes =
during
> probe or a regmap patch would do it.

So, can I remove=20
".reg_defaults =3D tas2783_reg_defaults," and tas2783_reg_defaults from the=
 code?

>=20
> > +	.cache_type =3D REGCACHE_RBTREE,
> > +	.use_single_read =3D true,
> > +	.use_single_write =3D true,
>=20
> REGCACHE_MAPLE is generally the most sensible choice for modern devices
> - it's a newer and fancier data structure underlying it and there's only =
a few
> cases with low end devices, mostly doing block I/O which this doesn't
> support, where the RBTREE cache is still better.

Accept

>=20
> > +	u16 dev_info;
> > +	int ret;
> > +
> > +	if (!tas_dev->sdw_peripheral) {
> > +		dev_err(tas_dev->dev, "%s: peripheral doesn't exist.\n",
> > +			__func__);
> > +		return;
> > +	}
> > +
> > +	dev_info =3D tas_dev->sdw_peripheral->bus->link_id |
> > +		tas_dev->sdw_peripheral->id.unique_id << 16;
>=20
> I'm kind of surprised dev_info works as a variable name without something
> getting upset that it aliases the function of the same name.

Accept