iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

gpio: pca953x: fix handling of automatic address incrementing

Browse Source 
Some of the chips supported by the pca953x driver need the most
significant bit in the address word set to automatically increment the
address pointer on subsequent reads and writes (example: PCA9505). With
this bit unset the same register is read multiple times on a multi-byte
read sequence. Other chips must not have this bit set and autoincrement
always (example: PCA9555).

Up to now this AI bit was interpreted to be part of the address, which
resulted in inconsistent regmap caching when a register was written with
AI set and then read without it. This happened for the PCA9505 in
pca953x_gpio_set_multiple() where pca953x_read_regs() bulk read from the
cache for registers 0x8-0xc and then wrote to registers 0x88-0x8c. (Side
note: reading 5 values from offset 0x8 yiels OP0 5 times because AI must
be set to get OP0-OP4, which is another bug that is resolved here as a
by-product.) The same problem happens when calls to gpio_set_value() and
gpio_set_array_value() were mixed.

With this patch the AI bit is always set for chips that support it. This
works as there are no code locations that make use of the behaviour with
AI unset (for the chips that support it).

Note that the call to pca953x_setup_gpio() had to be done a bit earlier
to make the NBANK macro work.

The history of this bug is a bit complicated. Commit b32cecb46b
("gpio: pca953x: Extract the register address mangling to single
function") changed which chips and functions are affected. Commit
3b00691cc4 ("gpio: pca953x: hack to fix 24 bit gpio expanders") used
some duct tape to make the driver at least appear to work. Commit
4942723276 ("gpio: pca953x: Perform basic regmap conversion")
introduced the caching. Commit b4818afeac ("gpio: pca953x: Add
set_multiple to allow multiple bits to be set in one write.") introduced
the .set_multiple() callback which didn't work for chips that need the
AI bit which was fixed later for some chips in 8958262af3 ("gpio:
pca953x: Repair multi-byte IO address increment on PCA9575"). So I'm
sorry, I don't know which commit I should pick for a Fixes: line.

Tested-by: Marcel Gudert <m.gudert@eckelmann.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
This commit is contained in:
Uwe Kleine-König 2020-04-20 20:27:51 +03:00 committed by Bartosz Golaszewski
parent 6f793485fc
commit bcf41dc480
1 changed files with 28 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
drivers/gpio/gpio-pca953x.c
View File

@ -308,8 +308,22 @@ static const struct regmap_config pca953x_i2c_regmap = {
.disable_locking = true,
.cache_type = REGCACHE_RBTREE,
/* REVISIT: should be 0x7f but some 24 bit chips use REG_ADDR_AI */
.max_register = 0xff,
.max_register = 0x7f,
};
static const struct regmap_config pca953x_ai_i2c_regmap = {
.reg_bits = 8,
.val_bits = 8,
.read_flag_mask = REG_ADDR_AI,
.write_flag_mask = REG_ADDR_AI,
.readable_reg = pca953x_readable_register,
.writeable_reg = pca953x_writeable_register,
.volatile_reg = pca953x_volatile_register,
.cache_type = REGCACHE_RBTREE,
.max_register = 0x7f,
};
static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off,
@ -320,18 +334,6 @@ static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off,
int pinctrl = (reg & PCAL_PINCTRL_MASK) << 1;
u8 regaddr = pinctrl | addr | (off / BANK_SZ);
/* Single byte read doesn't need AI bit set. */
if (!addrinc)
return regaddr;
/* Chips with 24 and more GPIOs always support Auto Increment */
if (write && NBANK(chip) > 2)
regaddr |= REG_ADDR_AI;
/* PCA9575 needs address-increment on multi-byte writes */
if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE)
regaddr |= REG_ADDR_AI;
return regaddr;
}
@ -882,6 +884,7 @@ static int pca953x_probe(struct i2c_client *client,
int ret;
u32 invert = 0;
struct regulator *reg;
const struct regmap_config *regmap_config;
chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
if (chip == NULL)
@ -944,7 +947,17 @@ static int pca953x_probe(struct i2c_client *client,
i2c_set_clientdata(client, chip);
chip->regmap = devm_regmap_init_i2c(client, &pca953x_i2c_regmap);
pca953x_setup_gpio(chip, chip->driver_data & PCA_GPIO_MASK);
if (NBANK(chip) > 2 || PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
dev_info(&client->dev, "using AI\n");
regmap_config = &pca953x_ai_i2c_regmap;
} else {
dev_info(&client->dev, "using no AI\n");
regmap_config = &pca953x_i2c_regmap;
}
chip->regmap = devm_regmap_init_i2c(client, regmap_config);
if (IS_ERR(chip->regmap)) {
ret = PTR_ERR(chip->regmap);
goto err_exit;
@ -975,7 +988,6 @@ static int pca953x_probe(struct i2c_client *client,
/* initialize cached registers from their original values.
* we can't share this chip with another i2c master.
*/
pca953x_setup_gpio(chip, chip->driver_data & PCA_GPIO_MASK);
if (PCA_CHIP_TYPE(chip->driver_data) == PCA953X_TYPE) {
chip->regs = &pca953x_regs;