stmpe: mfd & gpio updates
Subsystems (3)
v2 cover stmpe: mfd & gpio updates
Changes since V1:
- V2 of this patchset also includes, earlier sent patches, Acked-by Linus:
mfd/stmpe: separate out controller and interface specific code mfd/stmpe: Add support for SPI interface
- Issues with above patches are also fixed
- MODULE_DESCRIPTION and MODULE_AUTHOR are fixed in stmpe-spi and stmpe-i2c files
- gpio-stmpe patch is update to use stmpe->partnum instead of stmpe->variant->id_val
- Patch "mfd/stmpe: Move struct stmpe_variant_info to linux/mfd/stmpe.h" is dropped as it is no longer required.
Viresh Kumar (5):
mfd/stmpe: separate out controller and interface specific code mfd/stmpe: Add support for SPI interface
mfd/stmpe: Add support for stmpe variant 610 mfd/stmpe: ADD support for stmpe variant 801 gpio/gpio-stmpe: ADD support for stmpe variant 801
drivers/gpio/gpio-stmpe.c | 31 +++++- drivers/mfd/Kconfig | 19 ++++- drivers/mfd/Makefile | 2 + drivers/mfd/stmpe-i2c.c | 109 ++++++++++++++++++++ drivers/mfd/stmpe-spi.c | 150 +++++++++++++++++++++++++++ drivers/mfd/stmpe.c | 250 ++++++++++++++++++++++++++------------------- drivers/mfd/stmpe.h | 53 ++++++++++ include/linux/mfd/stmpe.h | 9 ++- 8 files changed, 512 insertions(+), 111 deletions(-) create mode 100644 drivers/mfd/stmpe-i2c.c create mode 100644 drivers/mfd/stmpe-spi.c
Signature · 1 line
1.7.2.2
PATCH v2 1/5 mfd/stmpe: separate out controller and interface specific code
Few STMPE controller can have register interface over SPI or I2C. Current implementation only supports I2C and all code is present in a single file stmpe.c. It would be better to separate out I2C interface specific code from controller specific code. Later SPI specific code can be added in a separate file.
This patch separates out I2C and controller specific code into separate files, making stmpe.c independent of I2C.
--- drivers/mfd/Kconfig | 11 ++++ drivers/mfd/Makefile | 1 + drivers/mfd/stmpe-i2c.c | 107 ++++++++++++++++++++++++++++++++++++ drivers/mfd/stmpe.c | 133 +++++++++++++++------------------------------ drivers/mfd/stmpe.h | 33 +++++++++++ include/linux/mfd/stmpe.h | 7 ++- 6 files changed, 200 insertions(+), 92 deletions(-) create mode 100644 drivers/mfd/stmpe-i2c.c
drivers/mfd/Kconfig +11 -0
@@ -293,6 +293,17 @@ config MFD_STMPE Keypad: stmpe-keypad Touchscreen: stmpe-ts +menu "STMPE Interface Drivers" +depends on MFD_STMPE + +config STMPE_I2C + bool "STMPE I2C Inteface" + depends on I2C + default y + help + This is used to enable I2C interface of STMPE +endmenu + config MFD_TC3589X bool "Support Toshiba TC35892 and variants" depends on I2C=y && GENERIC_HARDIRQS
drivers/mfd/Makefile +1 -0
@@ -16,6 +16,7 @@ obj-$(CONFIG_MFD_DM355EVM_MSP) += dm355evm_msp.o obj-$(CONFIG_MFD_TI_SSP) += ti-ssp.o obj-$(CONFIG_MFD_STMPE) += stmpe.o +obj-$(CONFIG_STMPE_I2C) += stmpe-i2c.o obj-$(CONFIG_MFD_TC3589X) += tc3589x.o obj-$(CONFIG_MFD_T7L66XB) += t7l66xb.o tmio_core.o obj-$(CONFIG_MFD_TC6387XB) += tc6387xb.o tmio_core.o
drivers/mfd/stmpe-i2c.c new file +107 -0
107 lines not on this page · raw message
drivers/mfd/stmpe.c +43 -90
223 lines not on this page · raw message
drivers/mfd/stmpe.h +33 -0
@@ -8,6 +8,14 @@#ifndef __STMPE_H#define __STMPE_H +#include <linux/device.h> +#include <linux/mfd/core.h> +#include <linux/mfd/stmpe.h> +#include <linux/printk.h> +#include <linux/types.h> + +extern const struct dev_pm_ops stmpe_dev_pm_ops; +#ifdef STMPE_DUMP_BYTES static inline void stmpe_dump_bytes (const char *str,const void *buf,size_t len)
@@ -67,6 +75,31 @@ struct stmpe_variant_info {int (*enable_autosleep)(struct stmpe *stmpe,int autosleep_timeout); }; +/** +* struct stmpe_client_info - i2c or spi specific routines/info +* @data: client specific data +* @read_byte: read single byte +* @write_byte: write single byte +* @read_block: read block or multiple bytes +* @write_block: write block or multiple bytes +* @init: client init routine, called during probe +*/ +struct stmpe_client_info { +void *data; +int irq; +void *client; +struct device *dev; +int (*read_byte)(struct stmpe *stmpe,u8 reg); +int (*write_byte)(struct stmpe *stmpe,u8 reg,u8 val); +int (*read_block)(struct stmpe *stmpe,u8 reg,u8 len,u8 *values); +int (*write_block)(struct stmpe *stmpe,u8 reg,u8 len, +const u8 *values); +void (*init)(struct stmpe *stmpe); +}; + +int stmpe_probe (struct stmpe_client_info *ci,int partnum); +int stmpe_remove (struct stmpe *stmpe); +#define STMPE_ICR_LSB_HIGH (1 << 2)#define STMPE_ICR_LSB_EDGE (1 << 1)#define STMPE_ICR_LSB_GIM (1 << 0)
include/linux/mfd/stmpe.h +5 -2
@@ -50,13 +50,15 @@ enum {struct stmpe_variant_info ; +struct stmpe_client_info ;/** * struct stmpe - STMPE MFD structure * @lock: lock protecting I/O operations * @irq_lock: IRQ bus lock * @dev: device, mostly for dev_dbg() -* @i2c: i2c client +* @client: client - i2c or spi +* @ci: client specific information * @partnum: part number * @variant: the detected STMPE model number * @regs: list of addresses of registers which are at different addresses on
@@ -72,7 +74,8 @@ struct stmpe {struct mutex lock;struct mutex irq_lock;struct device *dev; -struct i2c_client *i2c; +void *client; +struct stmpe_client_info *ci;enum stmpe_partnum partnum;struct stmpe_variant_info *variant;const u8 *regs;
Signature · 1 line
1.7.2.2
PATCH v2 2/5 mfd/stmpe: Add support for SPI interface
In reply to Viresh KumarFew STMPE controller can have register interface over SPI or I2C. Current implementation only supports I2C.
This patch adds support for SPI interface for accessing STMPE's address space.
--- drivers/mfd/Kconfig | 8 ++- drivers/mfd/Makefile | 1 + drivers/mfd/stmpe-spi.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++ drivers/mfd/stmpe.h | 1 + 4 files changed, 157 insertions(+), 1 deletions(-) create mode 100644 drivers/mfd/stmpe-spi.c
drivers/mfd/Kconfig +7 -1
@@ -272,7 +272,7 @@ config TWL6040_CORE config MFD_STMPE bool "Support STMicroelectronics STMPE" - depends on I2C=y && GENERIC_HARDIRQS + depends on (I2C=y || SPI_MASTER=y) && GENERIC_HARDIRQS select MFD_CORE help Support for the STMPE family of I/O Expanders from
@@ -302,6 +302,12 @@ config STMPE_I2C default y help This is used to enable I2C interface of STMPE + +config STMPE_SPI + bool "STMPE SPI Inteface" + depends on SPI_MASTER + help + This is used to enable SPI interface of STMPE endmenu config MFD_TC3589X
drivers/mfd/Makefile +1 -0
@@ -17,6 +17,7 @@ obj-$(CONFIG_MFD_TI_SSP) += ti-ssp.o obj-$(CONFIG_MFD_STMPE) += stmpe.o obj-$(CONFIG_STMPE_I2C) += stmpe-i2c.o +obj-$(CONFIG_STMPE_SPI) += stmpe-spi.o obj-$(CONFIG_MFD_TC3589X) += tc3589x.o obj-$(CONFIG_MFD_T7L66XB) += t7l66xb.o tmio_core.o obj-$(CONFIG_MFD_TC6387XB) += tc6387xb.o tmio_core.o
drivers/mfd/stmpe-spi.c new file +148 -0
148 lines not on this page · raw message
drivers/mfd/stmpe.h +1 -0
@@ -120,6 +120,7 @@ int stmpe_remove(struct stmpe *stmpe);#define STMPE811_REG_CHIP_ID 0x00#define STMPE811_REG_SYS_CTRL2 0x04 +#define STMPE811_REG_SPI_CFG 0x08#define STMPE811_REG_INT_CTRL 0x09#define STMPE811_REG_INT_EN 0x0A#define STMPE811_REG_INT_STA 0x0B
Signature · 1 line
1.7.2.2
PATCH v2 3/5 mfd/stmpe: Add support for stmpe variant 610
In reply to Viresh KumarSTMPE610 is very much like STMPE811, except the number of gpio pins, which is 8 in 811 and 6 in 610. This patch adds support for variant 610. STMPE610 will share most of the code with STMPE811.
--- drivers/mfd/stmpe-i2c.c | 1 + drivers/mfd/stmpe-spi.c | 1 + drivers/mfd/stmpe.c | 20 ++++++++++++++++++-- include/linux/mfd/stmpe.h | 1 + 4 files changed, 21 insertions(+), 2 deletions(-)
drivers/mfd/stmpe-i2c.c +1 -0
@@ -71,6 +71,7 @@ static int __devexit stmpe_i2c_remove(struct i2c_client *i2c) }static const struct i2c_device_id stmpe_i2c_id[] = { + {"stmpe610" ,STMPE610 }, {"stmpe811" ,STMPE811 }, {"stmpe1601" ,STMPE1601 }, {"stmpe2401" ,STMPE2401 },
drivers/mfd/stmpe-spi.c +1 -0
@@ -109,6 +109,7 @@ static int __devexit stmpe_spi_remove(struct spi_device *spi) }static const struct spi_device_id stmpe_spi_id[] = { + {"stmpe610" ,STMPE610 }, {"stmpe811" ,STMPE811 }, {"stmpe1601" ,STMPE1601 }, {"stmpe2401" ,STMPE2401 },
drivers/mfd/stmpe.c +18 -2
@@ -321,7 +321,7 @@ static struct mfd_cell stmpe_keypad_cell = { };/* -* Touchscreen (STMPE811) +* Touchscreen (STMPE811 or STMPE610) */ static struct resource stmpe_ts_resources[] = {
@@ -346,7 +346,7 @@ static struct mfd_cell stmpe_ts_cell = { };/* -* STMPE811 +* STMPE811 or STMPE610 */ static const u8 stmpe811_regs[] = {
@@ -417,6 +417,21 @@ static struct stmpe_variant_info stmpe811 = { .get_altfunc = stmpe811_get_altfunc, }; +/* Similar to 811, except number of gpios */ +static struct stmpe_variant_info stmpe610 = { + .name ="stmpe610" , + .id_val =0x0811 , + .id_mask =0xffff , + .num_gpios =6 , + .af_bits =1 , + .regs = stmpe811_regs, + .blocks = stmpe811_blocks, + .num_blocks =ARRAY_SIZE (stmpe811_blocks), + .num_irqs =STMPE811_NR_INTERNAL_IRQS , + .enable = stmpe811_enable, + .get_altfunc = stmpe811_get_altfunc, +}; +/* * STMPE1601 */
@@ -651,6 +666,7 @@ static struct stmpe_variant_info stmpe2403 = { };static struct stmpe_variant_info *stmpe_variant_info[] = { + [STMPE610 ] = &stmpe610, [STMPE811 ] = &stmpe811, [STMPE1601 ] = &stmpe1601, [STMPE2401 ] = &stmpe2401,
include/linux/mfd/stmpe.h +1 -0
@@ -20,6 +20,7 @@ enum stmpe_block { };enum stmpe_partnum { +STMPE610 ,STMPE811 ,STMPE1601 ,STMPE2401 ,
Signature · 1 line
1.7.2.2
Re: [PATCH V2 3/5] mfd/stmpe: Add support for stmpe variant 610
Quoted 5 lines · Viresh Kumar
STMPE610 is very much like STMPE811, except the number of gpio pins, which is 8 in 811 and 6 in 610. This patch adds support for variant 610. STMPE610 will share most of the code with STMPE811.
Signed-off-by Viresh Kumar <[email protected]>
Thanks,
Linus Walleij
PATCH v2 4/5 mfd/stmpe: ADD support for stmpe variant 801
In reply to Viresh KumarSTMPE801 is a GPIO expander. Registers for 801 are much different from other variants. This patch adds support for STMPE801 in stmpe mfd driver.
--- drivers/mfd/stmpe-i2c.c | 1 + drivers/mfd/stmpe-spi.c | 1 + drivers/mfd/stmpe.c | 97 +++++++++++++++++++++++++++++++++++++++------ drivers/mfd/stmpe.h | 19 +++++++++ include/linux/mfd/stmpe.h | 1 + 5 files changed, 106 insertions(+), 13 deletions(-)
drivers/mfd/stmpe-i2c.c +1 -0
@@ -72,6 +72,7 @@ static int __devexit stmpe_i2c_remove(struct i2c_client *i2c)static const struct i2c_device_id stmpe_i2c_id[] = { {"stmpe610" ,STMPE610 }, + {"stmpe801" ,STMPE801 }, {"stmpe811" ,STMPE811 }, {"stmpe1601" ,STMPE1601 }, {"stmpe2401" ,STMPE2401 },
drivers/mfd/stmpe-spi.c +1 -0
@@ -110,6 +110,7 @@ static int __devexit stmpe_spi_remove(struct spi_device *spi)static const struct spi_device_id stmpe_spi_id[] = { {"stmpe610" ,STMPE610 }, + {"stmpe801" ,STMPE801 }, {"stmpe811" ,STMPE811 }, {"stmpe1601" ,STMPE1601 }, {"stmpe2401" ,STMPE2401 },
drivers/mfd/stmpe.c +84 -13
151 lines not on this page · raw message
drivers/mfd/stmpe.h +19 -0
@@ -105,6 +105,25 @@ int stmpe_remove(struct stmpe *stmpe);#define STMPE_ICR_LSB_GIM (1 << 0)/* +* STMPE801 +*/ +#define STMPE801_ID 0x0108 +#define STMPE801_NR_INTERNAL_IRQS 1 + +#define STMPE801_REG_CHIP_ID 0x00 +#define STMPE801_REG_VERSION_ID 0x02 +#define STMPE801_REG_SYS_CTRL 0x04 +#define STMPE801_REG_GPIO_INT_EN 0x08 +#define STMPE801_REG_GPIO_INT_STA 0x09 +#define STMPE801_REG_GPIO_MP_STA 0x10 +#define STMPE801_REG_GPIO_SET_PIN 0x11 +#define STMPE801_REG_GPIO_DIR 0x12 + +#define STMPE801_REG_SYS_CTRL_RESET (1 << 7) +#define STMPE801_REG_SYS_CTRL_INT_EN (1 << 2) +#define STMPE801_REG_SYS_CTRL_INT_HI (1 << 0) + +/* * STMPE811 */
include/linux/mfd/stmpe.h +1 -0
@@ -21,6 +21,7 @@ enum stmpe_block {enum stmpe_partnum {STMPE610 , +STMPE801 ,STMPE811 ,STMPE1601 ,STMPE2401 ,
Signature · 1 line
1.7.2.2
PATCH v2 5/5 gpio/gpio-stmpe: ADD support for stmpe variant 801
In reply to Viresh KumarSTMPE801 is a GPIO expander. GPIO registers for 801 are slightly different from other variants. This patch adds support for STMPE801 in stmpe gpio driver.
--- drivers/gpio/gpio-stmpe.c | 31 ++++++++++++++++++++++++++++--- 1 files changed, 28 insertions(+), 3 deletions(-)
drivers/gpio/gpio-stmpe.c +28 -3
@@ -65,7 +65,15 @@ static void stmpe_gpio_set(struct gpio_chip *chip, unsigned offset, int val)u8 reg = stmpe->regs[which] - (offset /8 );u8 mask =1 << (offset %8 ); -stmpe_reg_write (stmpe, reg, mask); +/* +* Some variants have single register for gpio set/clear functionality. +* For them we need to write 0 to clear and 1 to set. +*/ +if (!val && (stmpe->regs[STMPE_IDX_GPSR_LSB ] == + stmpe->regs[STMPE_IDX_GPCR_LSB ])) +stmpe_set_bits (stmpe, reg, mask, ~mask); +else +stmpe_set_bits (stmpe, reg, mask, mask); } static int stmpe_gpio_direction_output(struct gpio_chip *chip,
@@ -125,10 +133,19 @@ static struct gpio_chip template_chip = {static int stmpe_gpio_irq_set_type (struct irq_data *d,unsigned int type) {struct stmpe_gpio *stmpe_gpio =irq_data_get_irq_chip_data (d); +struct stmpe *stmpe = stmpe_gpio->stmpe;int offset = d->irq - stmpe_gpio->irq_base;int regoffset = offset /8 ;int mask =1 << (offset %8 ); +/* STMPE801 doesn't have RE and FE registers */ +if (stmpe->partnum ==STMPE801 ) { +if (type ==IRQ_TYPE_LEVEL_LOW || type ==IRQ_TYPE_LEVEL_HIGH ) +return 0 ; +else +return -EINVAL ; + } +if (type ==IRQ_TYPE_LEVEL_LOW || type ==IRQ_TYPE_LEVEL_HIGH )return -EINVAL ;
@@ -165,6 +182,11 @@ static void stmpe_gpio_irq_sync_unlock(struct irq_data *d)int i, j;for (i =0 ; i <CACHE_NR_REGS ; i++) { +/* STMPE801 doesn't have RE and FE registers */ +if ((stmpe->partnum ==STMPE801 ) && + (i !=REG_IE )) +continue ; +for (j =0 ; j < num_banks; j++) {u8 old = stmpe_gpio->oldregs[i][j];u8 new = stmpe_gpio->regs[i][j];
@@ -241,8 +263,11 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev) }stmpe_reg_write (stmpe, statmsbreg + i, status[i]); -stmpe_reg_write (stmpe, stmpe->regs[STMPE_IDX_GPEDR_MSB ] + i, - status[i]); + +/* Edge detect register is not present on 801 */ +if (stmpe->partnum !=STMPE801 ) +stmpe_reg_write (stmpe, stmpe->regs[STMPE_IDX_GPEDR_MSB ] + + i, status[i]); }return IRQ_HANDLED ;
Signature · 1 line
1.7.2.2
Re: [PATCH V2 5/5] gpio/gpio-stmpe: ADD support for stmpe variant 801
Quoted 9 lines · Viresh Kumar
drivers/gpio/gpio-stmpe.c +0
-1@@ -65,7 +65,15 @@ static void stmpe_gpio_set(struct gpio_chip *chip, unsigned offset, int val) u8 reg = stmpe->regs[which] - (offset / 8); u8 mask = 1 << (offset % 8); - stmpe_reg_write(stmpe, reg, mask);
drivers/gpio/gpio-stmpe.c static void stmpe_gpio_set(struct gpio_chip *chip, unsigned offset, int val) · line 77
Quoted 10 lines
drivers/gpio/gpio-stmpe.c +9
-0+ /* + * Some variants have single register for gpio set/clear functionality. + * For them we need to write 0 to clear and 1 to set. + */ + if (!val && (stmpe->regs[STMPE_IDX_GPSR_LSB] == + stmpe->regs[STMPE_IDX_GPCR_LSB])) + stmpe_set_bits(stmpe, reg, mask, ~mask); + else + stmpe_set_bits(stmpe, reg, mask, mask); }
This code,
(1) for 801, when clearing one GPIO, sets all the others.
(2) for other devices, adds an an unnecessary read (within stmpe_set_bits()), which wasn't there before.
Please rework to something like:
if (stmpe->regs[...) stmpe_set_bits(stmpe, reg, mask, val ? mask : 0); else stmpe_reg_write(stmpe, reg, mask);
Quoted 5 lines
drivers/gpio/gpio-stmpe.c +0
-0static int stmpe_gpio_direction_output(struct gpio_chip *chip,@@ -125,10 +133,19 @@ static struct gpio_chip template_chip = { static int stmpe_gpio_irq_set_type(struct irq_data *d, unsigned int type) {
drivers/gpio/gpio-stmpe.c static struct gpio_chip template_chip = { · line 144
Quoted 10 lines
drivers/gpio/gpio-stmpe.c +5
-0struct stmpe_gpio *stmpe_gpio = irq_data_get_irq_chip_data(d); + struct stmpe *stmpe = stmpe_gpio->stmpe; int offset = d->irq - stmpe_gpio->irq_base; int regoffset = offset / 8; int mask = 1 << (offset % 8); + /* STMPE801 doesn't have RE and FE registers */ + if (stmpe->partnum == STMPE801) { + if (type == IRQ_TYPE_LEVEL_LOW || type == IRQ_TYPE_LEVEL_HIGH) + return 0;
This looks wrong. From the datasheet I see that it supports edges only, so perhaps you meant to say IRQ_TYPE_EDGE_* above.
In that case please reorganize this to add the return 0 after the existing check which excludes levels (below).
Quoted 6 lines
drivers/gpio/gpio-stmpe.c +4
-0+ else + return -EINVAL; + } + if (type == IRQ_TYPE_LEVEL_LOW || type == IRQ_TYPE_LEVEL_HIGH) return -EINVAL;
Re: [PATCH V2 5/5] gpio/gpio-stmpe: ADD support for stmpe variant 801
Quoted 15 lines · Rabin Vincent
drivers/gpio/gpio-stmpe.c static void stmpe_gpio_set(struct gpio_chip *chip, unsigned offset, int val) · line 70
Quoted 11 lines · Viresh Kumar
drivers/gpio/gpio-stmpe.c +5
-0@@ -65,7 +65,15 @@ static void stmpe_gpio_set(struct gpio_chip *chip, unsigned offset, int val) +if (!val && (stmpe->regs[STMPE_IDX_GPSR_LSB ] == + stmpe->regs[STMPE_IDX_GPCR_LSB ])) +stmpe_set_bits (stmpe, reg, mask, ~mask); +else +stmpe_set_bits (stmpe, reg, mask, mask); }This code,
Quoted 1 line
(1) for 801, when clearing one GPIO, sets all the others.
I assumed stmpe_set_bits will only affect bits which are 1 in mask and i was wrong. :(
Quoted 3 lines
(2) for other devices, adds an an unnecessary read (within
stmpe_set_bits()), which wasn't there before.
Correct.
Quoted 7 lines
Please rework to something like:
if (stmpe->regs[...) stmpe_set_bits(stmpe, reg, mask, val ? mask : 0); else stmpe_reg_write(stmpe, reg, mask);
Sure.
Quoted 11 lines
Quoted 11 lines
drivers/gpio/gpio-stmpe.c +1
-0static int stmpe_gpio_direction_output(struct gpio_chip *chip,@@ -125,10 +133,19 @@ static struct gpio_chip template_chip = {static int stmpe_gpio_irq_set_type (struct irq_data *d,unsigned int type) {struct stmpe_gpio *stmpe_gpio =irq_data_get_irq_chip_data (d); +struct stmpe *stmpe = stmpe_gpio->stmpe;int offset = d->irq - stmpe_gpio->irq_base;int regoffset = offset /8 ;int mask =1 << (offset %8 );
Quoted 8 lines
drivers/gpio/gpio-stmpe.c static struct gpio_chip template_chip = { · line 144
Quoted 4 lines
drivers/gpio/gpio-stmpe.c +4
-0+/* STMPE801 doesn't have RE and FE registers */ +if (stmpe->partnum ==STMPE801 ) { +if (type ==IRQ_TYPE_LEVEL_LOW || type ==IRQ_TYPE_LEVEL_HIGH ) +return 0 ;This looks wrong. From the datasheet I see that it supports edges only, so perhaps you meant to say IRQ_TYPE_EDGE_* above.
I meant to say IRQ_TYPE_LEVEL* only, but i was wrong. I didn't read the manual correctly. :(
Quoted 3 lines
In that case please reorganize this to add the return 0 after the existing check which excludes levels (below).
Sure
Signature · 1 line
viresh
Re: [PATCH V2 5/5] gpio/gpio-stmpe: ADD support for stmpe variant 801
Quoted 6 lines · Viresh Kumar
Quoted 1 line · Rabin Vincent
(1) for 801, when clearing one GPIO, sets all the others.
I assumed
stmpe_set_bitswill only affect bits which are 1 in mask and i was wrong. :(
That does look like a straight bug in stmpe_set_bits() which ought to be fixed regardless of anything else.
Quoted 6 lines
Quoted 2 lines
(2) for other devices, adds an an unnecessary read (within
stmpe_set_bits()), which wasn't there before.Correct.
Quoted 1 line
Please rework to something like:
Looking at the code in stmpe.c it might be as well to convert to regmap which would if nothing else save a bunch of custom code in the driver and let you use the cache code to eliminate the read. Probably that will help eliminate reads in other places too.
Re: [PATCH V2 5/5] gpio/gpio-stmpe: ADD support for stmpe variant 801
Quoted 5 lines · Mark Brown
Quoted 2 lines
Quoted 2 lines
I assumed
stmpe_set_bitswill only affect bits which are 1 in mask and i was wrong. :(That does look like a straight bug in
stmpe_set_bits()which ought to be fixed regardless of anything else.
I think so. I will fix it in a separate patch.
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
(2) for other devices, adds an an unnecessary read (within
stmpe_set_bits()), which wasn't there before.Correct.
Quoted 1 line
Quoted 1 line
Please rework to something like:
Looking at the code in
stmpe.cit might be as well to convert to regmap which would if nothing else save a bunch of custom code in the driver and let you use the cache code to eliminate the read. Probably that will help eliminate reads in other places too.
Ya, i already got this feedback from Rabin. I am a bit busy right now and so requested sameo to accept this patchset as it is. I will update drivers to use regmap as soon as i get some time.
Signature · 1 line
viresh
Re: [PATCH V2 5/5] gpio/gpio-stmpe: ADD support for stmpe variant 801
Hi Viresh,
Quoted 13 lines · Viresh Kumar
Quoted 5 lines · Mark Brown
Quoted 2 lines
Quoted 2 lines
I assumed
stmpe_set_bitswill only affect bits which are 1 in mask and i was wrong. :(That does look like a straight bug in
stmpe_set_bits()which ought to be fixed regardless of anything else.I think so. I will fix it in a separate patch.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
(2) for other devices, adds an an unnecessary read (within
stmpe_set_bits()), which wasn't there before.Correct.
Quoted 1 line
Quoted 1 line
Please rework to something like:
Quoted 8 lines
Quoted 4 lines
Looking at the code in
stmpe.cit might be as well to convert to regmap which would if nothing else save a bunch of custom code in the driver and let you use the cache code to eliminate the read. Probably that will help eliminate reads in other places too.Ya, i already got this feedback from Rabin. I am a bit busy right now and so requested sameo to accept this patchset as it is. I will update drivers to use regmap as soon as i get some time.
Ok, so I reverted your first 2 patches patchset since it seems it was problematic. Now, I think Rabin's concern on your patch #5 seems valid, and I'd like you to fix it before applying this patchset.
Cheers,
Samuel.
Signature · 2 lines
Intel Open Source Technology Centre http://oss.intel.com/
Re: [PATCH V2 5/5] gpio/gpio-stmpe: ADD support for stmpe variant 801
Quoted 3 lines · Samuel Ortiz
Ok, so I reverted your first 2 patches patchset since it seems it was problematic. Now, I think Rabin's concern on your patch #5 seems valid, and I'd like you to fix it before applying this patchset.
Which problem are you talking about? Issues in v2 of patch 5/5 or converting to regmap?
Earlier once are already fixed in V3 of 5/5. And for later one, i already confirmed to do it in a separate patchset.
Signature · 1 line
viresh
Re: [PATCH V2 5/5] gpio/gpio-stmpe: ADD support for stmpe variant 801
Hi Viresh,
Quoted 9 lines · Viresh Kumar
Quoted 3 lines · Samuel Ortiz
Ok, so I reverted your first 2 patches patchset since it seems it was problematic. Now, I think Rabin's concern on your patch #5 seems valid, and I'd like you to fix it before applying this patchset.
Which problem are you talking about? Issues in v2 of patch 5/5 or converting to regmap?
Earlier once are already fixed in V3 of 5/5. And for later one, i already confirmed to do it in a separate patchset.
I haven't received a V3 for 5/5. Could you please re-send it to me ?
Cheers,
Samuel.
Signature · 2 lines
Intel Open Source Technology Centre http://oss.intel.com/
Re: [PATCH V2 5/5] gpio/gpio-stmpe: ADD support for stmpe variant 801
Quoted 1 line · Samuel Ortiz
I haven't received a V3 for 5/5. Could you please re-send it to me ?
I sent it again. :)
Signature · 1 line
viresh
PATCH v3 5/5 gpio/gpio-stmpe: ADD support for stmpe variant 801
In reply to Viresh KumarSTMPE801 is a GPIO expander. GPIO registers for 801 are slightly different from other variants. This patch adds support for STMPE801 in stmpe gpio driver.
---
Changes since V2:
- stmpe_gpio_set() is updates, so that it doesn't configure other gpio pins by mistake.
- STMPE801 also supports Edge interrupts and not level ints. Code updated for this too.
drivers/gpio/gpio-stmpe.c | 25 ++++++++++++++++++++++--- 1 files changed, 22 insertions(+), 3 deletions(-)
drivers/gpio/gpio-stmpe.c +22 -3
@@ -65,7 +65,14 @@ static void stmpe_gpio_set(struct gpio_chip *chip, unsigned offset, int val)u8 reg = stmpe->regs[which] - (offset /8 );u8 mask =1 << (offset %8 ); -stmpe_reg_write (stmpe, reg, mask); +/* +* Some variants have single register for gpio set/clear functionality. +* For them we need to write 0 to clear and 1 to set. +*/ +if (stmpe->regs[STMPE_IDX_GPSR_LSB ] == stmpe->regs[STMPE_IDX_GPCR_LSB ]) +stmpe_set_bits (stmpe, reg, mask, val ? mask :0 ); +else +stmpe_reg_write (stmpe, reg, mask); } static int stmpe_gpio_direction_output(struct gpio_chip *chip,
@@ -132,6 +139,10 @@ static int stmpe_gpio_irq_set_type(struct irq_data *d, unsigned int type)if (type ==IRQ_TYPE_LEVEL_LOW || type ==IRQ_TYPE_LEVEL_HIGH )return -EINVAL ; +/* STMPE801 doesn't have RE and FE registers */ +if (stmpe_gpio->stmpe->partnum ==STMPE801 ) +return 0 ; +if (type ==IRQ_TYPE_EDGE_RISING ) stmpe_gpio->regs[REG_RE ][regoffset] |= mask;else
@@ -165,6 +176,11 @@ static void stmpe_gpio_irq_sync_unlock(struct irq_data *d)int i, j;for (i =0 ; i <CACHE_NR_REGS ; i++) { +/* STMPE801 doesn't have RE and FE registers */ +if ((stmpe->partnum ==STMPE801 ) && + (i !=REG_IE )) +continue ; +for (j =0 ; j < num_banks; j++) {u8 old = stmpe_gpio->oldregs[i][j];u8 new = stmpe_gpio->regs[i][j];
@@ -241,8 +257,11 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev) }stmpe_reg_write (stmpe, statmsbreg + i, status[i]); -stmpe_reg_write (stmpe, stmpe->regs[STMPE_IDX_GPEDR_MSB ] + i, - status[i]); + +/* Edge detect register is not present on 801 */ +if (stmpe->partnum !=STMPE801 ) +stmpe_reg_write (stmpe, stmpe->regs[STMPE_IDX_GPEDR_MSB ] + + i, status[i]); }return IRQ_HANDLED ;
Signature · 1 line
1.7.2.2
Re: [PATVCH V3 Resend 5/5] gpio/gpio-stmpe: ADD support for stmpe variant 801
Hi Viresh,
Quoted 11 lines · Viresh Kumar
STMPE801 is a GPIO expander. GPIO registers for 801 are slightly different from other variants. This patch adds support for STMPE801 in stmpe gpio driver.
Signed-off-by Bhupesh Sharma <[email protected]>Signed-off-by Pratyush Anand <[email protected]>Signed-off-by Viresh Kumar <[email protected]>---
Hi Samuel,
Please apply this patch instead of V2 5/5.
All 5 patches applied now, thanks.
Cheers,
Samuel.
Signature · 2 lines
Intel Open Source Technology Centre http://oss.intel.com/