Hi Linus: On 09:57 Mon 02 Sep , Linus Walleij wrote: > Hi Yixun, > > thanks for your patch! Overall the driver looks very good, it's using the > right helpers and abstractions etc. > > There is this thing that needs some elaboration: > > On Wed, Aug 28, 2024 at 1:31 PM Yixun Lan <dlan@xxxxxxxxxx> wrote: > > > +/* pin offset */ > > +#define PINID(x) ((x) + 1) > > + > > +#define GPIO_INVAL 0 > > +#define GPIO_00 PINID(0) > > +#define GPIO_01 PINID(1) > (...) > > So GPIO 00 has pin ID 1 actually etc. > yes, in current version > But why? > good question! from hw perspective, the GPIO_00 pinctrl register start at offset 0x4, see chap 3.3.1 of "Pin Information" section at [1] and in this version of patch, we are extracting pinid from register offset, using the algorithem pinid = (offset >> 2). this idea was something I borrowed from vendor's driver, and now you remind me this might be wrong.. > If there is no datasheet or other conflicting documentation, just > begin numbering the GPIOs at 1 instead of 0 to match the > hardware: > > #define GPIO_01 1 > #define GPIO_02 2 > as current patch version, there will be some non-linear mapping, such as #define GPIO_98 93 #define GPIO_99 92 .. #define GPIO_110 116 ... I think we could fix this by introducing a pinid_to_register_offset() function, which should drop the pinid = (offset >> 2) mapping, but instead, doing in the reverse way, retrive register offset from pinid, so idealy we should get a linear mapping of GPIO to pinid, like #define GPIO_00 0 #define GPIO_01 1 .. #define GPIO_127 127 I will work this in next patch version. > and all is fine. > > It's just very uninituitive for developers. I guess that there > is a reason, such as that the datasheet has stated that the pin > with pin ID 1 is GPIO 00, then this needs to be explained with > a comment in the code: "we are doing this because otherwise > the developers will see an offset of -1 between the number the > pin has in the datasheet and the number they put into the > device tree, while the hardware starts the numbering at 1, the > documentation starts the numbering at 0". > see above, a potential solution to fix this > It is common that engineers from analog electronics background > start numbering things from 1 while any computer science > engineer start the numbering at 0. So this is what you get when > an analog engineer designs the electronics and a computer > science engineer writes that datasheet and decides to "fix" > the problem. > true, things happens, sometimes there is gap in understanding between analog engineers and software developers.. Link: https://developer.spacemit.com/documentation?token=An1vwTwKaigaXRkYfwmcznTXned [1] -- Yixun Lan (dlan) Gentoo Linux Developer GPG Key ID AABEFD55
