RE: [EXTERNAL] Re: [PATCH v9] ASoc: tas2783: Add tas2783 codec driver

[Ding, Shenghao]

Hi Pierre-Louis
Thanks for your careful review.

> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> Sent: Friday, February 23, 2024 8:14 AM
> To: Ding, Shenghao <shenghao-ding@xxxxxx>; broonie@xxxxxxxxxx
> Cc: andriy.shevchenko@xxxxxxxxxxxxxxx; lgirdwood@xxxxxxxxx;
> perex@xxxxxxxx; 13916275206@xxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; liam.r.girdwood@xxxxxxxxx;
> bard.liao@xxxxxxxxx; mengdong.lin@xxxxxxxxx; yung-
> chuan.liao@xxxxxxxxxxxxxxx; Xu, Baojun <baojun.xu@xxxxxx>; Lu, Kevin
> <kevin-lu@xxxxxx>; tiwai@xxxxxxx; soyer@xxxxxx
> Subject: [EXTERNAL] Re: [PATCH v9] ASoc: tas2783: Add tas2783 codec driver
> 
> On 2/22/24 2: 40 AM, Shenghao Ding wrote: > 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 ZjQcmQRYFpfptBannerStart This message was
> sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source of
> this email and know the content is safe.
> 
> ZjQcmQRYFpfptBannerEnd
> On 2/22/24 2:40 AM, Shenghao Ding wrote:
> > 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 speaker protection algorithm. The integrated
> > speaker voltage and current sense provides for real-time monitoring of
> lodspeakers.
> 
> loudspeakers.
> 
> 
> > +static void tas2783_apply_calib(struct tasdevice_priv *tas_dev,
> > +	unsigned int *cali_data)
> > +{
> > +	struct regmap *map = tas_dev->regmap;
> > +	u8 *cali_start;
> > +	u16 offset;
> > +	int ret;
> > +
> > +	if (!tas_dev->sdw_peripheral) {
> > +		dev_err(tas_dev->dev, "%s: peripheral doesn't exist.\n",
> > +			__func__);
> > +		return;
> > +	}
> > +
> > +	offset = tas_dev->sdw_peripheral->id.unique_id -
> > +		TAS2783_UNIQUE_ID_BASE;
> > +	if (offset >= TAS2783_MAX_DEV_NUM) {
> > +		dev_err(tas_dev->dev, "%s: offset(%u) is out of range.\n",
> > +			 __func__, offset);
> > +		return;
> > +	}
> 
> I think we are at the 4th iteration where I have to repeat myself...
> 
> In the SoundWire spec, the unique_id is *LINK SPECIFIC*, and only used at
> the bus level within the context of a link to help avoid enumeration
> conflicts
> 
> If you are using the unique_id as a SYSTEM-UNIQUE value to lookup EFI
> data, this is a TI-specific requirement that needs to be documented.
> That also means you need to double-check for errors so make sure there
> are no board configurations where the same unique_id is used in multiple
> links, or by devices other than tas2783.
> 
> 
This code only covers the tas2783s sitting in the same bus link. As to cases of the
different SWD links, customer will be required to have the secondary development
on current code. I'm sure my customers have much knowledge to handle this.