Message ID | 1694569212-10080-1-git-send-email-wentong.wu@intel.com |
---|---|
Headers | show |
Series | Add Intel LJCA device driver | expand |
On 13.09.23 03:40, Wentong Wu wrote: > +struct ljca_bank_descriptor { > + u8 bank_id; > + u8 pin_num; > + > + /* 1 bit for each gpio, 1 means valid */ > + u32 valid_pins; No endianness? > +} __packed; Regards Oliver
> From: Oliver Neukum <oneukum@suse.com> > > On 13.09.23 03:40, Wentong Wu wrote: > > > +struct ljca_bank_descriptor { > > + u8 bank_id; > > + u8 pin_num; > > + > > + /* 1 bit for each gpio, 1 means valid */ > > + u32 valid_pins; > > No endianness? On both sides, the endianness is same. BTW, the code has been tested on several real productions. BR, Wentong > > > +} __packed; > > Regards > Oliver
Hi, On 9/14/23 03:05, Wu, Wentong wrote: >> From: Oliver Neukum <oneukum@suse.com> >> >> On 13.09.23 03:40, Wentong Wu wrote: >> >>> +struct ljca_bank_descriptor { >>> + u8 bank_id; >>> + u8 pin_num; >>> + >>> + /* 1 bit for each gpio, 1 means valid */ >>> + u32 valid_pins; >> >> No endianness? > > On both sides, the endianness is same. Right, but normally USB drivers are also written so that they can work on big-endian CPUs. I realize that this driver will likely never be used with a big-endian CPU but still it is good practice to make the driver work on big-endian CPUs too. Even if it is just to set a good example when other drivers copy the code. So this should be: struct ljca_bank_descriptor { u8 bank_id; u8 pin_num; /* 1 bit for each gpio, 1 means valid */ __le32 valid_pins; } __packed; And then when reading valid_pins you should use: u32 valid_pins = get_unaligned_le32(&ljca_bank_descriptor.valid_pins); On x86_64 the compiler should optimize all of this away to just a regular read. Regards, Hans
> From: Hans de Goede <hdegoede@redhat.com> > > Hi, > > On 9/14/23 03:05, Wu, Wentong wrote: > >> From: Oliver Neukum <oneukum@suse.com> > >> > >> On 13.09.23 03:40, Wentong Wu wrote: > >> > >>> +struct ljca_bank_descriptor { > >>> + u8 bank_id; > >>> + u8 pin_num; > >>> + > >>> + /* 1 bit for each gpio, 1 means valid */ > >>> + u32 valid_pins; > >> > >> No endianness? > > > > On both sides, the endianness is same. > > Right, but normally USB drivers are also written so that they can work on big- > endian CPUs. > > I realize that this driver will likely never be used with a big-endian CPU but still it > is good practice to make the driver work on big-endian CPUs too. Even if it is just > to set a good example when other drivers copy the code. Thanks, I agree the point here. And I will update the driver after the test, thanks again. BR, Wentong > > > So this should be: > > struct ljca_bank_descriptor { > u8 bank_id; > u8 pin_num; > > /* 1 bit for each gpio, 1 means valid */ > __le32 valid_pins; > } __packed; > > And then when reading valid_pins you should use: > > u32 valid_pins = get_unaligned_le32(&ljca_bank_descriptor.valid_pins); > > On x86_64 the compiler should optimize all of this away to just a regular read. > > Regards, > > Hans