| Message ID | 20250311040838.3937136-15-pierrick.bouvier@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | make system memory API available for common code | expand |
On 11/3/25 05:08, Pierrick Bouvier wrote: > we'll use it in system/memory.c. Having part of the commit description separated in its subject is a bit annoying. But then I'm probably using 20-years too old tools in my patch workflow. Only used in system/{memory,physmem}.c, worth move to a local system/memory-internal.h header? Or even simpler, move include/exec/memory-internal.h -> exec/memory-internal.h first. > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > include/exec/memory.h | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 60c0fb6ccd4..57661283684 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -3138,16 +3138,22 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, > MemTxResult address_space_set(AddressSpace *as, hwaddr addr, > uint8_t c, hwaddr len, MemTxAttrs attrs); > > -/* enum device_endian to MemOp. */ > -static inline MemOp devend_memop(enum device_endian end) > +/* returns true if end is big endian. */ > +static inline bool devend_big_endian(enum device_endian end) > { > QEMU_BUILD_BUG_ON(DEVICE_HOST_ENDIAN != DEVICE_LITTLE_ENDIAN && > DEVICE_HOST_ENDIAN != DEVICE_BIG_ENDIAN); > > - bool big_endian = (end == DEVICE_NATIVE_ENDIAN > - ? target_words_bigendian() > - : end == DEVICE_BIG_ENDIAN); > - return big_endian ? MO_BE : MO_LE; > + if (end == DEVICE_NATIVE_ENDIAN) { > + return target_words_bigendian(); > + } > + return end == DEVICE_BIG_ENDIAN; > +} > + > +/* enum device_endian to MemOp. */ > +static inline MemOp devend_memop(enum device_endian end) > +{ > + return devend_big_endian(end) ? MO_BE : MO_LE; > } > > /*
On 3/11/25 00:36, Philippe Mathieu-Daudé wrote: > On 11/3/25 05:08, Pierrick Bouvier wrote: >> we'll use it in system/memory.c. > > Having part of the commit description separated in its subject is a > bit annoying. But then I'm probably using 20-years too old tools in > my patch workflow. > > Only used in system/{memory,physmem}.c, worth move to a local > system/memory-internal.h header? Or even simpler, move > include/exec/memory-internal.h -> exec/memory-internal.h first. > Good point, I'll move them to the existing exec/memory-internal.h in an additional commit. >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> include/exec/memory.h | 18 ++++++++++++------ >> 1 file changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/include/exec/memory.h b/include/exec/memory.h >> index 60c0fb6ccd4..57661283684 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -3138,16 +3138,22 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, >> MemTxResult address_space_set(AddressSpace *as, hwaddr addr, >> uint8_t c, hwaddr len, MemTxAttrs attrs); >> >> -/* enum device_endian to MemOp. */ >> -static inline MemOp devend_memop(enum device_endian end) >> +/* returns true if end is big endian. */ >> +static inline bool devend_big_endian(enum device_endian end) >> { >> QEMU_BUILD_BUG_ON(DEVICE_HOST_ENDIAN != DEVICE_LITTLE_ENDIAN && >> DEVICE_HOST_ENDIAN != DEVICE_BIG_ENDIAN); >> >> - bool big_endian = (end == DEVICE_NATIVE_ENDIAN >> - ? target_words_bigendian() >> - : end == DEVICE_BIG_ENDIAN); >> - return big_endian ? MO_BE : MO_LE; >> + if (end == DEVICE_NATIVE_ENDIAN) { >> + return target_words_bigendian(); >> + } >> + return end == DEVICE_BIG_ENDIAN; >> +} >> + >> +/* enum device_endian to MemOp. */ >> +static inline MemOp devend_memop(enum device_endian end) >> +{ >> + return devend_big_endian(end) ? MO_BE : MO_LE; >> } >> >> /* >
On 3/11/25 00:36, Philippe Mathieu-Daudé wrote: > On 11/3/25 05:08, Pierrick Bouvier wrote: >> we'll use it in system/memory.c. > > Having part of the commit description separated in its subject is a > bit annoying. But then I'm probably using 20-years too old tools in > my patch workflow. Can you please share what would be the message you (or the tool) would prefer in this case? It's just a single line subject (saying "we extract the function") + an additional line justifying why it's needed. > > Only used in system/{memory,physmem}.c, worth move to a local > system/memory-internal.h header? Or even simpler, move > include/exec/memory-internal.h -> exec/memory-internal.h first. > >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> include/exec/memory.h | 18 ++++++++++++------ >> 1 file changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/include/exec/memory.h b/include/exec/memory.h >> index 60c0fb6ccd4..57661283684 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -3138,16 +3138,22 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, >> MemTxResult address_space_set(AddressSpace *as, hwaddr addr, >> uint8_t c, hwaddr len, MemTxAttrs attrs); >> >> -/* enum device_endian to MemOp. */ >> -static inline MemOp devend_memop(enum device_endian end) >> +/* returns true if end is big endian. */ >> +static inline bool devend_big_endian(enum device_endian end) >> { >> QEMU_BUILD_BUG_ON(DEVICE_HOST_ENDIAN != DEVICE_LITTLE_ENDIAN && >> DEVICE_HOST_ENDIAN != DEVICE_BIG_ENDIAN); >> >> - bool big_endian = (end == DEVICE_NATIVE_ENDIAN >> - ? target_words_bigendian() >> - : end == DEVICE_BIG_ENDIAN); >> - return big_endian ? MO_BE : MO_LE; >> + if (end == DEVICE_NATIVE_ENDIAN) { >> + return target_words_bigendian(); >> + } >> + return end == DEVICE_BIG_ENDIAN; >> +} >> + >> +/* enum device_endian to MemOp. */ >> +static inline MemOp devend_memop(enum device_endian end) >> +{ >> + return devend_big_endian(end) ? MO_BE : MO_LE; >> } >> >> /* >
diff --git a/include/exec/memory.h b/include/exec/memory.h index 60c0fb6ccd4..57661283684 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -3138,16 +3138,22 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, MemTxResult address_space_set(AddressSpace *as, hwaddr addr, uint8_t c, hwaddr len, MemTxAttrs attrs); -/* enum device_endian to MemOp. */ -static inline MemOp devend_memop(enum device_endian end) +/* returns true if end is big endian. */ +static inline bool devend_big_endian(enum device_endian end) { QEMU_BUILD_BUG_ON(DEVICE_HOST_ENDIAN != DEVICE_LITTLE_ENDIAN && DEVICE_HOST_ENDIAN != DEVICE_BIG_ENDIAN); - bool big_endian = (end == DEVICE_NATIVE_ENDIAN - ? target_words_bigendian() - : end == DEVICE_BIG_ENDIAN); - return big_endian ? MO_BE : MO_LE; + if (end == DEVICE_NATIVE_ENDIAN) { + return target_words_bigendian(); + } + return end == DEVICE_BIG_ENDIAN; +} + +/* enum device_endian to MemOp. */ +static inline MemOp devend_memop(enum device_endian end) +{ + return devend_big_endian(end) ? MO_BE : MO_LE; } /*
we'll use it in system/memory.c. Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- include/exec/memory.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)