> +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 in 0x8000..807F are readable. That's the usual non-SDCA vendor-specific areas ... > +static const struct regmap_config tasdevice_regmap = { > + .reg_bits = 32, > + .val_bits = 8, > + .readable_reg = tas2783_readable_register, > + .volatile_reg = tas2783_volatile_register, > + .max_register = 0x44ffffff, .... but here you show support for a much larger register set in SDCA space. I am having a hard-time believing that none of these SDCA registers are readable? > +static void tas2783_calibration(struct tasdevice_priv *tas_dev) > +{ > + efi_guid_t efi_guid = EFI_GUID(0x1f52d2a1, 0xbb3a, 0x457d, 0xbc, > + 0x09, 0x43, 0xa3, 0xf4, 0x31, 0x0a, 0x92); > + static efi_char16_t efi_name[] = L"CALI_DATA"; > + struct calibration_data cali_data; > + unsigned int *tmp_val; > + unsigned int crc; > + efi_status_t status; > + > + cali_data.total_sz = 0; > + > + status = efi.get_variable(efi_name, &efi_guid, NULL, > + &cali_data.total_sz, NULL); > + if (status == EFI_BUFFER_TOO_SMALL > + && cali_data.total_sz < TAS2783_MAX_CALIDATA_SIZE) { > + status = efi.get_variable(efi_name, &efi_guid, NULL, > + &cali_data.total_sz, > + cali_data.data); > + dev_dbg(tas_dev->dev, "%s: cali get %lx bytes result:%ld\n", > + __func__, cali_data.total_sz, status); > + } > + if (status != 0) { > + /* Failed got calibration data from EFI. */ > + dev_dbg(tas_dev->dev, "No calibration data in UEFI."); > + return; > + } > + > + tmp_val = (unsigned int *)cali_data.data; > + > + /* Check Calibrated Data V1 */ > + crc = crc32(~0, cali_data.data, TAS2783_CALIDATAV1_BYTE_SIZE) ^ ~0; > + if (crc == tmp_val[TAS2783_CALIDATAV1_CRC32_INDX]) { > + /* Date and time of when calibration was done. */ > + tas2783_apply_calibv1(tas_dev, tmp_val); > + dev_dbg(tas_dev->dev, "V1: %ptTs", Is this really needed/helpful? > + &tmp_val[TAS2783_CALIDATAV1_TIMESTAMP_INDX]); > + return; > + } > + > + /* 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. > + 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? > + &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?