On Fri, Sep 14, 2012 at 3:30 PM, Ashish Jangam <ashish.jangam@xxxxxxxxxxxxxxx> wrote: > This is the GPIO patch for the DA9055 PMIC. This patch has got > dependency on the DA9055 MFD core. > > This patch is functionally tested on SMDK6410 board. > > Signed-off-by: David Dajun Chen <dchen@xxxxxxxxxxx> > Signed-off-by: Ashish Jangam <ashish.jangam@xxxxxxxxxxxxxxx> This looks OK Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx> But I want Marks comment on this, for example: > +static int da9055_gpio_get(struct gpio_chip *gc, unsigned offset) > +{ > + struct da9055_gpio *gpio = to_da9055_gpio(gc); > + int gpio_direction = 0; > + int ret; > + > + /* Get GPIO direction */ > + ret = da9055_reg_read(gpio->da9055, (offset >> 1) + DA9055_REG_GPIO0_1); > + if (ret < 0) > + return ret; So unique functions to read/write registers (as I'm used to). But the parent driver is using regmap, sand you seem not to use the struct da9055 for anything else than passing reads/writes, so isn't it simpler to just pass the struct regmap * and use update_bits etc directly and remove a layer of indirection? I'm very uncertain but Mark will know the proper design pattern. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html