Re: [EXTERNAL] Re: [PATCH v11] ASoc: tas2783: Add tas2783 codec driver

From: Pierre-Louis Bossart
Date: Tue Mar 26 2024 - 11:14:56 EST



>>> +
>>> + /* 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.

I would use a dev_info with a clearer message that defaults are used,
e.g. along the lines of

dev_info(tas_dev->dev, "using default calibration data, no device found
in EFI data v2").

dev_warn would work as well if this is really not expected.