Re: [PATCH v3 4/5] pinctrl: add pinctrl/GPIO driver for Apple SoCs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux