[PATCH v3 0/3] Add support for gpio expander pca9505 used on Mirabox

[PATCH v3 0/3] Add support for gpio expander pca9505 used on Mirabox

[Gregory CLEMENT]

Hello,

This patch set adds the support for the i2c gpio expander pca9505 used
on the JTAG/GPIO box which can be connected to the Mirabox.

To be able to use the pca9505 I had to do several changes in the
driver. Indeed, until now the pca953x driver accessed all the bank of
a given register in a single command using only a 32 bits
variable. This expander comes with 40 GPIOs which no more fits in a 32
variable. This patch set makes the accesses to the registers more
generic by relying on an array of u8 variables. This fits exactly the
way the registers are represented in the hardware.

Once the per-bank representation was added, it was easier to introduce
helpers to access to a single register of a bank instead of reading or
writing all the banks for a given register. As the GPIO API allows
only the accesses to a single GPIO at a time there was no point to read
and write all the other banks. Hence it should help to decrease the
latency especially for the pca9505.

However as the block GPIO API from Roland Stigge is incoming I kept
the helpers used to access all the banks in the same time. I had to
make some modifications in the arguments that these functions
received, so it will have a conflict here. Currently my patch set is
based on v3.8-rc4, but I am willing to rebase onto gpio-for-next once
the GPIO block will be merged into it.

After the feedback from Maxime Ripard on the second version, I
continue to abuse of his good will as I didn't get any information on
the Mirabox about the interrupt pin. Thanks to him I managed to fix
the bug related to the interrupt part. Now the IRQ part have been also
validated! Thanks again to Maxime for his help.

I have also updated the branch gpio-pca9505 available at:
https://github.com/MISL-EBU-System-SW/mainline-public.git

Linus, now you can apply the first two patches on your git tree. I
kept your acked-by for the 3rd one, as I didn't change anything for
this one.

Thanks,

Changelog:
V1->V2:

- Fix the way to calculate the shift used to apply to register to
  access a given bank.
- Fix all the pending issue in the IRQ part which appeared once I have
  enable CONFIG_GPIO_PCA953X_IRQ!

V2->V3:

- Rebased on v3.8-rc4
- Fix the interrupt handler and not try to call an handler if there is
  no irq pending on a gpio bank


Gregory CLEMENT (3):
  gpio: pca953x: make the register access by GPIO bank
  gpio: pca953x: add support for pca9505
  arm: mvebu: enable gpio expander over i2c on Mirabox platform

 arch/arm/boot/dts/armada-370-mirabox.dts |   10 ++
 drivers/gpio/gpio-pca953x.c              |  283 +++++++++++++++++++-----------
 2 files changed, 187 insertions(+), 106 deletions(-)

-- 
1.7.9.5

[PATCH v3 0/3] Add support for gpio expander pca9505 used on Mirabox

[Gregory CLEMENT]

Hello,

This patch set adds the support for the i2c gpio expander pca9505 used
on the JTAG/GPIO box which can be connected to the Mirabox.

To be able to use the pca9505 I had to do several changes in the
driver. Indeed, until now the pca953x driver accessed all the bank of
a given register in a single command using only a 32 bits
variable. This expander comes with 40 GPIOs which no more fits in a 32
variable. This patch set makes the accesses to the registers more
generic by relying on an array of u8 variables. This fits exactly the
way the registers are represented in the hardware.

Once the per-bank representation was added, it was easier to introduce
helpers to access to a single register of a bank instead of reading or
writing all the banks for a given register. As the GPIO API allows
only the accesses to a single GPIO at a time there was no point to read
and write all the other banks. Hence it should help to decrease the
latency especially for the pca9505.

However as the block GPIO API from Roland Stigge is incoming I kept
the helpers used to access all the banks in the same time. I had to
make some modifications in the arguments that these functions
received, so it will have a conflict here. Currently my patch set is
based on v3.8-rc4, but I am willing to rebase onto gpio-for-next once
the GPIO block will be merged into it.

After the feedback from Maxime Ripard on the second version, I
continue to abuse of his good will as I didn't get any information on
the Mirabox about the interrupt pin. Thanks to him I managed to fix
the bug related to the interrupt part. Now the IRQ part have been also
validated! Thanks again to Maxime for his help.

I have also updated the branch gpio-pca9505 available at:
https://github.com/MISL-EBU-System-SW/mainline-public.git

Linus, now you can apply the first two patches on your git tree. I
kept your acked-by for the 3rd one, as I didn't change anything for
this one.

Thanks,

Changelog:
V1->V2:

- Fix the way to calculate the shift used to apply to register to
  access a given bank.
- Fix all the pending issue in the IRQ part which appeared once I have
  enable CONFIG_GPIO_PCA953X_IRQ!

V2->V3:

- Rebased on v3.8-rc4
- Fix the interrupt handler and not try to call an handler if there is
  no irq pending on a gpio bank


Gregory CLEMENT (3):
  gpio: pca953x: make the register access by GPIO bank
  gpio: pca953x: add support for pca9505
  arm: mvebu: enable gpio expander over i2c on Mirabox platform

 arch/arm/boot/dts/armada-370-mirabox.dts |   10 ++
 drivers/gpio/gpio-pca953x.c              |  283 +++++++++++++++++++-----------
 2 files changed, 187 insertions(+), 106 deletions(-)

-- 
1.7.9.5

[PATCH v3 1/3] gpio: pca953x: make the register access by GPIO bank

[Gregory CLEMENT]

Until now the pca953x driver accessed all the bank of a given register
in a single command using only a 32 bits variable. New expanders from
the pca53x family come with 40 GPIOs which no more fit in a 32
variable. This patch make access to the registers more generic by
relying on an array of u8 variables. This fits exactly the way the
registers are represented in the hardware.

It also adds helpers to access to a single register of a bank instead
of reading or writing all the banks for a given register.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Tested-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpio/gpio-pca953x.c |  281 +++++++++++++++++++++++++++----------------
 1 file changed, 175 insertions(+), 106 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index cc102d2..b35ba06 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -71,18 +71,23 @@ static const struct i2c_device_id pca953x_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pca953x_id);
 
+#define MAX_BANK 5
+#define BANK_SZ 8
+
+#define NBANK(chip) (chip->gpio_chip.ngpio / BANK_SZ)
+
 struct pca953x_chip {
 	unsigned gpio_start;
-	u32 reg_output;
-	u32 reg_direction;
+	u8 reg_output[MAX_BANK];
+	u8 reg_direction[MAX_BANK];
 	struct mutex i2c_lock;
 
 #ifdef CONFIG_GPIO_PCA953X_IRQ
 	struct mutex irq_lock;
-	u32 irq_mask;
-	u32 irq_stat;
-	u32 irq_trig_raise;
-	u32 irq_trig_fall;
+	u8 irq_mask[MAX_BANK];
+	u8 irq_stat[MAX_BANK];
+	u8 irq_trig_raise[MAX_BANK];
+	u8 irq_trig_fall[MAX_BANK];
 	int	 irq_base;
 	struct irq_domain *domain;
 #endif
@@ -93,33 +98,69 @@ struct pca953x_chip {
 	int	chip_type;
 };
 
-static int pca953x_write_reg(struct pca953x_chip *chip, int reg, u32 val)
+static int pca953x_read_single(struct pca953x_chip *chip, int reg, u32 *val,
+				int off)
+{
+	int ret;
+	int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
+	int offset = off / BANK_SZ;
+
+	ret = i2c_smbus_read_byte_data(chip->client,
+				(reg << bank_shift) + offset);
+	*val = ret;
+
+	if (ret < 0) {
+		dev_err(&chip->client->dev, "failed reading register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int pca953x_write_single(struct pca953x_chip *chip, int reg, u32 val,
+				int off)
+{
+	int ret = 0;
+	int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
+	int offset = off / BANK_SZ;
+
+	ret = i2c_smbus_write_byte_data(chip->client,
+					(reg << bank_shift) + offset, val);
+
+	if (ret < 0) {
+		dev_err(&chip->client->dev, "failed writing register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int pca953x_write_regs(struct pca953x_chip *chip, int reg, u8 *val)
 {
 	int ret = 0;
 
 	if (chip->gpio_chip.ngpio <= 8)
-		ret = i2c_smbus_write_byte_data(chip->client, reg, val);
-	else if (chip->gpio_chip.ngpio == 24) {
-		cpu_to_le32s(&val);
+		ret = i2c_smbus_write_byte_data(chip->client, reg, *val);
+	else if (chip->gpio_chip.ngpio >= 24) {
+		int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
 		ret = i2c_smbus_write_i2c_block_data(chip->client,
-						(reg << 2) | REG_ADDR_AI,
-						3,
-						(u8 *) &val);
+					(reg << bank_shift) | REG_ADDR_AI,
+					NBANK(chip), val);
 	}
 	else {
 		switch (chip->chip_type) {
 		case PCA953X_TYPE:
 			ret = i2c_smbus_write_word_data(chip->client,
-							reg << 1, val);
+							reg << 1, (u16) *val);
 			break;
 		case PCA957X_TYPE:
 			ret = i2c_smbus_write_byte_data(chip->client, reg << 1,
-							val & 0xff);
+							val[0]);
 			if (ret < 0)
 				break;
 			ret = i2c_smbus_write_byte_data(chip->client,
 							(reg << 1) + 1,
-							(val & 0xff00) >> 8);
+							val[1]);
 			break;
 		}
 	}
@@ -132,26 +173,24 @@ static int pca953x_write_reg(struct pca953x_chip *chip, int reg, u32 val)
 	return 0;
 }
 
-static int pca953x_read_reg(struct pca953x_chip *chip, int reg, u32 *val)
+static int pca953x_read_regs(struct pca953x_chip *chip, int reg, u8 *val)
 {
 	int ret;
 
 	if (chip->gpio_chip.ngpio <= 8) {
 		ret = i2c_smbus_read_byte_data(chip->client, reg);
 		*val = ret;
-	}
-	else if (chip->gpio_chip.ngpio == 24) {
-		*val = 0;
+	} else if (chip->gpio_chip.ngpio >= 24) {
+		int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
+
 		ret = i2c_smbus_read_i2c_block_data(chip->client,
-						(reg << 2) | REG_ADDR_AI,
-						3,
-						(u8 *) val);
-		le32_to_cpus(val);
+					(reg << bank_shift) | REG_ADDR_AI,
+					NBANK(chip), val);
 	} else {
 		ret = i2c_smbus_read_word_data(chip->client, reg << 1);
-		*val = ret;
+		val[0] = (u16)ret & 0xFF;
+		val[1] = (u16)ret >> 8;
 	}
-
 	if (ret < 0) {
 		dev_err(&chip->client->dev, "failed reading register\n");
 		return ret;
@@ -163,13 +202,13 @@ static int pca953x_read_reg(struct pca953x_chip *chip, int reg, u32 *val)
 static int pca953x_gpio_direction_input(struct gpio_chip *gc, unsigned off)
 {
 	struct pca953x_chip *chip;
-	uint reg_val;
+	u8 reg_val;
 	int ret, offset = 0;
 
 	chip = container_of(gc, struct pca953x_chip, gpio_chip);
 
 	mutex_lock(&chip->i2c_lock);
-	reg_val = chip->reg_direction | (1u << off);
+	reg_val = chip->reg_direction[off / BANK_SZ] | (1u << (off % BANK_SZ));
 
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
@@ -179,11 +218,11 @@ static int pca953x_gpio_direction_input(struct gpio_chip *gc, unsigned off)
 		offset = PCA957X_CFG;
 		break;
 	}
-	ret = pca953x_write_reg(chip, offset, reg_val);
+	ret = pca953x_write_single(chip, offset, reg_val, off);
 	if (ret)
 		goto exit;
 
-	chip->reg_direction = reg_val;
+	chip->reg_direction[off / BANK_SZ] = reg_val;
 	ret = 0;
 exit:
 	mutex_unlock(&chip->i2c_lock);
@@ -194,7 +233,7 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
 		unsigned off, int val)
 {
 	struct pca953x_chip *chip;
-	uint reg_val;
+	u8 reg_val;
 	int ret, offset = 0;
 
 	chip = container_of(gc, struct pca953x_chip, gpio_chip);
@@ -202,9 +241,11 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
 	mutex_lock(&chip->i2c_lock);
 	/* set output level */
 	if (val)
-		reg_val = chip->reg_output | (1u << off);
+		reg_val = chip->reg_output[off / BANK_SZ]
+			| (1u << (off % BANK_SZ));
 	else
-		reg_val = chip->reg_output & ~(1u << off);
+		reg_val = chip->reg_output[off / BANK_SZ]
+			& ~(1u << (off % BANK_SZ));
 
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
@@ -214,14 +255,14 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
 		offset = PCA957X_OUT;
 		break;
 	}
-	ret = pca953x_write_reg(chip, offset, reg_val);
+	ret = pca953x_write_single(chip, offset, reg_val, off);
 	if (ret)
 		goto exit;
 
-	chip->reg_output = reg_val;
+	chip->reg_output[off / BANK_SZ] = reg_val;
 
 	/* then direction */
-	reg_val = chip->reg_direction & ~(1u << off);
+	reg_val = chip->reg_direction[off / BANK_SZ] & ~(1u << (off % BANK_SZ));
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
 		offset = PCA953X_DIRECTION;
@@ -230,11 +271,11 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
 		offset = PCA957X_CFG;
 		break;
 	}
-	ret = pca953x_write_reg(chip, offset, reg_val);
+	ret = pca953x_write_single(chip, offset, reg_val, off);
 	if (ret)
 		goto exit;
 
-	chip->reg_direction = reg_val;
+	chip->reg_direction[off / BANK_SZ] = reg_val;
 	ret = 0;
 exit:
 	mutex_unlock(&chip->i2c_lock);
@@ -258,7 +299,7 @@ static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off)
 		offset = PCA957X_IN;
 		break;
 	}
