linux-kernel - Re: [PATCH 1/2] pinctrl: mediatek: fix direction control issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACRpkdb1pVQY0UG-_Z5ivyG6rPZq5YibYcRhzdnDMghfH8jMjA@mail.gmail.com>
Date:	Tue, 16 Feb 2016 13:28:52 +0100
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Yingjoe Chen <yingjoe.chen@...iatek.com>
Cc:	Hongzhou Yang <hongzhou.yang@...iatek.com>,
	Sherman Yin <syin@...adcom.com>,
	Biao Huang <biao.huang@...iatek.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"moderated list:ARM/Mediatek SoC support" 
	<linux-mediatek@...ts.infradead.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	srv_heupstream@...iatek.com, Eddie Huang <eddie.huang@...iatek.com>
Subject: Re: [PATCH 1/2] pinctrl: mediatek: fix direction control issue

On Tue, Feb 16, 2016 at 9:54 AM, Yingjoe Chen <yingjoe.chen@...iatek.com> wrote:

> To make it more clear, for a GPIO pin to get input on MTK chip, we must:
>
> - Enable input for the pad (input-enable)

This should be done by the pin control portions of the driver in
response to the PIN_CONFIG_INPUT_ENABLE generic pin
config setting.

Possibly *also* by just writing the hardware registers
from the callbacks right below here:

> - Set mux to GPIO mode

This should be done by just writing the hardware registers
in these callbacks from struct pinmux_ops:

        int (*gpio_request_enable) (struct pinctrl_dev *pctldev,
                                    struct pinctrl_gpio_range *range,
                                    unsigned offset);
        void (*gpio_disable_free) (struct pinctrl_dev *pctldev,
                                   struct pinctrl_gpio_range *range,
                                   unsigned offset);
        int (*gpio_set_direction) (struct pinctrl_dev *pctldev,
                                   struct pinctrl_gpio_range *range,
                                   unsigned offset,
                                   bool input);

In respons to these functions being called from the corresponding
GPIO driver:

extern int pinctrl_request_gpio(unsigned gpio);
extern void pinctrl_free_gpio(unsigned gpio);
extern int pinctrl_gpio_direction_input(unsigned gpio);
extern int pinctrl_gpio_direction_output(unsigned gpio);

> - Set GPIO direction to input

This should be done in these callbacks from struct gpio_chip:

        int                     (*direction_input)(struct gpio_chip *chip,
                                                unsigned offset);
        int                     (*direction_output)(struct gpio_chip *chip,
                                                unsigned offset, int value);

By just writing the hardware registers.

> Currently user need to add a dts node to set pinmux to GPIO mode with
> input-enable property, the driver need to call gpio_direction_input to
> set direction.

It seems like:

- Your GPIO driver is not calling the pinctrl_request_gpio/
  pinctrl_gpio_direction_* etc.

- Your pinmux driver is not implementing the GPIO-specifif
  .gpio_* functions

> Since input-enable and GPIO direction are 2 different things, we now
> think this patch is inappropriate. Instead, user should not required to
> add pinmux node in dts if all he want is use it as plain GPIO input.

If you sort out the above callbacks (also read
Documentation/pinctrl.txt about GPIO) I think it is
clear what you need to do.

Usually you should only need a special pinmux/pinconf node
for a GPIO line if you need to specify pull-up or similar electronic
things for that GPIO.

Yours,
Linus Walleij