On 5/31/20 9:41 PM, Peter Maydell wrote: > On Sun, 31 May 2020 at 18:54, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: >> >> Most CPUs can do 64-bit operations. Update the CPUReadMemoryFunc >> and CPUWriteMemoryFunc prototypes. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> >> --- >> include/exec/cpu-common.h | 4 ++-- >> hw/usb/hcd-musb.c | 12 ++++++------ >> 2 files changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h >> index b47e5630e7..5ac766e3b6 100644 >> --- a/include/exec/cpu-common.h >> +++ b/include/exec/cpu-common.h >> @@ -43,8 +43,8 @@ extern ram_addr_t ram_size; >> >> /* memory API */ >> >> -typedef void CPUWriteMemoryFunc(void *opaque, hwaddr addr, uint32_t value); >> -typedef uint32_t CPUReadMemoryFunc(void *opaque, hwaddr addr); >> +typedef void CPUWriteMemoryFunc(void *opaque, hwaddr addr, uint64_t value); >> +typedef uint64_t CPUReadMemoryFunc(void *opaque, hwaddr addr); > > I don't think the type of these functions has anything to do with the > CPU's capabilities, does it? The typedefs are a legacy remnant from before > the conversion to MemoryRegions: > * before MemoryRegions, devices provided separate functions for > byte/word/long reads and writes (64-bit writes were simply > impossible with the ancient APIs, which required a 3-element > function pointer array for read and the same for write) > * the initial MemoryRegion conversion introduced the new-style > "one read/write fn for all widths" APIs, but also supported > old-style six-function devices, for ease of conversion, using > MemoryRegionOps::old_mmio. > * in commit 62a0db942dec6ebfe we were finally able to drop the > old_mmio (having changed over the last devices using old-style). > (I see I forgot to delete the now-unused MemoryRegionMmio typedef.) > > The only remaining user of these typedefs is hw/usb/hcd-musb.c, > which is still not converted to QOM/qdev. It uses them to allow > its one user (hw/usb/tusb6010.c) to perform reads/writes on the > underlying musb registers.
Indeed you are correct, I have been short-sighted. > > There's no point in changing these typedefs to pass or return > a 64-bit data type, because their sole use is in the musb_read[] > and musb_write[] arrays, which only allow for 1, 2 or 4 byte > accesses, depending on which array element you use. > > Possible cleanups here: > Easy: > * delete the unused MmeoryRegionMmio > * move these typedefs into include/hw/usb.h and rename them > to MUSBReadFunc and MUSBWriteFunc, since that's all they're > used for now Agreed. > Tricky: > * convert the hw/usb/hcd-musb.c code to QOM/qdev, which would > include refactoring the current musb_read/musb_write so that > instead of the tusb6010.c code calling function entries in these > arrays the hcd-musb.c code exposed a MemoryRegion; the tusb6010 > code would access it via memory_region_dispatch_read/write Left as TODO. Thanks for reviewing this patch. > > thanks > -- PMM >