On Mon, May 15, 2023 at 06:18:07PM +0000, Kelvin.Cao@xxxxxxxxxxxxx wrote: > > I find this rather confusing, especially as some code literally > > switches on the op to fill in either set. > > It's a hardware interface, and not possible to change it at the point. > I guess I can make it look slightly better by grouping the related > names together: > > union { > struct { > __le32 saddr_lo; > __le32 saddr_hi; > }; > struct { > __le32 widata_lo; > __le32 widata_hi; > }; > }; The hardware interface is simply: __le32 field_lo; __le32 field_hi; hardware documentation might decide to give those fields two different names just to confuse you :) I think everyone else would be served better by: __le32 addr_lo; /* SADDR_LO/WIADDR_LO */ __le32 addr_hi; /* SADDR_HI/WIADDR_HI */