On 1 August 2018 at 16:14, Paolo Bonzini <pbonz...@redhat.com> wrote:
> Many of these are marked as "intentional/fix required" because they
> just need adding a fall through comment. This is exactly what this
> patch does, except for target/mips/translate.c where it is easier to
> duplicate the code, and hw/audio/sb16.c where I consulted the DOSBox
> sources and decide to just remove the LOG_UNIMP before the fallthrough.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
> index 5a4d32364e..665a7e75c5 100644
> --- a/hw/audio/sb16.c
> +++ b/hw/audio/sb16.c
> @@ -741,10 +741,8 @@ static void complete (SB16State *s)
> ldebug ("set time const %d\n", s->time_const);
> break;
>
> - case 0x42: /* FT2 sets output freq with this, go figure
> */
> - qemu_log_mask(LOG_UNIMP, "cmd 0x42 might not do what it think it"
> - " should\n");
> case 0x41:
> + case 0x42: /* FT2 sets output freq with this, go figure
> */
> s->freq = dsp_get_hilo (s);
> ldebug ("set freq %d\n", s->freq);
> break;
I agree with the outcome; see previous discussions
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02081.html
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02079.html
and this description of what the hardware does:
http://homepages.cae.wisc.edu/~brodskye/sb16doc/sb16doc.html#SamplingRate
We could improve the somewhat cryptic comment:
/*
* 0x41 is documented as setting the output sample rate,
* and 0x42 the input sample rate, but in fact SB16 hardware
* seems to have only a single sample rate under the hood,
* and some (buggy) guest programs such as FT2 will set the
* output rate using 0x42. Compare:
* http://homepages.cae.wisc.edu/~brodskye/sb16doc/sb16doc.html#SamplingRate
*/
thanks
-- PMM
