- Alsa-devel - mailman.alsa-project.org

[PATCH] ASoC: ak4458: add optional reset control to instead of gpio
by Shengjiu Wang 18 Oct '22

18 Oct '22

From: Viorel Suman <viorel.suman(a)nxp.com> Add optional reset control instead of GPIO to manage codec PDN pin. As there is reference counter for reset control, so need to remove one ak4458_reset in runtime resume to make the reference counter balance. Signed-off-by: Viorel Suman <viorel.suman(a)nxp.com> Signed-off-by: Shengjiu Wang <shengjiu.wang(a)nxp.com> --- sound/soc/codecs/ak4458.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/sound/soc/codecs/ak4458.c b/sound/soc/codecs/ak4458.c index 626310859814..ac328d598291 100644 --- a/sound/soc/codecs/ak4458.c +++ b/sound/soc/codecs/ak4458.c @@ -13,6 +13,7 @@ #include <linux/of_gpio.h> #include <linux/pm_runtime.h> #include <linux/regulator/consumer.h> +#include <linux/reset.h> #include <linux/slab.h> #include <sound/initval.h> #include <sound/pcm_params.h> @@ -46,6 +47,7 @@ struct ak4458_priv { struct device *dev; struct regmap *regmap; struct gpio_desc *reset_gpiod; + struct reset_control *reset; struct gpio_desc *mute_gpiod; int digfil; /* SSLOW, SD, SLOW bits */ int fs; /* sampling rate */ @@ -633,6 +635,12 @@ static void ak4458_reset(struct ak4458_priv *ak4458, bool active) if (ak4458->reset_gpiod) { gpiod_set_value_cansleep(ak4458->reset_gpiod, active); usleep_range(1000, 2000); + } else if (!IS_ERR_OR_NULL(ak4458->reset)) { + if (active) + reset_control_assert(ak4458->reset); + else + reset_control_deassert(ak4458->reset); + usleep_range(1000, 2000); } } @@ -668,7 +676,6 @@ static int __maybe_unused ak4458_runtime_resume(struct device *dev) if (ak4458->mute_gpiod) gpiod_set_value_cansleep(ak4458->mute_gpiod, 1); - ak4458_reset(ak4458, true); ak4458_reset(ak4458, false); regcache_cache_only(ak4458->regmap, false); @@ -748,6 +755,10 @@ static int ak4458_i2c_probe(struct i2c_client *i2c) ak4458->drvdata = of_device_get_match_data(&i2c->dev); + ak4458->reset = devm_reset_control_get_optional_shared(ak4458->dev, NULL); + if (IS_ERR(ak4458->reset)) + return PTR_ERR(ak4458->reset); + ak4458->reset_gpiod = devm_gpiod_get_optional(ak4458->dev, "reset", GPIOD_OUT_LOW); if (IS_ERR(ak4458->reset_gpiod)) -- 2.34.1

2 1

[PATCH v3 0/7] Create a new sound card to access MICFIL based on rpmsg channel
by Chancel Liu 18 Oct '22

18 Oct '22

At a previous time, we have successfully created a virtual sound card based on rpmsg. The sound card works under this mechanism Cortex-A core tells the Cortex-M core the format, rate, channel, .etc configuration of the PCM parameters and Cortex-M controls real hardware devices such as SAI and DMA. From the view of Linux side, the sound card is bound to a rpmsg channel through which it can access SAI. Here these patches are introduced to create a new virtual sound card to access MICFIL based on a new created rpmsg channel. It's easy to create a new rpmsg channel for MICFIL through rpmsg name service announcment. Also the other ASoC components bound to this rpmsg MICFIL sound card will be registered with these patches. If other sound cards using different hardware devices needs to be created over rpmsg in the future, these patches can be referred. changes in v3 - Delete the statement referring to linux drivers in dts changes in v2: - Rename property in bindings file according to Krzysztof's comments - Update codes and comments according to Shengjiu's comments Chancel Liu (7): ASoC: dt-bindings: fsl_rpmsg: Add a property to assign the rpmsg channel ASoC: imx-audio-rpmsg: Create rpmsg channel for MICFIL ASoC: imx-pcm-rpmsg: Register different platform drivers ASoC: imx-pcm-rpmsg: Multi-channel support for sound card based on rpmsg ASoC: fsl_rpmsg: Register different ASoC machine devices ASoC: fsl_rpmsg: Multi-channel support in CPU DAI driver ASoC: imx-rpmsg: Assign platform driver used by machine driver to link with .../devicetree/bindings/sound/fsl,rpmsg.yaml | 36 +++++++++++++++++-- sound/soc/fsl/fsl_rpmsg.c | 6 ++-- sound/soc/fsl/imx-audio-rpmsg.c | 3 +- sound/soc/fsl/imx-pcm-rpmsg.c | 10 ++++-- sound/soc/fsl/imx-rpmsg.c | 6 +++- 5 files changed, 51 insertions(+), 10 deletions(-) -- 2.25.1

4 15

[PATCH] ASoC: amd: Update Pink Sardine platform ACP register header
by Vijendar Mukunda 18 Oct '22

18 Oct '22

