lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 22 Dec 2022 09:50:07 -0600
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     wangweidong.a@...nic.com, lgirdwood@...il.com, broonie@...nel.org,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        perex@...ex.cz, tiwai@...e.com, ckeepax@...nsource.cirrus.com,
        rf@...nsource.cirrus.com, peter.ujfalusi@...ux.intel.com,
        james.schulman@...rus.com, flatmax@...tmax.com,
        ryan.lee.analog@...il.com, jonathan.albrieux@...il.com,
        tanureal@...nsource.cirrus.com, povik+lin@...ebit.org,
        13691752556@....com, cezary.rojewski@...el.com,
        stephan@...hold.net, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     liweilei@...nic.com, zhaolei@...nic.com, yijiangtao@...nic.com,
        zhangjianming@...nic.com, duanyibo@...nic.com
Subject: Re: [PATCH V7 3/5] ASoC: codecs: Aw883xx common function for ALSA
 Audio Driver


> +static int aw_dev_dsp_write_32bit(struct aw_device *aw_dev,
> +		unsigned short dsp_addr, unsigned int dsp_data)
> +{
> +	int ret;
> +	u16 temp_data = 0;

useless init

> +
> +	ret = regmap_write(aw_dev->regmap, AW_PID_2049_DSPMADD_REG, dsp_addr);
> +	if (ret < 0) {
> +		dev_err(aw_dev->dev, "%s write addr error, ret=%d", __func__, ret);
> +		return ret;
> +	}
> +
> +	temp_data = dsp_data & AW_DSP_16_DATA_MASK;
> +	ret = regmap_write(aw_dev->regmap, AW_PID_2049_DSPMDAT_REG, (u16)temp_data);
> +	if (ret < 0) {
> +		dev_err(aw_dev->dev, "%s write datal error, ret=%d", __func__, ret);
> +		return ret;
> +	}
> +
> +	temp_data = dsp_data >> 16;
> +	ret = regmap_write(aw_dev->regmap, AW_PID_2049_DSPMDAT_REG, (u16)temp_data);
> +	if (ret < 0) {
> +		dev_err(aw_dev->dev, "%s write datah error, ret=%d", __func__, ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int aw_dev_dsp_write(struct aw_device *aw_dev,
> +		unsigned short dsp_addr, unsigned int dsp_data, unsigned char data_type)
> +{
> +	int ret = 0;
> +	u32 reg_value;
> +
> +	mutex_lock(&aw_dev->dsp_lock);
> +	switch (data_type) {
> +	case AW_DSP_16_DATA:
> +		ret = aw_dev_dsp_write_16bit(aw_dev, dsp_addr, dsp_data);
> +		if (ret < 0)
> +			dev_err(aw_dev->dev, "write dsp_addr[0x%x] 16-bit dsp_data[0x%x] failed",
> +					(u32)dsp_addr, dsp_data);
> +		break;
> +	case AW_DSP_32_DATA:
> +		ret = aw_dev_dsp_write_32bit(aw_dev, dsp_addr, dsp_data);
> +		if (ret < 0)
> +			dev_err(aw_dev->dev, "write dsp_addr[0x%x] 32-bit dsp_data[0x%x] failed",
> +					(u32)dsp_addr, dsp_data);
> +		break;
> +	default:
> +		dev_err(aw_dev->dev, "data type[%d] unsupported", data_type);
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	/* clear dsp chip select state*/
> +	regmap_read(aw_dev->regmap, AW_PID_2049_ID_REG, &reg_value);
> +	mutex_unlock(&aw_dev->dsp_lock);
> +	return ret;
> +}
> +
> +static int aw_dev_dsp_read_16bit(struct aw_device *aw_dev,
> +		unsigned short dsp_addr, unsigned int *dsp_data)
> +{
> +	int ret;
> +	unsigned int temp_data = 0;
> +
> +	ret = regmap_write(aw_dev->regmap, AW_PID_2049_DSPMADD_REG, dsp_addr);
> +	if (ret < 0) {
> +		dev_err(aw_dev->dev, "%s write error, ret=%d", __func__, ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_read(aw_dev->regmap, AW_PID_2049_DSPMDAT_REG, &temp_data);
> +	if (ret < 0) {
> +		dev_err(aw_dev->dev, "%s read error, ret=%d", __func__, ret);
> +		return ret;
> +	}
> +
> +	*dsp_data = temp_data;
> +
> +	return 0;
> +}
> +
> +static int aw_dev_dsp_read_32bit(struct aw_device *aw_dev,
> +		unsigned short dsp_addr, unsigned int *dsp_data)
> +{
> +	int ret;
> +	unsigned int temp_data = 0;
> +
> +	ret = regmap_write(aw_dev->regmap, AW_PID_2049_DSPMADD_REG, dsp_addr);
> +	if (ret < 0) {
> +		dev_err(aw_dev->dev, "%s write error, ret=%d", __func__, ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_read(aw_dev->regmap, AW_PID_2049_DSPMDAT_REG, &temp_data);
> +	if (ret < 0) {
> +		dev_err(aw_dev->dev, "%s read error, ret=%d", __func__, ret);
> +		return ret;
> +	}
> +
> +	*dsp_data = temp_data;
> +
> +	ret = regmap_read(aw_dev->regmap, AW_PID_2049_DSPMDAT_REG, &temp_data);
> +	if (ret < 0) {
> +		dev_err(aw_dev->dev, "%s read error, ret=%d", __func__, ret);
> +		return ret;
> +	}
> +
> +	*dsp_data |= (temp_data << 16);
> +
> +	return 0;
> +}
> +
> +static int aw_dev_dsp_read(struct aw_device *aw_dev,
> +		unsigned short dsp_addr, unsigned int *dsp_data, unsigned char data_type)
> +{
> +	int ret = 0;
> +	u32 reg_value;
> +
> +	mutex_lock(&aw_dev->dsp_lock);
> +	switch (data_type) {
> +	case AW_DSP_16_DATA:
> +		ret = aw_dev_dsp_read_16bit(aw_dev, dsp_addr, dsp_data);
> +		if (ret < 0)
> +			dev_err(aw_dev->dev, "read dsp_addr[0x%x] 16-bit dsp_data[0x%x] failed",
> +					(u32)dsp_addr, *dsp_data);
> +		break;
> +	case AW_DSP_32_DATA:
> +		ret = aw_dev_dsp_read_32bit(aw_dev, dsp_addr, dsp_data);
> +		if (ret < 0)
> +			dev_err(aw_dev->dev, "read dsp_addr[0x%x] 32r-bit dsp_data[0x%x] failed",
> +					(u32)dsp_addr, *dsp_data);
> +		break;
> +	default:
> +		dev_err(aw_dev->dev, "data type[%d] unsupported", data_type);
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	/* clear dsp chip select state*/
> +	regmap_read(aw_dev->regmap, AW_PID_2049_ID_REG, &reg_value);
> +	mutex_unlock(&aw_dev->dsp_lock);
> +	return ret;
> +}
> +
> +
> +static int aw_dev_read_chipid(struct aw_device *aw_dev, u16 *chip_id)
> +{
> +	int ret = 0;

useless init

> +	int reg_val = 0;
> +
> +	ret = regmap_read(aw_dev->regmap, AW_CHIP_ID_REG, &reg_val);
> +	if (ret < 0)
> +		return -EIO;
> +
> +	dev_info(aw_dev->dev, "chip id = %x\n", reg_val);
> +	*chip_id = reg_val;
> +
> +	return 0;
> +}
> +
> +static void aw_dev_set_cfg_f0_fs(struct aw_device *aw_dev, unsigned int *f0_fs)
> +{
> +	unsigned int rate_data = 0;
> +	u32 fs = 0;
> +
> +	regmap_read(aw_dev->regmap, AW_PID_2049_I2SCTRL_REG, &rate_data);

usually you test the regmap results and here you don't, is it intentional?

> +
> +	switch (rate_data & (~AW_PID_2049_I2SSR_MASK)) {
> +	case AW_PID_2049_I2SSR_8_KHZ_VALUE:
> +		fs = 8000;
> +		break;
> +	case AW_PID_2049_I2SSR_16_KHZ_VALUE:
> +		fs = 16000;
> +		break;
> +	case AW_PID_2049_I2SSR_32_KHZ_VALUE:
> +		fs = 32000;
> +		break;
> +	case AW_PID_2049_I2SSR_44_KHZ_VALUE:
> +		fs = 44000;

did you mean 44100?

> +		break;
> +	case AW_PID_2049_I2SSR_48_KHZ_VALUE:
> +		fs = 48000;
> +		break;
> +	case AW_PID_2049_I2SSR_96_KHZ_VALUE:
> +		fs = 96000;
> +		break;
> +	case AW_PID_2049_I2SSR_192KHZ_VALUE:
> +		fs = 192000;
> +		break;
> +	default:
> +		fs = 48000;
> +		dev_err(aw_dev->dev,
> +			"rate can not support, use default 48kHz");
> +		break;
> +	}
> +
> +	dev_dbg(aw_dev->dev, "i2s fs:%d Hz", fs);
> +	*f0_fs = fs / 8;
> +
> +	aw_dev_dsp_write(aw_dev, AW_PID_2049_DSP_REG_CFGF0_FS, *f0_fs, AW_DSP_32_DATA);
> +}
> +
> +/*[9 : 6]: -6DB ; [5 : 0]: -0.125DB  real_value = value * 8 : 0.125db --> 1*/
> +static unsigned int reg_val_to_db(unsigned int value)
> +{
> +	return (((value >> AW_PID_2049_VOL_6DB_START) * AW_PID_2049_VOLUME_STEP_DB) +
> +			((value & 0x3f) % AW_PID_2049_VOLUME_STEP_DB));
> +}
> +
> +/*[9 : 6]: -6DB ; [5 : 0]: -0.125DB reg_value = value / step << 6 + value % step ; step = 6 * 8*/
> +static unsigned short db_to_reg_val(unsigned short value)
> +{
> +	return (((value / AW_PID_2049_VOLUME_STEP_DB) << AW_PID_2049_VOL_6DB_START) +
> +			(value % AW_PID_2049_VOLUME_STEP_DB));
> +}
> +
> +static int aw_dev_dsp_fw_check(struct aw_device *aw_dev)
> +{
> +	struct aw_prof_desc *set_prof_desc = NULL;
> +	struct aw_sec_data_desc *dsp_fw_desc = NULL;
> +	u16 base_addr = AW_PID_2049_DSP_FW_ADDR;
> +	u16 addr = base_addr;
> +	int ret, i;
> +	u32 dsp_val;
> +	u16 bin_val;
> +
> +	ret = aw883xx_dev_get_prof_data(aw_dev, aw_dev->prof_cur, &set_prof_desc);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*update reg*/
> +	dsp_fw_desc = &set_prof_desc->sec_desc[AW_DATA_TYPE_DSP_FW];
> +
> +	for (i = 0; i < AW_FW_CHECK_PART; i++) {
> +		ret = aw_dev_dsp_read(aw_dev, addr, &dsp_val, AW_DSP_16_DATA);
> +		if (ret  < 0) {
> +			dev_err(aw_dev->dev, "dsp read failed");
> +			return ret;
> +		}
> +
> +		bin_val = be16_to_cpup((void *)&dsp_fw_desc->data[2 * (addr - base_addr)]);
> +
> +		if (dsp_val != bin_val) {
> +			dev_err(aw_dev->dev, "fw check failed, addr[0x%x], read[0x%x] != bindata[0x%x]",
> +					addr, dsp_val, bin_val);
> +			return -EINVAL;
> +		}
> +
> +		addr += (dsp_fw_desc->len / 2) / AW_FW_CHECK_PART;
> +		if ((addr - base_addr) > dsp_fw_desc->len) {
> +			dev_err(aw_dev->dev, "fw check failed, addr[0x%x] too large", addr);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int aw_dev_set_volume(struct aw_device *aw_dev, unsigned int value)
> +{
> +	struct aw_volume_desc *vol_desc = &aw_dev->volume_desc;
> +	unsigned int reg_value = 0;
> +	u16 real_value = 0;
> +	u16 volume = 0;
> +
> +	volume = min((value + vol_desc->init_volume), (unsigned int)AW_PID_2049_MUTE_VOL);
> +	real_value = db_to_reg_val(volume);
> +
> +	/* cal real value */
> +	regmap_read(aw_dev->regmap, AW_PID_2049_SYSCTRL2_REG, &reg_value);
> +
> +	dev_dbg(aw_dev->dev, "value 0x%x , reg:0x%x", value, real_value);
> +
> +	/*[15 : 6] volume*/
> +	real_value = (real_value << AW_PID_2049_VOL_START_BIT) | (reg_value & AW_PID_2049_VOL_MASK);
> +
> +	/* write value */
> +	regmap_write(aw_dev->regmap, AW_PID_2049_SYSCTRL2_REG, real_value);
> +
> +	return 0;
> +}
> +
> +void aw883xx_dev_set_volume(struct aw_device *aw_dev, unsigned short set_vol)
> +{
> +	int ret = 0;
> +
> +	ret = aw_dev_set_volume(aw_dev, set_vol);
> +	if (ret < 0)
> +		dev_dbg(aw_dev->dev, "set volume failed");
> +}
> +EXPORT_SYMBOL_GPL(aw883xx_dev_set_volume);
> +
> +static void aw_dev_fade_in(struct aw_device *aw_dev)
> +{
> +	int i = 0;

useless init

> +	struct aw_volume_desc *desc = &aw_dev->volume_desc;
> +	int fade_step = aw_dev->fade_step;
> +	u16 fade_in_vol = desc->ctl_volume;
> +
> +	if (!aw_dev->fade_en)
> +		return;
> +
> +	if (fade_step == 0 || aw_dev->fade_in_time == 0) {
> +		aw_dev_set_volume(aw_dev, fade_in_vol);
> +		return;
> +	}
> +	/*volume up*/
> +	for (i = AW_PID_2049_MUTE_VOL; i >= fade_in_vol; i -= fade_step) {
> +		aw_dev_set_volume(aw_dev, i);
> +		usleep_range(aw_dev->fade_in_time, aw_dev->fade_in_time + 10);
> +	}
> +	if (i != fade_in_vol)
> +		aw_dev_set_volume(aw_dev, fade_in_vol);
> +}
> +
> +static void aw_dev_fade_out(struct aw_device *aw_dev)
> +{
> +	int i = 0;

useless init

> +	struct aw_volume_desc *desc = &aw_dev->volume_desc;
> +	int fade_step = aw_dev->fade_step;
> +
> +	if (!aw_dev->fade_en)
> +		return;
> +
> +	if (fade_step == 0 || aw_dev->fade_out_time == 0) {
> +		aw_dev_set_volume(aw_dev, AW_PID_2049_MUTE_VOL);
> +		return;
> +	}
> +
> +	for (i = desc->ctl_volume; i <= AW_PID_2049_MUTE_VOL; i += fade_step) {
> +		aw_dev_set_volume(aw_dev, i);
> +		usleep_range(aw_dev->fade_out_time, aw_dev->fade_out_time + 10);
> +	}
> +	if (i != AW_PID_2049_MUTE_VOL) {
> +		aw_dev_set_volume(aw_dev, AW_PID_2049_MUTE_VOL);
> +		usleep_range(aw_dev->fade_out_time, aw_dev->fade_out_time + 10);
> +	}
> +}
> +
> +static int aw_dev_modify_dsp_cfg(struct aw_device *aw_dev,
> +			unsigned int addr, unsigned int dsp_data, unsigned char data_type)
> +{
> +	u32 addr_offset = 0;

useless init

> +	u16 data1 = 0;
> +	u32 data2 = 0;

useless init and could be moved to lower scopes

> +	struct aw_sec_data_desc *crc_dsp_cfg = &aw_dev->crc_dsp_cfg;
> +
> +	dev_dbg(aw_dev->dev, "addr:0x%x, dsp_data:0x%x", addr, dsp_data);
> +
> +	addr_offset = (addr - AW_PID_2049_DSP_CFG_ADDR) * 2;
> +	if (addr_offset > crc_dsp_cfg->len) {
> +		dev_err(aw_dev->dev, "addr_offset[%d] > crc_dsp_cfg->len[%d]",
> +				addr_offset, crc_dsp_cfg->len);
> +		return -EINVAL;
> +	}
> +	switch (data_type) {
> +	case AW_DSP_16_DATA:
> +		data1 = cpu_to_le16((u16)dsp_data);
> +		memcpy(crc_dsp_cfg->data + addr_offset, (u8 *)&data1, 2);
> +		break;
> +	case AW_DSP_32_DATA:
> +		data2 = cpu_to_le32(dsp_data);
> +		memcpy(crc_dsp_cfg->data + addr_offset, (u8 *)&data2, 4);
> +		break;
> +	default:
> +		dev_err(aw_dev->dev, "data type[%d] unsupported", data_type);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int aw_dev_dsp_set_cali_re(struct aw_device *aw_dev)
> +{
> +	u32 cali_re = 0;
> +	int ret = 0;

useless inits

> +
> +	cali_re = AW_SHOW_RE_TO_DSP_RE((aw_dev->cali_desc.cali_re +
> +		aw_dev->cali_desc.ra), AW_PID_2049_DSP_RE_SHIFT);
> +
> +	/* set cali re to device */
> +	ret = aw_dev_dsp_write(aw_dev,
> +			AW_PID_2049_DSP_REG_CFG_ADPZ_RE, cali_re, AW_DSP_32_DATA);
> +	if (ret < 0) {
> +		dev_err(aw_dev->dev, "set cali re error");
> +		return ret;
> +	}
> +
> +	ret = aw_dev_modify_dsp_cfg(aw_dev, AW_PID_2049_DSP_REG_CFG_ADPZ_RE,
> +				cali_re, AW_DSP_32_DATA);
> +	if (ret < 0)
> +		dev_err(aw_dev->dev, "modify dsp cfg failed");
> +
> +	return ret;
> +
> +}

this file needs style fixes, stopping the review here.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