Skip to contentSkip to conversation

pca953x: support 16-pin devices

PATCH Superseded Chris Packham 4 participants 11 messages Dec 7, 2010 – Jan 13, 2011 GPIO

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 pca953x: support 16-pin devices

This adds support for for the PCA9535/PCA9539 family of gpio devices which have 16 output pins.

To let the driver know which devices are 16-pin it is necessary to define CONFIG_SYS_I2C_PCA953X_WIDTH in your board config file. This is used to create an array of {chip, ngpio} tuples that are used to determine the width of a particular chip. For backwards compatibility it is assumed that any chip not defined in CONFIG_SYS_I2C_PCA953X_WIDTH has 8 pins.
---

As suggested by Peter I've implemented the 16-pin support in the existing pca953x driver. So this is pretty much a re-write of the v1 patch. Is the commit message sufficient to document CONFIG_SYS_I2C_PCA953X_WIDTH or is there something under doc/ that I should be adding to.

 drivers/gpio/pca953x.c |  107 ++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 85 insertions(+), 22 deletions(-)
drivers/gpio/pca953x.c +84 -21
index 6e82bd6..491b79e 100644
@@ -17,8 +17,8 @@
  */
 
 /*
- * Driver for NXP's 4 and 8 bit I2C gpio expanders (eg pca9537, pca9557, etc)
- * TODO: support additional devices with more than 8-bits GPIO
+ * Driver for NXP's 4, 8 and 16 bit I2C gpio expanders (eg pca9537, pca9557,
+ * pca9539, etc)
  */
 
 #include <common.h>
@@ -38,22 +38,78 @@ enum {
 	PCA953X_CMD_INVERT,
 };
 
+#ifdef CONFIG_SYS_I2C_PCA953X_WIDTH
+struct chip_ngpio {
+	uint8_t chip;
+	uint8_t ngpio;
+};
+
+static struct chip_ngpio chip_ngpios[] = CONFIG_SYS_I2C_PCA953X_WIDTH;
+#define NUM_CHIP_GPIOS (sizeof(chip_ngpios) / sizeof(struct chip_ngpio))
+
+/*
+ * Determine the number of GPIO pins supported. If we don't know we assume
+ * 8 pins.
+ */
+static int ngpio(uint8_t chip)
+{
+	int i;
+
+	for (i = 0; i < NUM_CHIP_GPIOS; i++) {
+		if (chip_ngpios[i].chip == chip)
+			return chip_ngpios[i].ngpio;
+	}
+	return 8;
+}
+#else
+#define ngpio(chip)	8
+#endif
+
 /*
  * Modify masked bits in register
  */
 static int pca953x_reg_write(uint8_t chip, uint addr, uint mask, uint data)
 {
-	uint8_t val;
+	uint8_t  valb;
+	uint16_t valw;
 
-	if (i2c_read(chip, addr, 1, &val, 1))
-		return -1;
+	if (ngpio(chip) <= 8) {
+		if (i2c_read(chip, addr, 1, &valb, 1))
+			return -1;
+
+		valb &= ~mask;
+		valb |= data;
+
+		return i2c_write(chip, addr, 1, &valb, 1);
+	} else {
+		if (i2c_read(chip, addr << 1, 1, (u8*)&valw, 2))
+			return -1;
+
+		valw &= ~mask;
+		valw |= data;
+
+		return i2c_write(chip, addr << 1, 1, (u8*)&valw, 2);
+	}
+}
 
-	val &= ~mask;
-	val |= data;
+static int pca953x_reg_read(uint8_t chip, uint addr, uint *data)
+{
+	uint8_t  valb;
+	uint16_t valw;
 
-	return i2c_write(chip, addr, 1, &val, 1);
+	if (ngpio(chip) <= 8) {
+		if (i2c_read(chip, addr, 1, &valb, 1))
+			return -1;
+		*data = (int) valb;
+	} else {
+		if (i2c_read(chip, addr << 1, 1, (u8*)&valw, 2))
+			return -1;
+		*data = (int) valw;
+	}
+	return 0;
 }
 
