On Sat, Mar 01, 2025 at 12:30:01PM +0800, Shenghao Ding wrote:
> For calibration, basic version does not contain any calibration addresses,
> it depends on calibration tool to convey the addresses to the driver.
> Since Alpha and Beta firmware, all the calibration addresses are saved
> into the firmware.
...
> + for (i = 0; i < 20; i++) {
> + const unsigned char *dat = &data[24 * i];
> +
> + /*
> + * check whether current fct param is empty.
> + */
> + if (dat[23] != 1)
> + break;
> +
> + if (!strncmp(dat, "umg_SsmKEGCye", 20)) {
> + r->pow_reg = TASDEVICE_REG(dat[20], dat[21], dat[22]);
> + continue;
> + }
Instead of this lengthy approach, just replace the chain with
if (foo)
...
else if ...
...
It will reduce this by 2/3 of LoCs.
> + /* high 32-bit of real-time spk impedance */
> + if (!strncmp(dat, "iks_E0", 20)) {
> + r->r0_reg = TASDEVICE_REG(dat[20], dat[21], dat[22]);
> + continue;
> + }
> + /* inverse of real-time spk impedance */
> + if (!strncmp(dat, "yep_LsqM0", 20)) {
> + r->invr0_reg =
> + TASDEVICE_REG(dat[20], dat[21], dat[22]);
> + continue;
> + }
> + /* low 32-bit of real-time spk impedance */
> + if (!strncmp(dat, "oyz_U0_ujx", 20)) {
> + r->r0_low_reg =
> + TASDEVICE_REG(dat[20], dat[21], dat[22]);
> + continue;
> + }
> + /* Delta Thermal Limit */
> + if (!strncmp(dat, "iks_GC_GMgq", 20)) {
> + r->tlimit_reg =
> + TASDEVICE_REG(dat[20], dat[21], dat[22]);
> + continue;
> + }
> + /* Thermal data for PG 1.0 device */
> + if (!strncmp(dat, "gou_Yao", 20)) {
> + memcpy(p->thr, &dat[20], 3);
> + continue;
> + }
> + /* Pilot tone enable flag, usually the sine wave */
> + if (!strncmp(dat, "kgd_Wsc_Qsbp", 20)) {
> + memcpy(p->plt_flg, &dat[20], 3);
> + continue;
> + }
> + /* Pilot tone gain for calibration */
> + if (!strncmp(dat, "yec_CqseSsqs", 20)) {
> + memcpy(p->sin_gn, &dat[20], 3);
> + continue;
> + }
> + /* Pilot tone gain for calibration, useless in PG 2.0 */
> + if (!strncmp(dat, "iks_SogkGgog2", 20)) {
> + memcpy(p->sin_gn2, &dat[20], 3);
> + continue;
> + }
> + /* Thermal data for PG 2.0 device */
> + if (!strncmp(dat, "yec_Sae_Y", 20)) {
> + memcpy(p->thr2, &dat[20], 3);
> + continue;
> + }
> + /* Spk Equivalent Resistance in fixed-point format */
> + if (!strncmp(dat, "Re_Int", 20)) {
> + memcpy(p->r0_reg, &dat[20], 3);
> + continue;
> + }
> + /* Check whether the spk connection is open */
> + if (!strncmp(dat, "SigFlag", 20)) {
> + memcpy(p->tf_reg, &dat[20], 3);
> + continue;
> + }
> + /* check spk resonant frequency */
> + if (!strncmp(dat, "a1_Int", 20)) {
> + memcpy(p->a1_reg, &dat[20], 3);
> + continue;
> + }
> + /* check spk resonant frequency */
> + if (!strncmp(dat, "a2_Int", 20)) {
> + memcpy(p->a2_reg, &dat[20], 3);
> + continue;
> + }
> + }
> +}
...
> +static int fw_parse_fct_param_address(struct tasdevice_priv *tas_priv,
> + struct tasdevice_fw *tas_fmw, const struct firmware *fmw, int offset)
> +{
> + struct calidata *cali_data = &tas_priv->cali_data;
> + struct cali_reg *r = &cali_data->cali_reg_array;
> + const unsigned char *data = fmw->data;
> +
> + if (offset + 520 > fmw->size) {
> + dev_err(tas_priv->dev, "%s: File Size error\n", __func__);
> + return -1;
Use proper error code here. Check include/uapi/asm-generic/errno.h.
> + }
> +
> + /* skip reserved part */
> + offset += 40;
> +
> + fct_param_address_parser(r, tas_fmw, &data[offset]);
> +
> + offset += 480;
> +
> + return offset;
> +}
...
> offset = tas_priv->fw_parse_variable_header(tas_priv, fmw, offset);
> - if (offset < 0) {
> - ret = offset;
> - goto out;
> - }
> + if (offset < 0)
> + return offset;
> +
> offset = tas_priv->fw_parse_program_data(tas_priv, tas_fmw, fmw,
> offset);
> - if (offset < 0) {
> - ret = offset;
> - goto out;
> - }
> + if (offset < 0)
> + return offset;
> +
> offset = tas_priv->fw_parse_configuration_data(tas_priv,
> tas_fmw, fmw, offset);
> if (offset < 0)
> - ret = offset;
> + return offset;
The below...
> + if (tas_priv->fw_parse_fct_param_address) {
> + offset = tas_priv->fw_parse_fct_param_address(tas_priv,
> + tas_fmw, fmw, offset);
> + if (offset < 0)
> + ret = offset;
> + }
>
> -out:
> return ret;
...can be written in this form:
if (tas_priv->fw_parse_fct_param_address) {
offset = tas_priv->fw_parse_fct_param_address(tas_priv,
tas_fmw, fmw, offset);
if (offset < 0)
return offset;
}
return 0;
...
> +static void cali_reg_update(struct bulk_reg_val *p,
> + struct fct_param_address *t)
> +{
> + const int sum = ARRAY_SIZE(tas2781_cali_start_reg);
> + int reg = 0;
Unneeded assignment.
> + int j;
> +
> + for (j = 0; j < sum; j++) {
> + switch (tas2781_cali_start_reg[j].reg) {
> + case 0:
> + reg = TASDEVICE_REG(t->thr[0], t->thr[1], t->thr[2]);
> + break;
> + case TAS2781_PRM_PLT_FLAG_REG:
> + reg = TASDEVICE_REG(t->plt_flg[0], t->plt_flg[1],
> + t->plt_flg[2]);
> + break;
> + case TAS2781_PRM_SINEGAIN_REG:
> + reg = TASDEVICE_REG(t->sin_gn[0], t->sin_gn[1],
> + t->sin_gn[2]);
> + break;
> + case TAS2781_PRM_SINEGAIN2_REG:
> + reg = TASDEVICE_REG(t->sin_gn[0], t->sin_gn[1],
> + t->sin_gn[2]);
> + break;
> + default:
> + reg = 0;
> + break;
> + }
> + if (reg)
> + p[j].reg = reg;
> + }
> +}
--
With Best Regards,
Andy Shevchenko
