From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Shenghao Ding <13916275206@139.com>,
broonie@kernel.org, lgirdwood@gmail.com, perex@perex.cz
Cc: kevin-lu@ti.com, shenghao-ding@ti.com,
alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
x1077012@ti.com, peeyush@ti.com, navada@ti.com
Subject: Re: [PATCH v4] ASoC: tas2781: Add tas2781 driver
Date: Mon, 20 Mar 2023 10:47:08 -0500 [thread overview]
Message-ID: <aea0c730-898d-7326-d245-79bc124bca0d@linux.intel.com> (raw)
In-Reply-To: <20230320150726.20573-1-13916275206@139.com>
Quite a few issues with style, indentation, useless inits. There is one
change of convention between success/error codes, and confusions in the
end with the use of devm_ and manual error handling.
> +#include <linux/regmap.h>
> +#include <linux/i2c.h>
> +#include <linux/string.h>
> +#include <linux/crc8.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/firmware.h>
alphabetical order?
> +#include "tas2781.h"
> +
> +#define ERROR_PRAM_CRCCHK 0x0000000
> +#define ERROR_YRAM_CRCCHK 0x0000001
> +#define BINFILE_VER 0
> +#define DRV_VER 1
> +#define PPC_DRIVER_CRCCHK 0x00000200
indentation after define?
> +static int fw_parse_program_data_kernel(
> + struct tasdevice_priv *tas_dev, struct tasdevice_fw *tas_fmw,
> + const struct firmware *fmw, int offset)
> +{
> + struct tasdevice_prog *program;
> + unsigned int nr_program = 0;
useless initialization
> +
> + for (nr_program = 0; nr_program < tas_fmw->nr_programs; nr_program++) {
> + program = &(tas_fmw->programs[nr_program]);
> + if (offset + 64 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mpName error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 64;
> +
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnAppMode error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> +
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnPDMI2SMode error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnISnsPD error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnVSnsPD error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> + //skip 3-byte reserved
> + offset += 3;
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnPowerLDG error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> +
> + offset = fw_parse_data_kernel(tas_fmw, &(program->dev_data),
> + fmw, offset);
> + if (offset < 0)
> + goto out;
> + }
> +out:
> + return offset;
> +}
> +
> +static int fw_parse_configuration_data_kernel(
> + struct tasdevice_priv *tas_dev,
> + struct tasdevice_fw *tas_fmw, const struct firmware *fmw, int offset)
> +{
> + const unsigned char *data = fmw->data;
> + unsigned int nr_configs;
> + struct tasdevice_config *config;
> +
> + for (nr_configs = 0; nr_configs < tas_fmw->nr_configurations;
> + nr_configs++) {
one line?
> + config =
> + &(tas_fmw->configs[nr_configs]);
one line?
> + if (offset + 64 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mpName error\n",
> + __func__);
one line?
> +static int fw_parse_variable_header_kernel(
> + struct tasdevice_priv *tas_dev, const struct firmware *fmw, int offset)
> +{
> + struct tasdevice_fw *tas_fmw = tas_dev->fmw;
> + struct tasdevice_dspfw_hdr *fw_hdr = &(tas_fmw->fw_hdr);
> + const unsigned char *buf = fmw->data;
> + struct tasdevice_prog *program;
> + struct tasdevice_config *config;
> + unsigned int nr_program = 0, nr_configs = 0;
useless inits.
> + unsigned short max_confs = TASDEVICE_MAXCONFIG_NUM_KERNEL;
init also looks useless?
> +
> + if (offset + 2 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: File Size error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + fw_hdr->device_family = SMS_HTONS(buf[offset], buf[offset + 1]);
> + if (fw_hdr->device_family != 0) {
> + dev_err(tas_dev->dev, "ERROR:%s:not TAS device\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 2;
> + if (offset + 2 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnDevice error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + fw_hdr->device = SMS_HTONS(buf[offset], buf[offset + 1]);
> + if (fw_hdr->device >= TASDEVICE_DSP_TAS_MAX_DEVICE ||
> + fw_hdr->device == 6) {
> + dev_err(tas_dev->dev, "ERROR:%s: not support device %d\n",
> + __func__, fw_hdr->device);
> + offset = -1;
> + goto out;
> + }
> + offset += 2;
> + fw_hdr->ndev = deviceNumber[fw_hdr->device];
> +
> + if (fw_hdr->ndev != tas_dev->ndev) {
> + dev_err(tas_dev->dev,
> + "%s: ndev(%u) in dspbin dismatch ndev(%u) in DTS\n",
> + __func__, fw_hdr->ndev, tas_dev->ndev);
> + offset = -1;
> + goto out;
> + }
> +
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnPrograms error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + tas_fmw->nr_programs = SMS_HTONL(buf[offset], buf[offset + 1],
> + buf[offset + 2], buf[offset + 3]);
> + offset += 4;
> +
> + if (tas_fmw->nr_programs == 0 || tas_fmw->nr_programs >
> + TASDEVICE_MAXPROGRAM_NUM_KERNEL) {
> + dev_err(tas_dev->dev, "%s: mnPrograms is invalid\n", __func__);
> + offset = -1;
> + goto out;
> + }
> +
> + if (offset + 4 * TASDEVICE_MAXPROGRAM_NUM_KERNEL > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mpPrograms error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> +
> + tas_fmw->programs =
> + kcalloc(tas_fmw->nr_programs,
> + sizeof(struct tasdevice_prog), GFP_KERNEL);
indentation is off.
> + if (tas_fmw->programs == NULL) {
> + dev_err(tas_dev->dev, "%s: mpPrograms memory failed!\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> +
> + for (nr_program = 0; nr_program < tas_fmw->nr_programs; nr_program++) {
> + program = &(tas_fmw->programs[nr_program]);
> + program->prog_size = SMS_HTONL(buf[offset], buf[offset + 1],
> + buf[offset + 2], buf[offset + 3]);
> + offset += 4;
> + }
> + offset += (4 * (5 - nr_program));
> +
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnConfigurations error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + tas_fmw->nr_configurations = SMS_HTONL(buf[offset], buf[offset + 1],
> + buf[offset + 2], buf[offset + 3]);
> + offset += 4;
> + max_confs = (fw_hdr->ndev >= 4) ?
> + TASDEVICE_MAXCONFIG_NUM_KERNEL_MULTIPLE_AMPS :
> + TASDEVICE_MAXCONFIG_NUM_KERNEL;
> + if (tas_fmw->nr_configurations == 0 ||
> + tas_fmw->nr_configurations > max_confs) {
> + dev_err(tas_dev->dev, "%s: mnConfigurations is invalid\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> +
> + if (offset + 4 * max_confs > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mpConfigurations error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> +
> + tas_fmw->configs = kcalloc(tas_fmw->nr_configurations,
> + sizeof(struct tasdevice_config), GFP_KERNEL);
> + if (tas_fmw->configs == NULL) {
> + offset = -1;
> + goto out;
> + }
> +
> + for (nr_configs = 0; nr_configs < tas_fmw->nr_programs;
> + nr_configs++) {
> + config =
> + &(tas_fmw->configs[nr_configs]);
> + config->cfg_size = SMS_HTONL(buf[offset],
> + buf[offset + 1], buf[offset + 2], buf[offset + 3]);
> + offset += 4;
> + }
> +
> + offset += (4 * (max_confs - nr_configs));
> +out:
> + return offset;
> +}
> +
> +static int tasdevice_load_block_kernel(
> + struct tasdevice_priv *tasdevice, struct tasdev_blk *block)
> +{
> + int ret = 0;
> +
> + unsigned char *data = block->data;
> + unsigned int i = 0, length = 0;
init for i is useless, init for length is required. don't mix those cases.
> + const unsigned int blk_size = block->blk_size;
> + unsigned char dev_idx = 0;
> + struct tasdevice_dspfw_hdr *fw_hdr = &(tasdevice->fmw->fw_hdr);
> + struct tasdevice_fw_fixed_hdr *fw_fixed_hdr = &(fw_hdr->fixed_hdr);
> +
> + if (fw_fixed_hdr->ppcver >= PPC3_VERSION) {
> + switch (block->type) {
> + case MAIN_ALL_DEVICES_1X:
> + dev_idx = 0|0x80;
> + break;
> + case MAIN_DEVICE_A_1X:
> + dev_idx = 1|0x80;
> + break;
> + case COEFF_DEVICE_A_1X:
> + case PRE_DEVICE_A_1X:
> + dev_idx = 1|0xC0;
> + break;
> + case MAIN_DEVICE_B_1X:
> + dev_idx = 2|0x80;
> + break;
> + case COEFF_DEVICE_B_1X:
> + case PRE_DEVICE_B_1X:
> + dev_idx = 2|0xC0;
> + break;
> + case MAIN_DEVICE_C_1X:
> + dev_idx = 3|0x80;
> + break;
> + case COEFF_DEVICE_C_1X:
> + case PRE_DEVICE_C_1X:
> + dev_idx = 3|0xC0;
> + break;
> + case MAIN_DEVICE_D_1X:
> + dev_idx = 4|0x80;
> + break;
> + case COEFF_DEVICE_D_1X:
> + case PRE_DEVICE_D_1X:
> + dev_idx = 4|0xC0;
> + break;
> + default:
> + dev_info(tasdevice->dev,
> + "%s: load block: Other Type = 0x%02x\n",
> + __func__, block->type);
> + break;
> + }
> + } else {
> + switch (block->type) {
> + case MAIN_ALL_DEVICES:
> + dev_idx = 0|0x80;
> + break;
> + case MAIN_DEVICE_A:
> + dev_idx = 1|0x80;
> + break;
> + case COEFF_DEVICE_A:
> + case PRE_DEVICE_A:
> + dev_idx = 1|0xC0;
> + break;
> + case MAIN_DEVICE_B:
> + dev_idx = 2|0x80;
> + break;
> + case COEFF_DEVICE_B:
> + case PRE_DEVICE_B:
> + dev_idx = 2|0xC0;
> + break;
> + case MAIN_DEVICE_C:
> + dev_idx = 3|0x80;
> + break;
> + case COEFF_DEVICE_C:
> + case PRE_DEVICE_C:
> + dev_idx = 3|0xC0;
> + break;
> + case MAIN_DEVICE_D:
> + dev_idx = 4|0x80;
> + break;
> + case COEFF_DEVICE_D:
> + case PRE_DEVICE_D:
> + dev_idx = 4|0xC0;
> + break;
> + default:
> + dev_info(tasdevice->dev,
> + "%s: load block: Other Type = 0x%02x\n",
> + __func__, block->type);
> + break;
> + }
> + }
> +
> + for (i = 0; i < block->nr_subblocks; i++) {
> + int rc = tasdevice_process_block(tasdevice, data + length,
> + dev_idx, blk_size - length);
> + if (rc < 0) {
> + dev_err(tasdevice->dev,
> + "%s: ERROR:%u %u sublock write error\n",
> + __func__, length, blk_size);
> + break;
this will not return an error. should you use 'ret' instead of
'rc' here, or...
> + }
> + length += (unsigned int)rc;
> + if (blk_size < length) {
> + dev_err(tasdevice->dev,
> + "%s: ERROR:%u %u out of bounary\n",
typo: boundary. use a spell-checker!
> + __func__, length, blk_size);
> + break;
> + }
> + }
> +
> + return ret;
... return 0 and remove ret?
> +}
> +
> +static int fw_parse_variable_header_git(struct tasdevice_priv
> + *tas_dev, const struct firmware *fmw, int offset)
> +{
> + const unsigned char *buf = fmw->data;
> + struct tasdevice_fw *tas_fmw = tas_dev->fmw;
> + struct tasdevice_dspfw_hdr *fw_hdr = &(tas_fmw->fw_hdr);
> + int i = strlen((char *)&buf[offset]);
> +
> + i++;
'i' is not a great name for a length variable, best used for loops...
> +
> + if (offset + i > fmw->size) {
> + dev_err(tas_dev->dev, "%s: File Size error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> +
> + offset += i;
> +
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: File Size error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + fw_hdr->device_family = SMS_HTONL(buf[offset], buf[offset + 1],
> + buf[offset + 2], buf[offset + 3]);
> + if (fw_hdr->device_family != 0) {
> + dev_err(tas_dev->dev, "ERROR:%s: not TAS device\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 4;
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: File Size error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + fw_hdr->device = SMS_HTONL(buf[offset], buf[offset + 1],
> + buf[offset + 2], buf[offset + 3]);
> + if (fw_hdr->device >= TASDEVICE_DSP_TAS_MAX_DEVICE ||
> + fw_hdr->device == 6) {
> + dev_err(tas_dev->dev, "ERROR:%s: not support device %d\n",
> + __func__, fw_hdr->device);
> + offset = -1;
> + goto out;
> + }
> + offset += 4;
> + fw_hdr->ndev = deviceNumber[fw_hdr->device];
> + if (fw_hdr->ndev != tas_dev->ndev) {
> + dev_err(tas_dev->dev,
> + "%s: ndev(%u) in dspbin dismatch ndev(%u) in DTS\n",
> + __func__, fw_hdr->ndev,
> + tas_dev->ndev);
> + offset = -1;
> + }
> +
> +out:
> + return offset;
> +}
> +
> +static int fw_parse_variable_hdr_cal(struct tasdevice_priv *tas_dev,
> + struct tasdevice_fw *tas_fmw, const struct firmware *fmw, int offset)
> +{
> + const unsigned char *buf = fmw->data;
> + struct tasdevice_dspfw_hdr *fw_hdr = &(tas_fmw->fw_hdr);
> + int i = strlen((char *)&buf[offset]);
> +
> + i++;
same here.
> +
> + if (offset + i > fmw->size) {
> + dev_err(tas_dev->dev, "%s: File Size error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> +
> + offset += i;
> +
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnDeviceFamily error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + fw_hdr->device_family = SMS_HTONL(buf[offset], buf[offset + 1],
> + buf[offset + 2], buf[offset + 3]);
> + if (fw_hdr->device_family != 0) {
> + dev_err(tas_dev->dev, "ERROR:%s: not TAS device\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 4;
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnDevice error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + fw_hdr->device = SMS_HTONL(buf[offset], buf[offset + 1],
> + buf[offset + 2], buf[offset + 3]);
> + if (fw_hdr->device >= TASDEVICE_DSP_TAS_MAX_DEVICE ||
> + fw_hdr->device == 6) {
> + dev_err(tas_dev->dev, "ERROR:%s: not support device %d\n",
> + __func__, fw_hdr->device);
> + offset = -1;
> + goto out;
> + }
> + offset += 4;
> + fw_hdr->ndev = deviceNumber[fw_hdr->device];
> + if (fw_hdr->ndev != 1) {
> + dev_err(tas_dev->dev,
> + "%s: calbin must be 1, but currently ndev(%u)\n",
> + __func__, fw_hdr->ndev);
> + offset = -1;
> + }
> +
> +out:
> + return offset;
> +}
> +
> +static inline void tas2781_clear_calfirmware(struct tasdevice_fw
> + *tas_fmw)
> +{
> + int i = 0;
useless init
> + unsigned int blks = 0;
also useless
> +
> + if (tas_fmw->calibrations) {
> + struct tasdevice_calibration *calibration;
> +
> + for (i = 0; i < tas_fmw->nr_calibrations; i++) {
> + calibration = &(tas_fmw->calibrations[i]);
> + if (calibration) {
> + struct tasdevice_data *im =
> + &(calibration->dev_data);
> +
> + if (im->dev_blks) {
> + struct tasdev_blk *block;
> +
> + for (blks = 0; blks < im->nr_blk;
> + blks++) {
> + block = &(im->dev_blks[blks]);
> + kfree(block->data);
> + }
> + kfree(im->dev_blks);
> + }
> + }
> + }
> + kfree(tas_fmw->calibrations);
> + }
> + kfree(tas_fmw);
> +}
> +
> +static int fw_parse_block_data(struct tasdevice_fw *tas_fmw,
> + struct tasdev_blk *block, const struct firmware *fmw, int offset)
> +{
> + unsigned char *data = (unsigned char *)fmw->data;
> + int n;
> +
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_fmw->dev, "%s: mnType error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + block->type = SMS_HTONL(data[offset], data[offset + 1],
> + data[offset + 2], data[offset + 3]);
> + offset += 4;
> +
> + if (tas_fmw->fw_hdr.fixed_hdr.drv_ver >=
> + PPC_DRIVER_CRCCHK) {
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_fmw->dev, "%s: mbPChkSumPresent error\n",
> + __func__);
one line? same for all the code in this file.
> + offset = -1;
> + goto out;
> + }
> + block->is_pchksum_present = data[offset];
> + offset++;
> +
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_fmw->dev, "%s: mnPChkSum error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + block->pchksum = data[offset];
> + offset++;
> +
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_fmw->dev, "%s: mbYChkSumPresent error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + block->is_ychksum_present = data[offset];
> + offset++;
> +
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_fmw->dev, "%s: mnYChkSum error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + block->ychksum = data[offset];
> + offset++;
> + } else {
> + block->is_pchksum_present = 0;
> + block->is_ychksum_present = 0;
> + }
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_fmw->dev, "%s: mnCommands error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + block->nr_cmds = SMS_HTONL(data[offset], data[offset + 1],
> + data[offset + 2], data[offset + 3]);
> + offset += 4;
> +
> + n = block->nr_cmds * 4;
> + if (offset + n > fmw->size) {
> + dev_err(tas_fmw->dev,
> + "%s: File Size(%lu) error offset = %d n = %d\n",
> + __func__, (unsigned long)fmw->size, offset, n);
> + offset = -1;
> + goto out;
> + }
> + block->data = kmemdup(&data[offset], n, GFP_KERNEL);
> + if (block->data == NULL) {
> + offset = -1;
> + goto out;
> + }
> + offset += n;
> +out:
> + return offset;
> +}
> +
> +static int fw_parse_data(struct tasdevice_fw *tas_fmw,
> + struct tasdevice_data *img_data, const struct firmware *fmw,
> + int offset)
> +{
> + const unsigned char *data = (unsigned char *)fmw->data;
> + int n = 0;
useless init.
Also consider reverse xmas-tree order to make the code nicer.
> + unsigned int nr_block;
> +
> + if (offset + 64 > fmw->size) {
> + dev_err(tas_fmw->dev, "%s: mpName error\n", __func__);
> + n = -1;
> + goto out;
> + }
> + memcpy(img_data->name, &data[offset], 64);
> + offset += 64;
> +
> + n = strlen((char *)&data[offset]);
> + n++;
> + if (offset + n > fmw->size) {
> + dev_err(tas_fmw->dev, "%s: mpDescription error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += n;
> +
> + if (offset + 2 > fmw->size) {
> + dev_err(tas_fmw->dev, "%s: mnBlocks error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + img_data->nr_blk = SMS_HTONS(data[offset], data[offset + 1]);
> + offset += 2;
> +
> + img_data->dev_blks =
> + kcalloc(img_data->nr_blk, sizeof(struct tasdev_blk),
> + GFP_KERNEL);
> + if (img_data->dev_blks == NULL) {
> + dev_err(tas_fmw->dev, "%s: FW memory failed!\n", __func__);
> + goto out;
> + }
> + for (nr_block = 0; nr_block < img_data->nr_blk; nr_block++) {
> + offset = fw_parse_block_data(tas_fmw,
> + &(img_data->dev_blks[nr_block]), fmw, offset);
> + if (offset < 0) {
> + offset = -1;
> + goto out;
> + }
> + }
> +out:
> + return offset;
> +}
> +
> +static int fw_parse_calibration_data(struct tasdevice_priv *tas_dev,
> + struct tasdevice_fw *tas_fmw, const struct firmware *fmw, int offset)
> +{
> + unsigned char *data = (unsigned char *)fmw->data;
> + unsigned int n = 0;
> + unsigned int nr_calibration = 0;
useless inits
> + struct tasdevice_calibration *calibration = NULL;
useless as well
> +
> + if (offset + 2 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnCalibrations error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + tas_fmw->nr_calibrations = SMS_HTONS(data[offset], data[offset + 1]);
> + offset += 2;
> +
> + if (tas_fmw->nr_calibrations != 1) {
> + dev_err(tas_dev->dev,
> + "%s: only support one calibraiton(%d)!\n",
> + __func__, tas_fmw->nr_calibrations);
> + goto out;
> + }
> +
> + tas_fmw->calibrations =
> + kcalloc(tas_fmw->nr_calibrations,
> + sizeof(struct tasdevice_calibration), GFP_KERNEL);
> + if (tas_fmw->calibrations == NULL) {
> + dev_err(tas_dev->dev, "%s: mpCalibrations memory failed!\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + for (nr_calibration = 0; nr_calibration < tas_fmw->nr_calibrations;
> + nr_calibration++) {
> + if (offset + 64 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mpCalibrations error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + calibration = &(tas_fmw->calibrations[nr_calibration]);
> + offset += 64;
> +
> + n = strlen((char *)&data[offset]);
> + n++;
> + if (offset + n > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mpDescription error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += n;
> +
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev,
> + "%s: mnProgram error, offset = %d\n", __func__,
> + offset);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> +
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev,
> + "%s: mnConfiguration error, offset = %d\n",
> + __func__,
> + offset);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> +
> + offset = fw_parse_data(tas_fmw, &(calibration->dev_data), fmw,
> + offset);
> + if (offset < 0)
> + goto out;
> + }
> +
> +out:
> +
> + return offset;
> +}
> +
> +static int fw_parse_program_data(struct tasdevice_priv *tas_dev,
> + struct tasdevice_fw *tas_fmw, const struct firmware *fmw, int offset)
> +{
> + struct tasdevice_prog *program;
> + unsigned char *buf = (unsigned char *)fmw->data;
> + int nr_program = 0;
> +
> + if (offset + 2 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: File Size error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + tas_fmw->nr_programs = SMS_HTONS(buf[offset], buf[offset + 1]);
> + offset += 2;
> +
> + if (tas_fmw->nr_programs == 0) {
if (!tas_fmw->nr_programs)
> + dev_err(tas_dev->dev, "%s: mnPrograms is null, maybe calbin\n",
> + __func__);
> + //Do not "offset = -1;", because of calbin
Comment is not clear, and C comments are preferred.
> + goto out;
> + }
> +
> + tas_fmw->programs =
> + kcalloc(tas_fmw->nr_programs, sizeof(struct tasdevice_prog),
> + GFP_KERNEL);
> + if (tas_fmw->programs == NULL) {
> + offset = -1;
> + goto out;
> + }
> + for (nr_program = 0; nr_program < tas_fmw->nr_programs; nr_program++) {
> + int n = 0;
useless init
> +
> + program = &(tas_fmw->programs[nr_program]);
> + if (offset + 64 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mpName error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 64;
> +
> + n = strlen((char *)&buf[offset]);
> + n++;
> + if (offset + n > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mpDescription error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> +
> + offset += n;
> +
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnAppMode error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> +
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnPDMI2SMode error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnISnsPD error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnVSnsPD error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnPowerLDG error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> +
> + offset = fw_parse_data(tas_fmw, &(program->dev_data), fmw,
> + offset);
> + if (offset < 0)
> + goto out;
> + }
> +out:
> + return offset;
> +}
> +
> +static int fw_parse_configuration_data(
> + struct tasdevice_priv *tas_dev,
> + struct tasdevice_fw *tas_fmw,
> + const struct firmware *fmw, int offset)
> +{
> + unsigned char *data = (unsigned char *)fmw->data;
> + int n;
> + unsigned int n_configs;
> + struct tasdevice_config *config;
> +
> + if (offset + 2 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: File Size error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + tas_fmw->nr_configurations = SMS_HTONS(data[offset], data[offset + 1]);
> + offset += 2;
> +
> + if (tas_fmw->nr_configurations == 0) {
> + dev_err(tas_dev->dev, "%s: Configurations is zero\n",
> + __func__);
> + //Do not "offset = -1;", because of calbin
> + goto out;
> + }
> + tas_fmw->configs =
> + kcalloc(tas_fmw->nr_configurations,
> + sizeof(struct tasdevice_config), GFP_KERNEL);
> +
> + for (n_configs = 0; n_configs < tas_fmw->nr_configurations;
> + n_configs++) {
> + config =
> + &(tas_fmw->configs[n_configs]);
> + if (offset + 64 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: File Size error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + memcpy(config->name, &data[offset], 64);
> + offset += 64;
> +
> + n = strlen((char *)&data[offset]);
> + n++;
> + if (offset + n > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mpDescription error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> +
> + offset += n;
> + if (offset + 2 > fmw->size) {
> + dev_err(tas_dev->dev,
> + "%s: mnDevice_orientation error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 2;
> +
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mProgram error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> +
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnSamplingRate error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 4;
> +
> + if (offset + 1 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnPLLSrc error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset++;
> +
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnPLLSrcRate error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 4;
> +
> + if (offset + 2 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnFsRate error\n",
> + __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 2;
> +
> + offset = fw_parse_data(tas_fmw, &(config->dev_data),
> + fmw, offset);
> + if (offset < 0)
> + goto out;
> + }
> +out:
> + return offset;
> +}
> +
> +static int fw_parse_header(struct tasdevice_priv *tas_dev,
> + struct tasdevice_fw *tas_fmw, const struct firmware *fmw, int offset)
> +{
> + struct tasdevice_dspfw_hdr *fw_hdr = &(tas_fmw->fw_hdr);
> + struct tasdevice_fw_fixed_hdr *fw_fixed_hdr = &(fw_hdr->fixed_hdr);
> + const unsigned char *buf = (unsigned char *)fmw->data;
> + const unsigned char magic_number[] = { 0x35, 0x35, 0x35, 0x32 };
> +
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: File Size error\n", __func__);
> + offset = -EINVAL;
consistency: before you returned -1, now it's -EINVAL?
-1 should probably be avoided.
> + goto out;
> + }
> + if (memcmp(&buf[offset], magic_number, 4)) {
> + dev_err(tas_dev->dev, "%s: Magic number doesn't match",
> + __func__);
> + offset = -EINVAL;
> + goto out;
> + }
> + offset += 4;
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnFWSize error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + fw_fixed_hdr->fwsize = SMS_HTONL(buf[offset],
> + buf[offset + 1], buf[offset + 2], buf[offset + 3]);
> + offset += 4;
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: File Size error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + if (fw_fixed_hdr->fwsize != fmw->size) {
> + dev_err(tas_dev->dev, "File size not match, %lu %u",
> + (unsigned long)fmw->size, fw_fixed_hdr->fwsize);
> + offset = -1;
> + goto out;
> + }
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnChecksum error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 4;
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnPPCVersion error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + fw_fixed_hdr->ppcver = SMS_HTONL(buf[offset],
> + buf[offset + 1], buf[offset + 2], buf[offset + 3]);
> + offset += 4;
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnFWVersion error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 4;
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnDriverVersion error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + fw_fixed_hdr->drv_ver = SMS_HTONL(buf[offset],
> + buf[offset + 1], buf[offset + 2], buf[offset + 3]);
> + offset += 4;
> + if (offset + 4 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mnTimeStamp error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 4;
> + if (offset + 64 > fmw->size) {
> + dev_err(tas_dev->dev, "%s: mpDDCName error\n", __func__);
> + offset = -1;
> + goto out;
> + }
> + offset += 64;
> +
> + out:
> + return offset;
> +}
> +
> +static int check_inpage_yram(struct tas_crc *cd, unsigned char book,
> + unsigned char page, unsigned char reg, unsigned char len)
> +{
> + int ret = 0;
> +
> + if (book == TAS2781_YRAM_BOOK1) {
> + if (page == TAS2781_YRAM1_PAGE) {
> + if (reg >= TAS2781_YRAM1_START_REG) {
> + cd->offset = reg;
> + cd->len = len;
> + ret = 1;
> + } else if ((reg + len) > TAS2781_YRAM1_START_REG) {
> + cd->offset = TAS2781_YRAM1_START_REG;
> + cd->len =
> + len - (TAS2781_YRAM1_START_REG - reg);
> + ret = 1;
> + } else
> + ret = 0;
> + } else if (page == TAS2781_YRAM3_PAGE) {
> + if (reg > TAS2781_YRAM3_END_REG) {
> + ret = 0;
> + } else if (reg >= TAS2781_YRAM3_START_REG) {
> + if ((reg + len) > TAS2781_YRAM3_END_REG) {
> + cd->offset = reg;
> + cd->len =
> + TAS2781_YRAM3_END_REG - reg + 1;
> + ret = 1;
> + } else {
> + cd->offset = reg;
> + cd->len = len;
> + ret = 1;
> + }
> + } else {
> + if ((reg + (len-1)) <
> + TAS2781_YRAM3_START_REG)
> + ret = 0;
> + else {
> + cd->offset =
> + TAS2781_YRAM3_START_REG;
> + cd->len =
> + len - (TAS2781_YRAM3_START_REG - reg);
> + ret = 1;
> + }
> + }
> + }
> + } else if (book ==
> + TAS2781_YRAM_BOOK2) {
> + if (page == TAS2781_YRAM5_PAGE) {
> + if (reg > TAS2781_YRAM5_END_REG) {
> + ret = 0;
> + } else if (reg >= TAS2781_YRAM5_START_REG) {
> + if ((reg + len) > TAS2781_YRAM5_END_REG) {
> + cd->offset = reg;
> + cd->len =
> + TAS2781_YRAM5_END_REG - reg + 1;
> + ret = 1;
> + } else {
> + cd->offset = reg;
> + cd->len = len;
> + ret = 1;
> + }
> + } else {
> + if ((reg + (len-1)) <
> + TAS2781_YRAM5_START_REG)
> + ret = 0;
> + else {
> + cd->offset =
> + TAS2781_YRAM5_START_REG;
> + cd->len =
> + len - (TAS2781_YRAM5_START_REG - reg);
> + ret = 1;
> + }
> + }
> + }
> + } else
> + ret = 0;
> +
> + return ret;
and here you return a positive value on success and zero on error.
Usually we use the opposite convention.
> +}
> +
> +static int check_inblock_yram(struct tas_crc *cd, unsigned char book,
> + unsigned char page, unsigned char reg, unsigned char len)
> +{
> + int ret = 0;
> +
> + if (book == TAS2781_YRAM_BOOK1) {
> + if (page < TAS2781_YRAM2_START_PAGE)
> + ret = 0;
> + else if (page <= TAS2781_YRAM2_END_PAGE) {
> + if (reg > TAS2781_YRAM2_END_REG)
> + ret = 0;
> + else if (reg >= TAS2781_YRAM2_START_REG) {
> + cd->offset = reg;
> + cd->len = len;
> + ret = 1;
> + } else {
> + if ((reg + (len-1)) <
> + TAS2781_YRAM2_START_REG)
> + ret = 0;
> + else {
> + cd->offset =
> + TAS2781_YRAM2_START_REG;
> + cd->len =
> + reg + len - TAS2781_YRAM2_START_REG;
> + ret = 1;
> + }
> + }
> + } else
> + ret = 0;
> + } else if (book ==
> + TAS2781_YRAM_BOOK2) {
> + if (page < TAS2781_YRAM4_START_PAGE)
> + ret = 0;
> + else if (page <= TAS2781_YRAM4_END_PAGE) {
> + if (reg > TAS2781_YRAM2_END_REG)
> + ret = 0;
> + else if (reg >= TAS2781_YRAM2_START_REG) {
> + cd->offset = reg;
> + cd->len = len;
> + ret = 1;
> + } else {
> + if ((reg + (len-1))
> + < TAS2781_YRAM2_START_REG)
> + ret = 0;
> + else {
> + cd->offset =
> + TAS2781_YRAM2_START_REG;
> + cd->len =
> + reg + len - TAS2781_YRAM2_START_REG;
> + ret = 1;
> + }
> + }
> + } else
> + ret = 0;
> + } else
> + ret = 0;
> +
> + return ret;
> +}
same here, error/sucess are inverted?
> +
> +static int check_yram(struct tas_crc *cd, unsigned char book,
> + unsigned char page, unsigned char reg, unsigned char len)
> +{
> + int ret = 0;
useless init
> +
> + ret = check_inpage_yram(cd, book, page, reg, len);
> + if (ret == 0)
> + ret = check_inblock_yram(cd, book,
> + page, reg, len);
> +
> + return ret;
> +}
> +
> +static int do_singlereg_checksum(struct tasdevice_priv *tasdevice,
> + enum channel chl, unsigned char book, unsigned char page,
> + unsigned char reg, unsigned char val)
> +{
> + int ret = 0;
useless init
> + struct tas_crc crc_data;
> + unsigned int nData1 = 0;
useless init
> +
> + if ((book == TASDEVICE_BOOK_ID(TAS2781_SA_COEFF_SWAP_REG))
> + && (page == TASDEVICE_PAGE_ID(TAS2781_SA_COEFF_SWAP_REG))
> + && (reg >= TASDEVICE_PAGE_REG(TAS2781_SA_COEFF_SWAP_REG))
> + && (reg <= (TASDEVICE_PAGE_REG(
> + TAS2781_SA_COEFF_SWAP_REG) + 4))) {
> + /*DSP swap command, pass */
> + ret = 0;
> + goto end;
> + }
> +
> + ret = check_yram(&crc_data, book, page, reg, 1);
> + if (ret == 1) {
> + ret = tasdevice_dev_read(tasdevice, chl,
> + TASDEVICE_REG(book, page, reg), &nData1);
> + if (ret < 0)
> + goto end;
> +
> + if (nData1 != val) {
> + dev_err(tasdevice->dev,
> + "B[0x%x]P[0x%x]R[0x%x] W[0x%x], R[0x%x]\n",
> + book, page, reg,
> + val, nData1);
> + ret = -EAGAIN;
> + tasdevice->tasdevice[chl].err_code |=
> + ERROR_YRAM_CRCCHK;
> + ret = -EAGAIN;
> + goto end;
> + }
> +
> + ret = crc8(tasdevice->crc8_lkp_tbl, &val, 1, 0);
> + }
> +
> +end:
> + return ret;
> +}
> +
> +static int do_multireg_checksum(struct tasdevice_priv *tasdevice,
> + enum channel chn, unsigned char book, unsigned char page,
> + unsigned char reg, unsigned int len)
> +{
> + int ret = 0, i = 0;
i does not need to be initialized.
> + unsigned char crc_chksum = 0;
> + unsigned char nBuf1[128] = {0};
> + struct tas_crc crc_data;
> +
> + if ((reg + len-1) > 127) {
> + ret = -EINVAL;
> + dev_err(tasdevice->dev, "firmware error\n");
> + goto end;
> + }
> +
> + if ((book == TASDEVICE_BOOK_ID(TAS2781_SA_COEFF_SWAP_REG))
> + && (page == TASDEVICE_PAGE_ID(TAS2781_SA_COEFF_SWAP_REG))
> + && (reg == TASDEVICE_PAGE_REG(TAS2781_SA_COEFF_SWAP_REG))
> + && (len == 4)) {
> + /*DSP swap command, pass */
/* DSP swap
> + ret = 0;
> + goto end;
> + }
> +
> + ret = check_yram(&crc_data, book, page, reg, len);
> + if (ret == 1) {
> + if (len == 1) {
> + dev_err(tasdevice->dev, "firmware error\n");
> + ret = -EINVAL;
> + goto end;
> + } else {
> + ret = tasdevice_dev_bulk_read(tasdevice, chn,
> + TASDEVICE_REG(book, page, crc_data.offset),
> + nBuf1, crc_data.len);
> + if (ret < 0)
> + goto end;
> +
> + for (i = 0; i < crc_data.len; i++) {
> + if ((book == TASDEVICE_BOOK_ID(
> + TAS2781_SA_COEFF_SWAP_REG))
> + && (page == TASDEVICE_PAGE_ID(
> + TAS2781_SA_COEFF_SWAP_REG))
> + && ((i + crc_data.offset)
> + >= TASDEVICE_PAGE_REG(
> + TAS2781_SA_COEFF_SWAP_REG))
> + && ((i + crc_data.offset)
> + <= (TASDEVICE_PAGE_REG(
> + TAS2781_SA_COEFF_SWAP_REG)
> + + 4))) {
> + /*DSP swap command, bypass */
> + continue;
> + } else
> + crc_chksum +=
> + crc8(tasdevice->crc8_lkp_tbl,
> + &nBuf1[i], 1, 0);
> + }
> +
> + ret = crc_chksum;
> + }
> + }
> +
> +end:
> + return ret;
> +}
> +
> +static int tasdevice_load_block(struct tasdevice_priv *tas_dev,
> + struct tasdev_blk *block)
> +{
> + int ret = 0;
> + unsigned int nr_cmds = 0;
> + unsigned char book = 0;
> + unsigned char page = 0;
> + unsigned char offset = 0;
> + unsigned char val = 0;
> + unsigned int len = 0;
> + unsigned int sleep_dur = 0;
> + unsigned char crc_chksum = 0;
> + unsigned int nr_value = 0;
> + int nr_retry = 6;
> + unsigned char *data = block->data;
> + int chn = 0, chnend = 0;
I would guess most inits are not needed, except when they are...
> +
> + switch (block->type) {
> + case MAIN_ALL_DEVICES:
> + chn = 0;
> + chnend = tas_dev->ndev;
> + break;
> + case MAIN_DEVICE_A:
> + case COEFF_DEVICE_A:
> + case PRE_DEVICE_A:
> + chn = 0;
> + chnend = 1;
> + break;
> + case MAIN_DEVICE_B:
> + case COEFF_DEVICE_B:
> + case PRE_DEVICE_B:
> + chn = 1;
> + chnend = 2;
> + break;
> + case MAIN_DEVICE_C:
> + case COEFF_DEVICE_C:
> + case PRE_DEVICE_C:
> + chn = 2;
> + chnend = 3;
> + break;
> + case MAIN_DEVICE_D:
> + case COEFF_DEVICE_D:
> + case PRE_DEVICE_D:
> + chn = 3;
> + chnend = 4;
> + break;
> + default:
> + dev_dbg(tas_dev->dev, "load block: Other Type = 0x%02x\n",
> + block->type);
> + break;
> + }
> +
> + for (; chn < chnend; chn++) {
> + if (tas_dev->tasdevice[chn].is_loading == false)
> + continue;
> +start:
> + if (block->is_pchksum_present) {
> + ret = tasdevice_dev_write(tas_dev, chn,
> + TASDEVICE_I2CChecksum, 0);
> + if (ret < 0)
> + goto end;
> + }
> +
> + if (block->is_ychksum_present)
> + crc_chksum = 0;
> +
> + nr_cmds = 0;
> +
> + while (nr_cmds < block->nr_cmds) {
> + data = block->data + nr_cmds * 4;
> +
> + book = data[0];
> + page = data[1];
> + offset = data[2];
> + val = data[3];
> +
> + nr_cmds++;
> +
> + if (offset <= 0x7F) {
> + ret = tasdevice_dev_write(tas_dev, chn,
> + TASDEVICE_REG(book, page, offset),
> + val);
> + if (ret < 0)
> + goto end;
> + if (block->is_ychksum_present) {
> + ret = do_singlereg_checksum(tas_dev,
> + chn, book, page, offset,
> + val);
> + if (ret < 0)
> + goto check;
> + crc_chksum += (unsigned char)ret;
> + }
> + continue;
> + }
> + if (offset == 0x81) {
> + sleep_dur = (book << 8) + page;
> + msleep(sleep_dur);
> + continue;
> + }
> + if (offset == 0x85) {
> + data += 4;
> + len = (book << 8) + page;
> + book = data[0];
> + page = data[1];
> + offset = data[2];
> + if (len > 1) {
> + ret = tasdevice_dev_bulk_write(
> + tas_dev, chn,
> + TASDEVICE_REG(book, page,
> + offset), data + 3, len);
> + if (ret < 0)
> + goto end;
> + if (!block->is_ychksum_present)
> + goto next;
> + ret = do_multireg_checksum(
> + tas_dev, chn, book,
> + page, offset,
> + len);
> + if (ret < 0)
> + goto check;
> + crc_chksum +=
> + (unsigned char)ret;
> + } else {
> + ret = tasdevice_dev_write(tas_dev,
> + chn,
> + TASDEVICE_REG(book, page,
> + offset),
> + data[3]);
> + if (ret < 0)
> + goto end;
> + if (!block->is_ychksum_present)
> + goto next;
> + ret = do_singlereg_checksum(
> + tas_dev, chn, book,
> + page, offset,
> + data[3]);
> + if (ret < 0)
> + goto check;
> + crc_chksum +=
> + (unsigned char)ret;
> + }
> +next:
> + nr_cmds++;
> + if (len >= 2)
> + nr_cmds += ((len - 2) / 4) + 1;
> + }
> + }
> + if (block->is_pchksum_present) {
> + ret = tasdevice_dev_read(tas_dev, chn,
> + TASDEVICE_I2CChecksum, &nr_value);
> + if (ret < 0) {
> + dev_err(tas_dev->dev, "%s: Channel %d\n",
> + __func__, chn);
> + goto check;
> + }
> + if ((nr_value & 0xff) != block->pchksum) {
> + dev_err(tas_dev->dev,
> + "Block PChkSum Channel %d ", chn);
> + dev_err(tas_dev->dev,
> + "FW = 0x%x, Reg = 0x%x\n",
> + block->pchksum,
> + (nr_value & 0xff));
> + ret = -EAGAIN;
> + tas_dev->tasdevice[chn].err_code |=
> + ERROR_PRAM_CRCCHK;
> + goto check;
> + }
> + ret = 0;
> + tas_dev->tasdevice[chn].err_code &=
> + ~ERROR_PRAM_CRCCHK;
> + }
> +
> + if (block->is_ychksum_present) {
> + //TBD, open it when FW ready
> + dev_err(tas_dev->dev,
> + "Block YChkSum: FW = 0x%x, YCRC = 0x%x\n",
> + block->ychksum, crc_chksum);
> +
> + tas_dev->tasdevice[chn].err_code &=
> + ~ERROR_YRAM_CRCCHK;
> + ret = 0;
> + }
> +check:
> + if (ret == -EAGAIN) {
> + nr_retry--;
> + if (nr_retry > 0)
> + goto start;
> + else {
> + if ((block->type == MAIN_ALL_DEVICES)
> + || (block->type == MAIN_DEVICE_A)
> + || (block->type == MAIN_DEVICE_B)
> + || (block->type == MAIN_DEVICE_C)
> + || (block->type == MAIN_DEVICE_D))
> + tas_dev->tasdevice[chn].cur_prog = -1;
> + else
> + tas_dev->tasdevice[chn].cur_conf = -1;
> + nr_retry = 6;
> + }
> + }
> + }
> +end:
> + if (ret < 0)
> + dev_err(tas_dev->dev, "Block (%d) load error\n", block->type);
> +
> + return ret;
> +}
> +
> +
> +static int tasdevice_load_data(struct tasdevice_priv *tas_dev,
> + struct tasdevice_data *dev_data)
> +{
> + int ret = 0;
required init
> + unsigned int nr_block = 0;
> + struct tasdev_blk *block = NULL;
useless inits
> +
> + for (nr_block = 0; nr_block < dev_data->nr_blk; nr_block++) {
> + block = &(dev_data->dev_blks[nr_block]);
> + ret = tas_dev->tasdevice_load_block(tas_dev, block);
> + if (ret < 0)
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static int tasdevice_load_calibrated_data(
> + struct tasdevice_priv *tas_dev, struct tasdevice_data *dev_data)
> +{
> + int ret = 0;
> + unsigned int nr_block = 0;
> + struct tasdev_blk *block = NULL;
> +
> + for (nr_block = 0; nr_block < dev_data->nr_blk; nr_block++) {
> + block = &(dev_data->dev_blks[nr_block]);
> + ret = tasdevice_load_block(tas_dev, block);
> + if (ret < 0)
> + break;
> + }
> +
> + return ret;
> +}
> +
> +int tas2781_load_calibration(void *context,
> + char *file_name, enum channel i)
> +{
> + int ret = 0, offset = 0;
> + struct firmware fmw;
> + const struct firmware *fw_entry = NULL;
> + struct tasdevice_priv *tas_dev = (struct tasdevice_priv *)context;
> + struct tasdevice *tasdev = &(tas_dev->tasdevice[i]);
> + struct tasdevice_fw *tas_fmw = NULL;
> +
> + ret = request_firmware(&fw_entry, file_name, tas_dev->dev);
> + if (!ret) {
> + if (!fw_entry->size) {
> + dev_err(tas_dev->dev,
> + "%s: file read error: size = %lu\n",
> + __func__, (unsigned long)fw_entry->size);
> + goto out;
> + }
> + fmw.size = fw_entry->size;
> + fmw.data = fw_entry->data;
> + } else {
> + dev_info(tas_dev->dev,
> + "%s: Request firmware %s failed\n",
> + __func__, file_name);
> + goto out;
> + }
> +
> + tas_fmw = tasdev->cali_data_fmw = kcalloc(1,
> + sizeof(struct tasdevice_fw), GFP_KERNEL);
> + if (tasdev->cali_data_fmw == NULL) {
> + dev_err(tas_dev->dev, "%s: FW memory failed!\n", __func__);
> + ret = -1;
> + goto out;
> + }
> + tas_fmw->dev = tas_dev->dev;
> + offset = fw_parse_header(tas_dev, tas_fmw, &fmw, offset);
> + if (offset == -1) {
> + dev_err(tas_dev->dev, "%s: fw_parse_header EXIT!\n", __func__);
> + goto out;
> + }
> + offset = fw_parse_variable_hdr_cal(tas_dev, tas_fmw, &fmw, offset);
> + if (offset == -1) {
> + dev_err(tas_dev->dev,
> + "%s: fw_parse_variable_header_cal EXIT!\n", __func__);
> + goto out;
> + }
> + offset = fw_parse_program_data(tas_dev, tas_fmw, &fmw, offset);
> + if (offset == -1) {
> + dev_err(tas_dev->dev, "%s: fw_parse_program_data EXIT!\n",
> + __func__);
> + goto out;
> + }
> + offset = fw_parse_configuration_data(tas_dev, tas_fmw, &fmw,
> + offset);
> + if (offset == -1) {
> + dev_err(tas_dev->dev,
> + "%s: fw_parse_configuration_data EXIT!\n", __func__);
> + goto out;
> + }
> + offset = fw_parse_calibration_data(tas_dev,
> + tas_fmw, &fmw, offset);
> + if (offset == -1) {
> + dev_err(tas_dev->dev, "%s: fw_parse_calibration_data EXIT!\n",
> + __func__);
> + goto out;
> + }
> + tasdev->is_calibrated_data_loaded = true;
> +out:
> + if (fw_entry) {
> + release_firmware(fw_entry);
> + fw_entry = NULL;
> + }
> + return ret;
> +}
> +
> +int tasdevice_dspfw_ready(const struct firmware *fmw, void *context)
> +{
> + struct tasdevice_priv *tas_dev = (struct tasdevice_priv *) context;
> + struct tasdevice_fw *tas_fmw = NULL;
> + struct tasdevice_fw_fixed_hdr *fw_fixed_hdr = NULL;
> + int offset = 0, ret = 0;
> +
> + if (!fmw || !fmw->data) {
> + dev_err(tas_dev->dev, "%s: Failed to read firmware %s\n",
> + __func__, tas_dev->coef_binaryname);
> + ret = -1;
> + goto out;
> + }
> +
> + tas_dev->fmw = kcalloc(1,
> + sizeof(struct tasdevice_fw), GFP_KERNEL);
> + if (tas_dev->fmw == NULL) {
> + ret = -1;
> + goto out;
> + }
> + tas_fmw = tas_dev->fmw;
> + tas_fmw->dev = tas_dev->dev;
> + offset = fw_parse_header(tas_dev, tas_fmw, fmw, offset);
> +
> + if (offset == -1)
> + goto out;
> + fw_fixed_hdr = &(tas_fmw->fw_hdr.fixed_hdr);
> + switch (fw_fixed_hdr->drv_ver) {
> + case 0x301:
> + case 0x302:
> + case 0x502:
> + tas_dev->fw_parse_variable_header =
> + fw_parse_variable_header_kernel;
> + tas_dev->fw_parse_program_data =
> + fw_parse_program_data_kernel;
> + tas_dev->fw_parse_configuration_data =
> + fw_parse_configuration_data_kernel;
> + tas_dev->tasdevice_load_block =
> + tasdevice_load_block_kernel;
> + break;
> + case 0x202:
> + case 0x400:
> + tas_dev->fw_parse_variable_header =
> + fw_parse_variable_header_git;
> + tas_dev->fw_parse_program_data =
> + fw_parse_program_data;
> + tas_dev->fw_parse_configuration_data =
> + fw_parse_configuration_data;
> + tas_dev->tasdevice_load_block =
> + tasdevice_load_block;
> + break;
> + default:
> + if (fw_fixed_hdr->drv_ver == 0x100) {
> + if (fw_fixed_hdr->ppcver >= PPC3_VERSION) {
> + tas_dev->fw_parse_variable_header =
> + fw_parse_variable_header_kernel;
> + tas_dev->fw_parse_program_data =
> + fw_parse_program_data_kernel;
> + tas_dev->fw_parse_configuration_data =
> + fw_parse_configuration_data_kernel;
> + tas_dev->tasdevice_load_block =
> + tasdevice_load_block_kernel;
> + tas_dev->fw_parse_calibration_data = NULL;
> + } else {
> + switch (fw_fixed_hdr->ppcver) {
> + case 0x00:
> + tas_dev->fw_parse_variable_header =
> + fw_parse_variable_header_git;
> + tas_dev->fw_parse_program_data =
> + fw_parse_program_data;
> + tas_dev->fw_parse_configuration_data =
> + fw_parse_configuration_data;
> + tas_dev->fw_parse_calibration_data =
> + fw_parse_calibration_data;
> + tas_dev->tasdevice_load_block =
> + tasdevice_load_block;
> + break;
> + default:
> + dev_err(tas_dev->dev,
> + "%s: PPCVersion must be 0x0 or 0x%02x",
> + __func__, PPC3_VERSION);
> + dev_err(tas_dev->dev, " Current:0x%02x\n",
> + fw_fixed_hdr->ppcver);
> + offset = -1;
> + break;
> + }
> + }
> + } else {
> + dev_err(tas_dev->dev,
> + "%s: DriverVersion must be 0x0, 0x230 or above 0x230 ",
> + __func__);
> + dev_err(tas_dev->dev, "current is 0x%02x\n",
> + fw_fixed_hdr->drv_ver);
> + offset = -1;
> + }
> + break;
> + }
> +
> + offset = tas_dev->fw_parse_variable_header(tas_dev, fmw, offset);
> + if (offset == -1)
> + goto out;
> +
> + offset = tas_dev->fw_parse_program_data(tas_dev, tas_fmw, fmw,
> + offset);
> + if (offset < 0) {
> + ret = -1;
> + goto out;
> + }
> + offset = tas_dev->fw_parse_configuration_data(tas_dev,
> + tas_fmw, fmw, offset);
> + if (offset < 0)
> + ret = -1;
> +
> +out:
> + return ret;
> +}
> +
> +void tasdevice_calbin_remove(void *context)
> +{
> + struct tasdevice_priv *tas_dev = (struct tasdevice_priv *) context;
> + struct tasdevice *tasdev = NULL;
> + int i = 0;
last two inits are useless
> +
> + if (tas_dev) {
> + for (i = 0; i < tas_dev->ndev; i++) {
> + tasdev = &(tas_dev->tasdevice[i]);
> + if (tasdev->cali_data_fmw) {
> + tas2781_clear_calfirmware(
> + tasdev->cali_data_fmw);
> + tasdev->cali_data_fmw = NULL;
> + }
> + }
> + }
> +}
> +
> +void tasdevice_dsp_remove(void *context)
> +{
> + struct tasdevice_priv *tas_dev = (struct tasdevice_priv *) context;
> + int i;
> + struct tasdev_blk *blk;
> + unsigned int nr_blk;
> +
> + if (tas_dev->fmw) {
> + struct tasdevice_fw *tas_fmw = tas_dev->fmw;
> +
> + if (tas_fmw->programs) {
> + struct tasdevice_prog *program;
> +
> + for (i = 0; i < tas_fmw->nr_programs; i++) {
> + program = &(tas_fmw->programs[i]);
> + if (program) {
> + struct tasdevice_data
> + *im = &(program->dev_data);
> +
> + if (!im->dev_blks)
> + continue;
> +
> + for (nr_blk = 0; nr_blk < im->nr_blk;
> + nr_blk++) {
> + blk =
> + &(im->dev_blks[nr_blk]);
> + kfree(blk->data);
indentation is awful.
> + }
> + kfree(im->dev_blks);
> + }
> + }
> + kfree(tas_fmw->programs);
> + }
> +
> + if (tas_fmw->configs) {
> + struct tasdevice_config *config;
> +
> + for (i = 0; i < tas_fmw->nr_configurations; i++) {
> + config = &(tas_fmw->configs[i]);
> + if (config) {
> + struct tasdevice_data
> + *im = &(config->dev_data);
> +
> + if (!im->dev_blks)
> + continue;
> + for (nr_blk = 0; nr_blk < im->nr_blk;
> + nr_blk++) {
> + blk =
> + &(im->dev_blks[nr_blk]);
> + kfree(blk->data);
> + }
> + kfree(im->dev_blks);
> + }
> + }
> + kfree(tas_fmw->configs);
> + }
> + kfree(tas_fmw);
> + tas_dev->fmw = NULL;
> + }
> +}
> +
> +static int tas2781_set_calibration(void *context, enum channel i,
> + int n_calibration)
> +{
> + struct tasdevice_priv *tas_dev = (struct tasdevice_priv *) context;
> + int ret = 0;
> + struct tasdevice *tasdevice = &(tas_dev->tasdevice[i]);
> + struct tasdevice_fw *cal_fmw = tasdevice->cali_data_fmw;
> +
> + if ((!tas_dev->fmw->programs)
too many parentheses
> + || (!tas_dev->fmw->configs)) {
> + dev_err(tas_dev->dev, "%s, Firmware not loaded\n\r", __func__);
> + ret = 0;
> + goto out;
> + }
> +
> + if (n_calibration == 0xFF || (n_calibration == 0x100
> + && tasdevice->is_calibrated_data_loaded == false)) {
> + if (cal_fmw) {
> + tasdevice->is_calibrated_data_loaded = false;
> + tas2781_clear_calfirmware(cal_fmw);
> + cal_fmw = NULL;
> + }
> +
> + scnprintf(tas_dev->cal_binaryname[i], 64, "%s_cal_0x%02x.bin",
> + tas_dev->dev_name, tas_dev->tasdevice[i].dev_addr);
> + ret = tas2781_load_calibration(tas_dev,
> + tas_dev->cal_binaryname[i], i);
> + if (ret != 0) {
> + dev_err(tas_dev->dev,
> + "%s: load %s error, no-side effect\n",
> + __func__, tas_dev->cal_binaryname[i]);
> + ret = 0;
> + }
> + }
> + tasdevice->is_loading = true;
> + tasdevice->is_loaderr = false;
> +
> + if (cal_fmw) {
> + struct tasdevice_calibration *calibration =
> + cal_fmw->calibrations;
> +
> + if (calibration)
> + tasdevice_load_calibrated_data(tas_dev,
> + &(calibration->dev_data));
> + } else
> + dev_err(tas_dev->dev,
> + "%s: No calibrated data for device %d\n", __func__, i);
> +
> +out:
> + return ret;
> +}
> +
> +int tasdevice_select_tuningprm_cfg(void *context, int prm_no,
> + int cfg_no, int rca_conf_no)
> +{
> + struct tasdevice_priv *tas_dev = (struct tasdevice_priv *) context;
> + struct tasdevice_rca *rca = &(tas_dev->rcabin);
> + struct tasdevice_config_info **cfg_info = rca->cfg_info;
> + struct tasdevice_fw *tas_fmw = tas_dev->fmw;
> + struct tasdevice_config *conf = NULL;
> + struct tasdevice_prog *program = NULL;
> + int i = 0;
> + int status = 0;
> + int prog_status = 0;
check inits please.
> +
> + if (tas_fmw == NULL) {
if (!tas_fmw)
> + dev_err(tas_dev->dev, "%s: Firmware is NULL\n", __func__);
> + goto out;
> + }
> +
> + if (cfg_no >= tas_fmw->nr_configurations) {
> + dev_err(tas_dev->dev,
> + "%s: cfg(%d) is not in range of conf %u\n",
> + __func__, cfg_no, tas_fmw->nr_configurations);
> + goto out;
> + }
> +
> + if (prm_no >= tas_fmw->nr_programs) {
> + dev_err(tas_dev->dev,
> + "%s: prm(%d) is not in range of Programs %u\n",
> + __func__, prm_no, tas_fmw->nr_programs);
> + goto out;
> + }
> +
> + if (rca_conf_no > rca->ncfgs || rca_conf_no <= 0 ||
> + cfg_info == NULL) {
> + dev_err(tas_dev->dev,
> + "conf_no:%d should be in range from 0 to %u\n",
> + rca_conf_no, rca->ncfgs-1);
> + goto out;
> + }
> +
> + conf = &(tas_fmw->configs[cfg_no]);
> + for (i = 0; i < tas_dev->ndev; i++) {
> + if (cfg_info[rca_conf_no]->active_dev & (1 << i)) {
> + if (tas_dev->tasdevice[i].cur_prog != prm_no) {
> + tas_dev->tasdevice[i].cur_conf = -1;
> + tas_dev->tasdevice[i].is_loading = true;
> + prog_status++;
> + }
> + } else
> + tas_dev->tasdevice[i].is_loading = false;
> + tas_dev->tasdevice[i].is_loaderr = false;
> + }
> +
> + if (prog_status) {
> + program = &(tas_fmw->programs[prm_no]);
> + tasdevice_load_data(tas_dev, &(program->dev_data));
> + for (i = 0; i < tas_dev->ndev; i++) {
> + if (tas_dev->tasdevice[i].is_loaderr == true)
> + continue;
> + else if (tas_dev->tasdevice[i].is_loaderr == false
> + && tas_dev->tasdevice[i].is_loading == true) {
> + struct tasdevice_fw *cal_fmw =
> + tas_dev->tasdevice[i].cali_data_fmw;
> +
> + if (cal_fmw) {
> + struct tasdevice_calibration
> + *calibration =
> + cal_fmw->calibrations;
> +
> + if (calibration)
> + tasdevice_load_calibrated_data(
> + tas_dev,
> + &(calibration->dev_data));
> + }
> + tas_dev->tasdevice[i].cur_prog = prm_no;
> + }
> + }
> + }
> +
> + if (tas_dev->is_glb_calibrated_data_loaded == false) {
> + for (i = 0; i < tas_dev->ndev; i++)
> + tas2781_set_calibration(tas_dev, i, 0x100);
> + tas_dev->is_glb_calibrated_data_loaded = true;
> + /* No wise to reload calibrationdata everytime,
> + * this code will work once even if calibrated
> + * data still failed to be got
> + */
> + }
> +
> + for (i = 0; i < tas_dev->ndev; i++) {
> + if (tas_dev->tasdevice[i].cur_conf != cfg_no
> + && (cfg_info[rca_conf_no]->active_dev & (1 << i))
> + && (tas_dev->tasdevice[i].is_loaderr == false)) {
> + status++;
> + tas_dev->tasdevice[i].is_loading = true;
> + } else
> + tas_dev->tasdevice[i].is_loading = false;
> + }
> +
> + if (status) {
> + status = 0;
> + tasdevice_load_data(tas_dev, &(conf->dev_data));
> + for (i = 0; i < tas_dev->ndev; i++) {
> + if (tas_dev->tasdevice[i].is_loaderr == true) {
> + status |= 1 << (i + 4);
> + continue;
> + } else if (tas_dev->tasdevice[i].is_loaderr == false
> + && tas_dev->tasdevice[i].is_loading == true)
> + tas_dev->tasdevice[i].cur_conf = cfg_no;
> + }
> + } else
> + dev_err(tas_dev->dev,
> + "%s: No device is in active in conf %d\n",
> + __func__, rca_conf_no);
> +
> + status |= cfg_info[rca_conf_no]->active_dev;
> +out:
> + return prog_status;
> diff --git a/sound/soc/codecs/tas2781-i2c.c b/sound/soc/codecs/tas2781-i2c.c
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/init.h>
> +#include <linux/pm.h>
> +#include <linux/i2c.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +#include <linux/firmware.h>
> +#include <linux/of_gpio.h>
> +#include <linux/interrupt.h>
> +#include <sound/soc.h>
> +#include <sound/pcm_params.h>
> +#include <sound/tlv.h>
> +#include <linux/crc8.h>
alphabetical order?
> +
> +int tasdevice_process_block(void *context,
> + unsigned char *data, unsigned char dev_idx, int sublocksize)
> +{
> + struct tasdevice_priv *tas_dev =
> + (struct tasdevice_priv *)context;
> + unsigned char subblk_typ = data[1];
> + int subblk_offset = 2;
> + int chn = 0, chnend = 0;
> + int rc = 0;
> + int blktyp = dev_idx & 0xC0, idx = dev_idx & 0x3F;
separate lines, this is awful to read.
> + bool is_err = false;
> +
> + if (idx) {
> + chn = idx-1;
> + chnend = idx;
> + } else {
> + if (tas_dev->set_global_mode) {
> + chn = tas_dev->ndev;
> + chnend = tas_dev->ndev + 1;
> + } else {
> + chn = 0;
> + chnend = tas_dev->ndev;
> + }
> + }
> +
> + for (; chn < chnend; chn++) {
> + if (tas_dev->set_global_mode == NULL &&
!tas_dev->set_global_mode
> + tas_dev->tasdevice[chn].is_loading == false)
> + continue;
> +
> + is_err = false;
> + subblk_offset = 2;
> + switch (subblk_typ) {
> + case TASDEVICE_CMD_SING_W: {
> + int i = 0;
> + unsigned short len = SMS_HTONS(data[2], data[3]);
> +
> + subblk_offset += 2;
> + if (subblk_offset + 4 * len > sublocksize) {
> + dev_err(tas_dev->dev,
> + "process_block: Out of bounary\n");
> + is_err = true;
> + break;
> + }
> +
> + for (i = 0; i < len; i++) {
> + rc = tasdevice_dev_write(tas_dev, chn,
> + TASDEVICE_REG(data[subblk_offset],
> + data[subblk_offset + 1],
> + data[subblk_offset + 2]),
> + data[subblk_offset + 3]);
> + if (rc < 0) {
> + is_err = true;
> + dev_err(tas_dev->dev,
> + "process_block: single write error\n");
> + }
> + subblk_offset += 4;
> + }
> + }
> + break;
> + case TASDEVICE_CMD_BURST: {
> + unsigned short len =
> + SMS_HTONS(data[2], data[3]);
> +
> + subblk_offset += 2;
> + if (subblk_offset + 4 + len > sublocksize) {
> + dev_err(tas_dev->dev,
> + "%s: BST Out of bounary\n", __func__);
> + is_err = true;
> + break;
> + }
> + if (len % 4) {
> + dev_err(tas_dev->dev,
> + "%s:Bst-len(%u)not div by 4\n",
> + __func__, len);
> + break;
> + }
> +
> + rc = tasdevice_dev_bulk_write(tas_dev, chn,
> + TASDEVICE_REG(data[subblk_offset],
> + data[subblk_offset + 1],
> + data[subblk_offset + 2]),
> + &(data[subblk_offset + 4]),
> + len);
> + if (rc < 0) {
> + is_err = true;
> + dev_err(tas_dev->dev,
> + "%s: bulk_write error = %d\n",
> + __func__, rc);
> + }
> + subblk_offset += (len + 4);
> + }
> + break;
> + case TASDEVICE_CMD_DELAY: {
> + unsigned short delay_time = 0;
> +
> + if (subblk_offset + 2 > sublocksize) {
> + dev_err(tas_dev->dev,
> + "%s: deley Out of bounary\n",
> + __func__);
> + is_err = true;
> + break;
> + }
> + delay_time = SMS_HTONS(data[2], data[3]);
> + usleep_range(delay_time*1000, delay_time*1000);
> + subblk_offset += 2;
> + }
> + break;
> + case TASDEVICE_CMD_FIELD_W:
> + if (subblk_offset + 6 > sublocksize) {
> + dev_err(tas_dev->dev,
> + "%s: bit write Out of bounary\n",
> + __func__);
> + is_err = true;
> + break;
> + }
> + rc = tasdevice_dev_update_bits(tas_dev, chn,
> + TASDEVICE_REG(data[subblk_offset + 2],
> + data[subblk_offset + 3],
> + data[subblk_offset + 4]),
> + data[subblk_offset + 1],
> + data[subblk_offset + 5]);
> + if (rc < 0) {
> + is_err = true;
> + dev_err(tas_dev->dev,
> + "%s: update_bits error = %d\n",
> + __func__, rc);
> + }
> + subblk_offset += 6;
> + break;
> + default:
> + break;
> + };
> + if (is_err == true && blktyp != 0) {
> + if (blktyp == 0x80) {
> + tas_dev->tasdevice[chn].cur_prog = -1;
> + tas_dev->tasdevice[chn].cur_conf = -1;
> + } else
> + tas_dev->tasdevice[chn].cur_conf = -1;
> + }
> + }
> + return subblk_offset;
> +}
> +
> +static void tasdevice_select_cfg_blk(void *pContext, int conf_no,
> + unsigned char block_type)
> +{
> + struct tasdevice_priv *tas_dev =
> + (struct tasdevice_priv *) pContext;
> + struct tasdevice_rca *rca = &(tas_dev->rcabin);
> + struct tasdevice_config_info **cfg_info = rca->cfg_info;
> + int j = 0, k = 0, chn = 0, chnend = 0;
inits?
> +
> + if (conf_no >= rca->ncfgs || conf_no < 0 || NULL == cfg_info) {
> + dev_err(tas_dev->dev,
> + "conf_no should be not more than %u\n",
> + rca->ncfgs);
> + goto out;
> + }
> +
> + for (j = 0; j < (int)cfg_info[conf_no]->real_nblocks; j++) {
> + unsigned int length = 0, rc = 0;
> +
> + if (block_type > 5 || block_type < 2) {
> + dev_err(tas_dev->dev,
> + "ERROR!!!block_type should be in range from 2 to 5\n");
> + goto out;
> + }
> + if (block_type != cfg_info[conf_no]->blk_data[j]->block_type)
> + continue;
> +
> + for (k = 0; k < (int)cfg_info[conf_no]->blk_data[j]
> + ->n_subblks; k++) {
> + if (cfg_info[conf_no]->blk_data[j]->dev_idx) {
> + chn =
> + cfg_info[conf_no]->blk_data[j]->dev_idx
> + - 1;
> + chnend =
> + cfg_info[conf_no]->blk_data[j]->dev_idx;
> + } else {
> + chn = 0;
> + chnend = tas_dev->ndev;
> + }
> + for (; chn < chnend; chn++)
> + tas_dev->tasdevice[chn].is_loading = true;
> +
> + rc = tasdevice_process_block(tas_dev,
> + cfg_info[conf_no]->blk_data[j]->regdata +
> + length,
> + cfg_info[conf_no]->blk_data[j]->dev_idx,
> + cfg_info[conf_no]->blk_data[j]->block_size -
> + length);
> + length += rc;
> + if (cfg_info[conf_no]->blk_data[j]->block_size <
> + length) {
> + dev_err(tas_dev->dev,
> + "%s: ERROR:%u %u out of bounary\n",
> + __func__, length,
> + cfg_info[conf_no]->blk_data[j]
> + ->block_size);
> + break;
> + }
> + }
> + if (length != cfg_info[conf_no]->blk_data[j]->block_size)
> + dev_err(tas_dev->dev,
> + "%s: %u %u size is not same\n",
> + __func__, length,
> + cfg_info[conf_no]->blk_data[j]->block_size);
> +
> + }
> +
> +out:
> + return;
> +}
> +
> +static struct tasdevice_config_info *tasdevice_add_config(
> + void *context, unsigned char *config_data,
> + unsigned int config_size)
> +{
> + struct tasdevice_priv *tas_dev =
> + (struct tasdevice_priv *)context;
> + struct tasdevice_config_info *cfg_info = NULL;
> + int config_offset = 0, i = 0;
inits?
> +
> + cfg_info = kzalloc(
> + sizeof(struct tasdevice_config_info), GFP_KERNEL);
> + if (!cfg_info)
> + goto out;
> +
> + if (tas_dev->rcabin.fw_hdr.binary_version_num >= 0x105) {
> + if (config_offset + 64 > (int)config_size) {
> + dev_err(tas_dev->dev,
> + "add config: Out of bounary\n");
> + goto out;
> + }
> + config_offset += 64;
> + }
> +
> + if (config_offset + 4 > (int)config_size) {
> + dev_err(tas_dev->dev,
> + "add config: Out of bounary\n");
> + goto out;
> + }
> + cfg_info->nblocks =
> + SMS_HTONL(config_data[config_offset],
> + config_data[config_offset + 1],
> + config_data[config_offset + 2], config_data[config_offset + 3]);
> + config_offset += 4;
> +
> + cfg_info->blk_data = kcalloc(
> + cfg_info->nblocks, sizeof(struct tasdev_blk_data *),
> + GFP_KERNEL);
> + if (!cfg_info->blk_data)
> + goto out;
> +
> + cfg_info->real_nblocks = 0;
> + for (i = 0; i < (int)cfg_info->nblocks; i++) {
> + if (config_offset + 12 > config_size) {
> + dev_err(tas_dev->dev,
> + "add config: Out of bounary: i = %d nblocks = %u!\n",
> + i, cfg_info->nblocks);
> + break;
> + }
> + cfg_info->blk_data[i] = kzalloc(
> + sizeof(struct tasdev_blk_data), GFP_KERNEL);
> + if (!cfg_info->blk_data[i])
> + break;
> +
> + cfg_info->blk_data[i]->dev_idx = config_data[config_offset];
> + config_offset++;
> +
> + cfg_info->blk_data[i]->block_type = config_data[config_offset];
> + config_offset++;
> +
> + if (cfg_info->blk_data[i]->block_type ==
> + TASDEVICE_BIN_BLK_PRE_POWER_UP) {
> + if (cfg_info->blk_data[i]->dev_idx == 0) {
> + cfg_info->active_dev = 1;
> + } else {
> + cfg_info->active_dev =
> + 1 <<
> + (cfg_info->blk_data[i]->dev_idx - 1);
> + }
> + }
> + cfg_info->blk_data[i]->yram_checksum =
> + SMS_HTONS(config_data[config_offset],
> + config_data[config_offset + 1]);
> + config_offset += 2;
> + cfg_info->blk_data[i]->block_size =
> + SMS_HTONL(config_data[config_offset],
> + config_data[config_offset + 1],
> + config_data[config_offset + 2],
> + config_data[config_offset + 3]);
> + config_offset += 4;
> +
> + cfg_info->blk_data[i]->n_subblks =
> + SMS_HTONL(config_data[config_offset],
> + config_data[config_offset + 1],
> + config_data[config_offset + 2],
> + config_data[config_offset + 3]);
> +
> + config_offset += 4;
> + cfg_info->blk_data[i]->regdata = kzalloc(
> + cfg_info->blk_data[i]->block_size, GFP_KERNEL);
> + if (cfg_info->blk_data[i]->regdata == 0)
> + goto out;
> +
> + if (config_offset + cfg_info->blk_data[i]->block_size
> + > config_size) {
> + dev_err(tas_dev->dev,
> + "%s: block_size Out of bounary: i = %d blks = %u!\n",
> + __func__, i, cfg_info->nblocks);
> + break;
> + }
> + memcpy(cfg_info->blk_data[i]->regdata,
> + &config_data[config_offset],
> + cfg_info->blk_data[i]->block_size);
> + config_offset += cfg_info->blk_data[i]->block_size;
> + cfg_info->real_nblocks += 1;
> + }
> +out:
> + return cfg_info;
> +}
> +
> +// tas2781 contain book and page two-level regmap, especially book switching
> +// will set two registers, one is BXXPXXR00, the other is B0P0R7F, after
> +// switching to the correct book, then leverage the mechanism for paging to
> +// access the register. Custom control is primarily for regmap booking,
> +// paging depends on internal regmap mechanism
C comments?
> +static int tas2781_digital_getvol(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_component *codec
> + = snd_soc_kcontrol_component(kcontrol);
> + struct tasdevice_priv *tas_dev =
> + snd_soc_component_get_drvdata(codec);
> + struct soc_mixer_control *mc =
> + (struct soc_mixer_control *)kcontrol->private_value;
> + int val;
> + unsigned int invert = mc->invert;
> + int max = mc->max;
> + int ret = 0;
> +
> + /* Read the primary device as the whole */
> + ret = tasdevice_dev_read(tas_dev, 0, mc->reg, &val);
> + if (ret) {
> + dev_err(tas_dev->dev,
> + "%s, get digital vol error\n",
> + __func__);
> + goto out;
> + }
> +
> + if (val > max)
> + val = max;
> + if (invert)
> + val = max - val;
> + if (val < 0)
> + val = 0;
> + ucontrol->value.integer.value[0] = val;
> +
> +out:
> + return ret;
> +}
> +
> +static int tas2781_digital_putvol(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_component *codec
> + = snd_soc_kcontrol_component(kcontrol);
> + struct tasdevice_priv *tas_dev =
> + snd_soc_component_get_drvdata(codec);
> + struct soc_mixer_control *mc =
> + (struct soc_mixer_control *)kcontrol->private_value;
> + int val;
> + unsigned int invert = mc->invert;
> + int max = mc->max;
> + int i, ret = 0;
> +
> + val = ucontrol->value.integer.value[0];
> + if (val > max)
> + val = max;
> + if (invert)
> + val = max - val;
> + if (val < 0)
> + val = 0;
> +
> + if (tas_dev->set_global_mode != NULL) {
> + ret = tasdevice_dev_write(tas_dev, tas_dev->ndev,
> + mc->reg, (unsigned int)val);
> + if (ret)
> + dev_err(tas_dev->dev,
> + "%s, set digital vol error in global mode\n",
> + __func__);
indentation is awful
> + } else {
> + for (i = 0; i < tas_dev->ndev; i++) {
> + ret = tasdevice_dev_write(tas_dev, i,
> + mc->reg, (unsigned int)val);
> + if (ret)
> + dev_err(tas_dev->dev,
> + "%s, set digital vol error in device %d\n",
> + __func__, i);
> + }
> + }
> +
> + return 1;
is this correct in case of errors?
> +}
> +
> +static int tas2781_amp_getvol(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_component *codec
> + = snd_soc_kcontrol_component(kcontrol);
> + struct tasdevice_priv *tas_dev =
> + snd_soc_component_get_drvdata(codec);
> + struct soc_mixer_control *mc =
> + (struct soc_mixer_control *)kcontrol->private_value;
> + int val;
> + unsigned char mask = 0;
> + int max = mc->max;
> + int ret = 0;
> + unsigned int invert = mc->invert;
> +
> + /* Read the primary device */
> + ret = tasdevice_dev_read(tas_dev, 0, mc->reg, &val);
> + if (ret) {
> + dev_err(tas_dev->dev,
> + "%s, get AMP vol error\n",
> + __func__);
> + goto out;
> + }
> +
> + mask = (1 << fls(mc->max)) - 1;
> + mask <<= mc->shift;
> + val = (val & mask) >> mc->shift;
> + if (val > max)
> + val = max;
> + if (invert)
> + val = max - val;
> + if (val < 0)
> + val = 0;
> + ucontrol->value.integer.value[0] = val;
> +
> +out:
> + return ret;
> +}
> +
> +static int tas2781_amp_putvol(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_component *codec
> + = snd_soc_kcontrol_component(kcontrol);
> + struct tasdevice_priv *tas_dev =
> + snd_soc_component_get_drvdata(codec);
> + struct soc_mixer_control *mc =
> + (struct soc_mixer_control *)kcontrol->private_value;
> + int val;
> + int i, ret = 0;
> + unsigned char mask = 0;
> + int max = mc->max;
> + unsigned int invert = mc->invert;
> +
> + mask = (1 << fls(mc->max)) - 1;
> + mask <<= mc->shift;
> + val = ucontrol->value.integer.value[0];
> + if (val > max)
> + val = max;
> + if (invert)
> + val = max - val;
> + if (val < 0)
> + val = 0;
> + for (i = 0; i < tas_dev->ndev; i++) {
> + ret = tasdevice_dev_update_bits(tas_dev, i,
> + mc->reg, mask, (unsigned int)(val << mc->shift));
> + if (ret)
> + dev_err(tas_dev->dev,
> + "%s, set AMP vol error in device %d\n",
> + __func__, i);
> + }
same is this correct to return 1 even for errors?
> +
> + return 1;
> +}
> +static void tasdevice_rca_ready(const struct firmware *fmw,
> + void *context)
> +{
> + struct tasdevice_priv *tas_dev =
> + (struct tasdevice_priv *) context;
> + struct tasdevice_rca *rca = NULL;
> + struct tasdevice_rca_hdr *fw_hdr = NULL;
> + struct tasdevice_config_info **cfg_info = NULL;
> + const struct firmware *fw_entry = NULL;
> + unsigned char *buf = NULL;
> + int offset = 0, i = 0;
> + unsigned int total_config_sz = 0;
> + int ret = 0;
check inits.
> +
> + mutex_lock(&tas_dev->codec_lock);
> + rca = &(tas_dev->rcabin);
> + fw_hdr = &(rca->fw_hdr);
> + if (!fmw || !fmw->data) {
> + dev_err(tas_dev->dev,
> + "Failed to read %s, no side - effect on driver running\n",
> + tas_dev->rca_binaryname);
> + ret = -1;
> + goto out;
> + }
> + buf = (unsigned char *)fmw->data;
> +
> + fw_hdr->img_sz = SMS_HTONL(buf[offset], buf[offset + 1],
> + buf[offset + 2], buf[offset + 3]);
> + offset += 4;
> + if (fw_hdr->img_sz != fmw->size) {
> + dev_err(tas_dev->dev,
> + "File size not match, %d %u", (int)fmw->size,
> + fw_hdr->img_sz);
> + ret = -1;
> + goto out;
> + }
> +
> + fw_hdr->checksum = SMS_HTONL(buf[offset], buf[offset + 1],
> + buf[offset + 2], buf[offset + 3]);
> + offset += 4;
> + fw_hdr->binary_version_num = SMS_HTONL(buf[offset],
> + buf[offset + 1], buf[offset + 2], buf[offset + 3]);
> + if (fw_hdr->binary_version_num < 0x103) {
> + dev_err(tas_dev->dev,
> + "File version 0x%04x is too low",
> + fw_hdr->binary_version_num);
> + ret = -1;
> + goto out;
> + }
> + offset += 4;
> + fw_hdr->drv_fw_version = SMS_HTONL(buf[offset], buf[offset + 1],
> + buf[offset + 2], buf[offset + 3]);
> + offset += 8;
> + fw_hdr->plat_type = buf[offset];
> + offset += 1;
> + fw_hdr->dev_family = buf[offset];
> + offset += 1;
> + fw_hdr->reserve = buf[offset];
> + offset += 1;
> + fw_hdr->ndev = buf[offset];
> + offset += 1;
> + if (fw_hdr->ndev != tas_dev->ndev) {
> + dev_err(tas_dev->dev,
> + "ndev(%u) from rcabin and ndev(%u) from DTS does not match\n",
> + fw_hdr->ndev,
> + tas_dev->ndev);
> + ret = -1;
> + goto out;
> + }
> + if (offset + TASDEVICE_DEVICE_SUM > fw_hdr->img_sz) {
> + dev_err(tas_dev->dev,
> + "rca_ready: Out of bounary!\n");
> + ret = -1;
> + goto out;
> + }
> +
> + for (i = 0; i < TASDEVICE_DEVICE_SUM; i++, offset++)
> + fw_hdr->devs[i] = buf[offset];
> +
> + fw_hdr->nconfig = SMS_HTONL(buf[offset], buf[offset + 1],
> + buf[offset + 2], buf[offset + 3]);
> + offset += 4;
> +
> + for (i = 0; i < TASDEVICE_CONFIG_SUM; i++) {
> + fw_hdr->config_size[i] = SMS_HTONL(buf[offset],
> + buf[offset + 1], buf[offset + 2], buf[offset + 3]);
> + offset += 4;
> + total_config_sz += fw_hdr->config_size[i];
> + }
> +
> + if (fw_hdr->img_sz - total_config_sz != (unsigned int)offset) {
> + dev_err(tas_dev->dev, "Bin file error!\n");
> + ret = -1;
> + goto out;
> + }
> + cfg_info = kcalloc(fw_hdr->nconfig,
> + sizeof(struct tasdevice_config_info *),
> + GFP_KERNEL);
> +
> + if (!cfg_info) {
> + ret = -1;
> + goto out;
> + }
> + rca->cfg_info = cfg_info;
> + rca->ncfgs = 0;
> + for (i = 0; i < (int)fw_hdr->nconfig; i++) {
> + cfg_info[i] = tasdevice_add_config(context, &buf[offset],
> + fw_hdr->config_size[i]);
> + if (!cfg_info[i]) {
> + ret = -1;
> + break;
> + }
> + offset += (int)fw_hdr->config_size[i];
> + rca->ncfgs += 1;
> + }
> + tasdevice_create_controls(tas_dev);
> +
> + tasdevice_dsp_remove(tas_dev);
> + tasdevice_calbin_remove(tas_dev);
> + tas_dev->fw_state = TASDEVICE_DSP_FW_PENDING;
> + scnprintf(tas_dev->coef_binaryname, 64, "%s_coef.bin",
> + tas_dev->dev_name);
> + ret = request_firmware(&fw_entry, tas_dev->coef_binaryname,
> + tas_dev->dev);
> + if (!ret) {
> + ret = tasdevice_dspfw_ready(fw_entry, tas_dev);
> + release_firmware(fw_entry);
> + fw_entry = NULL;
> + } else {
> + tas_dev->fw_state = TASDEVICE_DSP_FW_FAIL;
> + dev_err(tas_dev->dev, "%s: load %s error\n", __func__,
> + tas_dev->coef_binaryname);
> + goto out;
> + }
> + tasdevice_dsp_create_control(tas_dev);
> +
> + tas_dev->fw_state = TASDEVICE_DSP_FW_ALL_OK;
> + tas_dev->is_glb_calibrated_data_loaded = true;
> + for (i = 0; i < tas_dev->ndev; i++) {
> + scnprintf(tas_dev->cal_binaryname[i], 64, "%s_cal_0x%02x.bin",
> + tas_dev->dev_name, tas_dev->tasdevice[i].dev_addr);
> + ret = tas2781_load_calibration(tas_dev,
> + tas_dev->cal_binaryname[i], i);
> + if (ret != 0) {
> + dev_err(tas_dev->dev,
> + "%s: load %s error, no-side effect\n",
> + __func__,
> + tas_dev->cal_binaryname[i]);
> + ret = 0;
> + tas_dev->is_glb_calibrated_data_loaded = false;
> + }
> + }
> +
> +out:
> + mutex_unlock(&tas_dev->codec_lock);
> + if (fmw)
> + release_firmware(fmw);
> +}
> +
> +static void tasdevice_config_info_remove(void *context)
> +{
> + struct tasdevice_priv *tas_dev =
> + (struct tasdevice_priv *) context;
> + struct tasdevice_rca *rca = &(tas_dev->rcabin);
> + struct tasdevice_config_info **cfg_info = rca->cfg_info;
> + int i = 0, j = 0;
inits
> +
> + mutex_lock(&tas_dev->dev_lock);
> + if (cfg_info) {
> + for (i = 0; i < rca->ncfgs; i++) {
> + if (cfg_info[i]) {
> + for (j = 0; j < (int)cfg_info[i]->real_nblocks;
> + j++) {
> + kfree(
> + cfg_info[i]->blk_data[j]->regdata);
> + kfree(cfg_info[i]->blk_data[j]);
> + }
> + kfree(cfg_info[i]->blk_data);
> + kfree(cfg_info[i]);
> + }
> + }
> + kfree(cfg_info);
> + }
> + mutex_unlock(&tas_dev->dev_lock);
> +}
> +
> +static void tasdevice_tuning_switch(
> + struct tasdevice_priv *tas_dev, int state)
> +{
> + struct tasdevice_fw *tas_fmw = tas_dev->fmw;
> + int profile_cfg_id = 0;
> + int is_set_glb_mode = 0;
> +
> + if (state == 0) {
> + if (tas_fmw) {
> + if (tas_dev->cur_prog >= tas_fmw->nr_programs)
> + /*bypass all in rca is profile id 0*/
> + profile_cfg_id = RCA_CONFIGID_BYPASS_ALL;
> + else {
> + /*dsp mode or tuning mode*/
> + profile_cfg_id =
> + tas_dev->rcabin.profile_cfg_id;
> +
> + is_set_glb_mode =
> + tasdevice_select_tuningprm_cfg(tas_dev,
> + tas_dev->cur_prog,
> + tas_dev->cur_conf,
> + profile_cfg_id);
> + if (tas_dev->set_global_mode != NULL)
> + tas_dev->set_global_mode(tas_dev);
> + }
> + } else
> + profile_cfg_id = RCA_CONFIGID_BYPASS_ALL;
> +
> + tasdevice_select_cfg_blk(tas_dev, profile_cfg_id,
> + TASDEVICE_BIN_BLK_PRE_POWER_UP);
> + } else
> + tasdevice_select_cfg_blk(tas_dev,
> + tas_dev->rcabin.profile_cfg_id,
> + TASDEVICE_BIN_BLK_PRE_SHUTDOWN);
> +}
> +
> +static int tasdevice_dapm_event(struct snd_soc_dapm_widget *w,
> + struct snd_kcontrol *kcontrol, int event)
> +{
> + struct snd_soc_component *codec = snd_soc_dapm_to_component(w->dapm);
> + struct tasdevice_priv *tas_dev = snd_soc_component_get_drvdata(codec);
> + int state = 1;
> +
> + /* Codec Lock Hold */
> + mutex_lock(&tas_dev->codec_lock);
> + if (event == SND_SOC_DAPM_PRE_PMD)
> + state = 0;
state = (event == SND_SOC_DAPM_PRE_PMD) ? 0 : 1;
> + tasdevice_tuning_switch(tas_dev, state);
> + /* Codec Lock Release*/
> + mutex_unlock(&tas_dev->codec_lock);
> +
> + return 0;
> +}
> +
> +
> +static void tas2781_reset(struct tasdevice_priv *tas_dev)
> +{
> + int ret = 0;
> + int i = 0;
> +
> + for (; i < tas_dev->ndev; i++) {
> + ret = tasdevice_dev_write(tas_dev, i,
> + TAS2871_REG_SWRESET,
> + TAS2871_REG_SWRESET_RESET);
> + if (ret < 0) {
> + dev_err(tas_dev->dev, "%s: chn %d reset fail, %d\n",
> + __func__, i, ret);
> + continue;
> + }
> + }
> + usleep_range(1000, 1050);
> +}
> +static int tasdevice_parse_dt(struct tasdevice_priv *tas_dev)
> +{
> + struct device_node *np = tas_dev->dev->of_node;
> + int rc = 0, i = 0, ndev = 0;
inits?
> + unsigned int dev_addrs[max_chn];
> + struct i2c_client *client = (struct i2c_client *)tas_dev->client;
> +
> + ndev = of_property_read_variable_u32_array(np, "ti,audio-slots",
> + dev_addrs, 0, ARRAY_SIZE(dev_addrs));
> + if (ndev <= 0) {
> + ndev = 1;
> + dev_addrs[0] = client->addr;
> + }
> +
> + for (i = 0; i < ndev; i++)
> + tas_dev->tasdevice[i].dev_addr = dev_addrs[i];
> +
> + if (ndev > 1) {
> + rc = of_property_read_u32(np, "ti,broadcast-addr",
> + &(tas_dev->glb_addr.dev_addr));
> + if (rc) {
> + dev_err(tas_dev->dev,
> + "Looking up node %s failed %d\n",
> + np->full_name, rc);
> + tas_dev->glb_addr.dev_addr = 0;
> + }
> + }
> +
> + tas_dev->ndev = ndev;
> +
> + tas_dev->rst_gpio = of_get_named_gpio(np, "reset-gpios", i);
> + if (gpio_is_valid(tas_dev->rst_gpio)) {
> + rc = gpio_request(tas_dev->rst_gpio, "reset");
> + if (!rc)
> + gpio_direction_output(tas_dev->rst_gpio, 1);
> + }
> +
> + strcpy(tas_dev->dev_name, tasdevice_id[tas_dev->chip_id].name);
> +
> + tas_dev->irq_info.irq_gpio = of_get_named_gpio(np,
> + "interrupts", 0);
> + if (gpio_is_valid(tas_dev->irq_info.irq_gpio)) {
> + rc = gpio_request(tas_dev->irq_info.irq_gpio,
> + "AUDEV-IRQ");
> + if (!rc) {
> + gpio_direction_input(
> + tas_dev->irq_info.irq_gpio);
> +
> + tas_dev->irq_info.irq =
> + gpio_to_irq(tas_dev->irq_info.irq_gpio);
> + } else
> + dev_err(tas_dev->dev,
> + "%s: GPIO %d request error\n",
> + __func__,
> + tas_dev->irq_info.irq_gpio);
> + } else
> + dev_err(tas_dev->dev,
> + "Looking up irq-gpio property in node %s failed %d\n",
> + np->full_name,
> + tas_dev->irq_info.irq_gpio);
> +
> + return 0;
> +}
> +static void tas2781_set_global_mode(struct tasdevice_priv *tas_dev)
> +{
> + int i = 0;
> + int ret = 0;
> +
> + for (; i < tas_dev->ndev; i++) {
> + ret = tasdevice_dev_update_bits(tas_dev, i,
> + TAS2871_MISC_CFG2, TAS2871_GLOBAL_ADDR_MASK,
> + TAS2871_GLOBAL_ADDR_ENABLE);
> + if (ret < 0) {
> + dev_err(tas_dev->dev,
> + "%s: chn %d set global fail, %d\n",
> + __func__, i, ret);
> + continue;
> + }
> + }
> +}
> +
> +static int tasdevice_init(struct tasdevice_priv *tas_dev)
> +{
> + int ret = 0, i = 0;
inits?
> +
> + tas_dev->cur_prog = -1;
> + tas_dev->cur_conf = -1;
> +
> + for (i = 0; i < tas_dev->ndev; i++) {
> + tas_dev->tasdevice[i].cur_book = -1;
> + tas_dev->tasdevice[i].cur_prog = -1;
> + tas_dev->tasdevice[i].cur_conf = -1;
> + }
> + mutex_init(&tas_dev->dev_lock);
> + if (tas_dev->glb_addr.dev_addr != 0
> + && tas_dev->glb_addr.dev_addr < 0x7F)
> + tas_dev->set_global_mode = tas2781_set_global_mode;
> + dev_set_drvdata(tas_dev->dev, tas_dev);
> +
> + mutex_init(&tas_dev->codec_lock);
> + ret = devm_snd_soc_register_component(tas_dev->dev,
> + &soc_codec_driver_tasdevice,
> + tasdevice_dai_driver, ARRAY_SIZE(tasdevice_dai_driver));
> + if (ret) {
> + dev_err(tas_dev->dev, "%s: codec register error:0x%08x\n",
> + __func__, ret);
> + goto out;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +static void tasdevice_remove(struct tasdevice_priv *tas_dev)
> +{
> + snd_soc_unregister_component(tas_dev->dev);
no. if you use devm_snd_soc_register_component, then don't manually
remove this component
> + if (gpio_is_valid(tas_dev->rst_gpio))
> + gpio_free(tas_dev->rst_gpio);
> +
> + if (gpio_is_valid(tas_dev->irq_info.irq_gpio))
> + gpio_free(tas_dev->irq_info.irq_gpio);
> +
> + mutex_destroy(&tas_dev->dev_lock);
> + mutex_destroy(&tas_dev->codec_lock);
> +}
> +
> +static int tasdevice_i2c_probe(struct i2c_client *i2c)
> +{
> + struct tasdevice_priv *tas_dev = NULL;
> + int ret = 0;
> + const struct i2c_device_id *id = i2c_match_id(tasdevice_id, i2c);
> +
> + tas_dev = devm_kzalloc(&i2c->dev, sizeof(*tas_dev), GFP_KERNEL);
> + if (!tas_dev) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + tas_dev->dev = &i2c->dev;
> + tas_dev->client = (void *)i2c;
> + tas_dev->chip_id = id->driver_data;
> +
> + if (i2c->dev.of_node)
> + ret = tasdevice_parse_dt(tas_dev);
> + else {
> + dev_err(tas_dev->dev, "No DTS info\n");
> + goto out;
> + }
> +
> + tas_dev->regmap = devm_regmap_init_i2c(i2c,
> + &tasdevice_regmap);
> + if (IS_ERR(tas_dev->regmap)) {
> + ret = PTR_ERR(tas_dev->regmap);
> + dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
> + ret);
> + goto out;
> + }
> + ret = tasdevice_init(tas_dev);
> +
> +out:
> + if (ret < 0 && tas_dev != NULL)
> + tasdevice_remove(tas_dev);
if you use devm then there's nothing to remove?
> + return ret;
> +
> +}
> +
> +static void tasdevice_i2c_remove(struct i2c_client *client)
> +{
> + struct tasdevice_priv *tas_dev = i2c_get_clientdata(client);
> +
> + if (tas_dev)
can this not be always true?
> + tasdevice_remove(tas_dev);
same, is this needed?
> +#define SMARTAMP_MODULE_NAME "tas2781"
> +#define TASDEVICE_RETRY_COUNT 3
> +#define TASDEVICE_ERROR_FAILED -2
can't you use an existing error code?
> +
> +#define TASDEVICE_RATES (SNDRV_PCM_RATE_44100 |\
> + SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_96000 |\
> + SNDRV_PCM_RATE_88200)
> +#define TASDEVICE_MAX_CHANNELS 8
> +
> +#define TASDEVICE_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
> + SNDRV_PCM_FMTBIT_S24_LE | \
> + SNDRV_PCM_FMTBIT_S32_LE)
> +
> +/*PAGE Control Register (available in page0 of each book) */
> +#define TASDEVICE_PAGE_SELECT 0x00
> +#define TASDEVICE_BOOKCTL_PAGE 0x00
> +#define TASDEVICE_BOOKCTL_REG 127
> +#define TASDEVICE_BOOK_ID(reg) (reg / (256 * 128))
> +#define TASDEVICE_PAGE_ID(reg) ((reg % (256 * 128)) / 128)
> +#define TASDEVICE_PAGE_REG(reg) ((reg % (256 * 128)) % 128)
> +#define TASDEVICE_PGRG(reg) ((reg % (256 * 128)))
> +#define TASDEVICE_REG(book, page, reg) (((book * 256 * 128) + \
> + (page * 128)) + reg)
> +
> + /*Software Reset */
intentation and spaces in all this file?
> +#define TAS2871_REG_SWRESET TASDEVICE_REG(0x0, 0X0, 0x02)
> +#define TAS2871_REG_SWRESET_RESET BIT(0)
> +
> + /* Enable Global addresses */
> +#define TAS2871_MISC_CFG2 TASDEVICE_REG(0x0, 0X0, 0x07)
> +#define TAS2871_GLOBAL_ADDR_MASK BIT(1)
> +#define TAS2871_GLOBAL_ADDR_ENABLE BIT(1)
> +
> + /*I2C Checksum */
> +#define TASDEVICE_I2CChecksum TASDEVICE_REG(0x0, 0x0, 0x7E)
> +
> + /* Volume control */
> +#define TAS2781_DVC_LVL TASDEVICE_REG(0x0, 0x0, 0x1A)
> +#define TAS2781_AMP_LEVEL TASDEVICE_REG(0x0, 0x0, 0x03)
> +#define TAS2781_AMP_LEVEL_MASK GENMASK(5, 1)
> +
> +#define TASDEVICE_CMD_SING_W 0x1
> +#define TASDEVICE_CMD_BURST 0x2
> +#define TASDEVICE_CMD_DELAY 0x3
> +#define TASDEVICE_CMD_FIELD_W 0x4
> +struct tasdevice_priv {
> + struct device *dev;
> + void *client;
> + struct regmap *regmap;
> + struct mutex codec_lock;
> + struct mutex dev_lock;
need comments on what they protect. checkpatch.pl would tell you that.
> + int rst_gpio;
> + struct tasdevice tasdevice[max_chn];
> + struct tasdevice_fw *fmw;
> + struct tasdevice_rca rcabin;
> + struct tasdevice_irqinfo irq_info;
> + struct tas_control tas_ctrl;
> + struct global_addr glb_addr;
> + int cur_prog;
> + int cur_conf;
> + unsigned int chip_id;
> + void (*set_global_mode)(struct tasdevice_priv *tas_dev);
> + int (*fw_parse_variable_header)(struct tasdevice_priv *tas_dev,
> + const struct firmware *fmw, int offset);
> + int (*fw_parse_program_data)(struct tasdevice_priv *tas_dev,
> + struct tasdevice_fw *tas_fmw,
> + const struct firmware *fmw, int offset);
> + int (*fw_parse_configuration_data)(struct tasdevice_priv *tas_dev,
> + struct tasdevice_fw *tas_fmw,
> + const struct firmware *fmw, int offset);
> + int (*tasdevice_load_block)(struct tasdevice_priv *tas_dev,
> + struct tasdev_blk *pBlock);
> + int (*fw_parse_calibration_data)(struct tasdevice_priv *tas_dev,
> + struct tasdevice_fw *tas_fmw,
> + const struct firmware *fmw, int offset);
> + int fw_state;
> + unsigned int magic_num;
> + unsigned char ndev;
> + unsigned char dev_name[32];
> + unsigned char rca_binaryname[64];
> + unsigned char coef_binaryname[64];
> + unsigned char cal_binaryname[max_chn][64];
> + unsigned char crc8_lkp_tbl[CRC8_TABLE_SIZE];
> + void *codec;
> + unsigned int sysclk;
> + bool is_glb_calibrated_data_loaded;
maybe reorganize, this looks like a dumping ground of lots of different
things.
next prev parent reply other threads:[~2023-03-20 15:56 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-20 15:07 [PATCH v4] ASoC: tas2781: Add tas2781 driver Shenghao Ding
2023-03-20 15:47 ` Pierre-Louis Bossart [this message]
2023-03-20 16:50 ` Mark Brown
2023-03-20 16:50 ` Mark Brown
2023-03-20 19:42 ` kernel test robot
2023-03-20 20:34 ` kernel test robot
2023-03-20 20:55 ` kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2023-03-12 10:48 Shenghao Ding
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aea0c730-898d-7326-d245-79bc124bca0d@linux.intel.com \
--to=pierre-louis.bossart@linux.intel.com \
--cc=13916275206@139.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=kevin-lu@ti.com \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=navada@ti.com \
--cc=peeyush@ti.com \
--cc=perex@perex.cz \
--cc=shenghao-ding@ti.com \
--cc=x1077012@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.