-	ret = pca953x_read_reg(chip, offset, &reg_val);
+	ret = pca953x_read_single(chip, offset, &reg_val, off);
 	mutex_unlock(&chip->i2c_lock);
 	if (ret < 0) {
 		/* NOTE:  diagnostic already emitted; that's all we should
@@ -274,16 +315,18 @@ static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off)
 static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val)
 {
 	struct pca953x_chip *chip;
-	u32 reg_val;
+	u8 reg_val;
 	int ret, offset = 0;
 
 	chip = container_of(gc, struct pca953x_chip, gpio_chip);
 
 	mutex_lock(&chip->i2c_lock);
 	if (val)
-		reg_val = chip->reg_output | (1u << off);
+		reg_val = chip->reg_output[off / BANK_SZ]
+			| (1u << (off % BANK_SZ));
 	else
-		reg_val = chip->reg_output & ~(1u << off);
+		reg_val = chip->reg_output[off / BANK_SZ]
+			& ~(1u << (off % BANK_SZ));
 
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
@@ -293,11 +336,11 @@ static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val)
 		offset = PCA957X_OUT;
 		break;
 	}
-	ret = pca953x_write_reg(chip, offset, reg_val);
+	ret = pca953x_write_single(chip, offset, reg_val, off);
 	if (ret)
 		goto exit;
 
-	chip->reg_output = reg_val;
+	chip->reg_output[off / BANK_SZ] = reg_val;
 exit:
 	mutex_unlock(&chip->i2c_lock);
 }
@@ -335,14 +378,14 @@ static void pca953x_irq_mask(struct irq_data *d)
 {
 	struct pca953x_chip *chip = irq_data_get_irq_chip_data(d);
 
-	chip->irq_mask &= ~(1 << d->hwirq);
+	chip->irq_mask[d->hwirq / BANK_SZ] &= ~(1 << (d->hwirq % BANK_SZ));
 }
 
 static void pca953x_irq_unmask(struct irq_data *d)
 {
 	struct pca953x_chip *chip = irq_data_get_irq_chip_data(d);
 
-	chip->irq_mask |= 1 << d->hwirq;
+	chip->irq_mask[d->hwirq / BANK_SZ] |= 1 << (d->hwirq % BANK_SZ);
 }
 
 static void pca953x_irq_bus_lock(struct irq_data *d)
@@ -355,17 +398,20 @@ static void pca953x_irq_bus_lock(struct irq_data *d)
 static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
 {
 	struct pca953x_chip *chip = irq_data_get_irq_chip_data(d);
-	u32 new_irqs;
-	u32 level;
+	u8 new_irqs;
+	int level, i;
 
 	/* Look for any newly setup interrupt */
-	new_irqs = chip->irq_trig_fall | chip->irq_trig_raise;
-	new_irqs &= ~chip->reg_direction;
-
-	while (new_irqs) {
-		level = __ffs(new_irqs);
-		pca953x_gpio_direction_input(&chip->gpio_chip, level);
-		new_irqs &= ~(1 << level);
+	for (i = 0; i < NBANK(chip); i++) {
+		new_irqs = chip->irq_trig_fall[i] | chip->irq_trig_raise[i];
+		new_irqs &= ~chip->reg_direction[i];
+
+		while (new_irqs) {
+			level = __ffs(new_irqs);
+			pca953x_gpio_direction_input(&chip->gpio_chip,
+							level + (BANK_SZ * i));
+			new_irqs &= ~(1 << level);
+		}
 	}
 
 	mutex_unlock(&chip->irq_lock);
@@ -374,7 +420,8 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
 static int pca953x_irq_set_type(struct irq_data *d, unsigned int type)
 {
 	struct pca953x_chip *chip = irq_data_get_irq_chip_data(d);
-	u32 mask = 1 << d->hwirq;
+	int bank_nb = d->hwirq / BANK_SZ;
+	u8 mask = 1 << (d->hwirq % BANK_SZ);
 
 	if (!(type & IRQ_TYPE_EDGE_BOTH)) {
 		dev_err(&chip->client->dev, "irq %d: unsupported type %d\n",
@@ -383,14 +430,14 @@ static int pca953x_irq_set_type(struct irq_data *d, unsigned int type)
 	}
 
 	if (type & IRQ_TYPE_EDGE_FALLING)
-		chip->irq_trig_fall |= mask;
+		chip->irq_trig_fall[bank_nb] |= mask;
 	else
-		chip->irq_trig_fall &= ~mask;
+		chip->irq_trig_fall[bank_nb] &= ~mask;
 
 	if (type & IRQ_TYPE_EDGE_RISING)
-		chip->irq_trig_raise |= mask;
+		chip->irq_trig_raise[bank_nb] |= mask;
 	else
-		chip->irq_trig_raise &= ~mask;
+		chip->irq_trig_raise[bank_nb] &= ~mask;
 
 	return 0;
 }
@@ -404,13 +451,13 @@ static struct irq_chip pca953x_irq_chip = {
 	.irq_set_type		= pca953x_irq_set_type,
 };
 
-static u32 pca953x_irq_pending(struct pca953x_chip *chip)
+static u8 pca953x_irq_pending(struct pca953x_chip *chip, u8 *pending)
 {
-	u32 cur_stat;
-	u32 old_stat;
-	u32 pending;
-	u32 trigger;
-	int ret, offset = 0;
+	u8 cur_stat[MAX_BANK];
+	u8 old_stat[MAX_BANK];
+	u8 pendings = 0;
+	u8 trigger[MAX_BANK], triggers = 0;
+	int ret, i, offset = 0;
 
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
@@ -420,45 +467,54 @@ static u32 pca953x_irq_pending(struct pca953x_chip *chip)
 		offset = PCA957X_IN;
 		break;
 	}
-	ret = pca953x_read_reg(chip, offset, &cur_stat);
+	ret = pca953x_read_regs(chip, offset, cur_stat);
 	if (ret)
 		return 0;
 
 	/* Remove output pins from the equation */
-	cur_stat &= chip->reg_direction;
+	for (i = 0; i < NBANK(chip); i++)
+		cur_stat[i] &= chip->reg_direction[i];
 
-	old_stat = chip->irq_stat;
-	trigger = (cur_stat ^ old_stat) & chip->irq_mask;
+	memcpy(old_stat, chip->irq_stat, NBANK(chip));
 
-	if (!trigger)
+	for (i = 0; i < NBANK(chip); i++) {
+		trigger[i] = (cur_stat[i] ^ old_stat[i]) & chip->irq_mask[i];
+		triggers += trigger[i];
+	}
+
+	if (!triggers)
 		return 0;
 
-	chip->irq_stat = cur_stat;
+	memcpy(chip->irq_stat, cur_stat, NBANK(chip));
 
-	pending = (old_stat & chip->irq_trig_fall) |
-		  (cur_stat & chip->irq_trig_raise);
-	pending &= trigger;
+	for (i = 0; i < NBANK(chip); i++) {
+		pending[i] = (old_stat[i] & chip->irq_trig_fall[i]) |
+			(cur_stat[i] & chip->irq_trig_raise[i]);
+		pending[i] &= trigger[i];
+		pendings += pending[i];
+	}
 
-	return pending;
+	return pendings;
 }
 
 static irqreturn_t pca953x_irq_handler(int irq, void *devid)
 {
 	struct pca953x_chip *chip = devid;
-	u32 pending;
-	u32 level;
-
-	pending = pca953x_irq_pending(chip);
+	u8 pending[MAX_BANK];
+	u8 level;
+	int i;
 
-	if (!pending)
+	if (!pca953x_irq_pending(chip, pending))
 		return IRQ_HANDLED;
 
-	do {
-		level = __ffs(pending);
-		handle_nested_irq(irq_find_mapping(chip->domain, level));
-
-		pending &= ~(1 << level);
-	} while (pending);
+	for (i = 0; i < NBANK(chip); i++) {
+		while (pending[i]) {
+			level = __ffs(pending[i]);
+			handle_nested_irq(irq_find_mapping(chip->domain,
+							level + (BANK_SZ * i)));
+			pending[i] &= ~(1 << level);
+		}
+	}
 
 	return IRQ_HANDLED;
 }
@@ -468,8 +524,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 			     int irq_base)
 {
 	struct i2c_client *client = chip->client;
-	int ret, offset = 0;
-	u32 temporary;
+	int ret, i, offset = 0;
 
 	if (irq_base != -1
 			&& (id->driver_data & PCA_INT)) {
@@ -483,8 +538,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 			offset = PCA957X_IN;
 			break;
 		}
-		ret = pca953x_read_reg(chip, offset, &temporary);
-		chip->irq_stat = temporary;
+		ret = pca953x_read_regs(chip, offset, chip->irq_stat);
 		if (ret)
 			goto out_failed;
 
@@ -493,7 +547,8 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 		 * interrupt.  We have to rely on the previous read for
 		 * this purpose.
 		 */
-		chip->irq_stat &= chip->reg_direction;
+		for (i = 0; i < NBANK(chip); i++)
+			chip->irq_stat[i] &= chip->reg_direction[i];
 		mutex_init(&chip->irq_lock);
 
 		chip->irq_base = irq_alloc_descs(-1, irq_base, chip->gpio_chip.ngpio, -1);
@@ -619,18 +674,24 @@ pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, u32 *invert)
 static int device_pca953x_init(struct pca953x_chip *chip, u32 invert)
 {
 	int ret;
+	u8 val[MAX_BANK];
 
-	ret = pca953x_read_reg(chip, PCA953X_OUTPUT, &chip->reg_output);
+	ret = pca953x_read_regs(chip, PCA953X_OUTPUT, chip->reg_output);
 	if (ret)
 		goto out;
 
-	ret = pca953x_read_reg(chip, PCA953X_DIRECTION,
-			       &chip->reg_direction);
+	ret = pca953x_read_regs(chip, PCA953X_DIRECTION,
+			       chip->reg_direction);
 	if (ret)
 		goto out;
 
 	/* set platform specific polarity inversion */
-	ret = pca953x_write_reg(chip, PCA953X_INVERT, invert);
+	if (invert)
+		memset(val, 0xFF, NBANK(chip));
+	else
+		memset(val, 0, NBANK(chip));
+
+	ret = pca953x_write_regs(chip, PCA953X_INVERT, val);
 out:
 	return ret;
 }
@@ -638,28 +699,36 @@ out:
 static int device_pca957x_init(struct pca953x_chip *chip, u32 invert)
 {
 	int ret;
-	u32 val = 0;
+	u8 val[MAX_BANK];
 
 	/* Let every port in proper state, that could save power */
-	pca953x_write_reg(chip, PCA957X_PUPD, 0x0);
-	pca953x_write_reg(chip, PCA957X_CFG, 0xffff);
-	pca953x_write_reg(chip, PCA957X_OUT, 0x0);
-
-	ret = pca953x_read_reg(chip, PCA957X_IN, &val);
+	memset(val, 0, NBANK(chip));
+	pca953x_write_regs(chip, PCA957X_PUPD, val);
+	memset(val, 0xFF, NBANK(chip));
+	pca953x_write_regs(chip, PCA957X_CFG, val);
+	memset(val, 0, NBANK(chip));
+	pca953x_write_regs(chip, PCA957X_OUT, val);
+
+	ret = pca953x_read_regs(chip, PCA957X_IN, val);
 	if (ret)
 		goto out;
-	ret = pca953x_read_reg(chip, PCA957X_OUT, &chip->reg_output);
+	ret = pca953x_read_regs(chip, PCA957X_OUT, chip->reg_output);
 	if (ret)
 		goto out;
-	ret = pca953x_read_reg(chip, PCA957X_CFG, &chip->reg_direction);
+	ret = pca953x_read_regs(chip, PCA957X_CFG, chip->reg_direction);
 	if (ret)
 		goto out;
 
 	/* set platform specific polarity inversion */
-	pca953x_write_reg(chip, PCA957X_INVRT, invert);
+	if (invert)
+		memset(val, 0xFF, NBANK(chip));
+	else
+		memset(val, 0, NBANK(chip));
+	pca953x_write_regs(chip, PCA957X_INVRT, val);
 
 	/* To enable register 6, 7 to controll pull up and pull down */
-	pca953x_write_reg(chip, PCA957X_BKEN, 0x202);
+	memset(val, 0x02, NBANK(chip));
+	pca953x_write_regs(chip, PCA957X_BKEN, val);
 
 	return 0;
 out:
-- 
1.7.9.5

[PATCH v3 1/3] gpio: pca953x: make the register access by GPIO bank

[Gregory CLEMENT]

Until now the pca953x driver accessed all the bank of a given register
in a single command using only a 32 bits variable. New expanders from
the pca53x family come with 40 GPIOs which no more fit in a 32
variable. This patch make access to the registers more generic by
relying on an array of u8 variables. This fits exactly the way the
registers are represented in the hardware.

It also adds helpers to access to a single register of a bank instead
of reading or writing all the banks for a given register.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Tested-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpio/gpio-pca953x.c |  281 +++++++++++++++++++++++++++----------------
 1 file changed, 175 insertions(+), 106 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index cc102d2..b35ba06 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -71,18 +71,23 @@ static const struct i2c_device_id pca953x_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pca953x_id);
 
+#define MAX_BANK 5
+#define BANK_SZ 8
+
+#define NBANK(chip) (chip->gpio_chip.ngpio / BANK_SZ)
+
 struct pca953x_chip {
 	unsigned gpio_start;
-	u32 reg_output;
-	u32 reg_direction;
+	u8 reg_output[MAX_BANK];
+	u8 reg_direction[MAX_BANK];
 	struct mutex i2c_lock;
 
 #ifdef CONFIG_GPIO_PCA953X_IRQ
 	struct mutex irq_lock;
-	u32 irq_mask;
-	u32 irq_stat;
-	u32 irq_trig_raise;
-	u32 irq_trig_fall;
+	u8 irq_mask[MAX_BANK];
+	u8 irq_stat[MAX_BANK];
+	u8 irq_trig_raise[MAX_BANK];
+	u8 irq_trig_fall[MAX_BANK];
 	int	 irq_base;
 	struct irq_domain *domain;
 #endif
@@ -93,33 +98,69 @@ struct pca953x_chip {
 	int	chip_type;
 };
 
-static int pca953x_write_reg(struct pca953x_chip *chip, int reg, u32 val)
+static int pca953x_read_single(struct pca953x_chip *chip, int reg, u32 *val,
+				int off)
+{
+	int ret;
+	int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
+	int offset = off / BANK_SZ;
+
+	ret = i2c_smbus_read_byte_data(chip->client,
+				(reg << bank_shift) + offset);
+	*val = ret;
+
+	if (ret < 0) {
+		dev_err(&chip->client->dev, "failed reading register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int pca953x_write_single(struct pca953x_chip *chip, int reg, u32 val,
+				int off)
+{
+	int ret = 0;
+	int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
+	int offset = off / BANK_SZ;
+
+	ret = i2c_smbus_write_byte_data(chip->client,
+					(reg << bank_shift) + offset, val);
+
+	if (ret < 0) {
+		dev_err(&chip->client->dev, "failed writing register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int pca953x_write_regs(struct pca953x_chip *chip, int reg, u8 *val)
 {
 	int ret = 0;
 
 	if (chip->gpio_chip.ngpio <= 8)
-		ret = i2c_smbus_write_byte_data(chip->client, reg, val);
-	else if (chip->gpio_chip.ngpio == 24) {
-		cpu_to_le32s(&val);
+		ret = i2c_smbus_write_byte_data(chip->client, reg, *val);
+	else if (chip->gpio_chip.ngpio >= 24) {
+		int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
 		ret = i2c_smbus_write_i2c_block_data(chip->client,
-						(reg << 2) | REG_ADDR_AI,
-						3,
-						(u8 *) &val);
+					(reg << bank_shift) | REG_ADDR_AI,
+					NBANK(chip), val);
 	}
 	else {
 		switch (chip->chip_type) {
 		case PCA953X_TYPE:
 			ret = i2c_smbus_write_word_data(chip->client,
-							reg << 1, val);
+							reg << 1, (u16) *val);
 			break;
 		case PCA957X_TYPE:
 			ret = i2c_smbus_write_byte_data(chip->client, reg << 1,
-							val & 0xff);
+							val[0]);
 			if (ret < 0)
 				break;
 			ret = i2c_smbus_write_byte_data(chip->client,
 							(reg << 1) + 1,
-							(val & 0xff00) >> 8);
+							val[1]);
 			break;
 		}
 	}
@@ -132,26 +173,24 @@ static int pca953x_write_reg(struct pca953x_chip *chip, int reg, u32 val)
 	return 0;
 }
 
-static int pca953x_read_reg(struct pca953x_chip *chip, int reg, u32 *val)
+static int pca953x_read_regs(struct pca953x_chip *chip, int reg, u8 *val)
 {
 	int ret;
 
 	if (chip->gpio_chip.ngpio <= 8) {
 		ret = i2c_smbus_read_byte_data(chip->client, reg);
 		*val = ret;
-	}
-	else if (chip->gpio_chip.ngpio == 24) {
-		*val = 0;
+	} else if (chip->gpio_chip.ngpio >= 24) {
+		int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
+
 		ret = i2c_smbus_read_i2c_block_data(chip->client,
-						(reg << 2) | REG_ADDR_AI,
-						3,
-						(u8 *) val);
-		le32_to_cpus(val);
+					(reg << bank_shift) | REG_ADDR_AI,
+					NBANK(chip), val);
 	} else {
 		ret = i2c_smbus_read_word_data(chip->client, reg << 1);
-		*val = ret;
+		val[0] = (u16)ret & 0xFF;
+		val[1] = (u16)ret >> 8;
 	}
-
 	if (ret < 0) {
 		dev_err(&chip->client->dev, "failed reading register\n");
 		return ret;
@@ -163,13 +202,13 @@ static int pca953x_read_reg(struct pca953x_chip *chip, int reg, u32 *val)
 static int pca953x_gpio_direction_input(struct gpio_chip *gc, unsigned off)
 {
 	struct pca953x_chip *chip;
-	uint reg_val;
+	u8 reg_val;
 	int ret, offset = 0;
 
 	chip = container_of(gc, struct pca953x_chip, gpio_chip);
 
 	mutex_lock(&chip->i2c_lock);
-	reg_val = chip->reg_direction | (1u << off);
+	reg_val = chip->reg_direction[off / BANK_SZ] | (1u << (off % BANK_SZ));
 
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
@@ -179,11 +218,11 @@ static int pca953x_gpio_direction_input(struct gpio_chip *gc, unsigned off)
 		offset = PCA957X_CFG;
 		break;
 	}
-	ret = pca953x_write_reg(chip, offset, reg_val);
+	ret = pca953x_write_single(chip, offset, reg_val, off);
 	if (ret)
 		goto exit;
 
-	chip->reg_direction = reg_val;
+	chip->reg_direction[off / BANK_SZ] = reg_val;
 	ret = 0;
 exit:
 	mutex_unlock(&chip->i2c_lock);
@@ -194,7 +233,7 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
 		unsigned off, int val)
 {
 	struct pca953x_chip *chip;
-	uint reg_val;
+	u8 reg_val;
 	int ret, offset = 0;
 
 	chip = container_of(gc, struct pca953x_chip, gpio_chip);
@@ -202,9 +241,11 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
 	mutex_lock(&chip->i2c_lock);
 	/* set output level */
 	if (val)
-		reg_val = chip->reg_output | (1u << off);
+		reg_val = chip->reg_output[off / BANK_SZ]
+			| (1u << (off % BANK_SZ));
 	else
-		reg_val = chip->reg_output & ~(1u << off);
+		reg_val = chip->reg_output[off / BANK_SZ]
+			& ~(1u << (off % BANK_SZ));
 
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
@@ -214,14 +255,14 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
 		offset = PCA957X_OUT;
 		break;
 	}
-	ret = pca953x_write_reg(chip, offset, reg_val);
+	ret = pca953x_write_single(chip, offset, reg_val, off);
 	if (ret)
 		goto exit;
 
-	chip->reg_output = reg_val;
+	chip->reg_output[off / BANK_SZ] = reg_val;
 
 	/* then direction */
-	reg_val = chip->reg_direction & ~(1u << off);
+	reg_val = chip->reg_direction[off / BANK_SZ] & ~(1u << (off % BANK_SZ));
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
 		offset = PCA953X_DIRECTION;
@@ -230,11 +271,11 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
 		offset = PCA957X_CFG;
 		break;
 	}
-	ret = pca953x_write_reg(chip, offset, reg_val);
+	ret = pca953x_write_single(chip, offset, reg_val, off);
 	if (ret)
 		goto exit;
 
-	chip->reg_direction = reg_val;
+	chip->reg_direction[off / BANK_SZ] = reg_val;
 	ret = 0;
 exit:
 	mutex_unlock(&chip->i2c_lock);
@@ -258,7 +299,7 @@ static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off)
 		offset = PCA957X_IN;
 		break;
 	}
-	ret = pca953x_read_reg(chip, offset, &reg_val);
+	ret = pca953x_read_single(chip, offset, &reg_val, off);
 	mutex_unlock(&chip->i2c_lock);
 	if (ret < 0) {
 		/* NOTE:  diagnostic already emitted; that's all we should
@@ -274,16 +315,18 @@ static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off)
 static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val)
 {
 	struct pca953x_chip *chip;
-	u32 reg_val;
+	u8 reg_val;
 	int ret, offset = 0;
 
 	chip = container_of(gc, struct pca953x_chip, gpio_chip);
 
 	mutex_lock(&chip->i2c_lock);
 	if (val)
-		reg_val = chip->reg_output | (1u << off);
+		reg_val = chip->reg_output[off / BANK_SZ]
+			| (1u << (off % BANK_SZ));
 	else
-		reg_val = chip->reg_output & ~(1u << off);
+		reg_val = chip->reg_output[off / BANK_SZ]
+			& ~(1u << (off % BANK_SZ));
 
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
@@ -293,11 +336,11 @@ static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val)
 		offset = PCA957X_OUT;
 		break;
 	}
-	ret = pca953x_write_reg(chip, offset, reg_val);
+	ret = pca953x_write_single(chip, offset, reg_val, off);
 	if (ret)
 		goto exit;
 
-	chip->reg_output = reg_val;
+	chip->reg_output[off / BANK_SZ] = reg_val;
 exit:
 	mutex_unlock(&chip->i2c_lock);
 }
@@ -335,14 +378,14 @@ static void pca953x_irq_mask(struct irq_data *d)
 {
 	struct pca953x_chip *chip = irq_data_get_irq_chip_data(d);
 
-	chip->irq_mask &= ~(1 << d->hwirq);
+	chip->irq_mask[d->hwirq / BANK_SZ] &= ~(1 << (d->hwirq % BANK_SZ));
 }
 
 static void pca953x_irq_unmask(struct irq_data *d)
 {
 	struct pca953x_chip *chip = irq_data_get_irq_chip_data(d);
 
-	chip->irq_mask |= 1 << d->hwirq;
+	chip->irq_mask[d->hwirq / BANK_SZ] |= 1 << (d->hwirq % BANK_SZ);
 }
 
 static void pca953x_irq_bus_lock(struct irq_data *d)
@@ -355,17 +398,20 @@ static void pca953x_irq_bus_lock(struct irq_data *d)
 static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
 {
 	struct pca953x_chip *chip = irq_data_get_irq_chip_data(d);
-	u32 new_irqs;
-	u32 level;
+	u8 new_irqs;
+	int level, i;
 
 	/* Look for any newly setup interrupt */
-	new_irqs = chip->irq_trig_fall | chip->irq_trig_raise;
-	new_irqs &= ~chip->reg_direction;
-
-	while (new_irqs) {
-		level = __ffs(new_irqs);
-		pca953x_gpio_direction_input(&chip->gpio_chip, level);
-		new_irqs &= ~(1 << level);
+	for (i = 0; i < NBANK(chip); i++) {
+		new_irqs = chip->irq_trig_fall[i] | chip->irq_trig_raise[i];
+		new_irqs &= ~chip->reg_direction[i];
+
+		while (new_irqs) {
+			level = __ffs(new_irqs);
+			pca953x_gpio_direction_input(&chip->gpio_chip,
+							level + (BANK_SZ * i));
+			new_irqs &= ~(1 << level);
+		}
 	}
 
 	mutex_unlock(&chip->irq_lock);
@@ -374,7 +420,8 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
 static int pca953x_irq_set_type(struct irq_data *d, unsigned int type)
 {
 	struct pca953x_chip *chip = irq_data_get_irq_chip_data(d);
-	u32 mask = 1 << d->hwirq;
+	int bank_nb = d->hwirq / BANK_SZ;
+	u8 mask = 1 << (d->hwirq % BANK_SZ);
 
 	if (!(type & IRQ_TYPE_EDGE_BOTH)) {
 		dev_err(&chip->client->dev, "irq %d: unsupported type %d\n",
@@ -383,14 +430,14 @@ static int pca953x_irq_set_type(struct irq_data *d, unsigned int type)
 	}
 
 	if (type & IRQ_TYPE_EDGE_FALLING)
-		chip->irq_trig_fall |= mask;
+		chip->irq_trig_fall[bank_nb] |= mask;
 	else
-		chip->irq_trig_fall &= ~mask;
+		chip->irq_trig_fall[bank_nb] &= ~mask;
 
 	if (type & IRQ_TYPE_EDGE_RISING)
-		chip->irq_trig_raise |= mask;
+		chip->irq_trig_raise[bank_nb] |= mask;
 	else
-		chip->irq_trig_raise &= ~mask;
+		chip->irq_trig_raise[bank_nb] &= ~mask;
 
 	return 0;
 }
@@ -404,13 +451,13 @@ static struct irq_chip pca953x_irq_chip = {
 	.irq_set_type		= pca953x_irq_set_type,
 };
 
-static u32 pca953x_irq_pending(struct pca953x_chip *chip)
+static u8 pca953x_irq_pending(struct pca953x_chip *chip, u8 *pending)
 {
-	u32 cur_stat;
-	u32 old_stat;
-	u32 pending;
-	u32 trigger;
-	int ret, offset = 0;
+	u8 cur_stat[MAX_BANK];
+	u8 old_stat[MAX_BANK];
+	u8 pendings = 0;
+	u8 trigger[MAX_BANK], triggers = 0;
+	int ret, i, offset = 0;
 
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
@@ -420,45 +467,54 @@ static u32 pca953x_irq_pending(struct pca953x_chip *chip)
 		offset = PCA957X_IN;
 		break;
 	}
-	ret = pca953x_read_reg(chip, offset, &cur_stat);
+	ret = pca953x_read_regs(chip, offset, cur_stat);
 	if (ret)
 		return 0;
 
 	/* Remove output pins from the equation */
-	cur_stat &= chip->reg_direction;
+	for (i = 0; i < NBANK(chip); i++)
+		cur_stat[i] &= chip->reg_direction[i];
 
-	old_stat = chip->irq_stat;
-	trigger = (cur_stat ^ old_stat) & chip->irq_mask;
+	memcpy(old_stat, chip->irq_stat, NBANK(chip));
 
-	if (!trigger)
+	for (i = 0; i < NBANK(chip); i++) {
+		trigger[i] = (cur_stat[i] ^ old_stat[i]) & chip->irq_mask[i];
+		triggers += trigger[i];
+	}
+
+	if (!triggers)
 		return 0;
 
-	chip->irq_stat = cur_stat;
+	memcpy(chip->irq_stat, cur_stat, NBANK(chip));
 
-	pending = (old_stat & chip->irq_trig_fall) |
-		  (cur_stat & chip->irq_trig_raise);
-	pending &= trigger;
+	for (i = 0; i < NBANK(chip); i++) {
+		pending[i] = (old_stat[i] & chip->irq_trig_fall[i]) |
+			(cur_stat[i] & chip->irq_trig_raise[i]);
+		pending[i] &= trigger[i];
+		pendings += pending[i];
+	}
 
-	return pending;
+	return pendings;
 }
 
 static irqreturn_t pca953x_irq_handler(int irq, void *devid)
 {
 	struct pca953x_chip *chip = devid;
-	u32 pending;
-	u32 level;
-
-	pending = pca953x_irq_pending(chip);
+	u8 pending[MAX_BANK];
+	u8 level;
+	int i;
 
-	if (!pending)
+	if (!pca953x_irq_pending(chip, pending))
 		return IRQ_HANDLED;
 
-	do {
-		level = __ffs(pending);
-		handle_nested_irq(irq_find_mapping(chip->domain, level));
-
-		pending &= ~(1 << level);
-	} while (pending);
+	for (i = 0; i < NBANK(chip); i++) {
+		while (pending[i]) {
+			level = __ffs(pending[i]);
+			handle_nested_irq(irq_find_mapping(chip->domain,
+							level + (BANK_SZ * i)));
+			pending[i] &= ~(1 << level);
+		}
+	}
 
 	return IRQ_HANDLED;
 }
@@ -468,8 +524,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 			     int irq_base)
 {
 	struct i2c_client *client = chip->client;
-	int ret, offset = 0;
-	u32 temporary;
+	int ret, i, offset = 0;
 
 	if (irq_base != -1
 			&& (id->driver_data & PCA_INT)) {
@@ -483,8 +538,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 			offset = PCA957X_IN;
 			break;
 		}
-		ret = pca953x_read_reg(chip, offset, &temporary);
-		chip->irq_stat = temporary;
+		ret = pca953x_read_regs(chip, offset, chip->irq_stat);
 		if (ret)
 			goto out_failed;
 
@@ -493,7 +547,8 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 		 * interrupt.  We have to rely on the previous read for
 		 * this purpose.
 		 */
-		chip->irq_stat &= chip->reg_direction;
+		for (i = 0; i < NBANK(chip); i++)
+			chip->irq_stat[i] &= chip->reg_direction[i];
 		mutex_init(&chip->irq_lock);
 
 		chip->irq_base = irq_alloc_descs(-1, irq_base, chip->gpio_chip.ngpio, -1);
@@ -619,18 +674,24 @@ pca953x_get_alt_pdata(struct i2c_client *client, int *gpio_base, u32 *invert)
 static int device_pca953x_init(struct pca953x_chip *chip, u32 invert)
 {
 	int ret;
+	u8 val[MAX_BANK];
 
-	ret = pca953x_read_reg(chip, PCA953X_OUTPUT, &chip->reg_output);
+	ret = pca953x_read_regs(chip, PCA953X_OUTPUT, chip->reg_output);
 	if (ret)
 		goto out;
 
-	ret = pca953x_read_reg(chip, PCA953X_DIRECTION,
-			       &chip->reg_direction);
+	ret = pca953x_read_regs(chip, PCA953X_DIRECTION,
+			       chip->reg_direction);
 	if (ret)
 		goto out;
 
 	/* set platform specific polarity inversion */
-	ret = pca953x_write_reg(chip, PCA953X_INVERT, invert);
+	if (invert)
+		memset(val, 0xFF, NBANK(chip));
+	else
+		memset(val, 0, NBANK(chip));
+
+	ret = pca953x_write_regs(chip, PCA953X_INVERT, val);
 out:
 	return ret;
 }
@@ -638,28 +699,36 @@ out:
 static int device_pca957x_init(struct pca953x_chip *chip, u32 invert)
 {
 	int ret;
-	u32 val = 0;
+	u8 val[MAX_BANK];
 
 	/* Let every port in proper state, that could save power */
-	pca953x_write_reg(chip, PCA957X_PUPD, 0x0);
-	pca953x_write_reg(chip, PCA957X_CFG, 0xffff);
-	pca953x_write_reg(chip, PCA957X_OUT, 0x0);
-
-	ret = pca953x_read_reg(chip, PCA957X_IN, &val);
+	memset(val, 0, NBANK(chip));
+	pca953x_write_regs(chip, PCA957X_PUPD, val);
+	memset(val, 0xFF, NBANK(chip));
+	pca953x_write_regs(chip, PCA957X_CFG, val);
+	memset(val, 0, NBANK(chip));
+	pca953x_write_regs(chip, PCA957X_OUT, val);
+
+	ret = pca953x_read_regs(chip, PCA957X_IN, val);
 	if (ret)
 		goto out;
-	ret = pca953x_read_reg(chip, PCA957X_OUT, &chip->reg_output);
+	ret = pca953x_read_regs(chip, PCA957X_OUT, chip->reg_output);
 	if (ret)
 		goto out;
-	ret = pca953x_read_reg(chip, PCA957X_CFG, &chip->reg_direction);
+	ret = pca953x_read_regs(chip, PCA957X_CFG, chip->reg_direction);
 	if (ret)
 		goto out;
 
 	/* set platform specific polarity inversion */
-	pca953x_write_reg(chip, PCA957X_INVRT, invert);
+	if (invert)
+		memset(val, 0xFF, NBANK(chip));
+	else
+		memset(val, 0, NBANK(chip));
+	pca953x_write_regs(chip, PCA957X_INVRT, val);
 
 	/* To enable register 6, 7 to controll pull up and pull down */
-	pca953x_write_reg(chip, PCA957X_BKEN, 0x202);
+	memset(val, 0x02, NBANK(chip));
+	pca953x_write_regs(chip, PCA957X_BKEN, val);
 
 	return 0;
 out:
-- 
1.7.9.5

[PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Gregory CLEMENT]

Now that pca953x driver can handle GPIO expanders with more than 32
bits this patch adds the support for the pca9505 which cam with 40
GPIOs.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/gpio/gpio-pca953x.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index b35ba06..3a68aed 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -46,6 +46,7 @@
 #define PCA957X_TYPE		0x2000
 
 static const struct i2c_device_id pca953x_id[] = {
+	{ "pca9505", 40 | PCA953X_TYPE | PCA_INT, },
 	{ "pca9534", 8  | PCA953X_TYPE | PCA_INT, },
 	{ "pca9535", 16 | PCA953X_TYPE | PCA_INT, },
 	{ "pca9536", 4  | PCA953X_TYPE, },
@@ -835,6 +836,7 @@ static int pca953x_remove(struct i2c_client *client)
 }
 
 static const struct of_device_id pca953x_dt_ids[] = {
+	{ .compatible = "nxp,pca9505", },
 	{ .compatible = "nxp,pca9534", },
 	{ .compatible = "nxp,pca9535", },
 	{ .compatible = "nxp,pca9536", },
-- 
1.7.9.5

[PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Gregory CLEMENT]

Now that pca953x driver can handle GPIO expanders with more than 32
bits this patch adds the support for the pca9505 which cam with 40
GPIOs.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/gpio/gpio-pca953x.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index b35ba06..3a68aed 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -46,6 +46,7 @@
 #define PCA957X_TYPE		0x2000
 
 static const struct i2c_device_id pca953x_id[] = {
+	{ "pca9505", 40 | PCA953X_TYPE | PCA_INT, },
 	{ "pca9534", 8  | PCA953X_TYPE | PCA_INT, },
 	{ "pca9535", 16 | PCA953X_TYPE | PCA_INT, },
 	{ "pca9536", 4  | PCA953X_TYPE, },
@@ -835,6 +836,7 @@ static int pca953x_remove(struct i2c_client *client)
 }
 
 static const struct of_device_id pca953x_dt_ids[] = {
+	{ .compatible = "nxp,pca9505", },
 	{ .compatible = "nxp,pca9534", },
 	{ .compatible = "nxp,pca9535", },
 	{ .compatible = "nxp,pca9536", },
-- 
1.7.9.5

[PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Gregory CLEMENT]

The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
expansion IC to provide 40-bit parallel input/output GPIOs. This patch
enable the use of this expander on the Mirabox.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/boot/dts/armada-370-mirabox.dts |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-mirabox.dts b/arch/arm/boot/dts/armada-370-mirabox.dts
index 3b40713..d9b32d3 100644
--- a/arch/arm/boot/dts/armada-370-mirabox.dts
+++ b/arch/arm/boot/dts/armada-370-mirabox.dts
@@ -52,5 +52,15 @@
 			phy = <&phy1>;
 			phy-mode = "rgmii-id";
 		};
+		i2c@d0011000 {
+			status = "okay";
+			clock-frequency = <100000>;
+			pca9505: pca9505@25 {
+				compatible = "nxp,pca9505";
+				gpio-controller;
+				#gpio-cells = <2>;
+				reg = <0x25>;
+			};
+		};
 	};
 };
-- 
1.7.9.5

[PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Gregory CLEMENT]

The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
expansion IC to provide 40-bit parallel input/output GPIOs. This patch
enable the use of this expander on the Mirabox.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/boot/dts/armada-370-mirabox.dts |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-mirabox.dts b/arch/arm/boot/dts/armada-370-mirabox.dts
index 3b40713..d9b32d3 100644
--- a/arch/arm/boot/dts/armada-370-mirabox.dts
+++ b/arch/arm/boot/dts/armada-370-mirabox.dts
@@ -52,5 +52,15 @@
 			phy = <&phy1>;
 			phy-mode = "rgmii-id";
 		};
+		i2c at d0011000 {
+			status = "okay";
+			clock-frequency = <100000>;
+			pca9505: pca9505 at 25 {
+				compatible = "nxp,pca9505";
+				gpio-controller;
+				#gpio-cells = <2>;
+				reg = <0x25>;
+			};
+		};
 	};
 };
