RE: [PATCH v3 2/3] gpio: Add support for TPS68470 GPIOs

From: Mani, Rajmohan
Date: Tue Jun 20 2017 - 18:17:07 EST


Hi Linus,

Thanks for your comments.

> On Mon, Jun 12, 2017 at 11:26 AM, Rajmohan Mani
> <rajmohan.mani@xxxxxxxxx> wrote:
>
> > This patch adds support for TPS68470 GPIOs.
> > There are 7 GPIOs and a few sensor related GPIOs.
> > These GPIOs can be requested and configured as appropriate.
> >
> > Signed-off-by: Rajmohan Mani <rajmohan.mani@xxxxxxxxx>
>
> This is some fine code.
> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
>
> Feel free to merge this through MFD or tell me if I should merge it if there are
> no compile-time dependencies.

I still have the following 3 outstanding comments from Andy (that includes one from Lee) on this patch series.

Main points (some I already told in an answer to Sakari's mail):

1. Consider 2 GPIO chips over 1.(Andy on GPIO driver)
I will look into this week and get back on the issue that I find with using 2 GPIO chips.

2. Fix FIXME(s) (Andy and Lee on MFD driver)
I will work on this once I am done with 1.

+ /* FIXME: configure these dynamically */
+ /* Enable Daisy Chain LDO and configure relevant GPIOs as output */
+ ret = regmap_write(regmap, TPS68470_REG_S_I2C_CTL, 2);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, TPS68470_REG_GPCTL4A, 2);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, TPS68470_REG_GPCTL5A, 2);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, TPS68470_REG_GPCTL6A, 2);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * When SDA and SCL are routed to GPIO1 and GPIO2, the mode
+ * for these GPIOs must be configured using their respective
+ * GPCTLxA registers as inputs with no pull-ups.
+ */
+ ret = regmap_write(regmap, TPS68470_REG_GPCTL1A, 0);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, TPS68470_REG_GPCTL2A, 0);
+ if (ret < 0)
+ return ret;
+
+ /* Enable daisy chain */
+ ret = regmap_update_bits(regmap, TPS68470_REG_S_I2C_CTL, 1, 1);
+ if (ret < 0)
+ return ret;

3. Move <include/i2c.h> from tps68470.h to tps68470.c (Andy on MFD driver)
This is done, but waiting on 1 and 2

Thanks
Raj