| 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
| ||
|
Message-ID: <bf84b93c-0b24-99f6-9c70-2f4c677cff18@linaro.org>
Date: Tue, 20 Dec 2022 17:07:07 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Kevin Lu <luminlong@....com>, lgirdwood@...il.com,
broonie@...nel.org, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org
Cc: alsa-devel@...a-project.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, shenghao-ding@...com,
kevin-lu@...com, navada@...com, peeyush@...com
Subject: Re: [PATCH] ALSA SoC: Texas Instruments TAS2781 Audio Smart Amp
On 20/12/2022 15:42, Kevin Lu wrote:
> The TAS2781 driver implements a flexible and configurable register setting
> for one, two, even multiple TAS2781 chips. All the register setting are in
> a bin file. Almost no specific register setting can be found in the code.
>
> Signed-off-by: Kevin Lu <luminlong@....com>
> ---
> sound/soc/codecs/Kconfig | 13 +
> sound/soc/codecs/Makefile | 2 +
> sound/soc/codecs/tas2781-dsp.c | 2483 ++++++++++++++++++++++++++++++++
> sound/soc/codecs/tas2781-dsp.h | 213 +++
> sound/soc/codecs/tas2781-i2c.c | 2143 +++++++++++++++++++++++++++
> sound/soc/codecs/tas2781.h | 208 +++
> 6 files changed, 5062 insertions(+)
> create mode 100644 sound/soc/codecs/tas2781-dsp.c
> create mode 100644 sound/soc/codecs/tas2781-dsp.h
> create mode 100644 sound/soc/codecs/tas2781-i2c.c
> create mode 100644 sound/soc/codecs/tas2781.h
Your threading and patch formatting is broken. Read before sending patches:
https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst
>
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 7022e6286..31d2d9594 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -222,6 +222,7 @@ config SND_SOC_ALL_CODECS
> imply SND_SOC_TAS2764
> imply SND_SOC_TAS2770
> imply SND_SOC_TAS2780
> + imply SND_SOC_TAS2781
> imply SND_SOC_TAS5086
> imply SND_SOC_TAS571X
> imply SND_SOC_TAS5720
> @@ -1573,6 +1574,18 @@ config SND_SOC_TAS2780
> Enable support for Texas Instruments TAS2780 high-efficiency
> digital input mono Class-D audio power amplifiers.
>
(...)
> +static struct i2c_driver tasdevice_i2c_driver = {
> + .driver = {
> + .name = "tas2781-codec",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(tasdevice_of_match),
> + .pm = &tasdevice_pm_ops,
> + },
> + .probe = tasdevice_i2c_probe,
> + .remove = tasdevice_i2c_remove,
> + .id_table = tasdevice_id,
> +};
> +
> +module_i2c_driver(tasdevice_i2c_driver);
> +
> +MODULE_AUTHOR("Shenghao Ding <shenghao-ding@...com>");
> +MODULE_AUTHOR("Kevin Lu <kevin-lu@...com>");
> +MODULE_DESCRIPTION("ASoC TAS2781 Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/sound/soc/codecs/tas2781.h b/sound/soc/codecs/tas2781.h
> new file mode 100644
> index 000000000..db4ec752b
> --- /dev/null
> +++ b/sound/soc/codecs/tas2781.h
> @@ -0,0 +1,208 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * ALSA SoC Texas Instruments TAS2781 Audio Smart Amplifier
> + *
> + * Copyright (C) 2022 - 2023 Texas Instruments Incorporated
> + * https://www.ti.com
> + *
> + * The TAS2781 driver implements a flexible and configurable register setting
> + * for one, two, even multiple TAS2781 chips. All the register setting are in
> + * a bin file. Almost no specific register setting can be found in the code.
> + *
> + * Author: Shenghao Ding <shenghao-ding@...com>
> + * Kevin Lu <kevin-lu@...com>
> + */
> +
> +#ifndef __TAS2781_H__
> +#define __TAS2781_H__
> +
> +#include "tas2781-dsp.h"
> +
> +#define SMARTAMP_MODULE_NAME ("tas2781")
This does not belong to headers.
> +#define MAX_LENGTH (128)
Why () ?
> +
> +#define TASDEVICE_RETRY_COUNT (3)
> +#define TASDEVICE_ERROR_FAILED (-2)
> +
> +#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) */
Missing space
> +#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_REG(book, page, reg) (((book * 256 * 128) + \
> + (page * 128)) + reg)
> +
> + /*Software Reset */
Wrong coding style.
> +#define TAS2871_REG_SWRESET TASDEVICE_REG(0x0, 0X0, 0x02)
> +#define TAS2871_REG_SWRESET_RESET (0x1 << 0)
> +
> + /* Enable Global addresses */
Wrong coding style.
> +#define TAS2871_MISC_CFG2 TASDEVICE_REG(0x0, 0X0, 0x07)
> +#define TAS2871_GLOBAL_MASK (0x1 << 1)
> +
> +#define SMS_HTONS(a, b) ((((a)&0x00FF)<<8) | \
> + ((b)&0x00FF))
> +#define SMS_HTONL(a, b, c, d) ((((a)&0x000000FF)<<24) |\
> + (((b)&0x000000FF)<<16) | \
> + (((c)&0x000000FF)<<8) | \
> + ((d)&0x000000FF))
> +
> + /*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)
> +
> +enum audio_device {
> + GENERAL_AUDEV = 0,
> + TAS2781 = 1,
> +};
> +
> +struct smartpa_gpio_info {
> + unsigned char ndev;
> + int mnResetGpio[MaxChn];
> +};
> +
> +#define SMS_HTONS(a, b) ((((a)&0x00FF)<<8) | ((b)&0x00FF))
> +#define SMS_HTONL(a, b, c, d) ((((a)&0x000000FF)<<24) | \
> + (((b)&0x000000FF)<<16) | (((c)&0x000000FF)<<8) | \
> + ((d)&0x000000FF))
> +
> +struct Tbookpage {
> + unsigned char mnBook;
> + unsigned char mnPage;
> +};
> +
> +struct Ttasdevice {
> + unsigned int mnDevAddr;
> + unsigned int mnErrCode;
> + short mnCurrentProgram;
> + short mnCurrentConfiguration;
> + short mnCurrentRegConf;
> + int mnIRQGPIO;
> + int mnResetGpio;
> + int mn_irq;
> + int PowerStatus;
> + bool bDSPBypass;
> + bool bIrq_enabled;
> + bool bLoading;
> + bool bLoaderr;
> + bool mbCalibrationLoaded;
> + struct Tbookpage mnBkPg;
> + struct TFirmware *mpCalFirmware;
> +};
> +
> +struct global_addr {
> + struct Tbookpage mnBkPg;
> + unsigned int mnDevAddr;
> + int ref_cnt;
> +};
> +
> +struct tas_control {
> + struct snd_kcontrol_new *tasdevice_profile_controls;
> + int nr_controls;
> +};
> +
> +struct tasdevice_irqinfo {
> + int mn_irq_gpio;
> + int mn_irq;
> + struct delayed_work irq_work;
> + bool mb_irq_enable;
> +};
> +
> +struct tasdevice_priv {
> + struct device *dev;
> + void *client;
> + struct regmap *regmap;
> + struct mutex codec_lock;
> + struct mutex dev_lock;
> + struct Ttasdevice tasdevice[MaxChn];
> + struct TFirmware *mpFirmware;
> + struct tasdevice_regbin mtRegbin;
> + struct smartpa_gpio_info mtRstGPIOs;
> + struct tasdevice_irqinfo mIrqInfo;
> + struct tas_control tas_ctrl;
> + struct global_addr glb_addr;
> + int mnCurrentProgram;
> + int mnCurrentConfiguration;
> + unsigned int chip_id;
> + int (*read)(struct tasdevice_priv *tas_dev, enum channel chn,
> + unsigned int reg, unsigned int *pValue);
> + int (*write)(struct tasdevice_priv *tas_dev, enum channel chn,
> + unsigned int reg, unsigned int Value);
> + int (*bulk_read)(struct tasdevice_priv *tas_dev, enum channel chn,
> + unsigned int reg, unsigned char *pData, unsigned int len);
> + int (*bulk_write)(struct tasdevice_priv *tas_dev, enum channel chn,
> + unsigned int reg, unsigned char *pData, unsigned int len);
> + int (*set_calibration)(void *tas_dev, enum channel chl,
> + int calibration);
> + void (*tas2781_reset)(struct tasdevice_priv *tas_dev);
> + void (*tas2781_set_global)(struct tasdevice_priv *tas_dev);
> + int (*fw_parse_variable_header)(struct tasdevice_priv *tas_dev,
> + const struct firmware *pFW, int offset);
> + int (*fw_parse_program_data)(struct tasdevice_priv *tas_dev,
> + struct TFirmware *pFirmware,
> + const struct firmware *pFW, int offset);
> + int (*fw_parse_configuration_data)(struct tasdevice_priv *tas_dev,
> + struct TFirmware *pFirmware,
> + const struct firmware *pFW, int offset);
> + int (*tasdevice_load_block)(struct tasdevice_priv *tas_dev,
> + struct TBlock *pBlock);
> + int (*fw_parse_calibration_data)(struct tasdevice_priv *tas_dev,
> + struct TFirmware *pFirmware,
> + const struct firmware *pFW, int offset);
> + void (*irq_work_func)(struct tasdevice_priv *tas_dev);
> + int fw_state;
> + unsigned int magic_num;
> + unsigned char ndev;
> + unsigned char dev_name[32];
> + unsigned char regbin_binaryname[64];
> + unsigned char dsp_binaryname[64];
> + unsigned char cal_binaryname[MaxChn][64];
> + bool mb_runtime_suspend;
> + struct delayed_work powercontrol_work;
> + void *codec;
> + int sysclk;
> + int pstream;
> + int cstream;
> + bool mbCalibrationLoaded;
You need to adjust your coding style to Linux. This is some C++ crap.
Best regards,
Krzysztof
Powered by blists - more mailing lists