-- 
1.7.9.5

Re: [PATCH v3 1/3] gpio: pca953x: make the register access by GPIO bank

[Linus Walleij]

On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> Until now the pca953x driver accessed all the bank of a given register
> in a single command using only a 32 bits variable. New expanders from
> the pca53x family come with 40 GPIOs which no more fit in a 32
> variable. This patch make access to the registers more generic by
> relying on an array of u8 variables. This fits exactly the way the
> registers are represented in the hardware.
>
> It also adds helpers to access to a single register of a bank instead
> of reading or writing all the banks for a given register.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Tested-by: Maxime Ripard <maxime.ripard@free-electrons.com>

This is a nice refactoring and tested too, so patch applied.

I have some nitpicks, may try to fix them with a follow-up
patch but it's certainly not super-important stuff.

Yours,
Linus Walleij

[PATCH v3 1/3] gpio: pca953x: make the register access by GPIO bank

[Linus Walleij]

On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> Until now the pca953x driver accessed all the bank of a given register
> in a single command using only a 32 bits variable. New expanders from
> the pca53x family come with 40 GPIOs which no more fit in a 32
> variable. This patch make access to the registers more generic by
> relying on an array of u8 variables. This fits exactly the way the
> registers are represented in the hardware.
>
> It also adds helpers to access to a single register of a bank instead
> of reading or writing all the banks for a given register.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Tested-by: Maxime Ripard <maxime.ripard@free-electrons.com>