Update Pink Sardine platform ACP register header with Soundwire Controller specific registers and other ACP registers. Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda(a)amd.com> --- include/sound/acp62_chip_offset_byte.h | 214 +++++++++++++++++++++++++ 1 file changed, 214 insertions(+) diff --git a/include/sound/acp62_chip_offset_byte.h b/include/sound/acp62_chip_offset_byte.h index f03992f81168..ca38f8a0966e 100644 --- a/include/sound/acp62_chip_offset_byte.h +++ b/include/sound/acp62_chip_offset_byte.h @@ -131,6 +131,23 @@ #define ACP_I2S_WAKE_EN 0x000145C #define ACP_SW1_WAKE_EN 0x0001460 +#define ACP_SW_I2S_ERROR_REASON 0x00018B4 +#define ACP_SW_POS_TRACK_I2S_TX_CTRL 0x00018B8 +#define ACP_SW_I2S_TX_DMA_POS 0x00018BC +#define ACP_SW_POS_TRACK_BT_TX_CTRL 0x00018C0 +#define ACP_SW_BT_TX_DMA_POS 0x00018C4 +#define ACP_SW_POS_TRACK_HS_TX_CTRL 0x00018C8 +#define ACP_SW_HS_TX_DMA_POS 0x00018CC +#define ACP_SW_POS_TRACK_I2S_RX_CTRL 0x00018D0 +#define ACP_SW_I2S_RX_DMA_POS 0x00018D4 +#define ACP_SW_POS_TRACK_BT_RX_CTRL 0x00018D8 +#define ACP_SW_BT_RX_DMA_POS 0x00018DC +#define ACP_SW_POS_TRACK_HS_RX_CTRL 0x00018E0 +#define ACP_SW_HS_RX_DMA_POS 0x00018E4 +#define ACP_ERROR_INTR_MASK1 0X0001974 +#define ACP_ERROR_INTR_MASK2 0X0001978 +#define ACP_ERROR_INTR_MASK3 0X000197C + /* Registers from ACP_P1_MISC block */ #define ACP_EXTERNAL_INTR_ENB 0x0001A00 #define ACP_EXTERNAL_INTR_CNTL 0x0001A04 @@ -154,6 +171,8 @@ #define ACP_P1_SW_BT_RX_DMA_POS 0x0001A9C #define ACP_P1_SW_POS_TRACK_HS_RX_CTRL 0x0001AA0 #define ACP_P1_SW_HS_RX_DMA_POS 0x0001AA4 +#define ACP_ERROR_INTR_MASK4 0X0001AEC +#define ACP_ERROR_INTR_MASK5 0X0001AF0 /* Registers from ACP_AUDIO_BUFFERS block */ #define ACP_I2S_RX_RINGBUFADDR 0x0002000 @@ -210,6 +229,24 @@ #define ACP_HS_TX_LINEARPOSITIONCNTR_HIGH 0x00020CC #define ACP_HS_TX_LINEARPOSITIONCNTR_LOW 0x00020D0 #define ACP_HS_TX_INTR_WATERMARK_SIZE 0x00020D4 +#define ACP_AUDIO_RX_RINGBUFADDR ACP_I2S_RX_RINGBUFADDR +#define ACP_AUDIO_RX_RINGBUFSIZE ACP_I2S_RX_RINGBUFSIZE +#define ACP_AUDIO_RX_LINKPOSITIONCNTR ACP_I2S_RX_LINKPOSITIONCNTR +#define ACP_AUDIO_RX_FIFOADDR ACP_I2S_RX_FIFOADDR +#define ACP_AUDIO_RX_FIFOSIZE ACP_I2S_RX_FIFOSIZE +#define ACP_AUDIO_RX_DMA_SIZE ACP_I2S_RX_DMA_SIZE +#define ACP_AUDIO_RX_LINEARPOSITIONCNTR_HIGH ACP_I2S_RX_LINEARPOSITIONCNTR_HIGH +#define ACP_AUDIO_RX_LINEARPOSITIONCNTR_LOW ACP_I2S_RX_LINEARPOSITIONCNTR_LOW +#define ACP_AUDIO_RX_INTR_WATERMARK_SIZE ACP_I2S_RX_INTR_WATERMARK_SIZE +#define ACP_AUDIO_TX_RINGBUFADDR ACP_I2S_TX_RINGBUFADDR +#define ACP_AUDIO_TX_RINGBUFSIZE ACP_I2S_TX_RINGBUFSIZE +#define ACP_AUDIO_TX_LINKPOSITIONCNTR ACP_I2S_TX_LINKPOSITIONCNTR +#define ACP_AUDIO_TX_FIFOADDR ACP_I2S_TX_FIFOADDR +#define ACP_AUDIO_TX_FIFOSIZE ACP_I2S_TX_FIFOSIZE +#define ACP_AUDIO_TX_DMA_SIZE ACP_I2S_TX_DMA_SIZE +#define ACP_AUDIO_TX_LINEARPOSITIONCNTR_HIGH ACP_I2S_TX_LINEARPOSITIONCNTR_HIGH +#define ACP_AUDIO_TX_LINEARPOSITIONCNTR_LOW ACP_I2S_TX_LINEARPOSITIONCNTR_LOW +#define ACP_AUDIO_TX_INTR_WATERMARK_SIZE ACP_I2S_TX_INTR_WATERMARK_SIZE /* Registers from ACP_I2S_TDM block */ #define ACP_I2STDM_IER 0x0002400 @@ -255,6 +292,102 @@ #define ACP_WOV_ERROR_STATUS_REGISTER 0x0002C68 #define ACP_PDM_CLKDIV 0x0002C6C +/* Registers from ACP_SW_SWCLK block */ +#define ACP_SW_EN 0x0003000 +#define ACP_SW_EN_STATUS 0x0003004 +#define ACP_SW_FRAMESIZE 0x0003008 +#define ACP_SW_SSP_COUNTER 0x000300C +#define ACP_SW_AUDIO_TX_EN 0x0003010 +#define ACP_SW_AUDIO_TX_EN_STATUS 0x0003014 +#define ACP_SW_AUDIO_TX_FRAME_FORMAT 0x0003018 +#define ACP_SW_AUDIO_TX_SAMPLEINTERVAL 0x000301C +#define ACP_SW_AUDIO_TX_HCTRL_DP0 0x0003020 +#define ACP_SW_AUDIO_TX_HCTRL_DP1 0x0003024 +#define ACP_SW_AUDIO_TX_HCTRL_DP2 0x0003028 +#define ACP_SW_AUDIO_TX_HCTRL_DP3 0x000302C +#define ACP_SW_AUDIO_TX_OFFSET_DP0 0x0003030 +#define ACP_SW_AUDIO_TX_OFFSET_DP1 0x0003034 +#define ACP_SW_AUDIO_TX_OFFSET_DP2 0x0003038 +#define ACP_SW_AUDIO_TX_OFFSET_DP3 0x000303C +#define ACP_SW_AUDIO_TX_CHANNEL_ENABLE_DP0 0x0003040 +#define ACP_SW_AUDIO_TX_CHANNEL_ENABLE_DP1 0x0003044 +#define ACP_SW_AUDIO_TX_CHANNEL_ENABLE_DP2 0x0003048 +#define ACP_SW_AUDIO_TX_CHANNEL_ENABLE_DP3 0x000304C +#define ACP_SW_BT_TX_EN 0x0003050 +#define ACP_SW_BT_TX_EN_STATUS 0x0003054 +#define ACP_SW_BT_TX_FRAME_FORMAT 0x0003058 +#define ACP_SW_BT_TX_SAMPLEINTERVAL 0x000305C +#define ACP_SW_BT_TX_HCTRL 0x0003060 +#define ACP_SW_BT_TX_OFFSET 0x0003064 +#define ACP_SW_BT_TX_CHANNEL_ENABLE_DP0 0x0003068 +#define ACP_SW_HEADSET_TX_EN 0x000306C +#define ACP_SW_HEADSET_TX_EN_STATUS 0x0003070 +#define ACP_SW_HEADSET_TX_FRAME_FORMAT 0x0003074 +#define ACP_SW_HEADSET_TX_SAMPLEINTERVAL 0x0003078 +#define ACP_SW_HEADSET_TX_HCTRL 0x000307C +#define ACP_SW_HEADSET_TX_OFFSET 0x0003080 +#define ACP_SW_HEADSET_TX_CHANNEL_ENABLE_DP0 0x0003084 +#define ACP_SW_AUDIO_RX_EN 0x0003088 +#define ACP_SW_AUDIO_RX_EN_STATUS 0x000308C +#define ACP_SW_AUDIO_RX_FRAME_FORMAT 0x0003090 +#define ACP_SW_AUDIO_RX_SAMPLEINTERVAL 0x0003094 +#define ACP_SW_AUDIO_RX_HCTRL_DP0 0x0003098 +#define ACP_SW_AUDIO_RX_HCTRL_DP1 0x000309C +#define ACP_SW_AUDIO_RX_HCTRL_DP2 0x0003100 +#define ACP_SW_AUDIO_RX_HCTRL_DP3 0x0003104 +#define ACP_SW_AUDIO_RX_OFFSET_DP0 0x0003108 +#define ACP_SW_AUDIO_RX_OFFSET_DP1 0x000310C +#define ACP_SW_AUDIO_RX_OFFSET_DP2 0x0003110 +#define ACP_SW_AUDIO_RX_OFFSET_DP3 0x0003114 +#define ACP_SW_AUDIO_RX_CHANNEL_ENABLE_DP0 0x0003118 +#define ACP_SW_AUDIO_RX_CHANNEL_ENABLE_DP1 0x000311C +#define ACP_SW_AUDIO_RX_CHANNEL_ENABLE_DP2 0x0003120 +#define ACP_SW_AUDIO_RX_CHANNEL_ENABLE_DP3 0x0003124 +#define ACP_SW_BT_RX_EN 0x0003128 +#define ACP_SW_BT_RX_EN_STATUS 0x000312C +#define ACP_SW_BT_RX_FRAME_FORMAT 0x0003130 +#define ACP_SW_BT_RX_SAMPLEINTERVAL 0x0003134 +#define ACP_SW_BT_RX_HCTRL 0x0003138 +#define ACP_SW_BT_RX_OFFSET 0x000313C +#define ACP_SW_BT_RX_CHANNEL_ENABLE_DP0 0x0003140 +#define ACP_SW_HEADSET_RX_EN 0x0003144 +#define ACP_SW_HEADSET_RX_EN_STATUS 0x0003148 +#define ACP_SW_HEADSET_RX_FRAME_FORMAT 0x000314C +#define ACP_SW_HEADSET_RX_SAMPLEINTERVAL 0x0003150 +#define ACP_SW_HEADSET_RX_HCTRL 0x0003154 +#define ACP_SW_HEADSET_RX_OFFSET 0x0003158 +#define ACP_SW_HEADSET_RX_CHANNEL_ENABLE_DP0 0x000315C +#define ACP_SW_BPT_PORT_EN 0x0003160 +#define ACP_SW_BPT_PORT_EN_STATUS 0x0003164 +#define ACP_SW_BPT_PORT_FRAME_FORMAT 0x0003168 +#define ACP_SW_BPT_PORT_SAMPLEINTERVAL 0x000316C +#define ACP_SW_BPT_PORT_HCTRL 0x0003170 +#define ACP_SW_BPT_PORT_OFFSET 0x0003174 +#define ACP_SW_BPT_PORT_CHANNEL_ENABLE 0x0003178 +#define ACP_SW_BPT_PORT_FIRST_BYTE_ADDR 0x000317C +#define ACP_SW_CLK_RESUME_CTRL 0x0003180 +#define ACP_SW_CLK_RESUME_DELAY_CNTR 0x0003184 +#define ACP_SW_BUS_RESET_CTRL 0x0003188 +#define ACP_SW_PRBS_ERR_STATUS 0x000318C +#define SW_IMM_CMD_UPPER_WORD 0x0003230 +#define SW_IMM_CMD_LOWER_QWORD 0x0003234 +#define SW_IMM_RESP_UPPER_WORD 0x0003238 +#define SW_IMM_RESP_LOWER_QWORD 0x000323C +#define SW_IMM_CMD_STS 0x0003240 +#define SW_BRA_BASE_ADDRESS 0x0003244 +#define SW_BRA_TRANSFER_SIZE 0x0003248 +#define SW_BRA_DMA_BUSY 0x000324C +#define SW_BRA_RESP 0x0003250 +#define SW_BRA_RESP_FRAME_ADDR 0x0003254 +#define SW_BRA_CURRENT_TRANSFER_SIZE 0x0003258 +#define SW_STATE_CHANGE_STATUS_0TO7 0x000325C +#define SW_STATE_CHANGE_STATUS_8TO11 0x0003260 +#define SW_STATE_CHANGE_STATUS_MASK_0TO7 0x0003264 +#define SW_STATE_CHANGE_STATUS_MASK_8TO11 0x0003268 +#define SW_CLK_FREQUENCY_CTRL 0x000326C +#define SW_ERROR_INTR_MASK 0x0003270 +#define SW_PHY_TEST_MODE_DATA_OFF 0x0003274 + /* Registers from ACP_P1_AUDIO_BUFFERS block */ #define ACP_P1_I2S_RX_RINGBUFADDR 0x0003A00 #define ACP_P1_I2S_RX_RINGBUFSIZE 0x0003A04 @@ -310,6 +443,87 @@ #define ACP_P1_HS_TX_LINEARPOSITIONCNTR_HIGH 0x0003ACC #define ACP_P1_HS_TX_LINEARPOSITIONCNTR_LOW 0x0003AD0 #define ACP_P1_HS_TX_INTR_WATERMARK_SIZE 0x0003AD4 +#define ACP_P1_AUDIO_RX_RINGBUFADDR ACP_P1_I2S_RX_RINGBUFADDR +#define ACP_P1_AUDIO_RX_RINGBUFSIZE ACP_P1_I2S_RX_RINGBUFSIZE +#define ACP_P1_AUDIO_RX_LINKPOSITIONCNTR ACP_P1_I2S_RX_LINKPOSITIONCNTR +#define ACP_P1_AUDIO_RX_FIFOADDR ACP_P1_I2S_RX_FIFOADDR +#define ACP_P1_AUDIO_RX_FIFOSIZE ACP_P1_I2S_RX_FIFOSIZE +#define ACP_P1_AUDIO_RX_DMA_SIZE ACP_P1_I2S_RX_DMA_SIZE +#define ACP_P1_AUDIO_RX_LINEARPOSITIONCNTR_HIGH ACP_P1_I2S_RX_LINEARPOSITIONCNTR_HIGH +#define ACP_P1_AUDIO_RX_LINEARPOSITIONCNTR_LOW ACP_P1_I2S_RX_LINEARPOSITIONCNTR_LOW +#define ACP_P1_AUDIO_RX_INTR_WATERMARK_SIZE ACP_P1_I2S_RX_INTR_WATERMARK_SIZE +#define ACP_P1_AUDIO_TX_RINGBUFADDR ACP_P1_I2S_TX_RINGBUFADDR +#define ACP_P1_AUDIO_TX_RINGBUFSIZE ACP_P1_I2S_TX_RINGBUFSIZE +#define ACP_P1_AUDIO_TX_LINKPOSITIONCNTR ACP_P1_I2S_TX_LINKPOSITIONCNTR +#define ACP_P1_AUDIO_TX_FIFOADDR ACP_P1_I2S_TX_FIFOADDR +#define ACP_P1_AUDIO_TX_FIFOSIZE ACP_P1_I2S_TX_FIFOSIZE +#define ACP_P1_AUDIO_TX_DMA_SIZE ACP_P1_I2S_TX_DMA_SIZE +#define ACP_P1_AUDIO_TX_LINEARPOSITIONCNTR_HIGH ACP_P1_I2S_TX_LINEARPOSITIONCNTR_HIGH +#define ACP_P1_AUDIO_TX_LINEARPOSITIONCNTR_LOW ACP_P1_I2S_TX_LINEARPOSITIONCNTR_LOW +#define ACP_P1_AUDIO_TX_INTR_WATERMARK_SIZE ACP_P1_I2S_TX_INTR_WATERMARK_SIZE + +/* Registers from ACP_P1_SW_SWCLK block */ +#define ACP_P1_SW_EN 0x0003C00 +#define ACP_P1_SW_EN_STATUS 0x0003C04 +#define ACP_P1_SW_FRAMESIZE 0x0003C08 +#define ACP_P1_SW_SSP_COUNTER 0x0003C0C +#define ACP_P1_SW_BT_TX_EN 0x0003C50 +#define ACP_P1_SW_BT_TX_EN_STATUS 0x0003C54 +#define ACP_P1_SW_BT_TX_FRAME_FORMAT 0x0003C58 +#define ACP_P1_SW_BT_TX_SAMPLEINTERVAL 0x0003C5C +#define ACP_P1_SW_BT_TX_HCTRL 0x0003C60 +#define ACP_P1_SW_BT_TX_OFFSET 0x0003C64 +#define ACP_P1_SW_BT_TX_CHANNEL_ENABLE_DP0 0x0003C68 +#define ACP_P1_SW_BT_RX_EN 0x0003D28 +#define ACP_P1_SW_BT_RX_EN_STATUS 0x0003D2C +#define ACP_P1_SW_BT_RX_FRAME_FORMAT 0x0003D30 +#define ACP_P1_SW_BT_RX_SAMPLEINTERVAL 0x0003D34 +#define ACP_P1_SW_BT_RX_HCTRL 0x0003D38 +#define ACP_P1_SW_BT_RX_OFFSET 0x0003D3C +#define ACP_P1_SW_BT_RX_CHANNEL_ENABLE_DP0 0x0003D40 +#define ACP_P1_SW_BPT_PORT_EN 0x0003D60 +#define ACP_P1_SW_BPT_PORT_EN_STATUS 0x0003D64 +#define ACP_P1_SW_BPT_PORT_FRAME_FORMAT 0x0003D68 +#define ACP_P1_SW_BPT_PORT_SAMPLEINTERVAL 0x0003D6C +#define ACP_P1_SW_BPT_PORT_HCTRL 0x0003D70 +#define ACP_P1_SW_BPT_PORT_OFFSET 0x0003D74 +#define ACP_P1_SW_BPT_PORT_CHANNEL_ENABLE 0x0003D78 +#define ACP_P1_SW_BPT_PORT_FIRST_BYTE_ADDR 0x0003D7C +#define ACP_P1_SW_CLK_RESUME_CTRL 0x0003D80 +#define ACP_P1_SW_CLK_RESUME_DELAY_CNTR 0x0003D84 +#define ACP_P1_SW_BUS_RESET_CTRL 0x0003D88 +#define ACP_P1_SW_PRBS_ERR_STATUS 0x0003D8C + +/* Registers from ACP_P1_SW_ACLK block */ +#define P1_SW_CORB_BASE_ADDRESS 0x0003E00 +#define P1_SW_CORB_WRITE_POINTER 0x0003E04 +#define P1_SW_CORB_READ_POINTER 0x0003E08 +#define P1_SW_CORB_CONTROL 0x0003E0C +#define P1_SW_CORB_SIZE 0x0003E14 +#define P1_SW_RIRB_BASE_ADDRESS 0x0003E18 +#define P1_SW_RIRB_WRITE_POINTER 0x0003E1C +#define P1_SW_RIRB_RESPONSE_INTERRUPT_COUNT 0x0003E20 +#define P1_SW_RIRB_CONTROL 0x0003E24 +#define P1_SW_RIRB_SIZE 0x0003E28 +#define P1_SW_RIRB_FIFO_MIN_THDL 0x0003E2C +#define P1_SW_IMM_CMD_UPPER_WORD 0x0003E30 +#define P1_SW_IMM_CMD_LOWER_QWORD 0x0003E34 +#define P1_SW_IMM_RESP_UPPER_WORD 0x0003E38 +#define P1_SW_IMM_RESP_LOWER_QWORD 0x0003E3C +#define P1_SW_IMM_CMD_STS 0x0003E40 +#define P1_SW_BRA_BASE_ADDRESS 0x0003E44 +#define P1_SW_BRA_TRANSFER_SIZE 0x0003E48 +#define P1_SW_BRA_DMA_BUSY 0x0003E4C +#define P1_SW_BRA_RESP 0x0003E50 +#define P1_SW_BRA_RESP_FRAME_ADDR 0x0003E54 +#define P1_SW_BRA_CURRENT_TRANSFER_SIZE 0x0003E58 +#define P1_SW_STATE_CHANGE_STATUS_0TO7 0x0003E5C +#define P1_SW_STATE_CHANGE_STATUS_8TO11 0x0003E60 +#define P1_SW_STATE_CHANGE_STATUS_MASK_0TO7 0x0003E64 +#define P1_SW_STATE_CHANGE_STATUS_MASK_8TO11 0x0003E68 +#define P1_SW_CLK_FREQUENCY_CTRL 0x0003E6C +#define P1_SW_ERROR_INTR_MASK 0x0003E70 +#define P1_SW_PHY_TEST_MODE_DATA_OFF 0x0003E74 /* Registers from ACP_SCRATCH block */ #define ACP_SCRATCH_REG_0 0x0010000 -- 2.25.1

