Hi Guy Thanks for your comments. Feedback inline
-----Original Message----- From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Sent: Tuesday, March 19, 2024 11:35 PM To: Ding, Shenghao shenghao-ding@ti.com; broonie@kernel.org Cc: andriy.shevchenko@linux.intel.com; lgirdwood@gmail.com; perex@perex.cz; 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; Lu, Kevin kevin-lu@ti.com; tiwai@suse.de; Xu, Baojun baojun.xu@ti.com; soyer@irl.hu; Baojun.Xu@fpt.com; Navada Kanyana, Mukund navada@ti.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 dataV2.");
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?