This is a nice refactoring and tested too, so patch applied.

I have some nitpicks, may try to fix them with a follow-up
patch but it's certainly not super-important stuff.

Yours,
Linus Walleij

Re: [PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Linus Walleij]

On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> Now that pca953x driver can handle GPIO expanders with more than 32
> bits this patch adds the support for the pca9505 which cam with 40
> GPIOs.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Patch applied, thanks.

But guys, this driver contains some horrific stuff, look at this:

static int pxa_gpio_nums(void)
{
        int count = 0;

#ifdef CONFIG_ARCH_PXA
        if (cpu_is_pxa25x()) {
#ifdef CONFIG_CPU_PXA26x
                count = 89;
                gpio_type = PXA26X_GPIO;
#elif defined(CONFIG_PXA25x)
                count = 84;
                gpio_type = PXA26X_GPIO;
#endif /* CONFIG_CPU_PXA26x */
        } else if (cpu_is_pxa27x()) {
                count = 120;
                gpio_type = PXA27X_GPIO;
        } else if (cpu_is_pxa93x()) {
                count = 191;
                gpio_type = PXA93X_GPIO;
        } else if (cpu_is_pxa3xx()) {
                count = 127;
                gpio_type = PXA3XX_GPIO;
        }
#endif /* CONFIG_ARCH_PXA */

#ifdef CONFIG_ARCH_MMP
        if (cpu_is_pxa168() || cpu_is_pxa910()) {
                count = 127;
                gpio_type = MMP_GPIO;
        } else if (cpu_is_mmp2()) {
                count = 191;
                gpio_type = MMP_GPIO;
        }
#endif /* CONFIG_ARCH_MMP */
        return count;
}

