Re: [PATCH v5 2/6] ASoC: sgtl5000: Improve VAG power and mute control

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2019-07-18 11:02, Oleksandr Suvorov wrote:
+enum {
+	HP_POWER_EVENT,
+	DAC_POWER_EVENT,
+	ADC_POWER_EVENT,
+	LAST_POWER_EVENT
+};
+
+static u16 mute_mask[] = {
+	SGTL5000_HP_MUTE,
+	SGTL5000_OUTPUTS_MUTE,
+	SGTL5000_OUTPUTS_MUTE
+};

If mute_mask[] is only used within common handler, you may consider declaring const array within said handler instead (did not check that myself). Otherwise, simple comment for the second _OUTPUTS_MUTE should suffice - its not self explanatory why you doubled that mask.

+
  /* sgtl5000 private structure in codec */
  struct sgtl5000_priv {
  	int sysclk;	/* sysclk rate */
@@ -137,8 +157,109 @@ struct sgtl5000_priv {
  	u8 micbias_voltage;
  	u8 lrclk_strength;
  	u8 sclk_strength;
+	u16 mute_state[LAST_POWER_EVENT];
  };

When I spoke of LAST enum constant, I did not really had this specific usage in mind.

From design perspective, _LAST_ does not exist and should never be referred to as "the next option" i.e.: new enum constant.
That is way preferred usage is:
u16 mute_state[ADC_POWER_EVENT+1;
-or-
u16 mute_state[LAST_POWER_EVENT+1];

Maybe I'm just being radical here :)

Czarek
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux