On Mon, 2016-08-22 at 15:45 +0200, Linus Walleij wrote: > On Fri, Aug 19, 2016 at 2:44 PM, Andrew Jeffery <andrew@xxxxxxxx> wrote: > > > > > +++ b/drivers/pinctrl/aspeed/Kconfig > > @@ -0,0 +1,8 @@ > > +config PINCTRL_ASPEED > > + bool > > + depends on (ARCH_ASPEED || COMPILE_TEST) && OF > > + select PINMUX > > + select PINCONF > > + select GENERIC_PINCONF > > + select MFD_SYSCON > > + select REGMAP_MMIO > Since this device is spawn from the syscon, should it not be > "depends on MFD_SYSCON"? > > (No big deal, if you think this is right then go with it.) I think that's a fair point, I will look at rearranging it. > > > > > +#include > What is this include for? Cruft from past iterations. Will remove. > > > > > +#include > > +#include > > +#include > > +#include > > +#include "../core.h" > > +#include "pinctrl-aspeed.h" > No #include ? > > Maybe some #includes are centralized to pinctrl-aspeed.h > I don't know, just make sure you don't have implicit includes. linux/regmap.h is included in pinctrl-aspeed.h > > > > > + if (regmap_read(map, desc->reg, &val) < 0) > > + return false; > > + > > + val &= ~desc->mask; > > + val |= pattern << __ffs(desc->mask); > > + > > + if (regmap_write(map, desc->reg, val) < 0) > > + return false; > Use: > > regmap_update_bits(map, > desc->reg, > desc->mask, > pattern << __ffs->desc->mask); > > Or something like that instead of reimplementing > mask-and-set. Regmap already knows how to do the > business. > > (Applied everywhere in the driver where you have a > mask-and-set like this). Right, I will clean that up. > > The expression core engine is still a complete mystery > for me, I will just trust you that it works as intended. Gah! However, thanks! I will clean up the issues above and re-send. Cheers, Andrew
Attachment:
signature.asc
Description: This is a digitally signed message part