This is totally killing all attempts at a single kernel for multiple
machines in this family. The above should not be #ifdef's but instead
use either platform data or the compatible property to figure out which
one to use.

It's fine to introduce new compatible= strings or device names to
distinguish between these.

As an example, in pinctrl-nomadik.c we have this:

static const struct of_device_id nmk_pinctrl_match[] = {
        {
                .compatible = "stericsson,nmk_pinctrl",
                .data = (void *)PINCTRL_NMK_DB8500,
        },
        {},
};

static const struct platform_device_id nmk_pinctrl_id[] = {
        { "pinctrl-stn8815", PINCTRL_NMK_STN8815 },
        { "pinctrl-db8500", PINCTRL_NMK_DB8500 },
        { "pinctrl-db8540", PINCTRL_NMK_DB8540 },
        { }
};

static struct platform_driver nmk_pinctrl_driver = {
        .driver = {
                .owner = THIS_MODULE,
                .name = "pinctrl-nomadik",
                .of_match_table = nmk_pinctrl_match,
        },
        .probe = nmk_pinctrl_probe,
        .id_table = nmk_pinctrl_id,
};


The first match is for DT boot the latter for using the platform
device name directly.

Then in the probefunction we do:

static int nmk_pinctrl_probe(struct platform_device *pdev)
{
        const struct platform_device_id *platid = platform_get_device_id(pdev);
(...)
        if (platid)
                version = platid->driver_data;
        else if (np) {
                const struct of_device_id *match;

                match = of_match_device(nmk_pinctrl_match, &pdev->dev);
                if (!match)
                        return -ENODEV;
                version = (unsigned int) match->data;
        }
(...)
        if (version == PINCTRL_NMK_STN8815)
                nmk_pinctrl_stn8815_init(&npct->soc);
        if (version == PINCTRL_NMK_DB8500)
                nmk_pinctrl_db8500_init(&npct->soc);
        if (version == PINCTRL_NMK_DB8540)
                nmk_pinctrl_db8540_init(&npct->soc);
}

