Skip to contentSkip to conversation

ASoC: max98090: Simplify max98090_readable_register implementation

PATCH In review Axel Lin 3 participants 4 messages Jul 27, 2015 – Jul 29, 2015

Subsystems (2)
lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 ASoC: max98090: Simplify max98090_readable_register implementation

The readable registers are in consecutive ranges:

	0x01 ~ 0x03, 0x0D ~ 0xD1, 0xFF

So simplify the implementation by specifying a range of consecutive values in a single case label.

---
 sound/soc/codecs/max98090.c | 71 ++-------------------------------------------
 1 file changed, 2 insertions(+), 69 deletions(-)
sound/soc/codecs/max98090.c +2 -69
index 91c9f9f..315c71f 100644
@@ -267,75 +267,8 @@ static bool max98090_volatile_register(struct device *dev, unsigned int reg)
 static bool max98090_readable_register(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
-	case M98090_REG_DEVICE_STATUS:
-	case M98090_REG_JACK_STATUS:
-	case M98090_REG_INTERRUPT_S:
-	case M98090_REG_RESERVED:
-	case M98090_REG_LINE_INPUT_CONFIG:
-	case M98090_REG_LINE_INPUT_LEVEL:
-	case M98090_REG_INPUT_MODE:
-	case M98090_REG_MIC1_INPUT_LEVEL:
-	case M98090_REG_MIC2_INPUT_LEVEL:
-	case M98090_REG_MIC_BIAS_VOLTAGE:
-	case M98090_REG_DIGITAL_MIC_ENABLE:
-	case M98090_REG_DIGITAL_MIC_CONFIG:
-	case M98090_REG_LEFT_ADC_MIXER:
-	case M98090_REG_RIGHT_ADC_MIXER:
-	case M98090_REG_LEFT_ADC_LEVEL:
-	case M98090_REG_RIGHT_ADC_LEVEL:
-	case M98090_REG_ADC_BIQUAD_LEVEL:
-	case M98090_REG_ADC_SIDETONE:
-	case M98090_REG_SYSTEM_CLOCK:
-	case M98090_REG_CLOCK_MODE:
-	case M98090_REG_CLOCK_RATIO_NI_MSB:
-	case M98090_REG_CLOCK_RATIO_NI_LSB:
-	case M98090_REG_CLOCK_RATIO_MI_MSB:
-	case M98090_REG_CLOCK_RATIO_MI_LSB:
-	case M98090_REG_MASTER_MODE:
-	case M98090_REG_INTERFACE_FORMAT:
-	case M98090_REG_TDM_CONTROL:
-	case M98090_REG_TDM_FORMAT:
-	case M98090_REG_IO_CONFIGURATION:
-	case M98090_REG_FILTER_CONFIG:
-	case M98090_REG_DAI_PLAYBACK_LEVEL:
-	case M98090_REG_DAI_PLAYBACK_LEVEL_EQ:
-	case M98090_REG_LEFT_HP_MIXER:
-	case M98090_REG_RIGHT_HP_MIXER:
-	case M98090_REG_HP_CONTROL:
-	case M98090_REG_LEFT_HP_VOLUME:
-	case M98090_REG_RIGHT_HP_VOLUME:
-	case M98090_REG_LEFT_SPK_MIXER:
-	case M98090_REG_RIGHT_SPK_MIXER:
-	case M98090_REG_SPK_CONTROL:
-	case M98090_REG_LEFT_SPK_VOLUME:
-	case M98090_REG_RIGHT_SPK_VOLUME:
-	case M98090_REG_DRC_TIMING:
-	case M98090_REG_DRC_COMPRESSOR:
-	case M98090_REG_DRC_EXPANDER:
-	case M98090_REG_DRC_GAIN:
-	case M98090_REG_RCV_LOUTL_MIXER:
-	case M98090_REG_RCV_LOUTL_CONTROL:
-	case M98090_REG_RCV_LOUTL_VOLUME:
-	case M98090_REG_LOUTR_MIXER:
-	case M98090_REG_LOUTR_CONTROL:
-	case M98090_REG_LOUTR_VOLUME:
-	case M98090_REG_JACK_DETECT:
-	case M98090_REG_INPUT_ENABLE:
-	case M98090_REG_OUTPUT_ENABLE:
-	case M98090_REG_LEVEL_CONTROL:
-	case M98090_REG_DSP_FILTER_ENABLE:
-	case M98090_REG_BIAS_CONTROL:
-	case M98090_REG_DAC_CONTROL:
-	case M98090_REG_ADC_CONTROL:
-	case M98090_REG_DEVICE_SHUTDOWN:
-	case M98090_REG_EQUALIZER_BASE ... M98090_REG_EQUALIZER_BASE + 0x68:
-	case M98090_REG_RECORD_BIQUAD_BASE ... M98090_REG_RECORD_BIQUAD_BASE + 0x0E:
-	case M98090_REG_DMIC3_VOLUME:
-	case M98090_REG_DMIC4_VOLUME:
-	case M98090_REG_DMIC34_BQ_PREATTEN:
-	case M98090_REG_RECORD_TDM_SLOT:
-	case M98090_REG_SAMPLE_RATE:
-	case M98090_REG_DMIC34_BIQUAD_BASE ... M98090_REG_DMIC34_BIQUAD_BASE + 0x0E:
+	case M98090_REG_DEVICE_STATUS ... M98090_REG_INTERRUPT_S:
+	case M98090_REG_LINE_INPUT_CONFIG ... 0xD1:
 	case M98090_REG_REVISION_ID:
 		return true;
 	default:
Signature · 1 line
2.1.0
Reviewed-by Tested-by on v1 1/1
#1 lore raw
Quoted 10 lines · Axel Lin

The readable registers are in consecutive ranges:

	0x01 ~ 0x03, 0x0D ~ 0xD1, 0xFF

So simplify the implementation by specifying a range of consecutive values in a single case label.

---
  sound/soc/codecs/max98090.c | 71 ++-------------------------------------------
  1 file changed, 2 insertions(+), 69 deletions(-)

One minor comment below.

sound/soc/codecs/max98090.c static bool max98090_volatile_register(struct device *dev, unsigned int reg) · line 273

Quoted 5 lines
sound/soc/codecs/max98090.c +2 -1
-	case M98090_REG_DMIC34_BIQUAD_BASE ... M98090_REG_DMIC34_BIQUAD_BASE + 0x0E:
+	case M98090_REG_DEVICE_STATUS ... M98090_REG_INTERRUPT_S:
+	case M98090_REG_LINE_INPUT_CONFIG ... 0xD1:
  	case M98090_REG_REVISION_ID:
  		return true;

I'd have preserved that magic M98090_REG_DMIC34_BIQUAD_BASE + 0x0E here. Last register before 0xff is 0xbd in the specification, M98090_REG_DMIC34_BIQUAD_BASE is 0xc3 in the max98090.h so it seems to me that 0xc3 ~ 0xd1 are undocumented registers.

Signature · 1 line
Jarkko
author
#2 lore raw

2015-07-27 18:08 GMT+08:00 Jarkko Nikula <[email protected]>:

Quoted 7 lines · Axel Lin
Quoted 6 lines

The readable registers are in consecutive ranges:

        0x01 ~ 0x03, 0x0D ~ 0xD1, 0xFF

So simplify the implementation by specifying a range of consecutive values in a single case label.

Quoted 8 lines
Quoted 6 lines
---
  sound/soc/codecs/max98090.c | 71
++-------------------------------------------
  1 file changed, 2 insertions(+), 69 deletions(-)

Thanks.

Quoted 11 lines

One minor comment below.

sound/soc/codecs/max98090.c static bool max98090_volatile_register(struct device *dev, unsigned int reg) · line 273

Quoted 6 lines

- case M98090_REG_DMIC34_BIQUAD_BASE ...
M98090_REG_DMIC34_BIQUAD_BASE + 0x0E:

sound/soc/codecs/max98090.c +2 -0
+       case M98090_REG_DEVICE_STATUS ... M98090_REG_INTERRUPT_S:
+       case M98090_REG_LINE_INPUT_CONFIG ... 0xD1:
        case M98090_REG_REVISION_ID:
                return true;
Quoted 4 lines

I'd have preserved that magic M98090_REG_DMIC34_BIQUAD_BASE + 0x0E here. Last register before 0xff is 0xbd in the specification, M98090_REG_DMIC34_BIQUAD_BASE is 0xc3 in the max98090.h so it seems to me that 0xc3 ~ 0xd1 are undocumented registers.

0xc3 ~ 0xd1 are undocumented registers for MAX98090, but it is documented in MAX98091 datasheet (SECONDARY RECORD PATH BIQUAD COEFFICIENT REGISTERS).

Regards,
Axel

maintainer
#3 lore raw

Applied "ASoC: max98090: Simplify max98090_readable_register implementation" to the asoc tree

In reply to Axel Lin

The patch

ASoC: max98090: Simplify max98090_readable_register implementation

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 62d6d47cb8bd5ace08a90a1cd78f6908e9f9c2d3 Mon Sep 17 00:00:00 2001

From: Axel Lin <[email protected]>
Date: Mon, 27 Jul 2015 09:39:43 +0800
Subject: [PATCH] ASoC: max98090: Simplify max98090_readable_register
implementation

The readable registers are in consecutive ranges:

	0x01 ~ 0x03, 0x0D ~ 0xD1, 0xFF

So simplify the implementation by specifying a range of consecutive values in a single case label.

---
 sound/soc/codecs/max98090.c | 71 ++-------------------------------------------
 1 file changed, 2 insertions(+), 69 deletions(-)
sound/soc/codecs/max98090.c +2 -69
index 1697340..17ab597 100644
@@ -267,75 +267,8 @@ static bool max98090_volatile_register(struct device *dev, unsigned int reg)
 static bool max98090_readable_register(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
-	case M98090_REG_DEVICE_STATUS:
-	case M98090_REG_JACK_STATUS:
-	case M98090_REG_INTERRUPT_S:
-	case M98090_REG_RESERVED:
-	case M98090_REG_LINE_INPUT_CONFIG:
-	case M98090_REG_LINE_INPUT_LEVEL:
-	case M98090_REG_INPUT_MODE:
-	case M98090_REG_MIC1_INPUT_LEVEL:
-	case M98090_REG_MIC2_INPUT_LEVEL:
-	case M98090_REG_MIC_BIAS_VOLTAGE:
-	case M98090_REG_DIGITAL_MIC_ENABLE:
-	case M98090_REG_DIGITAL_MIC_CONFIG:
-	case M98090_REG_LEFT_ADC_MIXER:
-	case M98090_REG_RIGHT_ADC_MIXER:
-	case M98090_REG_LEFT_ADC_LEVEL:
-	case M98090_REG_RIGHT_ADC_LEVEL:
-	case M98090_REG_ADC_BIQUAD_LEVEL:
-	case M98090_REG_ADC_SIDETONE:
-	case M98090_REG_SYSTEM_CLOCK:
-	case M98090_REG_CLOCK_MODE:
-	case M98090_REG_CLOCK_RATIO_NI_MSB:
-	case M98090_REG_CLOCK_RATIO_NI_LSB:
-	case M98090_REG_CLOCK_RATIO_MI_MSB:
-	case M98090_REG_CLOCK_RATIO_MI_LSB:
-	case M98090_REG_MASTER_MODE:
-	case M98090_REG_INTERFACE_FORMAT:
-	case M98090_REG_TDM_CONTROL:
-	case M98090_REG_TDM_FORMAT:
-	case M98090_REG_IO_CONFIGURATION:
-	case M98090_REG_FILTER_CONFIG:
-	case M98090_REG_DAI_PLAYBACK_LEVEL:
-	case M98090_REG_DAI_PLAYBACK_LEVEL_EQ:
-	case M98090_REG_LEFT_HP_MIXER:
-	case M98090_REG_RIGHT_HP_MIXER:
-	case M98090_REG_HP_CONTROL:
-	case M98090_REG_LEFT_HP_VOLUME:
-	case M98090_REG_RIGHT_HP_VOLUME:
-	case M98090_REG_LEFT_SPK_MIXER:
-	case M98090_REG_RIGHT_SPK_MIXER:
-	case M98090_REG_SPK_CONTROL:
-	case M98090_REG_LEFT_SPK_VOLUME:
-	case M98090_REG_RIGHT_SPK_VOLUME:
-	case M98090_REG_DRC_TIMING:
-	case M98090_REG_DRC_COMPRESSOR:
-	case M98090_REG_DRC_EXPANDER:
-	case M98090_REG_DRC_GAIN:
-	case M98090_REG_RCV_LOUTL_MIXER:
-	case M98090_REG_RCV_LOUTL_CONTROL:
-	case M98090_REG_RCV_LOUTL_VOLUME:
-	case M98090_REG_LOUTR_MIXER:
-	case M98090_REG_LOUTR_CONTROL:
-	case M98090_REG_LOUTR_VOLUME:
-	case M98090_REG_JACK_DETECT:
-	case M98090_REG_INPUT_ENABLE:
-	case M98090_REG_OUTPUT_ENABLE:
-	case M98090_REG_LEVEL_CONTROL:
-	case M98090_REG_DSP_FILTER_ENABLE:
-	case M98090_REG_BIAS_CONTROL:
-	case M98090_REG_DAC_CONTROL:
-	case M98090_REG_ADC_CONTROL:
-	case M98090_REG_DEVICE_SHUTDOWN:
-	case M98090_REG_EQUALIZER_BASE ... M98090_REG_EQUALIZER_BASE + 0x68:
-	case M98090_REG_RECORD_BIQUAD_BASE ... M98090_REG_RECORD_BIQUAD_BASE + 0x0E:
-	case M98090_REG_DMIC3_VOLUME:
-	case M98090_REG_DMIC4_VOLUME:
-	case M98090_REG_DMIC34_BQ_PREATTEN:
-	case M98090_REG_RECORD_TDM_SLOT:
-	case M98090_REG_SAMPLE_RATE:
-	case M98090_REG_DMIC34_BIQUAD_BASE ... M98090_REG_DMIC34_BIQUAD_BASE + 0x0E:
+	case M98090_REG_DEVICE_STATUS ... M98090_REG_INTERRUPT_S:
+	case M98090_REG_LINE_INPUT_CONFIG ... 0xD1:
 	case M98090_REG_REVISION_ID:
 		return true;
 	default:
Signature · 1 line
2.1.4