> On Feb 16, 2020, at 4:59 PM, BALATON Zoltan <bala...@eik.bme.hu> wrote: > > On Sun, 16 Feb 2020, Howard Spoelstra wrote: >> On Sun, Feb 16, 2020 at 5:32 PM John Arbuckle <programmingk...@gmail.com> >> wrote: >>> diff --git a/hw/audio/screamer.c b/hw/audio/screamer.c >>> new file mode 100644 >>> index 0000000000..ad4aba12eb >>> --- /dev/null >>> +++ b/hw/audio/screamer.c >>> @@ -0,0 +1,983 @@ >>> +/* >>> + * File: Screamer.c >>> + * Description: Implement the Screamer sound chip used in Apple >>> Macintoshes. >>> + * It works by filling a buffer, then playing the buffer. >>> + */ > > Do you need a copyright and license header here? Especially if this is not > all your original work but based on previous code (don't know if it is just > saying in case as I know Mark also had some similar patches before but not > sure how are those related if at all). If this contains code from somewhere > else then license and author of that code may need to be included too.
That is a good question. According to this page https://wiki.qemu.org/License, files that don't have licensing information default under the GNU GPL v2. I'm fine with that. > >>> +/* Called when the CPU writes to the memory addresses assigned to >>> Screamer */ >>> +static void screamer_mmio_write(void *opaque, hwaddr addr, uint64_t >>> raw_value, >>> + unsigned size) >>> +{ >>> + DPRINTF("screamer_mmio_write() called - size: %d\n", size); >>> + ScreamerState *state = opaque; >>> + uint32_t value = raw_value & 0xffffffff; >>> + addr = addr >> 4; >>> + >>> + switch (addr) { >>> + case SOUND_CONTROL_REG: >>> + set_sound_control_reg(state, value); >>> + break; >>> + case CODEC_CONTROL_REG: >>> + set_codec_control_reg(state, value); >>> + break; >>> + case CODEC_STATUS_REG: >>> + set_codec_status_reg(state, value); >>> + break; >>> + case CLIP_COUNT_REG: >>> + set_clip_count_reg(state, value); >>> + break; >>> + case BYTE_SWAP_REG: >>> + set_byte_swap_reg(state, value); >>> + break; >>> + case FRAME_COUNT_REG: >>> + set_frame_count_reg(state, value); >>> + break; >>> + default: >>> + DPRINTF("Unknown register write - addr:%llu\tvalue:%d\n", addr, >>> value); >>> + } >>> +} >>> >>> Hi, >> >> This patch will not compile without errors. Host is Fedora 31. >> The compiler suggests changing lines 839, 842 and 878 in screamer.c so the >> DPRINTF arguments use %lu instead of %llu. >> With that fixed, compiling completes succesfully. > > Replacing with %lu may fix 32bit build but would break 64bit one. Use > HWADDR_PRIx format string instead to print hwaddr but others will probably > tell to remove DPRINTFs alltogether when they are not needed any more and > replace the remaining few useful ones with traces if debugging is still > needed. I don't mind DPRINTFs that much at least until things are stable > enough but once the code is stable most DPRINTFs may not be needed any more. > > I can't really review the actual patch because I don't know audio in QEMU. > > Regards, > BALATON Zoltan Your HWADDR_PRIx suggestion was great. I am making a small patch to test out your suggestion. Thank you.