Surely a scheme like this must be possible to use to distinguish between
the different versions at runtime rather than using these #ifdefs?

Yours,
Linus Walleij

[PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Linus Walleij]

On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> Now that pca953x driver can handle GPIO expanders with more than 32
> bits this patch adds the support for the pca9505 which cam with 40
> GPIOs.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Patch applied, thanks.

But guys, this driver contains some horrific stuff, look at this:

static int pxa_gpio_nums(void)
{
        int count = 0;

#ifdef CONFIG_ARCH_PXA
        if (cpu_is_pxa25x()) {
#ifdef CONFIG_CPU_PXA26x
                count = 89;
                gpio_type = PXA26X_GPIO;
#elif defined(CONFIG_PXA25x)
                count = 84;
                gpio_type = PXA26X_GPIO;
#endif /* CONFIG_CPU_PXA26x */
        } else if (cpu_is_pxa27x()) {
                count = 120;
                gpio_type = PXA27X_GPIO;
        } else if (cpu_is_pxa93x()) {
                count = 191;
                gpio_type = PXA93X_GPIO;
        } else if (cpu_is_pxa3xx()) {
                count = 127;
                gpio_type = PXA3XX_GPIO;
        }
#endif /* CONFIG_ARCH_PXA */

#ifdef CONFIG_ARCH_MMP
        if (cpu_is_pxa168() || cpu_is_pxa910()) {
                count = 127;
                gpio_type = MMP_GPIO;
        } else if (cpu_is_mmp2()) {
                count = 191;
                gpio_type = MMP_GPIO;
        }
#endif /* CONFIG_ARCH_MMP */
        return count;
}

This is totally killing all attempts at a single kernel for multiple
machines in this family. The above should not be #ifdef's but instead
use either platform data or the compatible property to figure out which
one to use.

It's fine to introduce new compatible= strings or device names to
distinguish between these.

As an example, in pinctrl-nomadik.c we have this:

static const struct of_device_id nmk_pinctrl_match[] = {
        {
                .compatible = "stericsson,nmk_pinctrl",
                .data = (void *)PINCTRL_NMK_DB8500,
        },
        {},
};

static const struct platform_device_id nmk_pinctrl_id[] = {
        { "pinctrl-stn8815", PINCTRL_NMK_STN8815 },
        { "pinctrl-db8500", PINCTRL_NMK_DB8500 },
        { "pinctrl-db8540", PINCTRL_NMK_DB8540 },
        { }
};

static struct platform_driver nmk_pinctrl_driver = {
        .driver = {
                .owner = THIS_MODULE,
                .name = "pinctrl-nomadik",
                .of_match_table = nmk_pinctrl_match,
        },
        .probe = nmk_pinctrl_probe,
        .id_table = nmk_pinctrl_id,
};


The first match is for DT boot the latter for using the platform
device name directly.

Then in the probefunction we do:

static int nmk_pinctrl_probe(struct platform_device *pdev)
{
        const struct platform_device_id *platid = platform_get_device_id(pdev);
(...)
        if (platid)
                version = platid->driver_data;
        else if (np) {
                const struct of_device_id *match;

                match = of_match_device(nmk_pinctrl_match, &pdev->dev);
                if (!match)
                        return -ENODEV;
                version = (unsigned int) match->data;
        }
(...)
        if (version == PINCTRL_NMK_STN8815)
                nmk_pinctrl_stn8815_init(&npct->soc);
        if (version == PINCTRL_NMK_DB8500)
                nmk_pinctrl_db8500_init(&npct->soc);
        if (version == PINCTRL_NMK_DB8540)
                nmk_pinctrl_db8540_init(&npct->soc);
}

Surely a scheme like this must be possible to use to distinguish between
the different versions at runtime rather than using these #ifdefs?

Yours,
Linus Walleij

Re: [PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Linus Walleij]

On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
> through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
> expansion IC to provide 40-bit parallel input/output GPIOs. This patch
> enable the use of this expander on the Mirabox.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

So as requested I leave this patch for now, tell me if you want it
applied through pinctrl.

Yours,
Linus Walleij

[PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Linus Walleij]

On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
> through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
> expansion IC to provide 40-bit parallel input/output GPIOs. This patch
> enable the use of this expander on the Mirabox.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

So as requested I leave this patch for now, tell me if you want it
applied through pinctrl.

Yours,
Linus Walleij

Re: [PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Gregory CLEMENT]

On 01/25/2013 09:16 AM, Linus Walleij wrote:
> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
> 
>> Now that pca953x driver can handle GPIO expanders with more than 32
>> bits this patch adds the support for the pca9505 which cam with 40
>> GPIOs.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> 
> Patch applied, thanks.
> 
> But guys, this driver contains some horrific stuff, look at this:
> 
> static int pxa_gpio_nums(void)
> {
>         int count = 0;
> 
> #ifdef CONFIG_ARCH_PXA
>         if (cpu_is_pxa25x()) {
> #ifdef CONFIG_CPU_PXA26x
>                 count = 89;
>                 gpio_type = PXA26X_GPIO;
> #elif defined(CONFIG_PXA25x)
>                 count = 84;
>                 gpio_type = PXA26X_GPIO;
> #endif /* CONFIG_CPU_PXA26x */
>         } else if (cpu_is_pxa27x()) {
>                 count = 120;
>                 gpio_type = PXA27X_GPIO;
>         } else if (cpu_is_pxa93x()) {
>                 count = 191;
>                 gpio_type = PXA93X_GPIO;
>         } else if (cpu_is_pxa3xx()) {
>                 count = 127;
>                 gpio_type = PXA3XX_GPIO;
>         }
> #endif /* CONFIG_ARCH_PXA */
> 
> #ifdef CONFIG_ARCH_MMP
>         if (cpu_is_pxa168() || cpu_is_pxa910()) {
>                 count = 127;
>                 gpio_type = MMP_GPIO;
>         } else if (cpu_is_mmp2()) {
>                 count = 191;
>                 gpio_type = MMP_GPIO;
>         }
> #endif /* CONFIG_ARCH_MMP */
>         return count;
> }
> 
> This is totally killing all attempts at a single kernel for multiple
> machines in this family. The above should not be #ifdef's but instead
> use either platform data or the compatible property to figure out which
> one to use.
> 
> It's fine to introduce new compatible= strings or device names to
> distinguish between these.
> 
> As an example, in pinctrl-nomadik.c we have this:
> 
> static const struct of_device_id nmk_pinctrl_match[] = {
>         {
>                 .compatible = "stericsson,nmk_pinctrl",
>                 .data = (void *)PINCTRL_NMK_DB8500,
>         },
>         {},
> };
> 
> static const struct platform_device_id nmk_pinctrl_id[] = {
>         { "pinctrl-stn8815", PINCTRL_NMK_STN8815 },
>         { "pinctrl-db8500", PINCTRL_NMK_DB8500 },
>         { "pinctrl-db8540", PINCTRL_NMK_DB8540 },
>         { }
> };
> 
> static struct platform_driver nmk_pinctrl_driver = {
>         .driver = {
>                 .owner = THIS_MODULE,
>                 .name = "pinctrl-nomadik",
>                 .of_match_table = nmk_pinctrl_match,
>         },
>         .probe = nmk_pinctrl_probe,
>         .id_table = nmk_pinctrl_id,
> };
> 
> 
> The first match is for DT boot the latter for using the platform
> device name directly.
> 
> Then in the probefunction we do:
> 
> static int nmk_pinctrl_probe(struct platform_device *pdev)
> {
>         const struct platform_device_id *platid = platform_get_device_id(pdev);
> (...)
>         if (platid)
>                 version = platid->driver_data;
>         else if (np) {
>                 const struct of_device_id *match;
> 
>                 match = of_match_device(nmk_pinctrl_match, &pdev->dev);
>                 if (!match)
>                         return -ENODEV;
>                 version = (unsigned int) match->data;
>         }
> (...)
>         if (version == PINCTRL_NMK_STN8815)
>                 nmk_pinctrl_stn8815_init(&npct->soc);
>         if (version == PINCTRL_NMK_DB8500)
>                 nmk_pinctrl_db8500_init(&npct->soc);
>         if (version == PINCTRL_NMK_DB8540)
>                 nmk_pinctrl_db8540_init(&npct->soc);
> }
> 
> Surely a scheme like this must be possible to use to distinguish between
> the different versions at runtime rather than using these #ifdefs?
> 

Well, at the beginning I thought adding support for pca9505 was just a matter
of a couple of lines to add. Then I realized that I need to handle the 40 bits
case, and I ended up refactoring all access to the registers. So now I am on it,
it seems I am volunteer to continue to improve this driver.

However I won't be able to test it, the only PXA based platform I have is a
Zaurus SL-C3100 which embeds a PXA270 if I remember well, but I doubt it come
with gpio expander on i2c.

Gregory

[PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Gregory CLEMENT]

On 01/25/2013 09:16 AM, Linus Walleij wrote:
> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
> 
>> Now that pca953x driver can handle GPIO expanders with more than 32
>> bits this patch adds the support for the pca9505 which cam with 40
>> GPIOs.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> 
> Patch applied, thanks.
> 
> But guys, this driver contains some horrific stuff, look at this:
> 
> static int pxa_gpio_nums(void)
> {
>         int count = 0;
> 
> #ifdef CONFIG_ARCH_PXA
>         if (cpu_is_pxa25x()) {
> #ifdef CONFIG_CPU_PXA26x
>                 count = 89;
>                 gpio_type = PXA26X_GPIO;
> #elif defined(CONFIG_PXA25x)
>                 count = 84;
>                 gpio_type = PXA26X_GPIO;
> #endif /* CONFIG_CPU_PXA26x */
>         } else if (cpu_is_pxa27x()) {
>                 count = 120;
>                 gpio_type = PXA27X_GPIO;
>         } else if (cpu_is_pxa93x()) {
>                 count = 191;
>                 gpio_type = PXA93X_GPIO;
>         } else if (cpu_is_pxa3xx()) {
>                 count = 127;
>                 gpio_type = PXA3XX_GPIO;
>         }
> #endif /* CONFIG_ARCH_PXA */
> 
> #ifdef CONFIG_ARCH_MMP
>         if (cpu_is_pxa168() || cpu_is_pxa910()) {
>                 count = 127;
>                 gpio_type = MMP_GPIO;
>         } else if (cpu_is_mmp2()) {
>                 count = 191;
>                 gpio_type = MMP_GPIO;
>         }
> #endif /* CONFIG_ARCH_MMP */
>         return count;
> }
> 
> This is totally killing all attempts at a single kernel for multiple
> machines in this family. The above should not be #ifdef's but instead
> use either platform data or the compatible property to figure out which
> one to use.
> 
> It's fine to introduce new compatible= strings or device names to
> distinguish between these.
> 
> As an example, in pinctrl-nomadik.c we have this:
> 
> static const struct of_device_id nmk_pinctrl_match[] = {
>         {
>                 .compatible = "stericsson,nmk_pinctrl",
>                 .data = (void *)PINCTRL_NMK_DB8500,
>         },
>         {},
> };
> 
> static const struct platform_device_id nmk_pinctrl_id[] = {
>         { "pinctrl-stn8815", PINCTRL_NMK_STN8815 },
>         { "pinctrl-db8500", PINCTRL_NMK_DB8500 },
>         { "pinctrl-db8540", PINCTRL_NMK_DB8540 },
>         { }
> };
> 
> static struct platform_driver nmk_pinctrl_driver = {
>         .driver = {
>                 .owner = THIS_MODULE,
>                 .name = "pinctrl-nomadik",
>                 .of_match_table = nmk_pinctrl_match,
>         },
>         .probe = nmk_pinctrl_probe,
>         .id_table = nmk_pinctrl_id,
> };
> 
> 
> The first match is for DT boot the latter for using the platform
> device name directly.
> 
> Then in the probefunction we do:
> 
> static int nmk_pinctrl_probe(struct platform_device *pdev)
> {
>         const struct platform_device_id *platid = platform_get_device_id(pdev);
> (...)
>         if (platid)
>                 version = platid->driver_data;
>         else if (np) {
>                 const struct of_device_id *match;
> 
>                 match = of_match_device(nmk_pinctrl_match, &pdev->dev);
>                 if (!match)
>                         return -ENODEV;
>                 version = (unsigned int) match->data;
>         }
> (...)
>         if (version == PINCTRL_NMK_STN8815)
>                 nmk_pinctrl_stn8815_init(&npct->soc);
>         if (version == PINCTRL_NMK_DB8500)
>                 nmk_pinctrl_db8500_init(&npct->soc);
>         if (version == PINCTRL_NMK_DB8540)
>                 nmk_pinctrl_db8540_init(&npct->soc);
> }
> 
> Surely a scheme like this must be possible to use to distinguish between
> the different versions at runtime rather than using these #ifdefs?
> 

