Re: [Qemu-devel] [PATCH] hw/audio/sb16.c: Convert file to new logging API

[Programmingkid]

> On Jan 30, 2018, at 4:41 AM, Gerd Hoffmann <address@hidden> wrote:
> 
>> @@ -148,15 +142,16 @@ static int irq_of_magic (int magic)
>> #if 0
>> static void log_dsp (SB16State *dsp)
>> {
>> -    ldebug ("%s:%s:%d:%s:dmasize=%d:freq=%d:const=%d:speaker=%d\n",
>> -            dsp->fmt_stereo ? "Stereo" : "Mono",
>> -            dsp->fmt_signed ? "Signed" : "Unsigned",
>> -            dsp->fmt_bits,
>> -            dsp->dma_auto ? "Auto" : "Single",
>> -            dsp->block_size,
>> -            dsp->freq,
>> -            dsp->time_const,
>> -            dsp->speaker);
>> +    qemu_log_mask(LOG_UNIMP, "%s:%s:%d:%s:dmasize=%d:freq=%d:const=%d:"
>> +                  "speaker=%d\n",
>> +                  dsp->fmt_stereo ? "Stereo" : "Mono",
>> +                  dsp->fmt_signed ? "Signed" : "Unsigned",
>> +                  dsp->fmt_bits,
>> +                  dsp->dma_auto ? "Auto" : "Single",
>> +                  dsp->block_size,
>> +                  dsp->freq,
>> +                  dsp->time_const,
>> +                  dsp->speaker);
>> }
>> #endif
> 
> Hmm, dead code.  Any places which call log_dsp() ?

There are several places but they are all dead code. Do you want this removed?

> 
>>         case 0x42:              /* FT2 sets output freq with this, go figure 
>> */
>> #if 0
>> -            dolog ("cmd 0x42 might not do what it think it should\n");
>> +            qemu_log_mask(LOG_UNIMP, "cmd 0x42 might not do what it think 
>> it"
>> +                          " should\n");
>> #endif
> 
> More dead code.

Would you like this enabled or removed?

> 
>>         case 0xe2:
>> #ifdef DEBUG
>>             d0 = dsp_get_data (s);
>> -            dolog ("E2 = %#x\n", d0);
>> +            qemu_log_mask(LOG_UNIMP, "E2 = %#x\n", d0);
>> #endif
> 
> Conditional code.  Enable this unconditionally, now that we can switch
> the logging at runtime?
> 
>> #ifndef DEBUG_SB16_MOST
>>     if (s->mixer_nreg != 0x82) {
>> -        ldebug ("mixer_read[%#x] -> %#x\n",
>> -                s->mixer_nreg, s->mixer_regs[s->mixer_nreg]);
>> +        qemu_log_mask(LOG_UNIMP, "mixer_read[%#x] -> %#x\n", s->mixer_nreg,
>> +                      s->mixer_regs[s->mixer_nreg]);
>>     }
>> #else
>> -    ldebug ("mixer_read[%#x] -> %#x\n",
>> -            s->mixer_nreg, s->mixer_regs[s->mixer_nreg]);
>> +    qemu_log_mask(LOG_UNIMP, "mixer_read[%#x] -> %#x\n",
>> +                  s->mixer_nreg, s->mixer_regs[s->mixer_nreg]);
>> #endif
> 
> Same question here.

You want all of this enabled?

> 
>> #ifdef DEBUG_SB16_MOST
>> -    dolog ("pos:%06d %d till:%d len:%d\n",
>> -           dma_pos, free, till, dma_len);
>> +    qemu_log_mask(LOG_UNIMP, "pos:%06d %d till:%d len:%d\n", dma_pos, free,
>> +                  till, dma_len);
>> #endif
> 
> And here.
> 
>> #ifdef DEBUG_SB16_MOST
>> -    ldebug ("pos %5d free %5d size %5d till % 5d copy %5d written %5d size 
>> %5d\n",
>> -            dma_pos, free, dma_len, s->left_till_irq, copy, written,
>> -            s->block_size);
>> +    qemu_log_mask(LOG_UNIMP, "pos %5d free %5d size %5d till % 5d copy %5d"
>> +                  " written %5d size %5d\n", dma_pos, free, dma_len,
>> +                  s->left_till_irq, copy, written, s->block_size);
>> #endif
> 
> Again.
> 
> cheers,
>  Gerd

Thank you.