Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1478103 > unrolled thread
| Started by | Bartosz Golaszewski <bgolaszewski@baylibre.com> |
|---|---|
| First post | 2016-09-07 11:30 +0200 |
| Last post | 2016-09-07 13:40 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v2 5/5] gpio: pca953x: refactor pca953x_read_regs() Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-09-07 11:30 +0200
Re: [PATCH v2 5/5] gpio: pca953x: refactor pca953x_read_regs() Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-09-07 13:40 +0200
| From | Bartosz Golaszewski <bgolaszewski@baylibre.com> |
|---|---|
| Date | 2016-09-07 11:30 +0200 |
| Subject | [PATCH v2 5/5] gpio: pca953x: refactor pca953x_read_regs() |
| Message-ID | <seHl7-6jf-9@gated-at.bofh.it> |
Avoid the unnecessary if-else in pca953x_read_regs() by spltting the
routine into smaller, specialized functions and calling the right one
via a function pointer held in struct pca953x.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/gpio/gpio-pca953x.c | 55 ++++++++++++++++++++++++++++++++-------------
1 file changed, 39 insertions(+), 16 deletions(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 00bb2ea..e417c42 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -134,6 +134,7 @@ struct pca953x_chip {
const struct pca953x_offset *offset;
int (*write_regs)(struct pca953x_chip *, int, u8 *);
+ int (*read_regs)(struct pca953x_chip *, int, u8 *);
};
static int pca953x_read_single(struct pca953x_chip *chip, int reg, u32 *val,
@@ -219,24 +220,41 @@ static int pca953x_write_regs(struct pca953x_chip *chip, int reg, u8 *val)
return 0;
}
-static int pca953x_read_regs(struct pca953x_chip *chip, int reg, u8 *val)
+static int pca953x_read_regs_8(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) {
- int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
+ ret = i2c_smbus_read_byte_data(chip->client, reg);
+ *val = ret;
- ret = i2c_smbus_read_i2c_block_data(chip->client,
- (reg << bank_shift) | REG_ADDR_AI,
- NBANK(chip), val);
- } else {
- ret = i2c_smbus_read_word_data(chip->client, reg << 1);
- val[0] = (u16)ret & 0xFF;
- val[1] = (u16)ret >> 8;
- }
+ return ret;
+}
+
+static int pca953x_read_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
+{
+ int ret;
+
+ ret = i2c_smbus_read_word_data(chip->client, reg << 1);
+ val[0] = (u16)ret & 0xFF;
+ val[1] = (u16)ret >> 8;
+
+ return ret;
+}
+
+static int pca953x_read_regs_24(struct pca953x_chip *chip, int reg, u8 *val)
+{
+ int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
+
+ return i2c_smbus_read_i2c_block_data(chip->client,
+ (reg << bank_shift) | REG_ADDR_AI,
+ NBANK(chip), val);
+}
+
+static int pca953x_read_regs(struct pca953x_chip *chip, int reg, u8 *val)
+{
+ int ret;
+
+ ret = chip->read_regs(chip, reg, val);
if (ret < 0) {
dev_err(&chip->client->dev, "failed reading register\n");
return ret;
@@ -766,10 +784,15 @@ static int pca953x_probe(struct i2c_client *client,
*/
pca953x_setup_gpio(chip, chip->driver_data & PCA_GPIO_MASK);
- if (chip->gpio_chip.ngpio <= 8)
+ if (chip->gpio_chip.ngpio <= 8) {
chip->write_regs = pca953x_write_regs_8;
- else if (chip->gpio_chip.ngpio >= 24)
+ chip->read_regs = pca953x_read_regs_8;
+ } else if (chip->gpio_chip.ngpio >= 24) {
chip->write_regs = pca953x_write_regs_24;
+ chip->read_regs = pca953x_read_regs_24;
+ } else {
+ chip->read_regs = pca953x_read_regs_16;
+ }
if (PCA_CHIP_TYPE(chip->driver_data) == PCA953X_TYPE)
ret = device_pca953x_init(chip, invert);
--
2.7.4
[toc] | [next] | [standalone]
| From | Andy Shevchenko <andriy.shevchenko@linux.intel.com> |
|---|---|
| Date | 2016-09-07 13:40 +0200 |
| Message-ID | <seJmW-7z9-35@gated-at.bofh.it> |
| In reply to | #1478103 |
On Wed, 2016-09-07 at 11:24 +0200, Bartosz Golaszewski wrote:
> Avoid the unnecessary if-else in pca953x_read_regs() by spltting the
> routine into smaller, specialized functions and calling the right one
> via a function pointer held in struct pca953x.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> drivers/gpio/gpio-pca953x.c | 55 ++++++++++++++++++++++++++++++++--
> -----------
> 1 file changed, 39 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index 00bb2ea..e417c42 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -134,6 +134,7 @@ struct pca953x_chip {
> const struct pca953x_offset *offset;
>
> int (*write_regs)(struct pca953x_chip *, int, u8 *);
> + int (*read_regs)(struct pca953x_chip *, int, u8 *);
> };
>
> static int pca953x_read_single(struct pca953x_chip *chip, int reg,
> u32 *val,
> @@ -219,24 +220,41 @@ static int pca953x_write_regs(struct
> pca953x_chip *chip, int reg, u8 *val)
> return 0;
> }
>
> -static int pca953x_read_regs(struct pca953x_chip *chip, int reg, u8
> *val)
> +static int pca953x_read_regs_8(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) {
> - int bank_shift = fls((chip->gpio_chip.ngpio - 1) /
> BANK_SZ);
> + ret = i2c_smbus_read_byte_data(chip->client, reg);
> + *val = ret;
>
> - ret = i2c_smbus_read_i2c_block_data(chip->client,
> - (reg << bank_shift) |
> REG_ADDR_AI,
> - NBANK(chip), val);
> - } else {
> - ret = i2c_smbus_read_word_data(chip->client, reg <<
> 1);
> - val[0] = (u16)ret & 0xFF;
> - val[1] = (u16)ret >> 8;
> - }
> + return ret;
> +}
> +
> +static int pca953x_read_regs_16(struct pca953x_chip *chip, int reg,
> u8 *val)
> +{
> + int ret;
> +
> + ret = i2c_smbus_read_word_data(chip->client, reg << 1);
> + val[0] = (u16)ret & 0xFF;
> + val[1] = (u16)ret >> 8;
> +
> + return ret;
> +}
> +
> +static int pca953x_read_regs_24(struct pca953x_chip *chip, int reg,
> u8 *val)
> +{
> + int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
> +
> + return i2c_smbus_read_i2c_block_data(chip->client,
> + (reg << bank_shift) |
> REG_ADDR_AI,
> + NBANK(chip), val);
> +}
> +
> +static int pca953x_read_regs(struct pca953x_chip *chip, int reg, u8
> *val)
> +{
> + int ret;
> +
> + ret = chip->read_regs(chip, reg, val);
> if (ret < 0) {
> dev_err(&chip->client->dev, "failed reading
> register\n");
> return ret;
> @@ -766,10 +784,15 @@ static int pca953x_probe(struct i2c_client
> *client,
> */
> pca953x_setup_gpio(chip, chip->driver_data & PCA_GPIO_MASK);
>
> - if (chip->gpio_chip.ngpio <= 8)
> + if (chip->gpio_chip.ngpio <= 8) {
> chip->write_regs = pca953x_write_regs_8;
> - else if (chip->gpio_chip.ngpio >= 24)
> + chip->read_regs = pca953x_read_regs_8;
> + } else if (chip->gpio_chip.ngpio >= 24) {
> chip->write_regs = pca953x_write_regs_24;
> + chip->read_regs = pca953x_read_regs_24;
> + } else {
> + chip->read_regs = pca953x_read_regs_16;
> + }
For sake of consolidation stuff can we move write_regs_16 variants here?
It might require to refactor patch 3 as well
>
> if (PCA_CHIP_TYPE(chip->driver_data) == PCA953X_TYPE)
> ret = device_pca953x_init(chip, invert);
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web