Well, at the beginning I thought adding support for pca9505 was just a matter
of a couple of lines to add. Then I realized that I need to handle the 40 bits
case, and I ended up refactoring all access to the registers. So now I am on it,
it seems I am volunteer to continue to improve this driver.

However I won't be able to test it, the only PXA based platform I have is a
Zaurus SL-C3100 which embeds a PXA270 if I remember well, but I doubt it come
with gpio expander on i2c.

Gregory

Re: [PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Linus Walleij]

On Fri, Jan 25, 2013 at 9:36 AM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> Well, at the beginning I thought adding support for pca9505 was just a matter
> of a couple of lines to add. Then I realized that I need to handle the 40 bits
> case, and I ended up refactoring all access to the registers. So now I am on it,
> it seems I am volunteer to continue to improve this driver.

I like the sound of this ;-)

To get you started I just sent out two other patches you can consider
as RFC, they're regrettably not even compile-tested. I mainly wanted
to indicate what needs to be done so we can throw them away, just
wanted to give a hint.

> However I won't be able to test it, the only PXA based platform I have is a
> Zaurus SL-C3100 which embeds a PXA270 if I remember well, but I doubt it come
> with gpio expander on i2c.

Well I guess if there is nobody testing it, then nobody cares.
The world must be full of people with PXA platforms doing nothing
but regression testing...

Actually just days ago I asked Haoijan to help me testing a set of
patches for the PXA SPI controller, and he kindly helped out, so there
are some people booting these platforms, sometimes :-)

Yours,
Linus Walleij

[PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Linus Walleij]

On Fri, Jan 25, 2013 at 9:36 AM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> Well, at the beginning I thought adding support for pca9505 was just a matter
> of a couple of lines to add. Then I realized that I need to handle the 40 bits
> case, and I ended up refactoring all access to the registers. So now I am on it,
> it seems I am volunteer to continue to improve this driver.

I like the sound of this ;-)

To get you started I just sent out two other patches you can consider
as RFC, they're regrettably not even compile-tested. I mainly wanted
to indicate what needs to be done so we can throw them away, just
wanted to give a hint.

> However I won't be able to test it, the only PXA based platform I have is a
> Zaurus SL-C3100 which embeds a PXA270 if I remember well, but I doubt it come
> with gpio expander on i2c.

Well I guess if there is nobody testing it, then nobody cares.
The world must be full of people with PXA platforms doing nothing
but regression testing...

Actually just days ago I asked Haoijan to help me testing a set of
patches for the PXA SPI controller, and he kindly helped out, so there
are some people booting these platforms, sometimes :-)

Yours,
Linus Walleij

Re: [PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Jason Cooper]

Linus,

On Fri, Jan 25, 2013 at 09:17:57AM +0100, Linus Walleij wrote:
> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
> 
> > The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
> > through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
> > expansion IC to provide 40-bit parallel input/output GPIOs. This patch
> > enable the use of this expander on the Mirabox.
> >
> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> So as requested I leave this patch for now, tell me if you want it
> applied through pinctrl.

Which branch did you put the first two on?  I'm not seeing them.

thx,

Jason.

[PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Jason Cooper]

Linus,

On Fri, Jan 25, 2013 at 09:17:57AM +0100, Linus Walleij wrote:
> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
> 
> > The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
> > through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
> > expansion IC to provide 40-bit parallel input/output GPIOs. This patch
> > enable the use of this expander on the Mirabox.
> >
> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> So as requested I leave this patch for now, tell me if you want it
> applied through pinctrl.

Which branch did you put the first two on?  I'm not seeing them.

thx,

Jason.

Re: [PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Linus Walleij]

On Fri, Jan 25, 2013 at 1:46 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> On Fri, Jan 25, 2013 at 09:17:57AM +0100, Linus Walleij wrote:
>> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
>> <gregory.clement@free-electrons.com> wrote:
>>
>> > The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
>> > through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
>> > expansion IC to provide 40-bit parallel input/output GPIOs. This patch
>> > enable the use of this expander on the Mirabox.
>> >
>> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> So as requested I leave this patch for now, tell me if you want it
>> applied through pinctrl.
>
> Which branch did you put the first two on?  I'm not seeing them.

Not pushed yet, take it easy.

Yours,
Linus Walleij

[PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Linus Walleij]

On Fri, Jan 25, 2013 at 1:46 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> On Fri, Jan 25, 2013 at 09:17:57AM +0100, Linus Walleij wrote:
>> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
>> <gregory.clement@free-electrons.com> wrote:
>>
>> > The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
>> > through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
>> > expansion IC to provide 40-bit parallel input/output GPIOs. This patch
>> > enable the use of this expander on the Mirabox.
>> >
>> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> So as requested I leave this patch for now, tell me if you want it
>> applied through pinctrl.
>
> Which branch did you put the first two on?  I'm not seeing them.

Not pushed yet, take it easy.

Yours,
Linus Walleij

Re: [PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Linus Walleij]

On Fri, Jan 25, 2013 at 1:55 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Jan 25, 2013 at 1:46 PM, Jason Cooper <jason@lakedaemon.net> wrote:
>> On Fri, Jan 25, 2013 at 09:17:57AM +0100, Linus Walleij wrote:
>>> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
>>> <gregory.clement@free-electrons.com> wrote:
>>>
>>> > The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
>>> > through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
>>> > expansion IC to provide 40-bit parallel input/output GPIOs. This patch
>>> > enable the use of this expander on the Mirabox.
>>> >
>>> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> So as requested I leave this patch for now, tell me if you want it
>>> applied through pinctrl.
>>
>> Which branch did you put the first two on?  I'm not seeing them.
>
> Not pushed yet, take it easy.

Or yeah, they are there:
http://git.kernel.org/?p=linux/kernel/git/linusw/linux-gpio.git;a=shortlog;h=refs/heads/devel

Yours,
Linus Walleij

[PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Linus Walleij]

On Fri, Jan 25, 2013 at 1:55 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Jan 25, 2013 at 1:46 PM, Jason Cooper <jason@lakedaemon.net> wrote:
>> On Fri, Jan 25, 2013 at 09:17:57AM +0100, Linus Walleij wrote:
>>> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
>>> <gregory.clement@free-electrons.com> wrote:
>>>
>>> > The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
>>> > through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
>>> > expansion IC to provide 40-bit parallel input/output GPIOs. This patch
>>> > enable the use of this expander on the Mirabox.
>>> >
>>> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> So as requested I leave this patch for now, tell me if you want it
>>> applied through pinctrl.
>>
>> Which branch did you put the first two on?  I'm not seeing them.
>
> Not pushed yet, take it easy.

Or yeah, they are there:
http://git.kernel.org/?p=linux/kernel/git/linusw/linux-gpio.git;a=shortlog;h=refs/heads/devel

Yours,
Linus Walleij

Re: [PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Jason Cooper]

On Fri, Jan 25, 2013 at 01:55:04PM +0100, Linus Walleij wrote:
> On Fri, Jan 25, 2013 at 1:46 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> > On Fri, Jan 25, 2013 at 09:17:57AM +0100, Linus Walleij wrote:
> >> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
> >> <gregory.clement@free-electrons.com> wrote:
> >>
> >> > The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
> >> > through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
> >> > expansion IC to provide 40-bit parallel input/output GPIOs. This patch
> >> > enable the use of this expander on the Mirabox.
> >> >
> >> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> >> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> >>
> >> So as requested I leave this patch for now, tell me if you want it
> >> applied through pinctrl.
> >
> > Which branch did you put the first two on?  I'm not seeing them.
> 
> Not pushed yet, take it easy.

Ahh, ok.  Sorry, I wasn't trying to rush you.  I didn't check the
timestamps.  Just saw it in my inbox this morning and decided to work on
it before going to work.  I'll check later,

thx,

Jason.

[PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Jason Cooper]

On Fri, Jan 25, 2013 at 01:55:04PM +0100, Linus Walleij wrote:
> On Fri, Jan 25, 2013 at 1:46 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> > On Fri, Jan 25, 2013 at 09:17:57AM +0100, Linus Walleij wrote:
> >> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
> >> <gregory.clement@free-electrons.com> wrote:
> >>
> >> > The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
> >> > through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
> >> > expansion IC to provide 40-bit parallel input/output GPIOs. This patch
> >> > enable the use of this expander on the Mirabox.
> >> >
> >> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> >> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> >>
> >> So as requested I leave this patch for now, tell me if you want it
> >> applied through pinctrl.
> >
> > Which branch did you put the first two on?  I'm not seeing them.
> 
> Not pushed yet, take it easy.

Ahh, ok.  Sorry, I wasn't trying to rush you.  I didn't check the
timestamps.  Just saw it in my inbox this morning and decided to work on
it before going to work.  I'll check later,

thx,

Jason.

Re: [PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Jason Cooper]

On Fri, Jan 25, 2013 at 01:57:51PM +0100, Linus Walleij wrote:
> On Fri, Jan 25, 2013 at 1:55 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Fri, Jan 25, 2013 at 1:46 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> >> On Fri, Jan 25, 2013 at 09:17:57AM +0100, Linus Walleij wrote:
> >>> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
> >>> <gregory.clement@free-electrons.com> wrote:
> >>>
> >>> > The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
> >>> > through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
> >>> > expansion IC to provide 40-bit parallel input/output GPIOs. This patch
> >>> > enable the use of this expander on the Mirabox.
> >>> >
> >>> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> >>> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> >>>
> >>> So as requested I leave this patch for now, tell me if you want it
> >>> applied through pinctrl.
> >>
> >> Which branch did you put the first two on?  I'm not seeing them.
> >
> > Not pushed yet, take it easy.
> 
> Or yeah, they are there:
> http://git.kernel.org/?p=linux/kernel/git/linusw/linux-gpio.git;a=shortlog;h=refs/heads/devel

