2022-08-29 13:48:40

by Martyn Welch

Subject: [PATCH 4/5] gpio: pca953x: Swap if statements to save later complexity

A later patch in the series adds support for a further chip type that
shares some similarity with the PCA953X_TYPE. In order to keep the logic
simple, swap over the if and else portions where checks are made against
PCA953X_TYPE and instead check for PCA957X_TYPE.

Signed-off-by: Martyn Welch <[email protected]>
---
drivers/gpio/gpio-pca953x.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 41e7ff83a735..19a8eb94a629 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -293,13 +293,13 @@ static bool pca953x_readable_register(struct device *dev, unsigned int reg)
struct pca953x_chip *chip = dev_get_drvdata(dev);
u32 bank;

- if (PCA_CHIP_TYPE(chip->driver_data) == PCA953X_TYPE) {
- bank = PCA953x_BANK_INPUT | PCA953x_BANK_OUTPUT |
- PCA953x_BANK_POLARITY | PCA953x_BANK_CONFIG;
- } else {
+ if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
bank = PCA957x_BANK_INPUT | PCA957x_BANK_OUTPUT |
PCA957x_BANK_POLARITY | PCA957x_BANK_CONFIG |
PCA957x_BANK_BUSHOLD;
+ } else {
+ bank = PCA953x_BANK_INPUT | PCA953x_BANK_OUTPUT |
+ PCA953x_BANK_POLARITY | PCA953x_BANK_CONFIG;
}

if (chip->driver_data & PCA_PCAL) {
@@ -316,12 +316,12 @@ static bool pca953x_writeable_register(struct device *dev, unsigned int reg)
struct pca953x_chip *chip = dev_get_drvdata(dev);
u32 bank;

- if (PCA_CHIP_TYPE(chip->driver_data) == PCA953X_TYPE) {
- bank = PCA953x_BANK_OUTPUT | PCA953x_BANK_POLARITY |
- PCA953x_BANK_CONFIG;
- } else {
+ if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
bank = PCA957x_BANK_OUTPUT | PCA957x_BANK_POLARITY |
PCA957x_BANK_CONFIG | PCA957x_BANK_BUSHOLD;
+ } else {
+ bank = PCA953x_BANK_OUTPUT | PCA953x_BANK_POLARITY |
+ PCA953x_BANK_CONFIG;
}

if (chip->driver_data & PCA_PCAL)
@@ -336,10 +336,10 @@ static bool pca953x_volatile_register(struct device *dev, unsigned int reg)
struct pca953x_chip *chip = dev_get_drvdata(dev);
u32 bank;

- if (PCA_CHIP_TYPE(chip->driver_data) == PCA953X_TYPE)
- bank = PCA953x_BANK_INPUT;
- else
+ if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE)
bank = PCA957x_BANK_INPUT;
+ else
+ bank = PCA953x_BANK_INPUT;

if (chip->driver_data & PCA_PCAL)
bank |= PCAL9xxx_BANK_IRQ_STAT;
@@ -1069,13 +1069,12 @@ 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.
*/
-
- if (PCA_CHIP_TYPE(chip->driver_data) == PCA953X_TYPE) {
- chip->regs = &pca953x_regs;
- ret = device_pca95xx_init(chip, invert);
- } else {
+ if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
chip->regs = &pca957x_regs;
ret = device_pca957x_init(chip, invert);
+ } else {
+ chip->regs = &pca953x_regs;
+ ret = device_pca95xx_init(chip, invert);
}
if (ret)
goto err_exit;
--
2.35.1

2022-08-31 12:45:11

by Linus Walleij

Subject: Re: [PATCH 4/5] gpio: pca953x: Swap if statements to save later complexity

On Mon, Aug 29, 2022 at 3:39 PM Martyn Welch <[email protected]> wrote:

> A later patch in the series adds support for a further chip type that
> shares some similarity with the PCA953X_TYPE. In order to keep the logic
> simple, swap over the if and else portions where checks are made against
> PCA953X_TYPE and instead check for PCA957X_TYPE.
>
> Signed-off-by: Martyn Welch <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2022-08-31 21:18:01