2 1

Thanks to review question [PATCH V2 1/2] ASoc:codes:Add Awinic AW883XX audio amplifier driver
by wangweidong.a＠awinic.com 18 Oct '22

18 Oct '22

HI: Mark Brown Thank you very much for your help. I will correct these problems. Best regards, Weidong Wang On Mon, Oct 17, 2022 at 04:09:12PM +0800, wangweidong.a(a)awinic.com wrote: > Hi: Mark Brown > > Thank you for your suggestion. I will fix the problem you raised in > the next patch, but there is still a question here and I want to > discuss it with you > > This is rather too big to go through in one go so the review here is > very high level but that's probably a good level for initial review > here as there Please fix your mail client to clearly identify quoted text, as you can see above it's very dificult for me to tell where you've replied to my mail. > > + if (mute) { > > + aw883xx->pstream = AW883XX_STREAM_CLOSE; > > + cancel_delayed_work_sync(&aw883xx->start_work); > > + mutex_lock(&aw883xx->lock); > > + aw883xx_device_stop(aw883xx->aw_pa); > > + mutex_unlock(&aw883xx->lock); > > + } else { > > + aw883xx->pstream = AW883XX_STREAM_OPEN; > > + mutex_lock(&aw883xx->lock); > > + aw883xx_start(aw883xx, AW_ASYNC_START); > > + aw883xx_hold_dsp_spin_st(&aw883xx->aw_pa->spin_desc); > > + mutex_unlock(&aw883xx->lock); > > + } > > This doesn't look like a mute operation, it looks like it's starting > and stopping the DSP. > > Answer: This is a mute operation ,aw883xx_device_stop is called in th > aw883xx_mute function. This function not only executes the mute > function aw883xx_dev_mute, but also disables dsp and power down. This > is for the aw883xx chip low power optimization. Then it's not a mute function, the goal of the mute function is to run before all the power management code to minimise glitches during power management. Just implement the power management via the standard ASoC power management APIs. > > + aw883xx_dev_set_fade_time(ucontrol->value.integer.value[0], true); > > + > > + aw_pr_dbg("step time %ld", ucontrol->value.integer.value[0]); > > + return 0; > > +} > > If a control write changes a value it should return 1, you should run > the mixer-test selftest which will identify this and a number of other issues. tools/testing/selftests/alsa > Answer: Could you tell me what is mixer-test selftest? I have checked > other drivers, and there is no return 1. Are you *sure* there's none? Other drivers being buggy isn't a good reason to introduce more bugs.

