Hi Phillippe, On 8/16/19 7:58 PM, Philippe Mathieu-Daudé wrote: >On 8/16/19 8:28 AM, tony.ngu...@bt.com wrote: >> This patchset implements the IE (Invert Endian) bit in SPARCv9 MMU TTE. >> >> v7: >[...] >> - Re-declared many native endian devices as little or big endian. This is why >> v7 has +16 patches. > >Why are you doing that? What is the rational?
While collapsing the byte swaps, it was suggested in patch #11 of v5 that consistent use of MemOp simplified endian comparisons. This lead to the deprecation of enum device_endian by MemOp. As MO_TE is conditional upon NEED_CPU_H, the s/DEVICE_NATIVE_ENDIAN/MO_TE/ required changing some device object files from common-obj-* to obj-*. In patch #15 of v6 Paolo noted that most devices should not of been DEVICE_NATIVE_ENDIAN and hinted at a clean up. The +16 patches in v7 is the clean up effort. >Anyhow if this not required by your series, you should split it out of >it, and send it on your principal changes are merged. >I'm worried because this these new patches involve many subsystems (thus >maintainers) and reviewing them will now take a fair amount of time. Yes, lets split these patches out. They are very much a tangent to the series purpose. >> For each device declared with DEVICE_NATIVE_ENDIAN, find the set of >> targets from the set of target/hw/*/device.o. >> >> If the set of targets are all little or all big endian, re-declare >> the device endianness as DEVICE_LITTLE_ENDIAN or DEVICE_BIG_ENDIAN >> respectively. > >If only little endian targets use a device, that doesn't mean the device >is designed in little endian... > >Then if a big endian target plan to use this device, it will require >more work and you might have introduced regressions... > >I'm not sure this is a safe move. > >> This *naive* deduction may result in genuinely native endian devices >> being incorrectly declared as little or big endian, but should not >> introduce regressions for current targets. > Roger. Evidently too naive. TBH, most devices I've never heard of... Regards, Tony