On 7/26/19 6:36 AM, Richard Henderson wrote: > On 7/25/19 11:43 PM, tony.ngu...@bt.com wrote: >> } MemOp; >> >> +/* No-op while memory_region_dispatch_[read|write] is converted to MemOp */ >> +#define MEMOP_SIZE(op) (op) /* MemOp to size. */ >> +#define SIZE_MEMOP(ul) (ul) /* Size to MemOp. */ >> + > > This doesn't thrill me, because for 9 patches these macros don't do what they > say on the tin, but I'll accept it. > > I would prefer lower-case and that the real implementation in patch 10 be > inline functions with proper types instead of typeless macros. In particular, > "unsigned" not "unsigned long" as you imply from "ul" here, since that's what > was used ... > >> MemTxResult memory_region_dispatch_read(MemoryRegion *mr, >> hwaddr addr, >> uint64_t *pval, >> - unsigned size, >> + MemOp op, >> MemTxAttrs attrs);
Actually, I thought of something that would make me happier: Do not make any change to memory_region_dispatch_{read,write} now. Let the operand continue to be "unsigned size", because it still is, because of the no-op macros. Make the change to to the proper type at the same time that you flip the switch to use the proper conversion function. This will make patch 10 about 5 lines longer, but we'll have proper typing at all points in between. r~ > > ... here. > > With the name case change, > Reviewed-by: Richard Henderson <richard.hender...@linaro.org> > > > r~ >