Re: [PATCH v3 2/4] pinctrl: spacemit: add support for SpacemiT K1 SoC

[Linus Walleij]

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.

But why?

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

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".

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.

Yours,
Linus Walleij