Heiko, can you please look at this patch.

On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen [off-list ref] wrote:

From: chenjh <redacted>

Full name please.

RK805 has two configurable GPIOs that can be used for several
purposes. These are output only.

This driver is generic for other Rockchip PMICs to be added.

Signed-off-by: chenjh <redacted>

Dito.

Your commit message says they are output-only, yet you implement
.direction_input(). So what is is going to be?

+#include <linux/i2c.h>
+#include <linux/gpio.h>

Only use:
#include <linux/gpio/driver.h>

+/*
+ * @mode: supported modes for this gpio, i.e. OUTPUT_MODE, OUTPUT_MODE...

Are you saying this should be an enum or a set of flags?

+static int rk805_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+       int ret, val;
+       struct rk805_gpio *gpio = gpiochip_get_data(chip);
+
+       ret = regmap_read(gpio->rk808->regmap, gpio->pins[offset].reg, &val);
+       if (ret) {
+               dev_err(gpio->dev, "gpio%d not support output mode\n", offset);
+               return ret;
+       }
+
+       return (val & gpio->pins[offset].val_msk) ? 1 : 0;

Do this:

return !!(val & gpio->pins[offset].val_msk)

+static int rk805_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+       int ret;
+       struct rk805_gpio *gpio = gpiochip_get_data(chip);
+
+       /* switch to gpio mode */
+       if (gpio->pins[offset].func_mask) {
+               ret = regmap_update_bits(gpio->rk808->regmap,
+                                        gpio->pins[offset].reg,
+                                        gpio->pins[offset].func_mask,
+                                        gpio->pins[offset].func_mask);
+               if (ret) {
+                       dev_err(gpio->dev, "set gpio%d func failed\n", offset);
+                       return ret;
+               }
+       }
+
+       return 0;
+}

This is pin control. Why don't you implement a proper pin control
driver for this chip?

If you don't, this will just come back and haunt you.

Why not merge the driver into drivers/pinctrl/* and name it
pinctrl-rk805.c to begin with?

+static const struct gpio_chip rk805_chip = {
+       .label                  = "rk805-gpio",
+       .owner                  = THIS_MODULE,
+       .direction_input        = rk805_gpio_direction_input,
+       .direction_output       = rk805_gpio_direction_output,

Please implement .get_direction()

+       .get                    = rk805_gpio_get,
+       .set                    = rk805_gpio_set,
+       .request                = rk805_gpio_request,
+       .base                   = -1,
+       .ngpio                  = 2,
+       .can_sleep              = true,

Consider assigning the .names[] array some pin names.

Yours,
Linus Walleij