On Mon, Aug 8, 2016 at 7:46 AM, Keerthy <j-keerthy@xxxxxx> wrote: > Add driver for lp873x PMIC family GPOs. Two GPOs are supported > and can be configured in Open-drain output or Push-pull output. > > Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx> > Signed-off-by: Keerthy <j-keerthy@xxxxxx> Still looks pretty nice, just thought of some things if you're anyway working on the code: > + return regmap_update_bits(gpio->lp873->regmap, LP873X_REG_GPO_CTRL, > + BIT(offset * 4), value ? BIT(offset * 4) : 0); (...) (...) > + return val & BIT(offset * 4); (...) > + regmap_update_bits(gpio->lp873->regmap, LP873X_REG_GPO_CTRL, > + BIT(offset * 4), value ? BIT(offset * 4) : 0); (...) > + return regmap_update_bits(gpio->lp873->regmap, > + LP873X_REG_GPO_CTRL, > + BIT(offset * 4 + 2), > + BIT(offset * 4 + 2)); > + case LINE_MODE_PUSH_PULL: > + return regmap_update_bits(gpio->lp873->regmap, > + LP873X_REG_GPO_CTRL, > + BIT(offset * 4 + 2), 0); This 4 +2 etc business is a bit hard to understand, could you create a macro with a clever name that makes it more understandable? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