+
 /*
  * Set output value of IO pins in 'mask' to corresponding value in 'data'
  * 0 = low, 1 = high
@@ -86,9 +142,9 @@ int pca953x_set_dir(uint8_t chip, uint mask, uint data)
  */
 int pca953x_get_val(uint8_t chip)
 {
-	uint8_t val;
+	uint val;
 
-	if (i2c_read(chip, 0, 1, &val, 1))
+	if (pca953x_reg_read(chip, PCA953X_IN, &val) < 0)
 		return -1;
 
 	return (int)val;
@@ -102,37 +158,44 @@ int pca953x_get_val(uint8_t chip)
 static int pca953x_info(uint8_t chip)
 {
 	int i;
-	uint8_t data;
+	uint data;
+	int nr_gpio = ngpio(chip);
+	int msb     = nr_gpio - 1;
 
-	printf("pca953x@ 0x%x:\n\n", chip);
-	printf("gpio pins: 76543210\n");
+	printf("pca953x@ 0x%x (%d pins):\n\n", chip, nr_gpio);
+	printf("gpio pins: ");
+	for (i = msb; i >= 0; i--)
+		printf("%x",i);
+	printf("\n");
+	if (nr_gpio > 8)
+		printf("--------");
 	printf("-------------------\n");
 
-	if (i2c_read(chip, PCA953X_CONF, 1, &data, 1))
+	if (pca953x_reg_read(chip, PCA953X_CONF, &data) < 0)
 		return -1;
 	printf("conf:      ");
-	for (i = 7; i >= 0; i--)
+	for (i = msb; i >= 0; i--)
 		printf("%c", data & (1 << i) ? 'i' : 'o');
 	printf("\n");
 
-	if (i2c_read(chip, PCA953X_POL, 1, &data, 1))
+	if (pca953x_reg_read(chip, PCA953X_POL, &data) < 0)
 		return -1;
 	printf("invert:    ");
-	for (i = 7; i >= 0; i--)
+	for (i = msb; i >= 0; i--)
 		printf("%c", data & (1 << i) ? '1' : '0');
 	printf("\n");
 
-	if (i2c_read(chip, PCA953X_IN, 1, &data, 1))
+	if (pca953x_reg_read(chip, PCA953X_IN, &data) < 0)
 		return -1;
 	printf("input:     ");
-	for (i = 7; i >= 0; i--)
+	for (i = msb; i >= 0; i--)
 		printf("%c", data & (1 << i) ? '1' : '0');
 	printf("\n");
 
-	if (i2c_read(chip, PCA953X_OUT, 1, &data, 1))
+	if (pca953x_reg_read(chip, PCA953X_OUT, &data) < 0)
 		return -1;
 	printf("output:    ");
-	for (i = 7; i >= 0; i--)
+	for (i = msb; i >= 0; i--)
 		printf("%c", data & (1 << i) ? '1' : '0');
 	printf("\n");
 
Signature · 1 line
1.7.3.2
#1 lore raw

PATCH v2 1/1 pca953x: support 16-pin devices

The patch looks good. I had a few minor nitpicky style comments below:

Quoted 4 lines

As suggested by Peter I've implemented the 16-pin support in the existing pca953x driver. So this is pretty much a re-write of the v1 patch. Is the commit message sufficient to document CONFIG_SYS_I2C_PCA953X_WIDTH or is there something under doc/ that I should be adding to.

You could add a brief description to the top-level README file. There is currently a bit of info in the "GPIO Support" section that could be added to.

drivers/gpio/pca953x.c enum { · line 45

Quoted 10 lines

#include <common.h>

drivers/gpio/pca953x.c +5 -0
@@ -38,22 +38,78 @@ enum {
 	PCA953X_CMD_INVERT,
 };
 
+#ifdef CONFIG_SYS_I2C_PCA953X_WIDTH
+struct chip_ngpio {
+	uint8_t chip;
+	uint8_t ngpio;
+};

Since this structure is 953x-specific I'd rename chip_ngpio pca953x_chip_ngpio to clarify its a driver-specific structure and to prevent the (unlikely) name collision down the road.

The same comment applies to ngpio()->pca953x_ngpio(), chip_ngpios[]->pca953x_chip_ngpios[].

Quoted 5 lines
drivers/gpio/pca953x.c +5 -0
+static struct chip_ngpio chip_ngpios[] = CONFIG_SYS_I2C_PCA953X_WIDTH;
+#define NUM_CHIP_GPIOS (sizeof(chip_ngpios) / sizeof(struct chip_ngpio))
+
+/*
+ * Determine the number of GPIO pins supported. If we don't know we assume

drivers/gpio/pca953x.c enum { · line 61

Quoted 10 lines
drivers/gpio/pca953x.c +10 -0
+ * 8 pins.
+ */
+static int ngpio(uint8_t chip)
+{
+	int i;
+
+	for (i = 0; i < NUM_CHIP_GPIOS; i++) {
+		if (chip_ngpios[i].chip == chip)
+			return chip_ngpios[i].ngpio;
+	}

I'd remove the for loop braces above per the Linux CodingStyle documentation that U-Boot adheres to.

There should also be an empty line above the "return 8" below.

Quoted 4 lines
drivers/gpio/pca953x.c +4 -0
+	return 8;
+}
+#else
+#define ngpio(chip)	8

drivers/gpio/pca953x.c enum { · line 74

Quoted 10 lines
drivers/gpio/pca953x.c +4 -1
+#endif
+
 /*
  * Modify masked bits in register
  */
 static int pca953x_reg_write(uint8_t chip, uint addr, uint mask, uint data)
 {
-	uint8_t val;
+	uint8_t  valb;
+	uint16_t valw;

I'd remove one of the spaces between "valb" above. My understanding is spaces shouldn't be used for such vertical alignment.

Quoted 24 lines
drivers/gpio/pca953x.c +18 -4
-	if (i2c_read(chip, addr, 1, &val, 1))
-		return -1;
+	if (ngpio(chip) <= 8) {
+		if (i2c_read(chip, addr, 1, &valb, 1))
+			return -1;
+
+		valb &= ~mask;
+		valb |= data;
+
+		return i2c_write(chip, addr, 1, &valb, 1);
+	} else {
+		if (i2c_read(chip, addr << 1, 1, (u8*)&valw, 2))
+			return -1;
+
+		valw &= ~mask;
+		valw |= data;
+
+		return i2c_write(chip, addr << 1, 1, (u8*)&valw, 2);
+	}
+}
 
-	val &= ~mask;
-	val |= data;

drivers/gpio/pca953x.c enum { · line 103

Quoted 10 lines
drivers/gpio/pca953x.c +8 -1
+static int pca953x_reg_read(uint8_t chip, uint addr, uint *data)
+{
+	uint8_t  valb;
+	uint16_t valw;
 
-	return i2c_write(chip, addr, 1, &val, 1);
+	if (ngpio(chip) <= 8) {
+		if (i2c_read(chip, addr, 1, &valb, 1))
+			return -1;
+		*data = (int) valb;

The space in "(int) valb" should be removed. Same below.

Quoted 7 lines
drivers/gpio/pca953x.c +6 -0
+	} else {
+		if (i2c_read(chip, addr << 1, 1, (u8*)&valw, 2))
+			return -1;
+		*data = (int) valw;
+	}
+	return 0;
 }

<snip>

drivers/gpio/pca953x.c int pca953x_get_val(uint8_t chip) · line 172

Quoted 6 lines
drivers/gpio/pca953x.c +5 -0
+	for (i = msb; i >= 0; i--)
+		printf("%x",i);
+	printf("\n");
+	if (nr_gpio > 8)
+		printf("--------");
 	printf("-------------------\n");

What about changing the printing of '-'s to a for loop like for (i = 19 + nr_gpio; i >0; i++)

	puts("-");

I'll give the next iteration of the patch a shot, it looks like it should work just fine.

Best,
Peter

#2 lore raw

PATCH v2 1/1 pca953x: support 16-pin devices

Quoted 126 lines · Peter Tyser

The patch looks good. ?I had a few minor nitpicky style comments below:

Quoted 4 lines

As suggested by Peter I've implemented the 16-pin support in the existing pca953x driver. So this is pretty much a re-write of the v1 patch. Is the commit message sufficient to document CONFIG_SYS_I2C_PCA953X_WIDTH or is there something under doc/ that I should be adding to.

You could add a brief description to the top-level README file. ?There is currently a bit of info in the "GPIO Support" section that could be added to.

drivers/gpio/pca953x.c enum { · line 45

Quoted 10 lines

?#include <common.h>

drivers/gpio/pca953x.c +5 -0
@@ -38,22 +38,78 @@ enum {
 ? ? ? PCA953X_CMD_INVERT,
 ?};
 
+#ifdef CONFIG_SYS_I2C_PCA953X_WIDTH
+struct chip_ngpio {
+ ? ? uint8_t chip;
+ ? ? uint8_t ngpio;
+};

Since this structure is 953x-specific I'd rename chip_ngpio pca953x_chip_ngpio to clarify its a driver-specific structure and to prevent the (unlikely) name collision down the road.

The same comment applies to ngpio()->pca953x_ngpio(), chip_ngpios[]->pca953x_chip_ngpios[].

Quoted 5 lines
drivers/gpio/pca953x.c +5 -0
+static struct chip_ngpio chip_ngpios[] = CONFIG_SYS_I2C_PCA953X_WIDTH;
+#define NUM_CHIP_GPIOS (sizeof(chip_ngpios) / sizeof(struct chip_ngpio))
+
+/*
+ * Determine the number of GPIO pins supported. If we don't know we assume

drivers/gpio/pca953x.c enum {

Quoted 10 lines
drivers/gpio/pca953x.c +10 -0
+ * 8 pins.
+ */
+static int ngpio(uint8_t chip)
+{
+ ? ? int i;
+
+ ? ? for (i = 0; i < NUM_CHIP_GPIOS; i++) {
+ ? ? ? ? ? ? if (chip_ngpios[i].chip == chip)
+ ? ? ? ? ? ? ? ? ? ? return chip_ngpios[i].ngpio;
+ ? ? }

I'd remove the for loop braces above per the Linux CodingStyle documentation that U-Boot adheres to.

There should also be an empty line above the "return 8" below.

Quoted 6 lines
drivers/gpio/pca953x.c +6 -0
+ ? ? return 8;
+}
+#else
+#define ngpio(chip) ?8
+#endif
+
Quoted 8 lines

?/*
? * Modify masked bits in register
? */
?static int pca953x_reg_write(uint8_t chip, uint addr, uint mask, uint data) ?{
- ? ? uint8_t val;
+ ? ? uint8_t ?valb;
+ ? ? uint16_t valw;

I'd remove one of the spaces between "valb" above. ?My understanding is spaces shouldn't be used for such vertical alignment.

Quoted 24 lines

- ? ? if (i2c_read(chip, addr, 1, &val, 1))
- ? ? ? ? ? ? return -1;
+ ? ? if (ngpio(chip) <= 8) {
+ ? ? ? ? ? ? if (i2c_read(chip, addr, 1, &valb, 1))
+ ? ? ? ? ? ? ? ? ? ? return -1;

(quoted diff) +15 -2
+
+ ? ? ? ? ? ? valb &= ~mask;
+ ? ? ? ? ? ? valb |= data;
+
+ ? ? ? ? ? ? return i2c_write(chip, addr, 1, &valb, 1);
+ ? ? } else {
+ ? ? ? ? ? ? if (i2c_read(chip, addr << 1, 1, (u8*)&valw, 2))
+ ? ? ? ? ? ? ? ? ? ? return -1;
+
+ ? ? ? ? ? ? valw &= ~mask;
+ ? ? ? ? ? ? valw |= data;
+
+ ? ? ? ? ? ? return i2c_write(chip, addr << 1, 1, (u8*)&valw, 2);
+ ? ? }
+}
 
- ? ? val &= ~mask;
- ? ? val |= data;

Review

Quoted 10 lines
(quoted diff) +8 -1
+static int pca953x_reg_read(uint8_t chip, uint addr, uint *data)
+{
+ ? ? uint8_t ?valb;
+ ? ? uint16_t valw;
 
- ? ? return i2c_write(chip, addr, 1, &val, 1);
+ ? ? if (ngpio(chip) <= 8) {
+ ? ? ? ? ? ? if (i2c_read(chip, addr, 1, &valb, 1))
+ ? ? ? ? ? ? ? ? ? ? return -1;
+ ? ? ? ? ? ? *data = (int) valb;

The space in "(int) valb" should be removed. ?Same below.

Quoted 7 lines

+ ? ? } else {
+ ? ? ? ? ? ? if (i2c_read(chip, addr << 1, 1, (u8*)&valw, 2))
+ ? ? ? ? ? ? ? ? ? ? return -1;
+ ? ? ? ? ? ? *data = (int) valw;
+ ? ? }
+ ? ? return 0;
?}

<snip>

Quoted 6 lines

+ ? ? for (i = msb; i >= 0; i--)
+ ? ? ? ? ? ? printf("%x",i);
+ ? ? printf("\n");
+ ? ? if (nr_gpio > 8)
+ ? ? ? ? ? ? printf("--------");
? ? ? printf("-------------------\n");

What about changing the printing of '-'s to a for loop like for (i = 19 + nr_gpio; i >0; i++)
? ? ? ?puts("-");

Quoted 6 lines

I'll give the next iteration of the patch a shot, it looks like it should work just fine.

Best,
Peter

Hi Peter,

I'll try and get an updated patch through as soon as I can. I'm on a training course today and tomorrow but it won't take me long to make the changes you've suggested once I find some time.

Thanks,
Chris

#3 lore raw

PATCH v3 1/1 pca953x: support 16-pin devices

This adds support for for the PCA9535/PCA9539 family of gpio devices which have 16 output pins.

To let the driver know which devices are 16-pin it is necessary to define CONFIG_SYS_I2C_PCA953X_WIDTH in your board config file. This is used to create an array of {chip, ngpio} tuples that are used to determine the width of a particular chip. For backwards compatibility it is assumed that any chip not defined in CONFIG_SYS_I2C_PCA953X_WIDTH has 8 pins.

---
Changes since v2:
- I've addressed Peters style comments.
- I've added a blurb to README describing the new config option

 README                 |    4 ++
 drivers/gpio/pca953x.c |  111 ++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 92 insertions(+), 23 deletions(-)
README +4 -0
index 68f5fb0..831c5af 100644
@@ -746,6 +746,10 @@ The following options need to be configured:
 		CONFIG_PCA953X		- use NXP's PCA953X series I2C GPIO
 		CONFIG_PCA953X_INFO	- enable pca953x info command
 
+		The CONFIG_SYS_I2C_PCA953X_WIDTH option specifies a list of
+		chip-ngpio pairs that tell the PCA953X driver the number of
+		pins supported by a particular chip.
+
 		Note that if the GPIO device uses I2C, then the I2C interface
 		must also be configured. See I2C Support, below.
 
drivers/gpio/pca953x.c +87 -22

161 lines not on this page · raw message

Signature · 1 line
1.7.3.2
#4 lore raw

PATCH v3 1/1 pca953x: support 16-pin devices

Quoted 10 lines · Chris Packham

This adds support for for the PCA9535/PCA9539 family of gpio devices which have 16 output pins.

To let the driver know which devices are 16-pin it is necessary to define CONFIG_SYS_I2C_PCA953X_WIDTH in your board config file. This is used to create an array of {chip, ngpio} tuples that are used to determine the width of a particular chip. For backwards compatibility it is assumed that any chip not defined in CONFIG_SYS_I2C_PCA953X_WIDTH has 8 pins.

Looks good to me and works as advertised.

#5 lore raw

PATCH v3 1/1 pca953x: support 16-pin devices

Quoted 10 lines · Peter Tyser
Quoted 9 lines · Chris Packham

This adds support for for the PCA9535/PCA9539 family of gpio devices which have 16 output pins.

To let the driver know which devices are 16-pin it is necessary to define CONFIG_SYS_I2C_PCA953X_WIDTH in your board config file. This is used to create an array of {chip, ngpio} tuples that are used to determine the width of a particular chip. For backwards compatibility it is assumed that any chip not defined in CONFIG_SYS_I2C_PCA953X_WIDTH has 8 pins.

Quoted 7 lines
Quoted 1 line

Looks good to me and works as advertised.

There is one minor fixup we might want to squash into v3 (or if someone wants me to submit a v4 I can). It makes sense to have pca953x_ngpio as a function in both cases. The compiler will auto-inline the function so we won't see a increase in size and having a function instead of a macro allows the compiler to do proper type checking.

---8<---

From: Chris Packham <[email protected]>
Date: Wed, 15 Dec 2010 15:44:17 +1300
Subject: [PATCH] fixup! pca953x: support 16-pin devices

---
 drivers/gpio/pca953x.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
drivers/gpio/pca953x.c +4 -1
index c8f5403..359fdee 100644
@@ -65,7 +65,10 @@ static int pca953x_ngpio(uint8_t chip)
 	return 8;
 }
 #else
-#define pca953x_ngpio(chip)	8
+static int pca953x_ngpio(uint8_t chip)
+{
+	return 8;
+}
 #endif
 
 /*
Signature · 1 line
1.7.3.2
#6 lore raw

PATCH v3 1/1 pca953x: support 16-pin devices

Dear Chris Packham,

Quoted 7 lines · In message <[email protected]> you…

There is one minor fixup we might want to squash into v3 (or if someone wants me to submit a v4 I can). It makes sense to have pca953x_ngpio as a function in both cases. The compiler will auto-inline the function so we won't see a increase in size and having a function instead of a macro allows the compiler to do proper type checking.

Please send a v4. Thanks.

Best regards,

Wolfgang Denk

Signature · 4 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I've seen it. It's rubbish."          - Marvin the Paranoid Android
#7 lore raw

PATCH v4 1/1 pca953x: support 16-pin devices

This adds support for for the PCA9535/PCA9539 family of gpio devices which have 16 output pins.

To let the driver know which devices are 16-pin it is necessary to define CONFIG_SYS_I2C_PCA953X_WIDTH in your board config file. This is used to create an array of {chip, ngpio} tuples that are used to determine the width of a particular chip. For backwards compatibility it is assumed that any chip not defined in CONFIG_SYS_I2C_PCA953X_WIDTH has 8 pins.

---
Changes since v3:
- pca953x_ngpio is now a function in all cases.
- Added Peter's ACK to the commit message.

 README                 |    4 ++
 drivers/gpio/pca953x.c |  114 ++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 95 insertions(+), 23 deletions(-)
README +4 -0
index 68f5fb0..831c5af 100644
@@ -746,6 +746,10 @@ The following options need to be configured:
 		CONFIG_PCA953X		- use NXP's PCA953X series I2C GPIO
 		CONFIG_PCA953X_INFO	- enable pca953x info command
 
+		The CONFIG_SYS_I2C_PCA953X_WIDTH option specifies a list of
+		chip-ngpio pairs that tell the PCA953X driver the number of
+		pins supported by a particular chip.
+
 		Note that if the GPIO device uses I2C, then the I2C interface
 		must also be configured. See I2C Support, below.
 
drivers/gpio/pca953x.c +90 -22

164 lines not on this page · raw message

Signature · 1 line
1.7.3.2
#8 lore raw

PATCH v4 1/1 pca953x: support 16-pin devices

Hello Chris,

Sorry for the late reply, but just looked in patchwork and found that I am responsible for your patch, so ...

Quoted 13 lines · Chris Packham

This adds support for for the PCA9535/PCA9539 family of gpio devices which have 16 output pins.

To let the driver know which devices are 16-pin it is necessary to define CONFIG_SYS_I2C_PCA953X_WIDTH in your board config file. This is used to create an array of {chip, ngpio} tuples that are used to determine the width of a particular chip. For backwards compatibility it is assumed that any chip not defined in CONFIG_SYS_I2C_PCA953X_WIDTH has 8 pins.

Quoted 8 lines

---
Changes since v3:
- pca953x_ngpio is now a function in all cases.
- Added Peter's ACK to the commit message.

 README                 |    4 ++
 drivers/gpio/pca953x.c |  114 ++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 95 insertions(+), 23 deletions(-)

Compiles with actual u-boot the xpedite* boards, so added to u-boot-i2c.git

Thanks!

bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#9 lore raw

PATCH v4 1/1 pca953x: support 16-pin devices

Quoted 28 lines · Heiko Schocher

Hello Chris,

Sorry for the late reply, but just looked in patchwork and found that I am responsible for your patch, so ...

Quoted 13 lines · Chris Packham

This adds support for for the PCA9535/PCA9539 family of gpio devices which have 16 output pins.

To let the driver know which devices are 16-pin it is necessary to define CONFIG_SYS_I2C_PCA953X_WIDTH in your board config file. This is used to create an array of {chip, ngpio} tuples that are used to determine the width of a particular chip. For backwards compatibility it is assumed that any chip not defined in CONFIG_SYS_I2C_PCA953X_WIDTH has 8 pins.

Quoted 8 lines

---
Changes since v3:
- pca953x_ngpio is now a function in all cases.
- Added Peter's ACK to the commit message.

?README ? ? ? ? ? ? ? ? | ? ?4 ++
?drivers/gpio/pca953x.c | ?114 ++++++++++++++++++++++++++++++++++++++---------- ?2 files changed, 95 insertions(+), 23 deletions(-)

Quoted 7 lines

Compiles with actual u-boot the xpedite* boards, so added to u-boot-i2c.git

Thanks!

bye,
Heiko

Thanks I was about to poke the mailing list to find out where things had got to. Did I miss something on
http://www.denx.de/wiki/U-Boot/Patches that says look <here> list for a custodian and CC them?

#10 lore raw

PATCH v4 1/1 pca953x: support 16-pin devices

Quoted 34 lines · Chris Packham
Quoted 33 lines · Heiko Schocher

Hello Chris,

Sorry for the late reply, but just looked in patchwork and found that I am responsible for your patch, so ...

Quoted 13 lines · Chris Packham

This adds support for for the PCA9535/PCA9539 family of gpio devices which have 16 output pins.

To let the driver know which devices are 16-pin it is necessary to define CONFIG_SYS_I2C_PCA953X_WIDTH in your board config file. This is used to create an array of {chip, ngpio} tuples that are used to determine the width of a particular chip. For backwards compatibility it is assumed that any chip not defined in CONFIG_SYS_I2C_PCA953X_WIDTH has 8 pins.

Quoted 8 lines

---
Changes since v3:
- pca953x_ngpio is now a function in all cases.
- Added Peter's ACK to the commit message.

 README                 |    4 ++
 drivers/gpio/pca953x.c |  114 ++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 95 insertions(+), 23 deletions(-)

Compiles with actual u-boot the xpedite* boards, so added to u-boot-i2c.git

Thanks!

Quoted 7 lines
Quoted 2 lines

bye,
Heiko

Thanks I was about to poke the mailing list to find out where things had got to. Did I miss something on
http://www.denx.de/wiki/U-Boot/Patches that says look <here> list for a custodian and CC them?

There's a list of custodians at
http://www.denx.de/wiki/U-Boot/Custodians , but your GPIO driver doesn't really fall neatly into any one of their areas of responsibility, so it'd be tough to know who to CC. I added a comment to
http://www.denx.de/wiki/U-Boot/Patches to mention that appropriate maintainers should be CC-ed, as well as people who might be familiar with the change based on past commits for what its worth.

Best,
Peter