On Sat, Oct 16, 2021 at 4:18 PM Joey Gouly <joey.gouly@xxxxxxx> wrote: > This driver adds support for the pinctrl / GPIO hardware found > on some Apple SoCs. > > Co-developed-by: Stan Skowronek <stan@xxxxxxxxxxxxx> > Signed-off-by: Stan Skowronek <stan@xxxxxxxxxxxxx> > Signed-off-by: Joey Gouly <joey.gouly@xxxxxxx> Hi Joey, this looks really good. I started to write a reply but noticed that all my comments are already covered by Marc Z so I scrapped it: do what Marc says. This is interesting: > +#define REG_GPIOx_PULL_OFF 0 > +#define REG_GPIOx_PULL_DOWN 1 > +#define REG_GPIOx_PULL_UP_STRONG 2 > +#define REG_GPIOx_PULL_UP 3 Pull-up strong! Nice that you found these details. We don't have a generic pinconf binding for that but possibly the bias-pull-up argument can be used if we know how many ohms each is (preferred). But this is no problem for the moment because pin config is for implementing later (I assume). I bet this is going to be important to get right as well as it usually affects the power dissipation in suspend-to-RAM. > +static int apple_gpio_pinmux_enable(struct pinctrl_dev *pctldev, unsigned func, > + unsigned group) (...) > + .set_mux = apple_gpio_pinmux_enable, What about just naming it apple_gpio_pinmux_set()? Next iteration should be good to go I guess! Yours, Linus Walleij
