Hello,

we have a 16 bit value here, so we have to define whether bit0(containin the information for IO0.0) is in the first or the second byte. Since the PCA9555 does this encoding little endian, the conversion is allright.

Cheers Dirk Am Do., 11. Okt. 2018 um 07:42 Uhr schrieb Heiko Schocher hs@denx.de:

Hello Joakim,

Am 10.10.2018 um 19:34 schrieb Joakim Tjernlund:

...

This commit broke our pca953x usage(on ppc).

I wonder why gpio pins here has an endian, its not a number. If there must be an endian connected with this, should it not be a cpu_to_be16 instead, which will retain compatibility ?

Hmm.. good question, I think you are right. May dirk can do a test? I have no pca953x with 16bit for doing a test.

bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs@denx.de

Hi Jocke, On Thu, Oct 11, 2018 at 5:48 PM Joakim Tjernlund Joakim.Tjernlund@infinera.com wrote:

On Thu, 2018-10-11 at 16:11 +0200, Dirk Eibach wrote:

...

Hello,

we have a 16 bit value here, so we have to define whether bit0(containin the information for IO0.0) is in the first or the second byte. Since the PCA9555 does this encoding little endian, the conversion is allright.

You used it as some number but this is really just a bunch I/O pins. I haven't seen any endian conversion in the kernel driver, have you?

Actually, there is. See [1]:

static int pca953x_write_regs_16(struct pca953x_chip *chip, int reg, u8 *val) { u16 word = get_unaligned((u16 *)val);

return i2c_smbus_write_word_data(chip->client, reg << 1, word); }

And get_unaligned on powerpc (see [2]):

#ifdef __LITTLE_ENDIAN__ #define get_unaligned __get_unaligned_le #define put_unaligned __put_unaligned_le #else #define get_unaligned __get_unaligned_be #define put_unaligned __put_unaligned_be #endif

So the endianness conversion happens in the get_unaligned.

This is a bigger question than this driver really, so:

Does IO pins have an endian?

The end effect really is where in the final 16-bit word the values of each pin appear: If you read in little-endian order, the values of the first bank (register 0) will end up in the lower byte, and the values of the second bank (register 1) will end up in the higher byte (in big-endian order it's reversed).

Since the bits in each bank register look like this (see [3]):

Register 0: | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | | I0.7 | I0.6 | I0.5 | I0.4 | I0.3 | I0.2 | I0.1 | I0.0 |

Register 1: | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | | I1.7 | I1.6 | I1.5 | I1.4 | I1.3 | I1.2 | I1.1 | I1.0 |

You will end up with this final word if you read little-endian (byte at lowest address at lowest position in word):

| I1.7 | I1.6 | I1.5 | I1.4 | I1.3 | I1.2 | I1.1 | I1.0 | I0.7 | I0.6 | I0.5 | I0.4 | I0.3 | I0.2 | I0.1 | I0.0 |

Since you usually want to number the pins from 0 to 15 with bank 0 having 0-7 and bank 1 having 8-15, this is exactly what you want, since now

!!(word & (1 << NUM))

gives you the value of pin number NUM in this enumeration.

[1] https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpio-pca953x.c#L... [2] https://elixir.bootlin.com/linux/latest/source/arch/powerpc/include/asm/unal... [3] https://www.nxp.com/docs/en/data-sheet/PCA9555.pdf

Jocke

...

Cheers Dirk Am Do., 11. Okt. 2018 um 07:42 Uhr schrieb Heiko Schocher hs@denx.de:

...

Hello Joakim,

Am 10.10.2018 um 19:34 schrieb Joakim Tjernlund:

...

This commit broke our pca953x usage(on ppc).

I wonder why gpio pins here has an endian, its not a number. If there must be an endian connected with this, should it not be a cpu_to_be16 instead, which will retain compatibility ?

Hmm.. good question, I think you are right. May dirk can do a test? I have no pca953x with 16bit for doing a test.

bye, Heiko

Best regards, Mario