lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 26 Mar 2024 12:36:05 +0000
From: "Ding, Shenghao" <shenghao-ding@...com>
To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        "broonie@...nel.org" <broonie@...nel.org>
CC: "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "perex@...ex.cz"
	<perex@...ex.cz>,
        "13916275206@....com" <13916275206@....com>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "liam.r.girdwood@...el.com" <liam.r.girdwood@...el.com>,
        "bard.liao@...el.com" <bard.liao@...el.com>,
        "mengdong.lin@...el.com"
	<mengdong.lin@...el.com>,
        "yung-chuan.liao@...ux.intel.com"
	<yung-chuan.liao@...ux.intel.com>,
        "Lu, Kevin" <kevin-lu@...com>, "tiwai@...e.de" <tiwai@...e.de>,
        "Xu, Baojun" <baojun.xu@...com>, "soyer@....hu" <soyer@....hu>,
        "Baojun.Xu@....com" <Baojun.Xu@....com>,
        "Navada Kanyana, Mukund" <navada@...com>
Subject: RE: [EXTERNAL] Re: [PATCH v11] ASoc: tas2783: Add tas2783 codec
 driver

Hi Guy
Thanks for your comments. Feedback inline

> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> Sent: Tuesday, March 19, 2024 11:35 PM
> To: Ding, Shenghao <shenghao-ding@...com>; broonie@...nel.org
> Cc: andriy.shevchenko@...ux.intel.com; lgirdwood@...il.com;
> perex@...ex.cz; 13916275206@....com; alsa-devel@...a-project.org;
> linux-kernel@...r.kernel.org; liam.r.girdwood@...el.com;
> bard.liao@...el.com; mengdong.lin@...el.com; yung-
> chuan.liao@...ux.intel.com; Lu, Kevin <kevin-lu@...com>; tiwai@...e.de; Xu,
> Baojun <baojun.xu@...com>; soyer@....hu; Baojun.Xu@....com; Navada
> Kanyana, Mukund <navada@...com>
> Subject: [EXTERNAL] Re: [PATCH v11] ASoc: tas2783: Add tas2783 codec
> driver
> 
> > +static bool tas2783_readable_register(struct device *dev, > +
> > +unsigned int reg) > +{ > + switch (reg) { > + /* Page 0 */ > + case
> > +0x8000 .. . 0x807F: > + return true; > + default: > + return false;
> > +so only the registers
> 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
> 
> > +static bool tas2783_readable_register(struct device *dev,
................................................
> > +
> > +	/* Check Calibrated Data V2 */
> > +	if (tmp_val[0] == 2783) {
> > +		const struct calibdatav2_info calib_info = {
> > +			.number_of_devices = tmp_val[1],
> > +			.crc32_indx = 3 + tmp_val[1] * 6,
> > +			.byt_sz = 12 + tmp_val[1] * 24,
> > +			.cali_data = &tmp_val[3]
> > +		};
> > +
> > +		if (calib_info.number_of_devices >
> TAS2783_MAX_DEV_NUM ||
> > +			calib_info.number_of_devices == 0) {
> > +			dev_dbg(tas_dev->dev, "No dev in calibrated data
> V2.");
> 
> the log is not aligned with the first condition where you have too many
> devices.
> 
> It's not clear why it's not an error.
playback still work without calibrated data stored in UEFI, for example bypass mode.
Even if in case of bypass mode, algo can still work with default calibrated data.
So, not an error.
> 
> > +			return;
> > +		}
> > +		crc = crc32(~0, cali_data.data, calib_info.byt_sz)
> > +			^ ~0;
> > +		if (crc == tmp_val[calib_info.crc32_indx]) {
> > +			tas2783_apply_calibv2(tas_dev, &calib_info);
> > +			dev_dbg(tas_dev->dev, "V2: %ptTs",
> 
> same, is this needed?
Accepted.
> > +
> 	&tmp_val[TAS2783_CALIDATAV2_TIMESTAMP_INDX]);
> > +		} else {
> > +			dev_dbg(tas_dev->dev,
> > +				"V2: CRC 0x%08x not match 0x%08x\n",
> > +				crc, tmp_val[calib_info.crc32_indx]);
> 
> is this not an error?
> 
> > +		}
> > +	} else {
> > +		dev_err(tas_dev->dev, "Non-2783 chip\n");
> > +	}
> > +}
> 
> the error level seem inconsistent and it's not clear why errors are ignored?
Maybe set them as dev_warn?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