[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/3] hw/audio/ac97: Coding style fixes to avoid checkpatch
|
From: |
Víctor Colombo |
|
Subject: |
Re: [PATCH v2 1/3] hw/audio/ac97: Coding style fixes to avoid checkpatch errors |
|
Date: |
Fri, 22 Apr 2022 17:37:27 -0300 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 |
On 22/04/2022 12:14, BALATON Zoltan wrote:
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/audio/ac97.c | 727 ++++++++++++++++++++++++------------------------
1 file changed, 357 insertions(+), 370 deletions(-)
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 3cb8131060..0b1d8ce9c6 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -87,39 +87,39 @@ enum {
#define GC_CR 2 /* rw */
#define GC_VALID_MASK ((1 << 6) - 1)
-#define GS_MD3 (1<<17) /* rw */
-#define GS_AD3 (1<<16) /* rw */
-#define GS_RCS (1<<15) /* rwc */
-#define GS_B3S12 (1<<14) /* ro */
-#define GS_B2S12 (1<<13) /* ro */
-#define GS_B1S12 (1<<12) /* ro */
-#define GS_S1R1 (1<<11) /* rwc */
-#define GS_S0R1 (1<<10) /* rwc */
-#define GS_S1CR (1<<9) /* ro */
-#define GS_S0CR (1<<8) /* ro */
-#define GS_MINT (1<<7) /* ro */
-#define GS_POINT (1<<6) /* ro */
-#define GS_PIINT (1<<5) /* ro */
-#define GS_RSRVD ((1<<4)|(1<<3))
-#define GS_MOINT (1<<2) /* ro */
-#define GS_MIINT (1<<1) /* ro */
+#define GS_MD3 (1 << 17) /* rw */
+#define GS_AD3 (1 << 16) /* rw */
+#define GS_RCS (1 << 15) /* rwc */
+#define GS_B3S12 (1 << 14) /* ro */
+#define GS_B2S12 (1 << 13) /* ro */
+#define GS_B1S12 (1 << 12) /* ro */
+#define GS_S1R1 (1 << 11) /* rwc */
+#define GS_S0R1 (1 << 10) /* rwc */
+#define GS_S1CR (1 << 9) /* ro */
+#define GS_S0CR (1 << 8) /* ro */
+#define GS_MINT (1 << 7) /* ro */
+#define GS_POINT (1 << 6) /* ro */
+#define GS_PIINT (1 << 5) /* ro */
+#define GS_RSRVD ((1 << 4) | (1 << 3))
+#define GS_MOINT (1 << 2) /* ro */
+#define GS_MIINT (1 << 1) /* ro */
#define GS_GSCI 1 /* rwc */
What do you think about aligning the `/*` in the lines above?
It was aligned before.
...
- mixer_store (s, AC97_Reset , 0x0000); /* 6940 */
- mixer_store (s, AC97_Headphone_Volume_Mute , 0x0000);
- mixer_store (s, AC97_Master_Volume_Mono_Mute , 0x0000);
- mixer_store (s, AC97_Master_Tone_RL, 0x0000);
- mixer_store (s, AC97_PC_BEEP_Volume_Mute , 0x0000);
- mixer_store (s, AC97_Phone_Volume_Mute , 0x0000);
- mixer_store (s, AC97_Mic_Volume_Mute , 0x0000);
- mixer_store (s, AC97_Line_In_Volume_Mute , 0x0000);
- mixer_store (s, AC97_CD_Volume_Mute , 0x0000);
- mixer_store (s, AC97_Video_Volume_Mute , 0x0000);
- mixer_store (s, AC97_Aux_Volume_Mute , 0x0000);
- mixer_store (s, AC97_Record_Gain_Mic_Mute , 0x0000);
- mixer_store (s, AC97_General_Purpose , 0x0000);
- mixer_store (s, AC97_3D_Control , 0x0000);
- mixer_store (s, AC97_Powerdown_Ctrl_Stat , 0x000f);
+ dolog("mixer_reset\n");
+ memset(s->mixer_data, 0, sizeof(s->mixer_data));
+ memset(active, 0, sizeof(active));
+ mixer_store(s, AC97_Reset , 0x0000); /* 6940 */
+ mixer_store(s, AC97_Headphone_Volume_Mute , 0x0000);
+ mixer_store(s, AC97_Master_Volume_Mono_Mute , 0x0000);
+ mixer_store(s, AC97_Master_Tone_RL, 0x0000);
It was already like this before, but I think this might be a good
oportunity to fix this spaces before comma inconsistency here.
Personally I think the best approach would be to make all of them
like `AC97_Master_Tone_RL,` instead of something like
`AC97_Master_Tone_RL <multiple spaces> ,`
+ mixer_store(s, AC97_PC_BEEP_Volume_Mute , 0x0000);
+ mixer_store(s, AC97_Phone_Volume_Mute , 0x0000);
+ mixer_store(s, AC97_Mic_Volume_Mute , 0x0000);
+ mixer_store(s, AC97_Line_In_Volume_Mute , 0x0000);
+ mixer_store(s, AC97_CD_Volume_Mute , 0x0000);
+ mixer_store(s, AC97_Video_Volume_Mute , 0x0000);
+ mixer_store(s, AC97_Aux_Volume_Mute , 0x0000);
+ mixer_store(s, AC97_Record_Gain_Mic_Mute , 0x0000);
+ mixer_store(s, AC97_General_Purpose , 0x0000);
+ mixer_store(s, AC97_3D_Control , 0x0000);
+ mixer_store(s, AC97_Powerdown_Ctrl_Stat , 0x000f);
/*
* Sigmatel 9700 (STAC9700)
*/
- mixer_store (s, AC97_Vendor_ID1 , 0x8384);
- mixer_store (s, AC97_Vendor_ID2 , 0x7600); /* 7608 */
-
- mixer_store (s, AC97_Extended_Audio_ID , 0x0809);
- mixer_store (s, AC97_Extended_Audio_Ctrl_Stat, 0x0009);
- mixer_store (s, AC97_PCM_Front_DAC_Rate , 0xbb80);
- mixer_store (s, AC97_PCM_Surround_DAC_Rate , 0xbb80);
- mixer_store (s, AC97_PCM_LFE_DAC_Rate , 0xbb80);
- mixer_store (s, AC97_PCM_LR_ADC_Rate , 0xbb80);
- mixer_store (s, AC97_MIC_ADC_Rate , 0xbb80);
-
- record_select (s, 0);
- set_volume (s, AC97_Master_Volume_Mute, 0x8000);
- set_volume (s, AC97_PCM_Out_Volume_Mute, 0x8808);
- set_volume (s, AC97_Record_Gain_Mute, 0x8808);
-
- reset_voices (s, active);
+ mixer_store(s, AC97_Vendor_ID1 , 0x8384);
+ mixer_store(s, AC97_Vendor_ID2 , 0x7600); /* 7608 */
+
+ mixer_store(s, AC97_Extended_Audio_ID , 0x0809);
+ mixer_store(s, AC97_Extended_Audio_Ctrl_Stat, 0x0009);
+ mixer_store(s, AC97_PCM_Front_DAC_Rate , 0xbb80);
+ mixer_store(s, AC97_PCM_Surround_DAC_Rate , 0xbb80);
+ mixer_store(s, AC97_PCM_LFE_DAC_Rate , 0xbb80);
+ mixer_store(s, AC97_PCM_LR_ADC_Rate , 0xbb80);
+ mixer_store(s, AC97_MIC_ADC_Rate , 0xbb80);
So I would change all these lines above to remove all this spacing
before the comma. But that's just my opinion.
...
@@ -1216,15 +1203,15 @@ static const VMStateDescription vmstate_ac97 = {
.minimum_version_id = 2,
.post_load = ac97_post_load,
.fields = (VMStateField[]) {
- VMSTATE_PCI_DEVICE (dev, AC97LinkState),
- VMSTATE_UINT32 (glob_cnt, AC97LinkState),
- VMSTATE_UINT32 (glob_sta, AC97LinkState),
- VMSTATE_UINT32 (cas, AC97LinkState),
- VMSTATE_STRUCT_ARRAY (bm_regs, AC97LinkState, 3, 1,
+ VMSTATE_PCI_DEVICE(dev, AC97LinkState),
+ VMSTATE_UINT32(glob_cnt, AC97LinkState),
+ VMSTATE_UINT32(glob_sta, AC97LinkState),
+ VMSTATE_UINT32(cas, AC97LinkState),
+ VMSTATE_STRUCT_ARRAY(bm_regs, AC97LinkState, 3, 1,
vmstate_ac97_bm_regs, AC97BusMasterRegs),
With the change above, this line is not correctly aligned with the
previous one anymore.
...
@@ -1373,13 +1360,13 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
c[PCI_INTERRUPT_LINE] = 0x00; /* intr_ln interrupt line rw */
c[PCI_INTERRUPT_PIN] = 0x01; /* intr_pn interrupt pin ro */
- memory_region_init_io (&s->io_nam, OBJECT(s), &ac97_io_nam_ops, s,
+ memory_region_init_io(&s->io_nam, OBJECT(s), &ac97_io_nam_ops, s,
"ac97-nam", 1024);
Same thing here...
- memory_region_init_io (&s->io_nabm, OBJECT(s), &ac97_io_nabm_ops, s,
+ memory_region_init_io(&s->io_nabm, OBJECT(s), &ac97_io_nabm_ops, s,
"ac97-nabm", 256);
...And here
- pci_register_bar (&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nam);
- pci_register_bar (&s->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nabm);
- AUD_register_card ("ac97", &s->card);
+ pci_register_bar(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nam);
+ pci_register_bar(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nabm);
+ AUD_register_card("ac97", &s->card);
ac97_on_reset(DEVICE(s));
}
...
Besides that, I confirmed that the checkpatch was giving errors before
and is not complaining anymore with this patch applied.
-- Víctor