Ahh, I had your pinctrl tree as a remote, but not gpio.  Fixed, thanks.

Jason.

[PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Jason Cooper]

On Fri, Jan 25, 2013 at 01:57:51PM +0100, Linus Walleij wrote:
> On Fri, Jan 25, 2013 at 1:55 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Fri, Jan 25, 2013 at 1:46 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> >> On Fri, Jan 25, 2013 at 09:17:57AM +0100, Linus Walleij wrote:
> >>> On Tue, Jan 22, 2013 at 10:10 PM, Gregory CLEMENT
> >>> <gregory.clement@free-electrons.com> wrote:
> >>>
> >>> > The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
> >>> > through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
> >>> > expansion IC to provide 40-bit parallel input/output GPIOs. This patch
> >>> > enable the use of this expander on the Mirabox.
> >>> >
> >>> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> >>> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> >>>
> >>> So as requested I leave this patch for now, tell me if you want it
> >>> applied through pinctrl.
> >>
> >> Which branch did you put the first two on?  I'm not seeing them.
> >
> > Not pushed yet, take it easy.
> 
> Or yeah, they are there:
> http://git.kernel.org/?p=linux/kernel/git/linusw/linux-gpio.git;a=shortlog;h=refs/heads/devel

Ahh, I had your pinctrl tree as a remote, but not gpio.  Fixed, thanks.

Jason.

Re: [PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Gregory CLEMENT]

On 01/25/2013 09:51 AM, Linus Walleij wrote:
> On Fri, Jan 25, 2013 at 9:36 AM, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
> 
>> Well, at the beginning I thought adding support for pca9505 was just a matter
>> of a couple of lines to add. Then I realized that I need to handle the 40 bits
>> case, and I ended up refactoring all access to the registers. So now I am on it,
>> it seems I am volunteer to continue to improve this driver.
> 
> I like the sound of this ;-)

I was about to fix the issues you have pointed but I didn't find anything like

#ifdef CONFIG_ARCH_PXA
        if (cpu_is_pxa25x()) {
#ifdef CONFIG_CPU_PXA26x
                count = 89;
                gpio_type = PXA26X_GPIO;
#elif defined(CONFIG_PXA25x)


in the pca953x driver! I think you messed up with another patch set!

I saw that Haojian Zhuang have sent a patch set for gpio-pxa and
among this set the patch "[PATCH 06/10] gpio: pxa: define nr gpios
in platform data" seemed to exactly what you've expected.

> 
> To get you started I just sent out two other patches you can consider
> as RFC, they're regrettably not even compile-tested. I mainly wanted
> to indicate what needs to be done so we can throw them away, just
> wanted to give a hint.
> 
>> However I won't be able to test it, the only PXA based platform I have is a
>> Zaurus SL-C3100 which embeds a PXA270 if I remember well, but I doubt it come
>> with gpio expander on i2c.
> 
> Well I guess if there is nobody testing it, then nobody cares.
> The world must be full of people with PXA platforms doing nothing
> but regression testing...
> 
> Actually just days ago I asked Haoijan to help me testing a set of
> patches for the PXA SPI controller, and he kindly helped out, so there
> are some people booting these platforms, sometimes :-)
> 
> Yours,
> Linus Walleij
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Gregory CLEMENT]

On 01/25/2013 09:51 AM, Linus Walleij wrote:
> On Fri, Jan 25, 2013 at 9:36 AM, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
> 
>> Well, at the beginning I thought adding support for pca9505 was just a matter
>> of a couple of lines to add. Then I realized that I need to handle the 40 bits
>> case, and I ended up refactoring all access to the registers. So now I am on it,
>> it seems I am volunteer to continue to improve this driver.
> 
> I like the sound of this ;-)

I was about to fix the issues you have pointed but I didn't find anything like

#ifdef CONFIG_ARCH_PXA
        if (cpu_is_pxa25x()) {
#ifdef CONFIG_CPU_PXA26x
                count = 89;
                gpio_type = PXA26X_GPIO;
#elif defined(CONFIG_PXA25x)


in the pca953x driver! I think you messed up with another patch set!

I saw that Haojian Zhuang have sent a patch set for gpio-pxa and
among this set the patch "[PATCH 06/10] gpio: pxa: define nr gpios
in platform data" seemed to exactly what you've expected.

> 
> To get you started I just sent out two other patches you can consider
> as RFC, they're regrettably not even compile-tested. I mainly wanted
> to indicate what needs to be done so we can throw them away, just
> wanted to give a hint.
> 
>> However I won't be able to test it, the only PXA based platform I have is a
>> Zaurus SL-C3100 which embeds a PXA270 if I remember well, but I doubt it come
>> with gpio expander on i2c.
> 
> Well I guess if there is nobody testing it, then nobody cares.
> The world must be full of people with PXA platforms doing nothing
> but regression testing...
> 
> Actually just days ago I asked Haoijan to help me testing a set of
> patches for the PXA SPI controller, and he kindly helped out, so there
> are some people booting these platforms, sometimes :-)
> 
> Yours,
> Linus Walleij
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Haojian Zhuang]

On 27 January 2013 06:02, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> On 01/25/2013 09:51 AM, Linus Walleij wrote:
>> On Fri, Jan 25, 2013 at 9:36 AM, Gregory CLEMENT
>> <gregory.clement@free-electrons.com> wrote:
>>
>>> Well, at the beginning I thought adding support for pca9505 was just a matter
>>> of a couple of lines to add. Then I realized that I need to handle the 40 bits
>>> case, and I ended up refactoring all access to the registers. So now I am on it,
>>> it seems I am volunteer to continue to improve this driver.
>>
>> I like the sound of this ;-)
>
> I was about to fix the issues you have pointed but I didn't find anything like
>
> #ifdef CONFIG_ARCH_PXA
>         if (cpu_is_pxa25x()) {
> #ifdef CONFIG_CPU_PXA26x
>                 count = 89;
>                 gpio_type = PXA26X_GPIO;
> #elif defined(CONFIG_PXA25x)
>
>
> in the pca953x driver! I think you messed up with another patch set!
>
> I saw that Haojian Zhuang have sent a patch set for gpio-pxa and
> among this set the patch "[PATCH 06/10] gpio: pxa: define nr gpios
> in platform data" seemed to exactly what you've expected.
>

PCA953X is a GPIO expander that is relied on I2C bus. It's a device in
the cirucit,
not in the PXA chips. So there's no cpu related code in this driver.

Gregory's concern is that he found that this device is used on pxa27x
platform, and
he don't have the hardware to test. I also don't have pxa27x platform.
I think that
he can ping the volunteers in the mailing list.

Regards
Haojian

[PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Haojian Zhuang]

On 27 January 2013 06:02, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> On 01/25/2013 09:51 AM, Linus Walleij wrote:
>> On Fri, Jan 25, 2013 at 9:36 AM, Gregory CLEMENT
>> <gregory.clement@free-electrons.com> wrote:
>>
>>> Well, at the beginning I thought adding support for pca9505 was just a matter
>>> of a couple of lines to add. Then I realized that I need to handle the 40 bits
>>> case, and I ended up refactoring all access to the registers. So now I am on it,
>>> it seems I am volunteer to continue to improve this driver.
>>
>> I like the sound of this ;-)
>
> I was about to fix the issues you have pointed but I didn't find anything like
>
> #ifdef CONFIG_ARCH_PXA
>         if (cpu_is_pxa25x()) {
> #ifdef CONFIG_CPU_PXA26x
>                 count = 89;
>                 gpio_type = PXA26X_GPIO;
> #elif defined(CONFIG_PXA25x)
>
>
> in the pca953x driver! I think you messed up with another patch set!
>
> I saw that Haojian Zhuang have sent a patch set for gpio-pxa and
> among this set the patch "[PATCH 06/10] gpio: pxa: define nr gpios
> in platform data" seemed to exactly what you've expected.
>

PCA953X is a GPIO expander that is relied on I2C bus. It's a device in
the cirucit,
not in the PXA chips. So there's no cpu related code in this driver.

Gregory's concern is that he found that this device is used on pxa27x
platform, and
he don't have the hardware to test. I also don't have pxa27x platform.
I think that
he can ping the volunteers in the mailing list.

Regards
Haojian

Re: [PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Linus Walleij]

On Sat, Jan 26, 2013 at 11:02 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> I was about to fix the issues you have pointed but I didn't find anything like
>
> #ifdef CONFIG_ARCH_PXA
>         if (cpu_is_pxa25x()) {
> #ifdef CONFIG_CPU_PXA26x
>                 count = 89;
>                 gpio_type = PXA26X_GPIO;
> #elif defined(CONFIG_PXA25x)
>
> in the pca953x driver! I think you messed up with another patch set!

Yes I did.

Forget about this, thanks for the other nice patches!

Yours,
Linus Walleij

[PATCH v3 2/3] gpio: pca953x: add support for pca9505

[Linus Walleij]

On Sat, Jan 26, 2013 at 11:02 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> I was about to fix the issues you have pointed but I didn't find anything like
>
> #ifdef CONFIG_ARCH_PXA
>         if (cpu_is_pxa25x()) {
> #ifdef CONFIG_CPU_PXA26x
>                 count = 89;
>                 gpio_type = PXA26X_GPIO;
> #elif defined(CONFIG_PXA25x)
>
> in the pca953x driver! I think you messed up with another patch set!

Yes I did.

Forget about this, thanks for the other nice patches!

Yours,
Linus Walleij

Re: [PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Jason Cooper]

On Tue, Jan 22, 2013 at 10:10:25PM +0100, Gregory CLEMENT wrote:
> The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
> through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
> expansion IC to provide 40-bit parallel input/output GPIOs. This patch
> enable the use of this expander on the Mirabox.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  arch/arm/boot/dts/armada-370-mirabox.dts |   10 ++++++++++
>  1 file changed, 10 insertions(+)

Applied to mvebu/dt_deps with a dependency on gpio/for-next for:

      89f5df0 gpio: pca953x: add support for pca9505
      f5f0b7a gpio: pca953x: make the register access by GPIO bank

thx,

Jason.

[PATCH v3 3/3] arm: mvebu: enable gpio expander over i2c on Mirabox platform

[Jason Cooper]

On Tue, Jan 22, 2013 at 10:10:25PM +0100, Gregory CLEMENT wrote:
> The Globalscale Mirabox platform can be connected to the JTAG/GPIO box
> through the Multi-IO port. The GPIO box use the NXP PCA9505 I/O port
> expansion IC to provide 40-bit parallel input/output GPIOs. This patch
> enable the use of this expander on the Mirabox.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  arch/arm/boot/dts/armada-370-mirabox.dts |   10 ++++++++++
>  1 file changed, 10 insertions(+)

Applied to mvebu/dt_deps with a dependency on gpio/for-next for:

      89f5df0 gpio: pca953x: add support for pca9505
      f5f0b7a gpio: pca953x: make the register access by GPIO bank

thx,

Jason.