1 0

[PATCH v2 00/11] arm64/slimbus/dt-bindings: convert to DT Schema, minor cleanups
by Krzysztof Kozlowski 18 Oct '22

18 Oct '22

Hi, Changes since v1 ================ 1. Fix commit title typo (Steev). 2. Add Rb/Tb tags. Dependencies ============ No dependencies. Binding patches are independent from DTS. Best regards, Krzysztof Krzysztof Kozlowski (11): arm64: dts: qcom: sdm845: drop unused slimbus properties arm64: dts: qcom: msm8996: drop unused slimbus reg-mames arm64: dts: qcom: sdm845: correct slimbus children unit addresses arm64: dts: qcom: mms8996: correct slimbus children unit addresses arm64: dts: qcom: sdm845: drop unused slimbus dmas arm64: dts: qcom: msm8996: drop unused slimbus dmas arm64: dts: qcom: sdm845: align node names with DT schema arm64: dts: qcom: msm8996: align node names with DT schema dt-bindings: slimbus: convert bus description to DT schema dt-bindings: slimbus: qcom,slim: convert to DT schema dt-bindings: slimbus: qcom,slim-ngd: convert to DT schema .../devicetree/bindings/slimbus/bus.txt | 60 --------- .../bindings/slimbus/qcom,slim-ngd.yaml | 120 ++++++++++++++++++ .../bindings/slimbus/qcom,slim.yaml | 86 +++++++++++++ .../bindings/slimbus/slim-ngd-qcom-ctrl.txt | 84 ------------ .../bindings/slimbus/slim-qcom-ctrl.txt | 39 ------ .../devicetree/bindings/slimbus/slimbus.yaml | 95 ++++++++++++++ arch/arm64/boot/dts/qcom/msm8996.dtsi | 18 ++- arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 +-- 8 files changed, 315 insertions(+), 203 deletions(-) delete mode 100644 Documentation/devicetree/bindings/slimbus/bus.txt create mode 100644 Documentation/devicetree/bindings/slimbus/qcom,slim-ngd.yaml create mode 100644 Documentation/devicetree/bindings/slimbus/qcom,slim.yaml delete mode 100644 Documentation/devicetree/bindings/slimbus/slim-ngd-qcom-ctrl.txt delete mode 100644 Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt create mode 100644 Documentation/devicetree/bindings/slimbus/slimbus.yaml -- 2.34.1