by Andy Shevchenko

Subject: Re: [PATCH 4/5] gpio: pca953x: Swap if statements to save later complexity

On Mon, Aug 29, 2022 at 4:43 PM Martyn Welch <[email protected]> wrote:
>
> A later patch in the series adds support for a further chip type that
> shares some similarity with the PCA953X_TYPE. In order to keep the logic
> simple,

If you say so...
Reviewed-by: Andy Shevchenko <[email protected]>

> swap over the if and else portions where checks are made against
> PCA953X_TYPE and instead check for PCA957X_TYPE.
>
> Signed-off-by: Martyn Welch <[email protected]>
> ---
> drivers/gpio/gpio-pca953x.c | 31 +++++++++++++++----------------
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index 41e7ff83a735..19a8eb94a629 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -293,13 +293,13 @@ static bool pca953x_readable_register(struct device *dev, unsigned int reg)
> struct pca953x_chip *chip = dev_get_drvdata(dev);
> u32 bank;
>
> - if (PCA_CHIP_TYPE(chip->driver_data) == PCA953X_TYPE) {
> - bank = PCA953x_BANK_INPUT | PCA953x_BANK_OUTPUT |
> - PCA953x_BANK_POLARITY | PCA953x_BANK_CONFIG;
> - } else {
> + if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
> bank = PCA957x_BANK_INPUT | PCA957x_BANK_OUTPUT |
> PCA957x_BANK_POLARITY | PCA957x_BANK_CONFIG |
> PCA957x_BANK_BUSHOLD;
> + } else {
> + bank = PCA953x_BANK_INPUT | PCA953x_BANK_OUTPUT |
> + PCA953x_BANK_POLARITY | PCA953x_BANK_CONFIG;
> }
>
> if (chip->driver_data & PCA_PCAL) {
> @@ -316,12 +316,12 @@ static bool pca953x_writeable_register(struct device *dev, unsigned int reg)
> struct pca953x_chip *chip = dev_get_drvdata(dev);
> u32 bank;
>
> - if (PCA_CHIP_TYPE(chip->driver_data) == PCA953X_TYPE) {
> - bank = PCA953x_BANK_OUTPUT | PCA953x_BANK_POLARITY |
> - PCA953x_BANK_CONFIG;
> - } else {
> + if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
> bank = PCA957x_BANK_OUTPUT | PCA957x_BANK_POLARITY |
> PCA957x_BANK_CONFIG | PCA957x_BANK_BUSHOLD;
> + } else {
> + bank = PCA953x_BANK_OUTPUT | PCA953x_BANK_POLARITY |
> + PCA953x_BANK_CONFIG;
> }
>
> if (chip->driver_data & PCA_PCAL)
> @@ -336,10 +336,10 @@ static bool pca953x_volatile_register(struct device *dev, unsigned int reg)
> struct pca953x_chip *chip = dev_get_drvdata(dev);
> u32 bank;
>
> - if (PCA_CHIP_TYPE(chip->driver_data) == PCA953X_TYPE)
> - bank = PCA953x_BANK_INPUT;
> - else
> + if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE)
> bank = PCA957x_BANK_INPUT;
> + else
> + bank = PCA953x_BANK_INPUT;
>
> if (chip->driver_data & PCA_PCAL)
> bank |= PCAL9xxx_BANK_IRQ_STAT;
> @@ -1069,13 +1069,12 @@ 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.
> */
> -
> - if (PCA_CHIP_TYPE(chip->driver_data) == PCA953X_TYPE) {
> - chip->regs = &pca953x_regs;
> - ret = device_pca95xx_init(chip, invert);
> - } else {
> + if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
> chip->regs = &pca957x_regs;
> ret = device_pca957x_init(chip, invert);
> + } else {
> + chip->regs = &pca953x_regs;
> + ret = device_pca95xx_init(chip, invert);
> }
> if (ret)
> goto err_exit;
> --
> 2.35.1
>


--
With Best Regards,
Andy Shevchenko