Re: [PATCH 4/5] hw: Remove Marvell 88W8618 devices

On Thu, 20 Aug 2026 at 11:59, Philippe Mathieu-Daudé
<[email protected]> wrote:
>
> On 19/8/26 20:54, Peter Maydell wrote:
> > The mv88w8618 ethernet and audio devices were used only by the
> > now-removed musicpal machine, so can also be deleted.
> >
> > Signed-off-by: Peter Maydell <[email protected]>
> > ---
> >   hw/arm/Kconfig                 |   3 -
> >   hw/audio/marvell_88w8618.c     | 315 --------------------------
> >   hw/audio/meson.build           |   1 -
> >   hw/net/meson.build             |   1 -
> >   hw/net/mv88w8618_eth.c         | 403 ---------------------------------
> >   include/hw/net/mv88w8618_eth.h |  13 --
> >   6 files changed, 736 deletions(-)
> >   delete mode 100644 hw/audio/marvell_88w8618.c
> >   delete mode 100644 hw/net/mv88w8618_eth.c
> >   delete mode 100644 include/hw/net/mv88w8618_eth.h
>
>
> > -static void mv88w8618_audio_callback(void *opaque, int free_out, int free_in)
> > -{
> > -    mv88w8618_audio_state *s = opaque;
> > -    int16_t *codec_buffer;
> > -    QEMU_UNINITIALIZED int8_t buf[4096];
> > -    int8_t *mem_buffer;
> > -    int pos, block_size;
> > -
> > -    if (!(s->playback_mode & MP_AUDIO_PLAYBACK_EN)) {
> > -        return;
> > -    }
> > -    if (s->playback_mode & MP_AUDIO_16BIT_SAMPLE) {
> > -        free_out <<= 1;
> > -    }
> > -    if (!(s->playback_mode & MP_AUDIO_MONO)) {
> > -        free_out <<= 1;
> > -    }
> > -    block_size = s->threshold / 2;
> > -    if (free_out - s->last_free < block_size) {
> > -        return;
> > -    }
> > -    if (block_size > 4096) {
> > -        return;
> > -    }
> > -    physical_memory_read(s->target_buffer + s->play_pos, buf, block_size);
> > -    mem_buffer = buf;
> > -    if (s->playback_mode & MP_AUDIO_16BIT_SAMPLE) {
> > -        if (s->playback_mode & MP_AUDIO_MONO) {
> > -            codec_buffer = wm8750_dac_buffer(s->wm, block_size >> 1);
> > -            for (pos = 0; pos < block_size; pos += 2) {
> > -                *codec_buffer++ = *(int16_t *)mem_buffer;
> > -                *codec_buffer++ = *(int16_t *)mem_buffer;
>
> There seems to be an endianness bug here.

Not that it matters, since the code is being deleted, but I don't
think there is. In guest memory the sample data is 16-bit little
endian values; in this code path we copy it directly into the
DAC buffer. The wm8750 code then copies from the DAC buffer to
the audio backend, and it has created the backend with formats
with big_endian == false. This matches up with the 8-bit-sample
codepaths in this function, where because we calculate a
(host-order) 16-bit sample value from the guest's 8-bit sample
we need to use cpu_to_le16() to write the LE16 data into the DAC
buffer.

> Reviewed-by: Philippe Mathieu-Daudé <[email protected]>

thanks
-- PMM