Re: Enable buttons GPIO passthrough

[Linus Walleij <linus.walleij@linaro.org>]

Hi Kwin!

On Tue, Jan 22, 2019 at 11:39 AM Wang, Kuiying <kuiying.wang@intel.com> wrote:
>
> Hi Linus,
> Let me attach a draft patch, it will be easy to understand.
>
> if someone wanna enable passthrough just need to override like " gpio->chip.direction_passthrough = aspeed_gpio_dir_passthrough;"
> else passthrough is disabled.
>
> In app level, just need to echo "passthrough" to enable passthrough like "echo passthrough > /sys/class/gpio/gpio35/direction"

Sorry, but the sysfs ABI is deprecated since over three years
and we will not add any new interesting features to it. Most
major distributions don't even compile it in to the kernel
anymore.

Your only option to control anything like this from userspace would
be through the new gpio character device ABI, see examples
in tools/gpio to see how to use this new ABI.

>  static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>  {
>         struct aspeed_gpio *gpio = gpiochip_get_data(gc);
> @@ -1188,6 +1215,7 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
>         gpio->chip.parent = &pdev->dev;
>         gpio->chip.direction_input = aspeed_gpio_dir_in;
>         gpio->chip.direction_output = aspeed_gpio_dir_out;
> +       gpio->chip.direction_passthrough = aspeed_gpio_dir_passthrough;

As stated earlier, do not add new callbacks for this. Use the
existing .set_config, extend generic config options with
a passthrough attribute.

> +       else if (sysfs_streq(buf, "passthrough"))
> +               status = gpiod_direction_passthrough(desc);

NACK on this sorry. This is the wrong design approach.

The Aspeed GPIO driver already contains a .set_config
callback. Use this.

It is this function:

static int aspeed_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
                                  unsigned long config)
{
        unsigned long param = pinconf_to_config_param(config);
        u32 arg = pinconf_to_config_argument(config);

        if (param == PIN_CONFIG_INPUT_DEBOUNCE)
                return set_debounce(chip, offset, arg);
        else if (param == PIN_CONFIG_BIAS_DISABLE ||
                        param == PIN_CONFIG_BIAS_PULL_DOWN ||
                        param == PIN_CONFIG_DRIVE_STRENGTH)
                return pinctrl_gpio_set_config(offset, config);
        else if (param == PIN_CONFIG_DRIVE_OPEN_DRAIN ||
                        param == PIN_CONFIG_DRIVE_OPEN_SOURCE)
                /* Return -ENOTSUPP to trigger emulation, as per datasheet */
                return -ENOTSUPP;
        else if (param == PIN_CONFIG_PERSIST_STATE)
                return aspeed_gpio_reset_tolerance(chip, offset, arg);

Here add

else if (param == PIN_CONFIG_PASSTHROUGH)
   ....

And work from there.

Yours,
Linus Walleij