2 12

[PATCH 00/12] ASoC/qcom/remoteproc/arm64: Qualcomm ADSP DTS and binding fixes
by Krzysztof Kozlowski 18 Oct '22

18 Oct '22

Hi, Dependencies/merging ==================== 1. The DTS patches are independent. 2. The binding patches should come together, because of context changes. Could be one of: Qualcomm SoC, ASoC or DT tree. Best regards, Krzysztof Krzysztof Kozlowski (12): arm64: dts: qcom: sdm630: align APR services node names with dtschema arm64: dts: qcom: sdm845: align APR services node names with dtschema arm64: dts: qcom: sm8250: align APR services node names with dtschema arm64: dts: qcom: msm8996: fix APR services nodes arm64: dts: qcom: sdm845: align dai node names with dtschema arm64: dts: qcom: msm8996: align dai node names with dtschema arm64: dts: qcom: qrb5165-rb5: align dai node names with dtschema arm64: dts: qcom: sm8250: use generic name for LPASS clock controller dt-bindings: soc: qcom: apr: correct service children ASoC: dt-bindings: qcom,q6dsp-lpass-ports: cleanup example ASoC: dt-bindings: qcom,q6dsp-lpass-clocks: cleanup example dt-bindings: soc: qcom: apr: add missing properties .../bindings/soc/qcom/qcom,apr.yaml | 85 ++++++++++++++++--- .../sound/qcom,q6dsp-lpass-clocks.yaml | 30 +++---- .../sound/qcom,q6dsp-lpass-ports.yaml | 30 +++---- arch/arm64/boot/dts/qcom/msm8996.dtsi | 10 +-- arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 4 +- arch/arm64/boot/dts/qcom/sdm630.dtsi | 8 +- arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 2 +- .../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 2 +- .../boot/dts/qcom/sdm845-xiaomi-polaris.dts | 4 +- arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 +- arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 +-- 11 files changed, 124 insertions(+), 69 deletions(-) -- 2.34.1

5 17

controlC1 no longer show up after USBDEVFS_DISCONNECT
by GitHub issues - opened 17 Oct '22

17 Oct '22

