[PATCH V6 3/5] ASoC: codecs: aw883xx chip control logic, such as power on and off
Amadeusz Sławiński
amadeuszx.slawinski at linux.intel.com
Thu Dec 8 15:22:10 CET 2022
On 12/8/2022 1:23 PM, wangweidong.a at awinic.com wrote:
> From: Weidong Wang <wangweidong.a at awinic.com>
>
> The Awinic AW883XX is an I2S/TDM input, high efficiency
> digital Smart K audio amplifier with an integrated 10.25V
> smart boost convert
>
> Signed-off-by: Nick Li <liweilei at awinic.com>
> Signed-off-by: Bruce zhao <zhaolei at awinic.com>
> Signed-off-by: Weidong Wang <wangweidong.a at awinic.com>
> ---
> sound/soc/codecs/aw883xx/aw883xx_device.c | 1613 +++++++++++++++++++++
> sound/soc/codecs/aw883xx/aw883xx_device.h | 537 +++++++
> 2 files changed, 2150 insertions(+)
> create mode 100644 sound/soc/codecs/aw883xx/aw883xx_device.c
> create mode 100644 sound/soc/codecs/aw883xx/aw883xx_device.h
>
> diff --git a/sound/soc/codecs/aw883xx/aw883xx_device.c b/sound/soc/codecs/aw883xx/aw883xx_device.c
> new file mode 100644
> index 000000000000..f4419e1a2fed
> --- /dev/null
> +++ b/sound/soc/codecs/aw883xx/aw883xx_device.c
> @@ -0,0 +1,1613 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * aw883xx.c -- ALSA Soc AW883XX codec support
Soc -> SoC
> + *
> + * Copyright (c) 2022 AWINIC Technology CO., LTD
> + *
> + * Author: Bruce zhao <zhaolei at awinic.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/debugfs.h>
> +#include <linux/firmware.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/syscalls.h>
> +#include <linux/version.h>
> +#include <linux/uaccess.h>
> +#include <linux/workqueue.h>
Again, there seem to be unnecessary headers included.
> +#include <sound/core.h>
> +#include <sound/control.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +#include "aw883xx_data_type.h"
> +#include "aw883xx_device.h"
> +#include "aw883xx_bin_parse.h"
> +
> +int aw883xx_dev_set_volume(struct aw_device *aw_dev, unsigned short set_vol)
> +{
> + u16 hw_vol = 0;
> + int ret = -1;
As mentioned in previous patchset this mey lead to returning "-1"
somewhere which maps to -EPERM, just set it to -EINVAL if you want to
set it to something... same for the rest of the patchset (won't be
commenting each occurence in file.)
> + struct aw_volume_desc *vol_desc = &aw_dev->volume_desc;
> +
> + hw_vol = set_vol + vol_desc->init_volume;
> +
> + ret = aw_dev->ops.aw_set_hw_volume(aw_dev, hw_vol);
> + if (ret < 0) {
> + dev_err(aw_dev->dev, "set volume failed");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +int aw883xx_dev_get_volume(struct aw_device *aw_dev, unsigned short *get_vol)
> +{
> + int ret = -1;
> + u16 hw_vol = 0;
> + struct aw_volume_desc *vol_desc = &aw_dev->volume_desc;
> +
> + ret = aw_dev->ops.aw_get_hw_volume(aw_dev, &hw_vol);
> + if (ret < 0) {
> + dev_err(aw_dev->dev, "read volume failed");
> + return ret;
> + }
> +
> + if (hw_vol > vol_desc->init_volume)
> + *get_vol = hw_vol - vol_desc->init_volume;
> +
> + return 0;
> +}
> +
> +static void aw_dev_fade_in(struct aw_device *aw_dev)
> +{
> + int i = 0;
> + 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) {
> + aw883xx_dev_set_volume(aw_dev, fade_in_vol);
> + return;
> + }
> + /*volume up*/
> + for (i = desc->mute_volume; i >= fade_in_vol; i -= fade_step) {
> + aw883xx_dev_set_volume(aw_dev, i);
> + usleep_range(aw_dev->fade_in_time, aw_dev->fade_in_time + 10);
> + }
> + if (i != fade_in_vol)
> + aw883xx_dev_set_volume(aw_dev, fade_in_vol);
> +
> +}
> +
> +static void aw_dev_fade_out(struct aw_device *aw_dev)
> +{
> + int i = 0;
> + 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) {
> + aw883xx_dev_set_volume(aw_dev, desc->mute_volume);
> + return;
> + }
> +
> + for (i = desc->ctl_volume; i <= desc->mute_volume; i += fade_step) {
> + aw883xx_dev_set_volume(aw_dev, i);
> + usleep_range(aw_dev->fade_out_time, aw_dev->fade_out_time + 10);
> + }
> + if (i != desc->mute_volume) {
> + aw883xx_dev_set_volume(aw_dev, desc->mute_volume);
> + usleep_range(aw_dev->fade_out_time, aw_dev->fade_out_time + 10);
> + }
> +}
> +
> +static uint64_t aw_dev_dsp_crc32_reflect(uint64_t ref, unsigned char ch)
> +{
> + int i;
> + uint64_t value = 0;
> +
> + for (i = 1; i < (ch + 1); i++) {
> + if (ref & 1)
> + value |= 1 << (ch - i);
> +
> + ref >>= 1;
> + }
> +
> + return value;
> +}
> +
> +static unsigned int aw_dev_calc_dsp_cfg_crc32(unsigned char *buf, unsigned int len)
> +{
> + u8 i;
> + u32 crc = 0xffffffff;
> +
> + while (len--) {
> + for (i = 1; i != 0; i <<= 1) {
> + if ((crc & 0x80000000) != 0) {
> + crc <<= 1;
> + crc ^= 0x1EDC6F41;
> + } else {
> + crc <<= 1;
> + }
> +
> + if ((*buf & i) != 0)
> + crc ^= 0x1EDC6F41;
> + }
> + buf++;
> + }
> +
> + return (aw_dev_dsp_crc32_reflect(crc, 32)^0xffffffff);
> +}
> +
> +static int aw_dev_set_dsp_crc32(struct aw_device *aw_dev)
> +{
> + u32 crc_value = 0;
> + u32 crc_data_len = 0;
> + int ret = -1;
> + struct aw_sec_data_desc *crc_dsp_cfg = &aw_dev->crc_dsp_cfg;
> + struct aw_dsp_crc_desc *desc = &aw_dev->dsp_crc_desc;
> +
> + /*get crc data len*/
> + crc_data_len = (desc->dsp_reg - aw_dev->dsp_mem_desc.dsp_cfg_base_addr) * 2;
> + if (crc_data_len > crc_dsp_cfg->len) {
> + dev_err(aw_dev->dev, "crc data len :%d > cfg_data len:%d",
> + crc_data_len, crc_dsp_cfg->len);
> + return -EINVAL;
> + }
> +
> + if (crc_data_len % 4 != 0) {
> + dev_err(aw_dev->dev, "The crc data len :%d unsupport", crc_data_len);
> + return -EINVAL;
> + }
> +
> + crc_value = aw_dev_calc_dsp_cfg_crc32(crc_dsp_cfg->data, crc_data_len);
> +
> + dev_dbg(aw_dev->dev, "crc_value:0x%x", crc_value);
> + ret = aw_dev->ops.aw_dsp_write(aw_dev, desc->dsp_reg, crc_value,
> + desc->data_type);
> + if (ret < 0) {
> + dev_err(aw_dev->dev, "set dsp crc value failed");
> + return ret;
> + }
> +
> + return 0;
> +}
Similarly to crc8, Linux implements crc32
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/crc32.h
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/crc32.c
no need to implement your own.
> +
> +static void aw_dev_dsp_crc_check_enable(struct aw_device *aw_dev, bool flag)
> +{
> + struct aw_dsp_crc_desc *dsp_crc_desc = &aw_dev->dsp_crc_desc;
> + struct aw883xx *aw883xx = aw_dev->private_data;
> + int ret;
> +
> + if (flag) {
> + ret = aw_dev->ops.aw_reg_write_bits(aw883xx->regmap, dsp_crc_desc->ctl_reg,
> + ~dsp_crc_desc->ctl_mask, dsp_crc_desc->ctl_enable);
> + if (ret < 0) {
> + dev_err(aw_dev->dev, "enable dsp crc failed");
> + return;
> + }
> + } else {
> + ret = aw_dev->ops.aw_reg_write_bits(aw883xx->regmap, dsp_crc_desc->ctl_reg,
> + ~dsp_crc_desc->ctl_mask, dsp_crc_desc->ctl_disable);
> + if (ret < 0) {
> + dev_err(aw_dev->dev, "close dsp crc failed");
> + return;
> + }
> + }
> +}
> +
(...)
> +
> +void aw883xx_dev_memclk_select(struct aw_device *aw_dev, unsigned char flag)
> +{
> + struct aw_memclk_desc *desc = &aw_dev->memclk_desc;
> + struct aw883xx *aw883xx = aw_dev->private_data;
> + int ret = -1;
> +
> + switch (flag) {
> + case AW_DEV_MEMCLK_PLL:
> + ret = aw_dev->ops.aw_reg_write_bits(aw883xx->regmap, desc->reg,
> + ~desc->mask, desc->mcu_hclk);
> + if (ret < 0)
> + dev_err(aw_dev->dev, "memclk select pll failed");
> + break;
> + case AW_DEV_MEMCLK_OSC:
> + ret = aw_dev->ops.aw_reg_write_bits(aw883xx->regmap, desc->reg,
> + ~desc->mask, desc->osc_clk);
> + if (ret < 0)
> + dev_err(aw_dev->dev, "memclk select OSC failed");
> + break;
> + default:
> + dev_err(aw_dev->dev, "unknown memclk config, flag=0x%x", flag);
> + break;
> + }
> +}
> +
> +int aw883xx_dev_get_dsp_status(struct aw_device *aw_dev)
> +{
> + int ret = -1;
> + unsigned int reg_val = 0;
> + struct aw_watch_dog_desc *desc = &aw_dev->watch_dog_desc;
> + struct aw883xx *aw883xx = aw_dev->private_data;
> +
> + aw_dev->ops.aw_reg_read(aw883xx->regmap, desc->reg, ®_val);
> + if (reg_val & (~desc->mask))
> + ret = 0;
> +
> + return ret;
Here is example of what I'm talking about, when talking about setting
"ret = -1" you can return -1 here, and in call stack it gets mixed with
kernel return values, like -EINVAL so it can be potentially interpreted
as -EPERM somewhere.
> +}
> +
> +static int aw_dev_get_vmax(struct aw_device *aw_dev, unsigned int *vmax)
> +{
> + int ret = -1;
> + struct aw_vmax_desc *desc = &aw_dev->vmax_desc;
> +
> + ret = aw_dev->ops.aw_dsp_read(aw_dev, desc->dsp_reg, vmax, desc->data_type);
> + if (ret < 0) {
> + dev_err(aw_dev->dev, "get vmax failed");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
(...)
> +
> +static int aw_dev_dsp_fw_update(struct aw_device *aw_dev,
> + unsigned char *data, unsigned int len)
> +{
> + struct aw_dsp_mem_desc *dsp_mem_desc = &aw_dev->dsp_mem_desc;
> +
> + dev_dbg(aw_dev->dev, "dsp firmware len:%d", len);
> +
> + if (len && (data != NULL)) {
> + aw_dev_dsp_container_update(aw_dev,
> + data, len, dsp_mem_desc->dsp_fw_base_addr);
> + aw_dev->dsp_fw_len = len;
> + } else {
> + dev_err(aw_dev->dev, "dsp firmware data is null or len is 0");
> + return -EPERM;
> + }
> +
> + return 0;
> +}
> +
> +static int aw_dev_copy_to_crc_dsp_cfg(struct aw_device *aw_dev,
> + unsigned char *data, unsigned int size)
> +{
> + struct aw_sec_data_desc *crc_dsp_cfg = &aw_dev->crc_dsp_cfg;
> + int ret;
> +
> + if (!crc_dsp_cfg->data) {
> + crc_dsp_cfg->data = devm_kzalloc(aw_dev->dev, size, GFP_KERNEL);
> + if (!crc_dsp_cfg->data)
> + return -ENOMEM;
> + crc_dsp_cfg->len = size;
> + } else if (crc_dsp_cfg->len < size) {
> + devm_kfree(aw_dev->dev, crc_dsp_cfg->data);
> + crc_dsp_cfg->data = NULL;
> + crc_dsp_cfg->data = devm_kzalloc(aw_dev->dev, size, GFP_KERNEL);
No need for NULL assignment above, the variable gets set one line later
anyway.
> + if (!crc_dsp_cfg->data) {
> + dev_err(aw_dev->dev, "error allocating memory");
> + return -ENOMEM;
> + }
> + crc_dsp_cfg->len = size;
> + }
> + memcpy(crc_dsp_cfg->data, data, size);
> + ret = aw883xx_dev_dsp_data_order(aw_dev, crc_dsp_cfg->data, size);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
(...)
> diff --git a/sound/soc/codecs/aw883xx/aw883xx_device.h b/sound/soc/codecs/aw883xx/aw883xx_device.h
> new file mode 100644
> index 000000000000..505cef1fd3e6
> --- /dev/null
> +++ b/sound/soc/codecs/aw883xx/aw883xx_device.h
> @@ -0,0 +1,537 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * aw883xx.c -- ALSA Soc AW883XX codec support
> + *
> + * Copyright (c) 2022 AWINIC Technology CO., LTD
> + *
> + * Author: Bruce zhao <zhaolei at awinic.com>
> + */
> +
> +#ifndef __AW883XX_DEVICE_FILE_H__
> +#define __AW883XX_DEVICE_FILE_H__
> +
> +#include "aw883xx_data_type.h"
> +#include "aw883xx.h"
> +
> +
> +#define AW_DEV_DEFAULT_CH (0)
> +#define AW_DEV_DSP_CHECK_MAX (5)
> +
> +/*
> + * DSP I2C WRITES
> + */
> +#define AW_DSP_I2C_WRITES
> +#define AW_MAX_RAM_WRITE_BYTE_SIZE (128)
> +#define AW_DSP_ODD_NUM_BIT_TEST (0x5555)
> +#define AW_DSP_EVEN_NUM_BIT_TEST (0xAAAA)
> +#define AW_DSP_ST_CHECK_MAX (2)
> +#define AW_FADE_IN_OUT_DEFAULT (0)
> +#define AW_CALI_DELAY_CACL(value) ((value * 32) / 48)
> +#define AW_CALI_RE_MAX (15000)
> +#define AW_CALI_RE_MIN (4000)
> +
> +#define AW_GET_MIN_VALUE(value1, value2) \
> + ((value1) > (value2) ? (value2) : (value1))
> +
> +#define AW_GET_MAX_VALUE(value1, value2) \
> + ((value1) > (value2) ? (value1) : (value2))
Linux already has min and max macros?
(...)
More information about the Alsa-devel
mailing list