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 20:42, Cezary Rojewski wrote:
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

Forgive me for double posting. Comment above is targeted towards:

>> +enum {
>> +    HP_POWER_EVENT,
>> +    DAC_POWER_EVENT,
>> +    ADC_POWER_EVENT,
>> +    LAST_POWER_EVENT
>> +};

as LAST_POWER_EVENT is not assigned explicitly to ADC_POWER_EVENT and thus generates implicit "new option" of value 3.

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