alsa-project/alsa-lib issue #276 was opened from robertwu1: After running the following IOCTL on some MIDI devices, the control C1 device node is gone. ~~~ struct usbdevfs_ioctl ctl; ctl.ifno = interface; // MIDI interface ctl.ioctl_code = USBDEVFS_DISCONNECT; ctl.data = NULL; return ioctl(device->fd, USBDEVFS_IOCTL, &ctl); ~~~ Under /dev/snd, the following disappears: controlC1, midiC1D0, pcmC1D0c, pcmC1D0p. MidiControl, AudioStreaming, and AudioControl are under separate interfaces. If some of these were to disappear, I expect only midiC1D0 to disappear. I've tested the following devices: /dev/snd looks the same after USBDEVFS_DISCONNECT: PreSonus AudioBox 96, Alesis iO4, Komplete Audio 6. controlC1, midiC1D0, pcmC1D0c, pcmC1D0p are gone: PreSonus Studio 26c, Yamaha AG06. Here is a public Android bug of users having issues of this on Android: https://issuetracker.google.com/issues/251050530 Issue URL : https://github.com/alsa-project/alsa-lib/issues/276 Repository URL: https://github.com/alsa-project/alsa-lib

1 0

[rft, PATCH v2 00/36] pinctrl: Clean up and add missed headers
by Andy Shevchenko 17 Oct '22

17 Oct '22

Currently the header inclusion inside the pinctrl headers seems more arbitrary than logical. This series is basically out of two parts: - add missed headers to the pin control drivers / users - clean up the headers of pin control subsystem The idea is to have this series to be pulled after -rc1 by the GPIO and pin control subsystems, so all new drivers will utilize cleaned up headers of the pin control. Please, review and comment. Changelog v2: - added preparatory patches: all, but last (LKP) - added missed forward declaration to the last patch (LKP) Andy Shevchenko (36): gpiolib: tegra186: Add missed header(s) gpiolib: cdev: Add missed header(s) media: c8sectpfe: Add missed header(s) pinctrl: actions: Add missed header(s) pinctrl: aspeed: Add missed header(s) pinctrl: at91: Add missed header(s) pinctrl: axp209: Add missed header(s) pinctrl: bcm: Add missed header(s) pinctrl: cygnus-mux: Add missed header(s) pinctrl: imx: Add missed header(s) pinctrl: ingenic: Add missed header(s) pinctrl: k210: Add missed header(s) pinctrl: lochnagar: Add missed header(s) pinctrl: mediatek: Add missed header(s) pinctrl: mvebu: Add missed header(s) pinctrl: npcm7xx: Add missed header(s) pinctrl: ocelot: Add missed header(s) pinctrl: qcom: Add missed header(s) pinctrl: renesas: Add missed header(s) pinctrl: samsung: Add missed header(s) pinctrl: single: Add missed header(s) pinctrl: spear: Add missed header(s) pinctrl: sprd: Add missed header(s) pinctrl: st: Add missed header(s) pinctrl: starfive: Add missed header(s) pinctrl: stm32: Add missed header(s) pinctrl: stmfx: Add missed header(s) pinctrl: tegra: Add missed header(s) pinctrl: ti-iodelay: Add missed header(s) pinctrl: uniphier: Add missed header(s) pinctrl: zynqmp: Add missed header(s) pinctrl: cherryview: Add missed header(s) pinctrl: lynxpoint: Add missed header(s) pinctrl: merrifield: Add missed header(s) pinctrl: intel: Add missed header(s) pinctrl: Clean up headers drivers/gpio/gpio-tegra186.c | 3 +- drivers/gpio/gpiolib-cdev.c | 6 ++-- .../st/sti/c8sectpfe/c8sectpfe-core.c | 8 +++-- drivers/pinctrl/actions/pinctrl-owl.c | 10 +++--- drivers/pinctrl/aspeed/pinctrl-aspeed.c | 1 + drivers/pinctrl/bcm/pinctrl-bcm281xx.c | 13 +++++--- drivers/pinctrl/bcm/pinctrl-cygnus-mux.c | 9 ++++-- drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 12 ++++--- drivers/pinctrl/bcm/pinctrl-ns2-mux.c | 8 +++-- drivers/pinctrl/bcm/pinctrl-nsp-mux.c | 8 +++-- drivers/pinctrl/cirrus/pinctrl-lochnagar.c | 6 ++-- drivers/pinctrl/core.c | 19 ++++++------ drivers/pinctrl/core.h | 12 ++++++- drivers/pinctrl/devicetree.h | 6 ++++ drivers/pinctrl/freescale/pinctrl-imx.c | 8 +++-- drivers/pinctrl/intel/pinctrl-cherryview.c | 6 ++-- drivers/pinctrl/intel/pinctrl-intel.c | 6 ++-- drivers/pinctrl/intel/pinctrl-lynxpoint.c | 6 ++-- drivers/pinctrl/intel/pinctrl-merrifield.c | 4 ++- drivers/pinctrl/mediatek/pinctrl-moore.c | 3 ++ drivers/pinctrl/mediatek/pinctrl-paris.c | 5 +++ drivers/pinctrl/mvebu/pinctrl-mvebu.c | 14 +++++---- drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c | 11 ++++--- drivers/pinctrl/pinconf.h | 10 ++++++ drivers/pinctrl/pinctrl-at91-pio4.c | 10 ++++-- drivers/pinctrl/pinctrl-at91.c | 16 +++++----- drivers/pinctrl/pinctrl-axp209.c | 8 +++-- drivers/pinctrl/pinctrl-ingenic.c | 10 +++--- drivers/pinctrl/pinctrl-k210.c | 12 ++++--- drivers/pinctrl/pinctrl-ocelot.c | 10 +++--- drivers/pinctrl/pinctrl-single.c | 5 ++- drivers/pinctrl/pinctrl-st.c | 21 ++++++++----- drivers/pinctrl/pinctrl-stmfx.c | 2 ++ drivers/pinctrl/pinctrl-utils.h | 5 +++ drivers/pinctrl/pinctrl-zynqmp.c | 4 ++- drivers/pinctrl/pinmux.c | 17 +++++----- drivers/pinctrl/pinmux.h | 11 +++++++ drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 8 +++-- drivers/pinctrl/qcom/pinctrl-spmi-mpp.c | 8 +++-- drivers/pinctrl/renesas/pinctrl-rzg2l.c | 7 +++-- drivers/pinctrl/renesas/pinctrl-rzv2m.c | 4 ++- drivers/pinctrl/renesas/pinctrl.c | 8 +++-- drivers/pinctrl/samsung/pinctrl-samsung.c | 11 ++++--- drivers/pinctrl/spear/pinctrl-spear.c | 6 ++-- drivers/pinctrl/sprd/pinctrl-sprd.c | 6 ++-- .../starfive/pinctrl-starfive-jh7100.c | 2 ++ drivers/pinctrl/stm32/pinctrl-stm32.c | 16 +++++----- drivers/pinctrl/tegra/pinctrl-tegra.c | 6 ++-- drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 8 +++-- .../pinctrl/uniphier/pinctrl-uniphier-core.c | 8 +++-- include/linux/pinctrl/consumer.h | 31 +++++++++---------- include/linux/pinctrl/devinfo.h | 6 ++-- include/linux/pinctrl/machine.h | 8 +++-- include/linux/pinctrl/pinconf-generic.h | 23 ++++++++------ include/linux/pinctrl/pinctrl.h | 18 +++++------ include/linux/pinctrl/pinmux.h | 5 ++- 56 files changed, 328 insertions(+), 186 deletions(-) base-commit: 9d157c89c5569f0ef560b7a5b2d7bf59ae98499c -- 2.35.1

11 61

Reply to review question [PATCH V2 1/2] ASoc:codes:Add Awinic AW883XX audio amplifier driver
by wangweidong.a＠awinic.com 17 Oct '22

17 Oct '22

Hi: Mark Brown Thank you for your suggestion. I will fix the problem you raised in the next patch, but there is still a question here and I want to discuss it with you This is rather too big to go through in one go so the review here is very high level but that's probably a good level for initial review here as there are a lot of stylistic and structural problems. This doesn't really look like a Linux driver, it's not following the standard patterns or using standard frameworks in normal ways. It'd be good for you to look at how other drivers for similar devices are implemented and ensure that this driver looks similar, where a driver does something very different to other drivers that's usually a sign there's something up. > --- > sound/soc/codecs/Kconfig | 10 + > sound/soc/codecs/Makefile | 4 + > sound/soc/codecs/aw883xx/aw883xx.c | 2734 +++++++++++++++++++++++ > sound/soc/codecs/aw883xx/aw883xx.h | 213 ++ > sound/soc/codecs/aw883xx/aw883xx_bin_parse.c | 1392 ++++++++++++ > sound/soc/codecs/aw883xx/aw883xx_bin_parse.h | 145 ++ > sound/soc/codecs/aw883xx/aw883xx_calib.c | 2534 +++++++++++++++++++++ > sound/soc/codecs/aw883xx/aw883xx_calib.h | 154 ++ > sound/soc/codecs/aw883xx/aw883xx_data_type.h | 157 ++ > sound/soc/codecs/aw883xx/aw883xx_device.c | 1734 ++++++++++++++ > sound/soc/codecs/aw883xx/aw883xx_device.h | 557 +++++ > sound/soc/codecs/aw883xx/aw883xx_init.c | 649 ++++++ > sound/soc/codecs/aw883xx/aw883xx_log.h | 32 + > sound/soc/codecs/aw883xx/aw883xx_monitor.c | 1197 ++++++++++ > sound/soc/codecs/aw883xx/aw883xx_monitor.h | 167 ++ > sound/soc/codecs/aw883xx/aw883xx_pid_2049_reg.h | 2388 ++++++++++++++++++++ > sound/soc/codecs/aw883xx/aw883xx_spin.c | 807 +++++++ > sound/soc/codecs/aw883xx/aw883xx_spin.h | 80 + > 18 files changed, 14954 insertions(+) This should as I previously suggested probably be split into multiple patches, adding the build integration at the end to preserve bisection. Answer: I modified and sent the patch to patch v3 as required > +#define AW883XX_DRIVER_VERSION "v1.3.0" The versioning for the overall kernel is enough, we don't track versions for individual drivers. Answer: I will fix this problem in patch v3. > +static unsigned int g_aw883xx_dev_cnt; static > +DEFINE_MUTEX(g_aw883xx_lock); static struct aw_container > +*g_awinic_cfg; If you're using globals there's something wrong with your driver design, everything should be working with the driver data. Answer: I will not use global variables in patch v3 > +static const char *const aw883xx_switch[] = {"Disable", "Enable"}; Simple on/off switches should be normal Switch controls not enums. Answer: I will fix this problem in patch v3. > +static int aw883xx_platform_init(struct aw883xx *aw883xx) { #ifdef > +AW_QCOM_PLATFORM > + aw883xx->aw_pa->platform = AW_QCOM; > + return 0; > +#elif defined AW_MTK_PLATFORM > + aw883xx->aw_pa->platform = AW_MTK; > + return 0; > +#elif defined AW_SPRD_PLATFORM > + aw883xx->aw_pa->platform = AW_SPRD; > + return 0; > +#else > + return -EINVAL; > +#endif > +} This is not how we do configuration for platforms in Linux, we build single kernel images that support multiple systems so build time configuration really doesn't work, and in any case selecting by SoC vendor isn't going to account for how people design their individual boards. The driver should use some combination of DT and ACPI to figure out whatever configuration it needs, though a lot of what's here looks like it should just be in the machine driver. The driver for a device should just support that device. Answer: I will fix this problem in patch v3. > +/* > + * aw883xx append suffix sound channel information */ static void > +*aw883xx_devm_kstrdup(struct device *dev, char *buf) { > + char *str = NULL; > + > + str = devm_kzalloc(dev, strlen(buf) + 1, GFP_KERNEL); > + if (!str) > + return str; > + memcpy(str, buf, strlen(buf)); > + return str; > +} This is just devm_kstrdup(). Answer: I will fix this problem in patch v3. > +/* > + * aw883xx distinguish between codecs and components by version */ > +#ifdef AW_KERNEL_VER_OVER_4_19_1 Don't include any support for old kernels in upstream submissions. Answer: I will fix this problem in patch v3. > +int aw883xx_i2c_writes(struct aw883xx *aw883xx, > + uint8_t reg_addr, uint8_t *buf, uint16_t len) { > + int ret = -1; > + uint8_t *data = NULL; > + > + data = kmalloc(len + 1, GFP_KERNEL); > + if (data == NULL) > + return -ENOMEM; > + > + data[0] = reg_addr; > + memcpy(&data[1], buf, len); > + > + ret = i2c_master_send(aw883xx->i2c, data, len + 1); I can't see why this isn't just using regmap. All the actual I/O appears to be via a 16 bit regmap. Answer: I will fix this problem in patch v3. > +/* > + * aw883xx interrupt > + */ > +static void aw883xx_interrupt_work(struct work_struct *work) { > + struct aw883xx *aw883xx = container_of(work, > + struct aw883xx, interrupt_work.work); > + int16_t reg_value = 0; > + int ret = -1; > + > + aw_dev_info(aw883xx->dev, "enter"); The driver should be quiet in normal operation unless thre is some error or it's reading back identifying information from the hardware. Answer: I will fix this problem in patch v3. > + /*read reg value*/ > + ret = aw883xx_dev_get_int_status(aw883xx->aw_pa, &reg_value); > + if (ret < 0) > + aw_dev_err(aw883xx->dev, "get init_reg value failed"); > + else > + aw_dev_info(aw883xx->dev, "int value 0x%x", reg_value); > + > + /*clear init reg*/ > + aw883xx_dev_clear_int_status(aw883xx->aw_pa); > + > + /*unmask interrupt*/ > + aw883xx_dev_set_intmask(aw883xx->aw_pa, true); } This doesn't actually do anything with the interrupt? Answer: I will fix this problem in patch v3. > +static int aw883xx_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) > +{ > + aw_snd_soc_codec_t *codec = aw883xx_get_codec(dai); > + > + aw_dev_info(codec->dev, "fmt=0x%x", fmt); > + > + return 0; > +} Remove empty functions, that includes that only log information. Answer: I will fix this problem in patch v3. > +static int aw883xx_mute(struct snd_soc_dai *dai, int mute, int > +stream) { > + aw_snd_soc_codec_t *codec = aw883xx_get_codec(dai); > + struct aw883xx *aw883xx = > + aw_componet_codec_ops.codec_get_drvdata(codec); > + > + aw_dev_info(aw883xx->dev, "mute state=%d", mute); > + > + if (stream != SNDRV_PCM_STREAM_PLAYBACK) { > + aw_dev_info(aw883xx->dev, "capture"); > + return 0; > + } > + > + if (mute) { > + aw883xx->pstream = AW883XX_STREAM_CLOSE; > + cancel_delayed_work_sync(&aw883xx->start_work); > + mutex_lock(&aw883xx->lock); > + aw883xx_device_stop(aw883xx->aw_pa); > + mutex_unlock(&aw883xx->lock); > + } else { > + aw883xx->pstream = AW883XX_STREAM_OPEN; > + mutex_lock(&aw883xx->lock); > + aw883xx_start(aw883xx, AW_ASYNC_START); > + aw883xx_hold_dsp_spin_st(&aw883xx->aw_pa->spin_desc); > + mutex_unlock(&aw883xx->lock); > + } This doesn't look like a mute operation, it looks like it's starting and stopping the DSP. Answer: This is a mute operation ,aw883xx_device_stop is called in th aw883xx_mute function. This function not only executes the mute function aw883xx_dev_mute, but also disables dsp and power down. This is for the aw883xx chip low power optimization. > +static int aw883xx_dai_drv_append_suffix(struct aw883xx *aw883xx, > + struct snd_soc_dai_driver *dai_drv, > + int num_dai) I can't tell what all this append_suffix() stuff is supposed to do but it should probably be removed, if this is about distinguishing multiple devices then the core already has support for that which should be used instead. Answer:I will delete this part in patch v3 > +static int aw883xx_set_fade_in_time(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) { > + struct soc_mixer_control *mc = > + (struct soc_mixer_control *)kcontrol->private_value; > + > + if ((ucontrol->value.integer.value[0] > mc->max) || > + (ucontrol->value.integer.value[0] < mc->min)) { > + aw_pr_dbg("set val %ld overflow %d or less than :%d", > + ucontrol->value.integer.value[0], mc->max, mc->min); > + return 0; > + } > + aw883xx_dev_set_fade_time(ucontrol->value.integer.value[0], true); > + > + aw_pr_dbg("step time %ld", ucontrol->value.integer.value[0]); > + return 0; > +} If a control write changes a value it should return 1, you should run the mixer-test selftest which will identify this and a number of other issues. Answer: Could you tell me what is mixer-test selftest? I have checked other drivers, and there is no return 1. > +static int aw883xx_request_firmware_file(struct aw883xx *aw883xx) { > + const struct firmware *cont = NULL; > + struct aw_container *aw_cfg = NULL; > + int ret = -1; > + int i; > + > + aw883xx->aw_pa->fw_status = AW_DEV_FW_FAILED; > + > + for (i = 0; i < AW_REQUEST_FW_RETRIES; i++) { > + ret = request_firmware(&cont, AW883XX_ACF_FILE, aw883xx->dev); Why are there retries here? Other drivers that load firmware don't do that, and if it's required I'd expect the firmware framework to do it rather than have individual drivers open code it. Answer: I'll remove the retry code, and fix this problem in patch v3. > +static void aw883xx_fw_wrok(struct work_struct *work) { wrok? Answer: I will fix this problem in patch v3. > +static void aw883xx_load_fw(struct aw883xx *aw883xx) { #ifdef > +AW_SYNC_LOAD > + /*sync loading*/ > + aw883xx_request_firmware_file(aw883xx); > +#else > + /*async loading*/ > + queue_delayed_work(aw883xx->work_queue, > + &aw883xx->acf_work, > + msecs_to_jiffies(AW883XX_LOAD_FW_DELAY_TIME)); > +#endif Why would this be a build time configuration? Answer: I will fix this problem in patch v3. > +/* flag addr_bytes data_bytes reg_num reg_addr*/ static int > +aw883xx_awrw_parse_buf(struct aw883xx *aw883xx, const char *buf, > +size_t count) { Remove all these sysfs APIs, they appear to be a combination of open coding features you shuld get from regmap and providing audio controls which should go via the ALSA API. Answer: I will fix this problem in patch v3. > +#define BigLittleSwap16(A) ((((uint16_t)(A) & 0xff00) >> 8) | \ > + (((uint16_t)(A) & 0x00ff) << 8)) > + > +#define BigLittleSwap32(A) ((((uint32_t)(A) & 0xff000000) >> 24) | \ > + (((uint32_t)(A) & 0x00ff0000) >> 8) | \ > + (((uint32_t)(A) & 0x0000ff00) << 8) | \ > + (((uint32_t)(A) & 0x000000ff) << 24)) Don't open code stuff like this, use the standard kernel interfaces and use naming styles that follow the kernel coding style. Answer: I will fix this problem in patch v3. > +static int aw_check_data_version(struct aw_bin *bin, int bin_num) { > + int i = 0; > + > + DBG("enter\n"); Use the kernel logging framework (though as previously indicated you shouldn't be logging noise like this). Answer: I will delete this part in patch v3. > +static void aw_fs_write(struct file *file, char *buf, size_t count, > +loff_t *pos) { #ifdef AW_KERNEL_VER_OVER_5_4_0 > + kernel_write(file, buf, count, pos); #else > + vfs_write(file, buf, count, pos); > +#endif > +} It is extremely worrying to see filesystem operations in a CODEC driver, this needs a very clear explanation as to what it's doing and why. Answer: I will delete this part in patch v3.

2 1

[PATCH -next] ALSA: hda/realtek: simplify the return of comp_bind()
by Yang Yingliang 17 Oct '22

17 Oct '22

After commit 23904f7b2518 ("ALSA: hda: cs35l41: Remove suspend/resume hda hooks"), the return of comp_bind() can be simplified. No functional changed. Signed-off-by: Yang Yingliang <yangyingliang(a)huawei.com> --- sound/pci/hda/patch_realtek.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index e6c4bb5fa041..7c177426bf30 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -6654,13 +6654,8 @@ static int comp_bind(struct device *dev) { struct hda_codec *cdc = dev_to_hda_codec(dev); struct alc_spec *spec = cdc->spec; - int ret; - ret = component_bind_all(dev, spec->comps); - if (ret) - return ret; - - return 0; + return component_bind_all(dev, spec->comps); } static void comp_unbind(struct device *dev) -- 2.25.1

2 1