On Tue, Apr 09, 2024 at 10:48:15AM +0800, Baojun Xu wrote:
Firmware download and parser lib for tas2781, it work for spi device with a single firmware binary file.
I believe this also can benefit from comments given against previous patches.
...
im = &(calibration->dev_data);
Unneeded parentheses.
if (!im->dev_blks)continue;for (blks = 0; blks < im->nr_blk; blks++) {block = &(im->dev_blks[blks]);if (!block)continue;kfree(block->data);}kfree(im->dev_blks);- }
- kfree(tas_fmw->calibrations);
+out:
- kfree(tas_fmw);
It may gain if you use cleanup.h from day 1.
+}
+void tasdevice_spi_calbin_remove(void *context) +{
- struct tasdevice_priv *tas_priv = (struct tasdevice_priv *) context;
Casting is not needed.
- struct tasdevice *tasdev;
- if (!tas_priv)
return;
How is this not a dead code?
- tasdev = &(tas_priv->tasdevice);
- if (tasdev->cali_data_fmw) {
tas2781_clear_calfirmware(tasdev->cali_data_fmw);tasdev->cali_data_fmw = NULL;- }
+}
...
+void tasdevice_spi_config_info_remove(void *context) +{
- struct tasdevice_priv *tas_priv = (struct tasdevice_priv *) context;
- struct tasdevice_rca *rca = &(tas_priv->rcabin);
- struct tasdevice_config_info **ci = rca->cfg_info;
- int i, j;
- if (!ci)
return;- for (i = 0; i < rca->ncfgs; i++) {
if (!ci[i])continue;if (ci[i]->blk_data) {for (j = 0; j < (int)ci[i]->real_nblocks; j++) {
Oh, explicit castings should be _rarely_ used. What's the problem with making j to be the same type as real_nblocks?
if (!ci[i]->blk_data[j])continue;kfree(ci[i]->blk_data[j]->regdata);kfree(ci[i]->blk_data[j]);}kfree(ci[i]->blk_data);}kfree(ci[i]);- }
- kfree(ci);
+}
...
- if (cfg_no >= 0
&& (tas_priv->tasdevice.cur_conf != cfg_no)&& (cfg_info[rca_conf_no]->active_dev & 1)&& (tas_priv->tasdevice.is_loaderr == false)) {
This is unparsable. Please, use postfix style and proper indentation.
if (foo && bar ...) { ...stuff...; }
status++;tas_priv->tasdevice.is_loading = true;- } else {
tas_priv->tasdevice.is_loading = false;- }
...
- if (state == 0) {
if (tas_priv->cur_prog < tas_fmw->nr_programs) {/*dsp mode or tuning mode*/profile_cfg_id = tas_priv->rcabin.profile_cfg_id;tasdevice_spi_select_tuningprm_cfg(tas_priv,tas_priv->cur_prog, tas_priv->cur_conf,profile_cfg_id);}tasdevice_spi_select_cfg_blk(tas_priv, profile_cfg_id,TASDEVICE_BIN_BLK_PRE_POWER_UP);- } else
tasdevice_spi_select_cfg_blk(tas_priv, profile_cfg_id,TASDEVICE_BIN_BLK_PRE_SHUTDOWN);
Out of a sudden different style (no {} in 'else' branch). Try to be consistent in style everywhere.