On Mon, Feb 21, 2022 at 01:48:12PM +0100, Nuno Sá wrote: > On Sun, 2022-02-20 at 13:32 +0200, Andy Shevchenko wrote: > > On Fri, Feb 18, 2022 at 02:51:28PM +0100, Nuno Sá wrote: > > > On Mon, 2022-02-14 at 15:49 +0200, Andy Shevchenko wrote: > > > > On Mon, Feb 07, 2022 at 09:19:46PM +0100, Nuno Sá wrote: > > > > > On Mon, 2022-02-07 at 13:09 +0200, Andy Shevchenko wrote: > > > > > > On Sun, Feb 06, 2022 at 01:19:59PM +0000, Sa, Nuno wrote: > > > > > > > > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> > > > > > > > > Sent: Saturday, February 5, 2022 6:30 PM > > > > > > > > On Fri, Jan 21, 2022 at 03:24:59PM +0100, Nuno Sá wrote: ... > > > > > > > > > + ret = kstrtou16(buf, 10, &val); > > > > > > > > > > > > > > > > In other function you have long, here u16. I would expect that > > > > > > > > the types are of the same class, e.g. if here you have u16, > > > > > > > > then there something like s32 / s64. Or here something like > > > > > > > > unsigned short. > > > > > > > > > > > > > > > > A bit of elaboration why u16 is chosen here? > > > > > > > > > > > > > > Well, I never really saw any enforcement here to be honest > > > > > > > (rather than using stdint types...). So I pretty much just use > > > > > > > these in unsigned types because I'm lazy and u16 is faster to > > > > > > > type than unsigned short... In this case, unless Jonathan really > > > > > > > asks for it, I prefer not to go all over the driver and change > > > > > > > this... > > > > > > > > > > > > This is about consistency. It may work as is, but it feels not good > > > > > > when for int (or unsigned int) one uses fixed-width types. Also > > > > > > it's non- written advice to use fixed-width variables when it's > > > > > > about programming registers or so, for the rest, use POD types. > > > > > > Ok, going a bit back in the discussion, you argued that in one place I > > > was using long while here u16. Well, in the place I'm using long, that > > > was on purpose because that value is to be compared against an array of > > > longs (which has to be long because it depends on CCF rates). I guess I > > > can als0 use s64, but there is also a reason why long was used. > > > > > > In the u16 case, we really want to have 2 bytes because I'm going to use > > > that value to write the dac code which is 2 bytes. > > > > Okay, that's what I want to hear. If it's indeed goes to be a value to the > > register, then it's fine. > > > > Perhaps a comment? > > I guess you mean to have a comment to state that here we have fixed > size type (as opposed to long, used in another place), because we > directly use the value on a register write? > > Asking it because I'm not planning to add comments in all the places > where I have fixed size types for register read/writes... Thinking more about it and now I'm convinced that using the value that goes to the register in ABI is bad idea (means that user space must not care about the size or contents of the hardware register and should be abstract representation of the HW). OTOH this seems to be "raw" value of something. So, I maybe missed the convention in IIO about this kind of values WRT the variable types used on ABI side. That said, I leave it to Jonathan since I'm not convinced that u16 is a proper choice here. > > > > > I can understand your reasoning but again this is something that I > > > > > never really saw being enforced. So, I'm more than ok to change it if > > > > > it really becomes something that we will try to "enforce" in IIO. > > > > > Otherwise it just feels as a random nitpick :). > > > > > > > > No, this is about consistency and common sense. If you define type uXX, > > > > we have an API for that exact type. It's confusing why POD type APIs > > > > are used with fixed-width types or vise versa. > > > > > > > > Moreover (which is pure theoretical, though) some architectures might > > > > have no (mutual) equivalency between these types. -- With Best Regards, Andy Shevchenko
