On Mon, 2023-05-15 at 22:00 -0700, Christoph Hellwig wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > 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 */ > It's simple and clean, thanks! Kelvin