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

[PATCH 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-rc2, but I am willing to rebase onto gpio-for-next once
the GPIO block will be merged into it.

I also expected some tested-by as I was only able to test the pca9505
and I didn't test the IRQ part.

Thanks!

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              |  235 +++++++++++++++++++-----------
 2 files changed, 160 insertions(+), 85 deletions(-)

-- 
1.7.9.5

[PATCH 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>
---
 drivers/gpio/gpio-pca953x.c |  233 +++++++++++++++++++++++++++----------------
 1 file changed, 148 insertions(+), 85 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index cc102d2..aa9b6b2 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -71,18 +71,22 @@ static const struct i2c_device_id pca953x_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pca953x_id);
 
+#define MAX_BANK 5
+
+#define NBANK(chip) (chip->gpio_chip.ngpio / 8)
+
 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 +97,70 @@ 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) - 3;
+	int offset = off / 8;
+	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 reg=%X\n",
+			reg);
+		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) - 3;
+	int offset = off / 8;
+
+	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) - 3;
 		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,28 +173,28 @@ 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) - 3;
 		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),
+					(u8 *) 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");
+		dev_err(&chip->client->dev,
+			"failed reading register reg=%X\n", reg);
 		return ret;
 	}
 
@@ -163,13 +204,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 / 8] | (1u << (off % 8));
 
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
@@ -179,11 +220,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 / 8] = reg_val;
 	ret = 0;
 exit:
 	mutex_unlock(&chip->i2c_lock);
@@ -194,7 +235,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 +243,9 @@ 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 / 8] | (1u << (off % 8));
 	else
-		reg_val = chip->reg_output & ~(1u << off);
+		reg_val = chip->reg_output[off / 8] & ~(1u << (off % 8));
 
 	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 / 8] = reg_val;
 
 	/* then direction */
-	reg_val = chip->reg_direction & ~(1u << off);
+	reg_val = chip->reg_direction[off / 8] & ~(1u << (off % 8));
 	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 / 8] = 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,16 @@ 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 / 8] | (1u << (off % 8));
 	else
-		reg_val = chip->reg_output & ~(1u << off);
+		reg_val = chip->reg_output[off / 8] & ~(1u << (off % 8));
 
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
@@ -293,11 +334,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 / 8] = reg_val;
 exit:
 	mutex_unlock(&chip->i2c_lock);
 }
@@ -406,11 +447,11 @@ static struct irq_chip pca953x_irq_chip = {
 
 static u32 pca953x_irq_pending(struct pca953x_chip *chip)
 {
-	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 pending[MAX_BANK];
+	u8 trigger[MAX_BANK];
+	int ret, i, offset = 0;
 
 	switch (chip->chip_type) {
 	case PCA953X_TYPE:
@@ -420,24 +461,29 @@ 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));
+
+	for (i = 0; i < NBANK(chip); i++)
+		trigger[i] = (cur_stat[i] ^ old_stat[i]) & chip->irq_mask[i];
 
 	if (!trigger)
 		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];
+	}
 
 	return pending;
 }
@@ -445,20 +491,21 @@ static u32 pca953x_irq_pending(struct pca953x_chip *chip)
 static irqreturn_t pca953x_irq_handler(int irq, void *devid)
 {
 	struct pca953x_chip *chip = devid;
-	u32 pending;
-	u32 level;
+	u8 *pending;
+	u8 level;
 
 	pending = pca953x_irq_pending(chip);
 
 	if (!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++) {
+		do {
+			level = __ffs(pending[i]);
+			handle_nested_irq(irq_find_mapping(chip->domain,
+							    level + (8 * i)));
+			pending[i] &= ~(1 << level);
+		} while (pending[i]);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -468,8 +515,8 @@ 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;
+	u8 *temporary;
 
 	if (irq_base != -1
 			&& (id->driver_data & PCA_INT)) {
@@ -483,8 +530,9 @@ 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, temporary);
+		for (i = 0; i < NBANK(chip); i++)
+			chip->irq_stat[i] = temporary[i];
 		if (ret)
 			goto out_failed;
 
@@ -493,7 +541,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 +668,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 +693,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 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 aa9b6b2..8bf70a2 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, },
@@ -829,6 +830,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 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>
---
 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

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

[Gregory CLEMENT]

On 01/06/2013 06:34 PM, Gregory CLEMENT wrote:
> 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-rc2, but I am willing to rebase onto gpio-for-next once
> the GPIO block will be merged into it.
> 
> I also expected some tested-by as I was only able to test the pca9505
> and I didn't test the IRQ part.

For those who are interested the branch gpio-pca9505 is available at:
https://github.com/MISL-EBU-System-SW/mainline-public.git

This branch is based on v3.8-rc2 plus some fixes related to mvebu
plateform and of course this patch set.

> 
> Thanks!
> 
> 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              |  235 +++++++++++++++++++-----------
>  2 files changed, 160 insertions(+), 85 deletions(-)
> 


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

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

[Maxime Ripard]

Hi Gregory,

Le 06/01/2013 18:34, Gregory CLEMENT a écrit :
> +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) - 3;
>  		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;
>  		}
>  	}

I just tested your patch on a PCA9555 on the Crystalfontz CFA-10049, and
it doesn't work anywore. It returns ENXIO now when you use this
function. As discussed on IRC, it seems that the fix would be to replace
all the calls to fls(chip->gpio_chip.ngpio) by fls(chip->gpio_chip.ngpio
- 1).

Also, all the irq handling is not compiling anymore for trivial errors
(variables not defined, incompatible types, etc.). So obviously, I
couldn't test the changes you made on the IRQ related functions.

Maxime

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

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

[Linus Walleij]

On Sun, Jan 6, 2013 at 6:34 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>

Applied, thanks.

Yours,
Linus Walleij

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

[Linus Walleij]

On Sun, Jan 6, 2013 at 6:34 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>

Pls merge through the Marvell tree or whatever ARM subtree makes sense.

Yours,
Linus Walleij

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

[Gregory CLEMENT]

On 01/10/2013 12:15 PM, Linus Walleij wrote:
> On Sun, Jan 6, 2013 at 6:34 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>
> 
> Applied, thanks.
> 
Hello Linus,

I think it is a little too early to apply this patch because the 1st
one still have bug and is not yet applied.

So it should be better to wait for that the 1st patch is fine.

Thanks.

> 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 2/3] gpio: pca953x: add support for pca9505

[Linus Walleij]

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

> On 01/10/2013 12:15 PM, Linus Walleij wrote:
>> On Sun, Jan 6, 2013 at 6:34 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>
>>
>> Applied, thanks.
>>
> Hello Linus,
>
> I think it is a little too early to apply this patch because the 1st
> one still have bug and is not yet applied.

OK pulled it out. Could have been an issue with Grant's and my synchronized
trees but we were lucky this time.

Yours,
Linus Walleij