| Message ID | 959CAFA1E282D14FB901BE9A7BF4E7724E41D8EE@shsmsx102.ccr.corp.intel.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Enable buttons GPIO passthrough | expand |
On Tue, 11 Dec 2018 at 18:32, Wang, Kuiying <kuiying.wang@intel.com> wrote: > > Hi Joel/Andrew, > > I write a drive to enable GPIO passthrough for buttons (like power/reset/id button) as following attached patch. > > Do you think it is acceptable? > > Or we could do it in pinmux and extend gpio driver? Design passthrough state except in/out. I think that this direction would be better than a misc driver. I've added Linus, the maintainer for these subsystems, and the linux-gpio mailing list to cc. Cheers, Joel > > What’s your suggestions? > > > > Thanks Kwin. > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > index f2062546250c..e94ee86820d3 100644 > > --- a/drivers/misc/Kconfig > > +++ b/drivers/misc/Kconfig > > @@ -4,6 +4,12 @@ > > menu "Misc devices" > > +config GPIO_PASS_THROUGH > > + tristate "GPIO Pass Through" > > + depends on (ARCH_ASPEED || COMPILE_TEST) > > + help > > + Enable this for buttons GPIO pass through. > > + > > config SENSORS_LIS3LV02D > > tristate > > depends on INPUT > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > > index bb89694e6b4b..13b8b8edbb70 100644 > > --- a/drivers/misc/Makefile > > +++ b/drivers/misc/Makefile > > @@ -61,3 +61,4 @@ obj-$(CONFIG_ASPEED_LPC_SIO) += aspeed-lpc-sio.o > > obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o > > obj-$(CONFIG_OCXL) += ocxl/ > > obj-$(CONFIG_MISC_RTSX) += cardreader/ > > +obj-$(CONFIG_GPIO_PASS_THROUGH) += gpio-passthrough.o > > diff --git a/drivers/misc/gpio-passthrough.c b/drivers/misc/gpio-passthrough.c > > new file mode 100644 > > index 000000000000..0126fc08ae55 > > --- /dev/null > > +++ b/drivers/misc/gpio-passthrough.c > > @@ -0,0 +1,260 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2018 Intel Corporation > > +*/ > > + > > +#include "gpio-passthrough.h" > > + > > +struct aspeed_gpio_pass_through_dev { > > + struct miscdevice miscdev; > > + unsigned int addr; > > + unsigned int size; > > +}; > > + > > +static struct aspeed_gpio_pass_through_dev ast_cdev_gpio_pass_through; > > + > > +static long ast_passthru_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > > +{ > > + long ret = 0; > > + struct passthru_ioctl_data passthru_data; > > + > > + if (cmd == GPIO_IOC_PASSTHRU) > > + { > > + if (copy_from_user(&passthru_data, > > + (void __user*)arg, sizeof(passthru_data))) > > + return -EFAULT; > > + if (passthru_data.idx >= GPIO_PASSTHRU_MAX) > > + return -EINVAL; > > + > > + switch (passthru_data.cmd) { > > + case SET_GPIO_PASSTHRU_ENABLE: > > + ast_set_passthru_enable(passthru_data.idx, > > + passthru_data.data); > > + break; > > + case GET_GPIO_PASSTHRU_ENABLE: > > + passthru_data.data = ast_get_passthru_enable(passthru_data.idx); > > + if (copy_to_user((void __user*)arg, &passthru_data, > > + sizeof(passthru_data))) > > + ret = -EFAULT; > > + break; > > + > > + case SET_GPIO_PASSTHRU_OUT: > > + ast_set_passthru_out(passthru_data.idx, passthru_data.data); > > + break; > > + > > + default: > > + ret = -EINVAL; > > + break; > > + } > > + } > > + return ret; > > + > > +} > > + > > +static int ast_passthru_open(struct inode *inode, struct file *filp) > > +{ > > + return container_of(filp->private_data, > > + struct aspeed_gpio_pass_through_dev, miscdev); > > +} > > + > > +static const struct file_operations ast_gpio_pth_fops = { > > + .owner = THIS_MODULE, > > + .llseek = no_llseek, > > + .unlocked_ioctl = ast_passthru_ioctl, > > + .open = ast_passthru_open, > > +}; > > + > > +static struct miscdevice ast_gpio_pth_miscdev = { > > + .minor = MISC_DYNAMIC_MINOR, > > + .name = GPIO_PASS_THROUGH_NAME, > > + .fops = &ast_gpio_pth_fops, > > +}; > > + > > +static u32 ast_scu_base = IO_ADDRESS(AST_SCU_BASE); > > + > > +static inline u32 ast_scu_read(u32 reg) > > +{ > > + return readl((void *)(ast_scu_base + reg)); > > +} > > + > > +static inline void ast_scu_write(u32 val, u32 reg) > > +{ > > +#ifdef CONFIG_AST_SCU_LOCK > > + writel(SCU_PROTECT_UNLOCK, (void *)(ast_scu_base + AST_SCU_PROTECT)); > > + writel(val, (void *)(ast_scu_base + reg)); > > + writel(0x000000AA, (void *)(ast_scu_base + AST_SCU_PROTECT)); > > +#else > > + writel(SCU_PROTECT_UNLOCK, (void *)(ast_scu_base + AST_SCU_PROTECT)); > > + writel(val, (void *)(ast_scu_base + reg)); > > +#endif > > +} > > + > > +static int gpio_pass_through_probe(struct platform_device *pdev) > > +{ > > + struct aspeed_gpio_pass_through_dev *gpio_pth_dev = &ast_cdev_gpio_pass_through; > > + struct device *dev = &pdev->dev; > > + struct resource *rc; > > + > > + dev_set_drvdata(&pdev->dev, gpio_pth_dev); > > + rc = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!rc) { > > + dev_err(dev, "Fail to platform_get_resource\n"); > > + return -ENXIO; > > + } > > + gpio_pth_dev->addr = rc->start; > > + gpio_pth_dev->size = resource_size(rc); > > + gpio_pth_dev->miscdev = ast_gpio_pth_miscdev; > > + ast_passthru_init(); > > + printk("GPIO PASS THROUGH DRIVER is loaded \n"); > > + return misc_register(&gpio_pth_dev->miscdev); > > +} > > + > > +static int gpio_pass_through_remove(struct platform_device *pdev) > > +{ > > + struct aspeed_gpio_pass_through_dev *gpio_pth_dev = > > + dev_get_drvdata(&pdev->dev); > > + misc_deregister(&gpio_pth_dev->miscdev); > > + printk("GPIO PASS THROUGH DRIVER is removing \n"); > > + > > + return 0; > > +} > > + > > +static struct platform_driver gpio_pass_through_driver = { > > + .probe = gpio_pass_through_probe, > > + .remove = gpio_pass_through_remove, > > + .driver = { > > + .name = "gpio-pass-through", > > + .owner = THIS_MODULE, > > + > > + }, > > +}; > > + > > +/* GPIOE group only */ > > +struct gpio_passthru { > > + u32 passthru_mask; > > + u16 pin_in; > > + u16 pin_out; > > +}; > > + > > +static struct gpio_passthru passthru_settings[GPIO_PASSTHRU_MAX] = { > > + [GPIO_PASSTHRU0] = { > > + .passthru_mask = (1 << 12), /* SCU8C[12] */ > > + .pin_in = PGPIO_PIN(GPIOE, 0), > > + .pin_out = PGPIO_PIN(GPIOE, 1), > > + }, > > + > > + [GPIO_PASSTHRU1] = { > > + .passthru_mask = (1 << 13), /* SCU8C[13] */ > > + .pin_in = PGPIO_PIN(GPIOE, 2), > > + .pin_out = PGPIO_PIN(GPIOE, 3), > > + }, > > + > > + [GPIO_PASSTHRU2] = { > > + .passthru_mask = (1 << 14), /* SCU8C[14] */ > > + .pin_in = PGPIO_PIN(GPIOE, 4), > > + .pin_out = PGPIO_PIN(GPIOE, 5), > > + }, > > + > > + [GPIO_PASSTHRU3] = { > > + .passthru_mask = (1 << 15), /* SCU8C[15] */ > > + .pin_in = PGPIO_PIN(GPIOE, 6), > > + .pin_out = PGPIO_PIN(GPIOE, 7), > > + }, > > +}; > > + > > +static void ast_set_passthru_enable( > > + unsigned short idx, unsigned int enable) > > +{ > > + u32 val; > > + unsigned long flags; > > + struct gpio_passthru *passthru = &passthru_settings[idx]; > > + > > + local_irq_save(flags); > > + > > + val = ast_scu_read(AST_SCU_FUN_PIN_CTRL4); > > + if (enable) > > + val |= (passthru->passthru_mask); > > + else > > + val &= ~(passthru->passthru_mask); > > + ast_scu_write(val, AST_SCU_FUN_PIN_CTRL4); > > + > > + local_irq_restore(flags); > > +} > > + > > +static unsigned int ast_get_passthru_enable(unsigned short idx) > > +{ > > + unsigned int enable; > > + unsigned long flags; > > + struct gpio_passthru *passthru = &passthru_settings[idx]; > > + > > + local_irq_save(flags); > > + > > + enable = (ast_scu_read(AST_SCU_FUN_PIN_CTRL4) & passthru->passthru_mask) != 0 ? 1 : 0; > > + > > + local_irq_restore(flags); > > + > > + return enable; > > +} > > + > > +static void ast_set_passthru_out( > > + unsigned short idx, unsigned int val) > > +{ > > + unsigned long flags; > > + struct gpio_passthru *passthru = &passthru_settings[idx]; > > + > > + local_irq_save(flags); > > + > > + /* Disable PASSTHRU */ > > + val = ast_scu_read(AST_SCU_FUN_PIN_CTRL4); > > + val &= ~(passthru->passthru_mask); > > + ast_scu_write(val, AST_SCU_FUN_PIN_CTRL4); > > + > > + local_irq_restore(flags); > > +} > > + > > +static void ast_passthru_init(void) > > +{ > > + int i; > > + u32 val; > > + unsigned long flags; > > + struct gpio_passthru *passthru; > > + > > + local_irq_save(flags); > > + > > + /* 1. Enable GPIOE pin mode, SCU80[16:23] = 00 */ > > + ast_scu_write(ast_scu_read(AST_SCU_FUN_PIN_CTRL1) & (~0x00FF0000), > > + AST_SCU_FUN_PIN_CTRL1); > > + > > + /* 2. Enable them by setting SCU8C[12:15] */ > > + for (i = 0; i < GPIO_PASSTHRU_MAX; i++) { > > + passthru = &passthru_settings[i]; > > + > > + val = ast_scu_read(AST_SCU_FUN_PIN_CTRL4); > > + val |= passthru->passthru_mask; > > + ast_scu_write(val, AST_SCU_FUN_PIN_CTRL4); > > + } > > + > > + /************************************************************** > > + * 3. Disable HWTrap for GPIOE pass-through mode > > + * > > + * Hardware strap register (SCU70) programming method. > > + * #Write '1' to SCU70 can set the specific bit with value '1' > > + * Write '0' has no effect. > > + * #Write '1' to SCU7C can clear the specific bit of SCU70 to > > + * value '0'. Write '0' has no effect. > > + **************************************************************/ > > + if (ast_scu_read(AST_SCU_HW_STRAP1) & (0x1 << 22)) > > + ast_scu_write((0x1 << 22), AST_SCU_REVISION_ID); > > + > > + local_irq_restore(flags); > > + > > + printk("HW_STRAP1 = 0x%08X\n", ast_scu_read(AST_SCU_HW_STRAP1)); > > +} > > + > > +module_platform_driver(gpio_pass_through_driver); > > + > > +MODULE_AUTHOR("Kuiying Wang <kuiying.wang@intel.com>"); > > +MODULE_DESCRIPTION("GPIO Pass Through Control Driver for all buttons like Power/Reset/ID button"); > > +MODULE_LICENSE("GPL v2"); > > + > > diff --git a/drivers/misc/gpio-passthrough.h b/drivers/misc/gpio-passthrough.h > > new file mode 100644 > > index 000000000000..a7274b8ab31e > > --- /dev/null > > +++ b/drivers/misc/gpio-passthrough.h > > @@ -0,0 +1,60 @@ > > +#ifndef __GPIO_PASS_THROUGH_H__ > > +#define __GPIO_PASS_THROUGH_H__ > > + > > +#include <linux/kernel.h> > > +#include <linux/miscdevice.h> > > +#include <linux/uaccess.h> > > +#include <linux/module.h> > > +#include <linux/of_platform.h> > > +#include <linux/mm.h> > > +#include <asm/io.h> > > + > > +#define PGPIO_PIN(PORT, PIN) (((PORT) << 3) | ((PIN) & 0x07)) > > +#define GPIOE 4 > > +#define AST_SCU_BASE 0x1E6E2000 /* SCU */ > > +#define AST_SCU_PROTECT 0x00 /* protection key register */ > > +#define SCU_PROTECT_UNLOCK 0x1688A8A8 > > +#define AST_SCU_FUN_PIN_CTRL4 0x8C /* Multi-function Pin Control#4*/ > > +#define AST_SCU_FUN_PIN_CTRL1 0x80 /* Multi-function Pin Control#1*/ > > +#define AST_SCU_HW_STRAP1 0x70 /* hardware strapping register */ > > +#define AST_SCU_REVISION_ID 0x7C /* Silicon revision ID register */ > > +#define GPIO_PASS_THROUGH_NAME "gpiopassthrough" > > +#define IO_ADDRESS(x) (x) > > + > > +enum GPIO_PASSTHRU_INDEX { > > + GPIO_PASSTHRU0 = 0, /* GPIOE0 -> GPIOE1 */ > > + GPIO_PASSTHRU1, /* GPIOE2 -> GPIOE3 */ > > + GPIO_PASSTHRU2, /* GPIOE4 -> GPIOE5 */ > > + GPIO_PASSTHRU3, /* GPIOE6 -> GPIOE7 */ > > + > > + GPIO_PASSTHRU_MAX > > +}; > > + > > +enum GPIO_PASSTHRU_CMD { > > + SET_GPIO_PASSTHRU_ENABLE = 0, > > + GET_GPIO_PASSTHRU_ENABLE, > > + GET_GPIO_PASSTHRU_IN, > > + SET_GPIO_PASSTHRU_OUT, /* !!! The PASSTHRU will be disabled !!! */ > > +}; > > + > > +struct passthru_ioctl_data { > > + unsigned short cmd; > > + unsigned short idx; > > + unsigned int data; > > +}; > > + > > +static void ast_set_passthru_enable( > > + unsigned short idx, unsigned int enable); > > +static unsigned int ast_get_passthru_enable(unsigned short idx); > > +static void ast_set_passthru_out( > > + unsigned short idx, unsigned int val); > > +static void ast_passthru_init(void); > > +static inline u32 ast_scu_read(u32 reg); > > +static inline void ast_scu_write(u32 val, u32 reg); > > + > > +/* IOCTL */ > > +#define GPIO_IOC_BASE 'G' > > +#define GPIO_IOC_PASSTHRU _IOWR(GPIO_IOC_BASE, 1, struct passthru_ioctl_data) > > + > > + > > +#endif > > -- > > 2.16.2 > > > > Thanks, > > Kwin. > >
Hi Linus, Who could help and work w/ me on this? Thanks, Kwin. -----Original Message----- From: Joel Stanley [mailto:joel@jms.id.au] Sent: Thursday, December 13, 2018 9:21 AM To: Wang, Kuiying <kuiying.wang@intel.com>; Linus Walleij <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; Andrew Jeffery <andrew@aj.id.au> Cc: Andrew Geissler <geissonator@gmail.com>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Mauery, Vernon <vernon.mauery@intel.com>; Feist, James <james.feist@intel.com>; Yoo, Jae Hyun <jae.hyun.yoo@intel.com> Subject: Re: Enable buttons GPIO passthrough On Tue, 11 Dec 2018 at 18:32, Wang, Kuiying <kuiying.wang@intel.com> wrote: > > Hi Joel/Andrew, > > I write a drive to enable GPIO passthrough for buttons (like power/reset/id button) as following attached patch. > > Do you think it is acceptable? > > Or we could do it in pinmux and extend gpio driver? Design passthrough state except in/out. I think that this direction would be better than a misc driver. I've added Linus, the maintainer for these subsystems, and the linux-gpio mailing list to cc. Cheers, Joel > > What’s your suggestions? > > > > Thanks Kwin. > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > index f2062546250c..e94ee86820d3 100644 > > --- a/drivers/misc/Kconfig > > +++ b/drivers/misc/Kconfig > > @@ -4,6 +4,12 @@ > > menu "Misc devices" > > +config GPIO_PASS_THROUGH > > + tristate "GPIO Pass Through" > > + depends on (ARCH_ASPEED || COMPILE_TEST) > > + help > > + Enable this for buttons GPIO pass through. > > + > > config SENSORS_LIS3LV02D > > tristate > > depends on INPUT > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > > index bb89694e6b4b..13b8b8edbb70 100644 > > --- a/drivers/misc/Makefile > > +++ b/drivers/misc/Makefile > > @@ -61,3 +61,4 @@ obj-$(CONFIG_ASPEED_LPC_SIO) += aspeed-lpc-sio.o > > obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o > > obj-$(CONFIG_OCXL) += ocxl/ > > obj-$(CONFIG_MISC_RTSX) += cardreader/ > > +obj-$(CONFIG_GPIO_PASS_THROUGH) += gpio-passthrough.o > > diff --git a/drivers/misc/gpio-passthrough.c > b/drivers/misc/gpio-passthrough.c > > new file mode 100644 > > index 000000000000..0126fc08ae55 > > --- /dev/null > > +++ b/drivers/misc/gpio-passthrough.c > > @@ -0,0 +1,260 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2018 Intel Corporation > > +*/ > > + > > +#include "gpio-passthrough.h" > > + > > +struct aspeed_gpio_pass_through_dev { > > + struct miscdevice miscdev; > > + unsigned int addr; > > + unsigned int size; > > +}; > > + > > +static struct aspeed_gpio_pass_through_dev > +ast_cdev_gpio_pass_through; > > + > > +static long ast_passthru_ioctl(struct file *filp, unsigned int cmd, > +unsigned long arg) > > +{ > > + long ret = 0; > > + struct passthru_ioctl_data passthru_data; > > + > > + if (cmd == GPIO_IOC_PASSTHRU) > > + { > > + if (copy_from_user(&passthru_data, > > + (void __user*)arg, > + sizeof(passthru_data))) > > + return -EFAULT; > > + if (passthru_data.idx >= > + GPIO_PASSTHRU_MAX) > > + return -EINVAL; > > + > > + switch (passthru_data.cmd) { > > + case SET_GPIO_PASSTHRU_ENABLE: > > + > + ast_set_passthru_enable(passthru_data.idx, > > + > + passthru_data.data); > > + break; > > + case GET_GPIO_PASSTHRU_ENABLE: > > + passthru_data.data = > + ast_get_passthru_enable(passthru_data.idx); > > + if (copy_to_user((void > + __user*)arg, &passthru_data, > > + > + sizeof(passthru_data))) > > + ret = -EFAULT; > > + break; > > + > > + case SET_GPIO_PASSTHRU_OUT: > > + > + ast_set_passthru_out(passthru_data.idx, passthru_data.data); > > + break; > > + > > + default: > > + ret = -EINVAL; > > + break; > > + } > > + } > > + return ret; > > + > > +} > > + > > +static int ast_passthru_open(struct inode *inode, struct file *filp) > > +{ > > + return container_of(filp->private_data, > > + struct aspeed_gpio_pass_through_dev, > + miscdev); > > +} > > + > > +static const struct file_operations ast_gpio_pth_fops = { > > + .owner = THIS_MODULE, > > + .llseek = no_llseek, > > + .unlocked_ioctl = ast_passthru_ioctl, > > + .open = ast_passthru_open, > > +}; > > + > > +static struct miscdevice ast_gpio_pth_miscdev = { > > + .minor = MISC_DYNAMIC_MINOR, > > + .name = GPIO_PASS_THROUGH_NAME, > > + .fops = &ast_gpio_pth_fops, > > +}; > > + > > +static u32 ast_scu_base = IO_ADDRESS(AST_SCU_BASE); > > + > > +static inline u32 ast_scu_read(u32 reg) > > +{ > > + return readl((void *)(ast_scu_base + reg)); > > +} > > + > > +static inline void ast_scu_write(u32 val, u32 reg) > > +{ > > +#ifdef CONFIG_AST_SCU_LOCK > > + writel(SCU_PROTECT_UNLOCK, (void *)(ast_scu_base + > + AST_SCU_PROTECT)); > > + writel(val, (void *)(ast_scu_base + reg)); > > + writel(0x000000AA, (void *)(ast_scu_base + AST_SCU_PROTECT)); > > +#else > > + writel(SCU_PROTECT_UNLOCK, (void *)(ast_scu_base + > + AST_SCU_PROTECT)); > > + writel(val, (void *)(ast_scu_base + reg)); > > +#endif > > +} > > + > > +static int gpio_pass_through_probe(struct platform_device *pdev) > > +{ > > + struct aspeed_gpio_pass_through_dev *gpio_pth_dev = &ast_cdev_gpio_pass_through; > > + struct device *dev = &pdev->dev; > > + struct resource *rc; > > + > > + dev_set_drvdata(&pdev->dev, gpio_pth_dev); > > + rc = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!rc) { > > + dev_err(dev, "Fail to > + platform_get_resource\n"); > > + return -ENXIO; > > + } > > + gpio_pth_dev->addr = rc->start; > > + gpio_pth_dev->size = resource_size(rc); > > + gpio_pth_dev->miscdev = ast_gpio_pth_miscdev; > > + ast_passthru_init(); > > + printk("GPIO PASS THROUGH DRIVER is loaded \n"); > > + return misc_register(&gpio_pth_dev->miscdev); > > +} > > + > > +static int gpio_pass_through_remove(struct platform_device *pdev) > > +{ > > + struct aspeed_gpio_pass_through_dev *gpio_pth_dev = > > + > + dev_get_drvdata(&pdev->dev); > > + misc_deregister(&gpio_pth_dev->miscdev); > > + printk("GPIO PASS THROUGH DRIVER is removing \n"); > > + > > + return 0; > > +} > > + > > +static struct platform_driver gpio_pass_through_driver = { > > + .probe = gpio_pass_through_probe, > > + .remove = gpio_pass_through_remove, > > + .driver = { > > + .name = "gpio-pass-through", > > + .owner = THIS_MODULE, > > + > > + }, > > +}; > > + > > +/* GPIOE group only */ > > +struct gpio_passthru { > > + u32 passthru_mask; > > + u16 pin_in; > > + u16 pin_out; > > +}; > > + > > +static struct gpio_passthru passthru_settings[GPIO_PASSTHRU_MAX] = { > > + [GPIO_PASSTHRU0] = { > > + .passthru_mask = (1 << > + 12), /* SCU8C[12] */ > > + .pin_in = PGPIO_PIN(GPIOE, 0), > > + .pin_out = PGPIO_PIN(GPIOE, 1), > > + }, > > + > > + [GPIO_PASSTHRU1] = { > > + .passthru_mask = (1 << > + 13), /* SCU8C[13] */ > > + .pin_in = PGPIO_PIN(GPIOE, 2), > > + .pin_out = PGPIO_PIN(GPIOE, 3), > > + }, > > + > > + [GPIO_PASSTHRU2] = { > > + .passthru_mask = (1 << > + 14), /* SCU8C[14] */ > > + .pin_in = PGPIO_PIN(GPIOE, 4), > > + .pin_out = PGPIO_PIN(GPIOE, 5), > > + }, > > + > > + [GPIO_PASSTHRU3] = { > > + .passthru_mask = (1 << > + 15), /* SCU8C[15] */ > > + .pin_in = PGPIO_PIN(GPIOE, 6), > > + .pin_out = PGPIO_PIN(GPIOE, 7), > > + }, > > +}; > > + > > +static void ast_set_passthru_enable( > > + unsigned short idx, > + unsigned int enable) > > +{ > > + u32 val; > > + unsigned long flags; > > + struct gpio_passthru *passthru = > + &passthru_settings[idx]; > > + > > + local_irq_save(flags); > > + > > + val = ast_scu_read(AST_SCU_FUN_PIN_CTRL4); > > + if (enable) > > + val |= (passthru->passthru_mask); > > + else > > + val &= ~(passthru->passthru_mask); > > + ast_scu_write(val, AST_SCU_FUN_PIN_CTRL4); > > + > > + local_irq_restore(flags); > > +} > > + > > +static unsigned int ast_get_passthru_enable(unsigned short idx) > > +{ > > + unsigned int enable; > > + unsigned long flags; > > + struct gpio_passthru *passthru = > + &passthru_settings[idx]; > > + > > + local_irq_save(flags); > > + > > + enable = (ast_scu_read(AST_SCU_FUN_PIN_CTRL4) & > + passthru->passthru_mask) != 0 ? 1 : 0; > > + > > + local_irq_restore(flags); > > + > > + return enable; > > +} > > + > > +static void ast_set_passthru_out( > > + unsigned short idx, > + unsigned int val) > > +{ > > + unsigned long flags; > > + struct gpio_passthru *passthru = > + &passthru_settings[idx]; > > + > > + local_irq_save(flags); > > + > > + /* Disable PASSTHRU */ > > + val = ast_scu_read(AST_SCU_FUN_PIN_CTRL4); > > + val &= ~(passthru->passthru_mask); > > + ast_scu_write(val, AST_SCU_FUN_PIN_CTRL4); > > + > > + local_irq_restore(flags); > > +} > > + > > +static void ast_passthru_init(void) > > +{ > > + int i; > > + u32 val; > > + unsigned long flags; > > + struct gpio_passthru *passthru; > > + > > + local_irq_save(flags); > > + > > + /* 1. Enable GPIOE pin mode, SCU80[16:23] = 00 */ > > + ast_scu_write(ast_scu_read(AST_SCU_FUN_PIN_CTRL1) & > + (~0x00FF0000), > > + AST_SCU_FUN_PIN_CTRL1); > > + > > + /* 2. Enable them by setting SCU8C[12:15] */ > > + for (i = 0; i < GPIO_PASSTHRU_MAX; i++) { > > + passthru = &passthru_settings[i]; > > + > > + val = > + ast_scu_read(AST_SCU_FUN_PIN_CTRL4); > > + val |= passthru->passthru_mask; > > + ast_scu_write(val, > + AST_SCU_FUN_PIN_CTRL4); > > + } > > + > > + > + /************************************************************** > > + * 3. Disable HWTrap for GPIOE pass-through mode > > + * > > + * Hardware strap register (SCU70) programming method. > > + * #Write '1' to SCU70 can set the specific bit with value '1' > > + * Write '0' has no effect. > > + * #Write '1' to SCU7C can clear the specific bit of SCU70 to > > + * value '0'. Write '0' has no effect. > > + > + **************************************************************/ > > + if (ast_scu_read(AST_SCU_HW_STRAP1) & (0x1 << 22)) > > + ast_scu_write((0x1 << 22), > + AST_SCU_REVISION_ID); > > + > > + local_irq_restore(flags); > > + > > + printk("HW_STRAP1 = 0x%08X\n", > + ast_scu_read(AST_SCU_HW_STRAP1)); > > +} > > + > > +module_platform_driver(gpio_pass_through_driver); > > + > > +MODULE_AUTHOR("Kuiying Wang <kuiying.wang@intel.com>"); > > +MODULE_DESCRIPTION("GPIO Pass Through Control Driver for all buttons > +like Power/Reset/ID button"); > > +MODULE_LICENSE("GPL v2"); > > + > > diff --git a/drivers/misc/gpio-passthrough.h > b/drivers/misc/gpio-passthrough.h > > new file mode 100644 > > index 000000000000..a7274b8ab31e > > --- /dev/null > > +++ b/drivers/misc/gpio-passthrough.h > > @@ -0,0 +1,60 @@ > > +#ifndef __GPIO_PASS_THROUGH_H__ > > +#define __GPIO_PASS_THROUGH_H__ > > + > > +#include <linux/kernel.h> > > +#include <linux/miscdevice.h> > > +#include <linux/uaccess.h> > > +#include <linux/module.h> > > +#include <linux/of_platform.h> > > +#include <linux/mm.h> > > +#include <asm/io.h> > > + > > +#define PGPIO_PIN(PORT, PIN) (((PORT) << 3) | ((PIN) & 0x07)) > > +#define GPIOE 4 > > +#define AST_SCU_BASE 0x1E6E2000 /* SCU */ > > +#define AST_SCU_PROTECT 0x00 /* protection key register */ > > +#define SCU_PROTECT_UNLOCK 0x1688A8A8 > > +#define AST_SCU_FUN_PIN_CTRL4 0x8C /* Multi-function Pin Control#4*/ > > +#define AST_SCU_FUN_PIN_CTRL1 0x80 /* Multi-function Pin Control#1*/ > > +#define AST_SCU_HW_STRAP1 0x70 /* hardware strapping register */ > > +#define AST_SCU_REVISION_ID 0x7C /* Silicon revision ID register */ > > +#define GPIO_PASS_THROUGH_NAME "gpiopassthrough" > > +#define IO_ADDRESS(x) (x) > > + > > +enum GPIO_PASSTHRU_INDEX { > > + GPIO_PASSTHRU0 = 0, /* GPIOE0 -> GPIOE1 */ > > + GPIO_PASSTHRU1, /* GPIOE2 -> GPIOE3 */ > > + GPIO_PASSTHRU2, /* GPIOE4 -> GPIOE5 */ > > + GPIO_PASSTHRU3, /* GPIOE6 -> GPIOE7 */ > > + > > + GPIO_PASSTHRU_MAX > > +}; > > + > > +enum GPIO_PASSTHRU_CMD { > > + SET_GPIO_PASSTHRU_ENABLE = 0, > > + GET_GPIO_PASSTHRU_ENABLE, > > + GET_GPIO_PASSTHRU_IN, > > + SET_GPIO_PASSTHRU_OUT, /* !!! The PASSTHRU will be > + disabled !!! */ > > +}; > > + > > +struct passthru_ioctl_data { > > + unsigned short cmd; > > + unsigned short idx; > > + unsigned int data; > > +}; > > + > > +static void ast_set_passthru_enable( > > + unsigned short idx, > + unsigned int enable); > > +static unsigned int ast_get_passthru_enable(unsigned short idx); > > +static void ast_set_passthru_out( > > + unsigned short idx, > + unsigned int val); > > +static void ast_passthru_init(void); > > +static inline u32 ast_scu_read(u32 reg); > > +static inline void ast_scu_write(u32 val, u32 reg); > > + > > +/* IOCTL */ > > +#define GPIO_IOC_BASE 'G' > > +#define GPIO_IOC_PASSTHRU _IOWR(GPIO_IOC_BASE, 1, struct passthru_ioctl_data) > > + > > + > > +#endif > > -- > > 2.16.2 > > > > Thanks, > > Kwin. > >
On Thu, Dec 13, 2018 at 2:21 AM Joel Stanley <joel@jms.id.au> wrote: > On Tue, 11 Dec 2018 at 18:32, Wang, Kuiying <kuiying.wang@intel.com> wrote: > > > > Hi Joel/Andrew, > > > > I write a drive to enable GPIO passthrough for buttons (like power/reset/id button) as > > following attached patch. > > > > Do you think it is acceptable? > > > > Or we could do it in pinmux and extend gpio driver? Design passthrough state > > except in/out. > > I think that this direction would be better than a misc driver. I've > added Linus, the maintainer for these subsystems, and the linux-gpio > mailing list to cc. I am sorry but I'm confused about this. It's a very terse explanation of the problem and a bunch of code. Can you folks please define what exactly you mean with a "GPIO passthrough" and what you are trying to achieve with this on these buttons? Yours, Linus Walleij
On Fri, 11 Jan 2019, at 19:31, Linus Walleij wrote: > On Thu, Dec 13, 2018 at 2:21 AM Joel Stanley <joel@jms.id.au> wrote: > > On Tue, 11 Dec 2018 at 18:32, Wang, Kuiying <kuiying.wang@intel.com> wrote: > > > > > > Hi Joel/Andrew, > > > > > > I write a drive to enable GPIO passthrough for buttons (like power/reset/id button) as > > > following attached patch. > > > > > > Do you think it is acceptable? > > > > > > Or we could do it in pinmux and extend gpio driver? Design passthrough state > > > except in/out. > > > > I think that this direction would be better than a misc driver. I've > > added Linus, the maintainer for these subsystems, and the linux-gpio > > mailing list to cc. > > I am sorry but I'm confused about this. It's a very terse explanation > of the problem and a bunch of code. > > Can you folks please define what exactly you mean with a > "GPIO passthrough" and what you are trying to achieve with > this on these buttons? On the Aspeed BMC SoCs, several GPIO banks (D and E) can be configured so the pins in the low half of a bank of 8 pins are input, and the pins in the high half of the bank are output. The line state of pins 0 <= i < 4 is reflected on pin i + 4. This way it appears as though the BMC is not present in the circuit. This is useful when the BMC may not have firmware of its own, or if it does, if the BMC firmware is not required on the critical path of the host's boot process. If or when the BMC firmware boots up, it may choose to break the pass-through behaviour in order to provide its own out-of-band management capabilities. This can be used for things like a box's power button as mentioned: If the BMC is not up, poke the host directly, otherwise, poke the BMC firmware and have it poke the host. The pass-through settings are tied to the SoC's pinmux. Intuitively it should be integrated into the pinmux driver, but IIRC there was some ugly coupling of the function that didn't have a neat solution with the currently-separate GPIO and pinmux drivers. Hope that helps. Andrew
On Mon, Jan 14, 2019 at 3:59 AM Andrew Jeffery <andrew@aj.id.au> wrote: > On Fri, 11 Jan 2019, at 19:31, Linus Walleij wrote: > > I am sorry but I'm confused about this. It's a very terse explanation > > of the problem and a bunch of code. > > > > Can you folks please define what exactly you mean with a > > "GPIO passthrough" and what you are trying to achieve with > > this on these buttons? > > On the Aspeed BMC SoCs, several GPIO banks (D and E) can be > configured so the pins in the low half of a bank of 8 pins are > input, and the pins in the high half of the bank are output. The > line state of pins 0 <= i < 4 is reflected on pin i + 4. This way it > appears as though the BMC is not present in the circuit. > > This is useful when the BMC may not have firmware of its own, > or if it does, if the BMC firmware is not required on the critical > path of the host's boot process. If or when the BMC firmware > boots up, it may choose to break the pass-through behaviour > in order to provide its own out-of-band management capabilities. > > This can be used for things like a box's power button as > mentioned: If the BMC is not up, poke the host directly, > otherwise, poke the BMC firmware and have it poke the host. > > The pass-through settings are tied to the SoC's pinmux. Intuitively > it should be integrated into the pinmux driver, but IIRC there was > some ugly coupling of the function that didn't have a neat solution > with the currently-separate GPIO and pinmux drivers. > > Hope that helps. Yes I get it now! Thanks a lot! That's a very neat hardware trick, I like how they think. Pin config in the pin control driver is indeed the recommended way to go, if possible. If not possible, so that this particular trick needs to be handled on the GPIO side of the world, I would recommend to work with adding it as generic GPIO line config on the GPIO side. That means, document passthrough in include/linux/pinctrl/pinconf-generic.h Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt so it is a generic config in the pin control world, then also make it a GPIO-only binding: Documentation/devicetree/bindings/gpio/gpio.txt include/dt-bindings/gpio/gpio.h And parse it as an argument for the second cell in the GPIO phandle, then manage it in the GPIO driver .set_config() callback without calling into the pin config backend. Thomas Petazzoni is currently looking into using some of the standard pin config set ups on the pure GPIO path so keep him in CC. I think it will be necessary to go down this path for some line configurations rather than forcibly keep GPIO and pin control electronic settings separate. My idea as maintainer is that I want to keep the available configs together so that it can be seamlessly switched between the GPIO front-end and a pin control back-end. Yours, Linus Walleij
Hi Linus, Thanks a lot for your sharing and suggestion. My proposal to enable buttons GPIO pass-through based on pinmux driver as following: 1. remove codes about SCU7C clearance (from line 194 to line 236) in aspeed_sig_expr_set function in pinctrl-aspeed.c file. 2. create 2 new functions in pinctrl-aspeed.c file: “aspeed_gpio_passthrough_enable” and “aspeed_gpio_passthrough_disable” 3. bind button gpio to pinctrl Please feel free to correct me and share your preferred solution. Thanks, Kwin. -----Original Message----- From: Linus Walleij [mailto:linus.walleij@linaro.org] Sent: Monday, January 14, 2019 4:19 PM To: Andrew Jeffery <andrew@aj.id.au>; Thomas Petazzoni <thomas.petazzoni@bootlin.com> Cc: Joel Stanley <joel@jms.id.au>; Wang, Kuiying <kuiying.wang@intel.com>; open list:GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>; Andrew Geissler <geissonator@gmail.com>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Mauery, Vernon <vernon.mauery@intel.com>; Feist, James <james.feist@intel.com>; Yoo, Jae Hyun <jae.hyun.yoo@intel.com> Subject: Re: Enable buttons GPIO passthrough On Mon, Jan 14, 2019 at 3:59 AM Andrew Jeffery <andrew@aj.id.au> wrote: > On Fri, 11 Jan 2019, at 19:31, Linus Walleij wrote: > > I am sorry but I'm confused about this. It's a very terse > > explanation of the problem and a bunch of code. > > > > Can you folks please define what exactly you mean with a "GPIO > > passthrough" and what you are trying to achieve with this on these > > buttons? > > On the Aspeed BMC SoCs, several GPIO banks (D and E) can be configured > so the pins in the low half of a bank of 8 pins are input, and the > pins in the high half of the bank are output. The line state of pins 0 > <= i < 4 is reflected on pin i + 4. This way it appears as though the > BMC is not present in the circuit. > > This is useful when the BMC may not have firmware of its own, or if it > does, if the BMC firmware is not required on the critical path of the > host's boot process. If or when the BMC firmware boots up, it may > choose to break the pass-through behaviour in order to provide its own > out-of-band management capabilities. > > This can be used for things like a box's power button as > mentioned: If the BMC is not up, poke the host directly, otherwise, > poke the BMC firmware and have it poke the host. > > The pass-through settings are tied to the SoC's pinmux. Intuitively it > should be integrated into the pinmux driver, but IIRC there was some > ugly coupling of the function that didn't have a neat solution with > the currently-separate GPIO and pinmux drivers. > > Hope that helps. Yes I get it now! Thanks a lot! That's a very neat hardware trick, I like how they think. Pin config in the pin control driver is indeed the recommended way to go, if possible. If not possible, so that this particular trick needs to be handled on the GPIO side of the world, I would recommend to work with adding it as generic GPIO line config on the GPIO side. That means, document passthrough in include/linux/pinctrl/pinconf-generic.h Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt so it is a generic config in the pin control world, then also make it a GPIO-only binding: Documentation/devicetree/bindings/gpio/gpio.txt include/dt-bindings/gpio/gpio.h And parse it as an argument for the second cell in the GPIO phandle, then manage it in the GPIO driver .set_config() callback without calling into the pin config backend. Thomas Petazzoni is currently looking into using some of the standard pin config set ups on the pure GPIO path so keep him in CC. I think it will be necessary to go down this path for some line configurations rather than forcibly keep GPIO and pin control electronic settings separate. My idea as maintainer is that I want to keep the available configs together so that it can be seamlessly switched between the GPIO front-end and a pin control back-end. Yours, Linus Walleij
Hi Linus,
Based on pinmux to do passthrough is too complex and no reference, I have another 2 simple proposals to do button GPIO passthrough as following:
1. extend "passthrough" to the "direction" property of gpio, use "value" to control it be disabled/enabled.
This solution needs:
Add "passthrough" parsing in gpiolib-sysfs.c
Add " direction_passthrough" supported in gpio_chip struct in include/linux/gpio/driver.h
Add "aspeed_gpio_dir_passthrough" function in gpio-aspeed.c file to map "passthrough" direction.
Modify aspeed_gpio_set function in gpio-aspeed.c file to modify register according to the "value" to disable/enable pass-through.
2. extend "value" property of gpio to support the third value "2", which means pass-through is enabled, otherwise passthrough is disabled.
This solution is very simple to support pass-through, just need:
Modify aspeed_gpio_set function in gpio-aspeed.c file. When value is set to "2" enable passthrough via modify corresponding register, otherwise disable passthrough.
Do you agree to use one simple solution to do passthrough?
Thanks,
Kwin.
-----Original Message-----
From: Wang, Kuiying
Sent: Tuesday, January 15, 2019 3:21 PM
To: 'Linus Walleij' <linus.walleij@linaro.org>; Andrew Jeffery <andrew@aj.id.au>; Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Joel Stanley <joel@jms.id.au>; open list:GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>; Andrew Geissler <geissonator@gmail.com>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Mauery, Vernon <vernon.mauery@intel.com>; Feist, James <james.feist@intel.com>; Yoo, Jae Hyun <jae.hyun.yoo@intel.com>
Subject: RE: Enable buttons GPIO passthrough
Hi Linus,
Thanks a lot for your sharing and suggestion.
My proposal to enable buttons GPIO pass-through based on pinmux driver as following:
1. remove codes about SCU7C clearance (from line 194 to line 236) in aspeed_sig_expr_set function in pinctrl-aspeed.c file.
2. create 2 new functions in pinctrl-aspeed.c file: “aspeed_gpio_passthrough_enable” and “aspeed_gpio_passthrough_disable”
3. bind button gpio to pinctrl
Please feel free to correct me and share your preferred solution.
Thanks,
Kwin.
-----Original Message-----
From: Linus Walleij [mailto:linus.walleij@linaro.org]
Sent: Monday, January 14, 2019 4:19 PM
To: Andrew Jeffery <andrew@aj.id.au>; Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Joel Stanley <joel@jms.id.au>; Wang, Kuiying <kuiying.wang@intel.com>; open list:GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>; Andrew Geissler <geissonator@gmail.com>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Mauery, Vernon <vernon.mauery@intel.com>; Feist, James <james.feist@intel.com>; Yoo, Jae Hyun <jae.hyun.yoo@intel.com>
Subject: Re: Enable buttons GPIO passthrough
On Mon, Jan 14, 2019 at 3:59 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> On Fri, 11 Jan 2019, at 19:31, Linus Walleij wrote:
> > I am sorry but I'm confused about this. It's a very terse
> > explanation of the problem and a bunch of code.
> >
> > Can you folks please define what exactly you mean with a "GPIO
> > passthrough" and what you are trying to achieve with this on these
> > buttons?
>
> On the Aspeed BMC SoCs, several GPIO banks (D and E) can be configured
> so the pins in the low half of a bank of 8 pins are input, and the
> pins in the high half of the bank are output. The line state of pins 0
> <= i < 4 is reflected on pin i + 4. This way it appears as though the
> BMC is not present in the circuit.
>
> This is useful when the BMC may not have firmware of its own, or if it
> does, if the BMC firmware is not required on the critical path of the
> host's boot process. If or when the BMC firmware boots up, it may
> choose to break the pass-through behaviour in order to provide its own
> out-of-band management capabilities.
>
> This can be used for things like a box's power button as
> mentioned: If the BMC is not up, poke the host directly, otherwise,
> poke the BMC firmware and have it poke the host.
>
> The pass-through settings are tied to the SoC's pinmux. Intuitively it
> should be integrated into the pinmux driver, but IIRC there was some
> ugly coupling of the function that didn't have a neat solution with
> the currently-separate GPIO and pinmux drivers.
>
> Hope that helps.
Yes I get it now! Thanks a lot!
That's a very neat hardware trick, I like how they think.
Pin config in the pin control driver is indeed the recommended way to go, if possible.
If not possible, so that this particular trick needs to be handled on the GPIO side of the world, I would recommend to work with adding it as generic GPIO line config on the GPIO side.
That means, document passthrough in
include/linux/pinctrl/pinconf-generic.h
Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
so it is a generic config in the pin control world, then also make it a GPIO-only binding:
Documentation/devicetree/bindings/gpio/gpio.txt
include/dt-bindings/gpio/gpio.h
And parse it as an argument for the second cell in the GPIO phandle, then manage it in the GPIO driver
.set_config() callback without calling into the pin config backend.
Thomas Petazzoni is currently looking into using some of the standard pin config set ups on the pure GPIO path so keep him in CC. I think it will be necessary to go down this path for some line configurations rather than forcibly keep GPIO and pin control electronic settings separate.
My idea as maintainer is that I want to keep the available configs together so that it can be seamlessly switched between the GPIO front-end and a pin control back-end.
Yours,
Linus Walleij
Hi Kwin! On Tue, Jan 15, 2019 at 8:21 AM Wang, Kuiying <kuiying.wang@intel.com> wrote: > My proposal to enable buttons GPIO pass-through based on pinmux driver as following: > 1. remove codes about SCU7C clearance (from line 194 to line 236) in > aspeed_sig_expr_set function in pinctrl-aspeed.c file. > 2. create 2 new functions in pinctrl-aspeed.c file: “aspeed_gpio_passthrough_enable” > and “aspeed_gpio_passthrough_disable” These are coding implementation issues that you need to discuss with the maintainer of pinctrl-aspeed.c, Andrew. > 3. bind button gpio to pinctrl This will just be a pin control state like "default" or any other stare. What you need to do first is establish a standard for this, so augment the pin control bindings and the generic parser code to handle this and provide a new enumerator back to .set_config in the pin config portion of the pin control driver. So: Step 1: standardize pin pass through for everyone Step 2: implement support for this in the Aspeed driver I think using a name like "gpio-passthrough" is misleading, something describing what is actually happening, like "latch" or just "passthrough" is better. Yours, Linus Walleij
Hi Kwin, > Based on pinmux to do passthrough is too complex and no reference, Don't let that discourage you! We are some of the world's most important software engineers in the kernel, and we don't just do the simple stuff, like filling in the blanks guided by previous example, we do the really complicated stuff of inventing the frameworks. > I have another 2 simple proposals to do button GPIO passthrough as following: > 1. extend "passthrough" to the "direction" property of gpio, use "value" to > control it be disabled/enabled. This is what I described in my first reply. A .set_config() option. But it still needs to patch the DT bindings for generic pin config and add a globally uniquie enumerator for this config to pinconf-generic.h since GPIO is reusing these generics, so begin with that in any case. > 2. extend "value" property of gpio to support the third value "2", > which means pass-through is enabled, otherwise passthrough is disabled. No, sorry, this is the wrong idea. .set_value() sets the GPIO line as high or low, it is essentially boolean and it is an integer only for historical reasons. .set_config() is what we use to set up debounce or open drain and such electronic configurations on GPIO lines, and this should be done there, unless Andrew prefers that it is gets done in the pin control driver. Yours, Linus Walleij
Hi Joel, Do agree to use proposal #1? (extend "passthrough" to the "direction" property of gpio, use "value" to control it be disabled/enabled.) All, Any other comments? Thanks, Kwin. -----Original Message----- From: Linus Walleij [mailto:linus.walleij@linaro.org] Sent: Tuesday, January 15, 2019 8:10 PM To: Wang, Kuiying <kuiying.wang@intel.com> Cc: Andrew Jeffery <andrew@aj.id.au>; Thomas Petazzoni <thomas.petazzoni@bootlin.com>; Joel Stanley <joel@jms.id.au>; open list:GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>; Andrew Geissler <geissonator@gmail.com>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Mauery, Vernon <vernon.mauery@intel.com>; Feist, James <james.feist@intel.com>; Yoo, Jae Hyun <jae.hyun.yoo@intel.com> Subject: Re: Enable buttons GPIO passthrough Hi Kwin, > Based on pinmux to do passthrough is too complex and no reference, Don't let that discourage you! We are some of the world's most important software engineers in the kernel, and we don't just do the simple stuff, like filling in the blanks guided by previous example, we do the really complicated stuff of inventing the frameworks. > I have another 2 simple proposals to do button GPIO passthrough as following: > 1. extend "passthrough" to the "direction" property of gpio, use > "value" to control it be disabled/enabled. This is what I described in my first reply. A .set_config() option. But it still needs to patch the DT bindings for generic pin config and add a globally uniquie enumerator for this config to pinconf-generic.h since GPIO is reusing these generics, so begin with that in any case. > 2. extend "value" property of gpio to support the third value "2", > which means pass-through is enabled, otherwise passthrough is disabled. No, sorry, this is the wrong idea. .set_value() sets the GPIO line as high or low, it is essentially boolean and it is an integer only for historical reasons. .set_config() is what we use to set up debounce or open drain and such electronic configurations on GPIO lines, and this should be done there, unless Andrew prefers that it is gets done in the pin control driver. Yours, Linus Walleij
Hi Kwin, On Wed, Jan 16, 2019 at 3:30 PM Wang, Kuiying <kuiying.wang@intel.com> wrote: > Hi Joel, > Do agree to use proposal #1? (extend "passthrough" to the "direction" property of gpio, use "value" to control it be disabled/enabled.) > > All, > Any other comments? I think you maybe misunderstood my previous mail, or I misunderstood yours :) Make sure that passthrough mode is set up in the .set_config() callback, nothing else. Do not change the signatures of the direction functions or set_value() functions in gpiolib. But maybe you are talking about implementation details in the specific GPIO driver for Aspeed? Then I just missed that part, sorry. Yours, Linus Walleij
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"
diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index 2342e154029b..1091ceded76a 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -482,6 +482,33 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
return 0;
}
+static int aspeed_gpio_dir_passthrough(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ struct aspeed_gpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_gpio_bank *bank = to_bank(offset);
+ void __iomem *addr = bank_reg(gpio, bank, reg_dir);
+ unsigned long flags;
+ bool copro;
+ u32 reg;
+ printk("kwin::aspeed_gpio_dir_passthrough");
+ //TODO enable_passthrouhg();
+ spin_lock_irqsave(&gpio->lock, flags);
+
+ reg = ioread32(addr);
+ reg |= GPIO_BIT(offset);
+
+ copro = aspeed_gpio_copro_request(gpio, offset);
+ //__aspeed_gpio_set(gc, offset, val);
+ iowrite32(reg, addr);
+
+ if (copro)
+ aspeed_gpio_copro_release(gpio, offset);
+ spin_unlock_irqrestore(&gpio->lock, flags);
+
+ return 0;
+}
+
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;
gpio->chip.get_direction = aspeed_gpio_get_direction;
gpio->chip.request = aspeed_gpio_request;
gpio->chip.free = aspeed_gpio_free;
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 3dbaf489a8a5..5c78067ad250 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -89,6 +89,8 @@ static ssize_t direction_store(struct device *dev,
status = gpiod_direction_output_raw(desc, 0);
else if (sysfs_streq(buf, "in"))
status = gpiod_direction_input(desc);
+ else if (sysfs_streq(buf, "passthrough"))
+ status = gpiod_direction_passthrough(desc);
else
status = -EINVAL;
@@ -300,6 +302,8 @@ static ssize_t edge_store(struct device *dev,
}
static DEVICE_ATTR_RW(edge);
+
+
/* Caller holds gpiod-data mutex. */
static int gpio_sysfs_set_active_low(struct device *dev, int value)
{
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a8e01d99919c..530573bd8711 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2536,6 +2536,33 @@ int gpiod_direction_input(struct gpio_desc *desc)
}
EXPORT_SYMBOL_GPL(gpiod_direction_input);
+
+int gpiod_direction_passthrough(struct gpio_desc *desc)
+{
+ struct gpio_chip *chip;
+ int status = -EINVAL;
+
+ VALIDATE_DESC(desc);
+ chip = desc->gdev->chip;
+
+ if (!chip->direction_passthrough) {
+ gpiod_warn(desc,
+ "%s: missing direction_passthrough() operations\n",
+ __func__);
+ return -EIO;
+ }
+
+ status = chip->direction_passthrough(chip, gpio_chip_hwgpio(desc));
+ if (status == 0)
+ clear_bit(FLAG_IS_OUT, &desc->flags);
+
+ trace_gpio_direction(desc_to_gpio(desc), 1, status);
+
+ return status;
+}
+EXPORT_SYMBOL_GPL(gpiod_direction_passthrough);
+
+
static int gpio_set_drive_single_ended(struct gpio_chip *gc, unsigned offset,
enum pin_config_param mode)
{
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index a4d5eb37744a..07e30f9e3bce 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -245,6 +245,8 @@ struct gpio_chip {
unsigned offset);
int (*direction_output)(struct gpio_chip *chip,
unsigned offset, int value);
+ int (*direction_passthrough)(struct gpio_chip *chip,
+ unsigned offset);
int (*get)(struct gpio_chip *chip,
unsigned offset);
int (*get_multiple)(struct gpio_chip *chip,
Thanks,
Kwin.
-----Original Message-----
From: Linus Walleij [mailto:linus.walleij@linaro.org]
Sent: Monday, January 21, 2019 9:10 PM
To: Wang, Kuiying <kuiying.wang@intel.com>
Cc: Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Thomas Petazzoni <thomas.petazzoni@bootlin.com>; open list:GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>; Andrew Geissler <geissonator@gmail.com>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Mauery, Vernon <vernon.mauery@intel.com>; Feist, James <james.feist@intel.com>; Yoo, Jae Hyun <jae.hyun.yoo@intel.com>; Nguyen, Hai V <hai.v.nguyen@intel.com>; Khetan, Sharad <sharad.khetan@intel.com>
Subject: Re: Enable buttons GPIO passthrough
Hi Kwin,
On Wed, Jan 16, 2019 at 3:30 PM Wang, Kuiying <kuiying.wang@intel.com> wrote:
> Hi Joel,
> Do agree to use proposal #1? (extend "passthrough" to the "direction"
> property of gpio, use "value" to control it be disabled/enabled.)
>
> All,
> Any other comments?
I think you maybe misunderstood my previous mail, or I misunderstood yours :)
Make sure that passthrough mode is set up in the .set_config() callback, nothing else.
Do not change the signatures of the direction functions or set_value() functions in gpiolib.
But maybe you are talking about implementation details in the specific GPIO driver for Aspeed? Then I just missed that part, sorry.
Yours,
Linus Walleij
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
Hi Linus,
I got your points.
Below patch is based gpio character device and extend .set_config() property to support pass-through. If you have more comments please feel free to reply me.
And I plan to add passthrough setting for gpioset of libgpiod, patch is attached. If you agree I will upstream it together w/ this pass-through patch.
BTW: I have 2 questions hope could get your answer:
1. why we have to switch to gpio character device and what's the advantage of it?
2. As you mentioned " sysfs ABI is deprecated since over three years ", why sysfs ABI is still in use and most of cases are still using sysfs ABI? And when sysfs ABI is removed totally?
Please share all the related information (including web page/video/PPT/etc.) ASAP. :)
diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index 2342e154029b..bd917dca48a6 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -988,12 +988,19 @@ static int set_debounce(struct gpio_chip *chip, unsigned int offset,
return disable_debounce(chip, offset);
}
+static int aspeed_gpio_pass_through(struct gpio_chip *chip, unsigned int offset,
+ unsigned long usecs)
+{
+ printk("kwin::aspeed_gpio_pass_through is called");
+ return 0;
+}
+
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);
-
+ printk("kwin:: aspeed_gpio_set_config");
if (param == PIN_CONFIG_INPUT_DEBOUNCE)
return set_debounce(chip, offset, arg);
else if (param == PIN_CONFIG_BIAS_DISABLE ||
@@ -1006,6 +1013,8 @@ static int aspeed_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
return -ENOTSUPP;
else if (param == PIN_CONFIG_PERSIST_STATE)
return aspeed_gpio_reset_tolerance(chip, offset, arg);
+ else if (param == PIN_CONFIG_PASS_THROUGH)
+ return aspeed_gpio_pass_through((chip, offset, arg);
return -ENOTSUPP;
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a8e01d99919c..1d2a2f0eb144 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -579,6 +579,8 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
set_bit(FLAG_OPEN_DRAIN, &desc->flags);
if (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE)
set_bit(FLAG_OPEN_SOURCE, &desc->flags);
+ if (lflags & GPIOHANDLE_REQUEST_PASS_THROUGH)
+ set_bit(FLAG_PASS_THROUGH, &desc->flags);
ret = gpiod_set_transitory(desc, false);
if (ret < 0)
@@ -598,6 +600,10 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
ret = gpiod_direction_input(desc);
if (ret)
goto out_free_descs;
+ } else if (lflags & GPIOHANDLE_REQUEST_PASS_THROUGH) {
+ ret = gpiod_direction_pass_through(desc);
+ if (ret)
+ goto out_free_descs;
}
dev_dbg(&gdev->dev, "registered chardev handle for line %d\n",
offset);
@@ -2643,6 +2649,41 @@ int gpiod_direction_output(struct gpio_desc *desc, int value)
}
EXPORT_SYMBOL_GPL(gpiod_direction_output);
+/**
+ * gpiod_direction_pass_through - set the GPIO direction to pass-through
+ * @desc: GPIO to set to pass-through
+ *
+ * Set the direction of the passed GPIO to passthrough.
+ *
+ * Return 0 in case of success, else an error code.
+ */
+int gpiod_direction_pass_through(struct gpio_desc *desc)
+{
+ struct gpio_chip *gc;
+ printk("kwin:: gpiod_direction_pass_through");
+ VALIDATE_DESC(desc);
+ /* GPIOs used for IRQs shall not be set as pass-through */
+ if (test_bit(FLAG_USED_AS_IRQ, &desc->flags)) {
+ gpiod_err(desc,
+ "%s: tried to set a GPIO tied to an IRQ as pass-through\n",
+ __func__);
+ return -EIO;
+ }
+ gc = desc->gdev->chip;
+ if (test_bit(FLAG_PASS_THROUGH, &desc->flags)) {
+ gpio_set_drive_single_ended(gc, gpio_chip_hwgpio(desc),
+ PIN_CONFIG_PASS_THROUGH);
+ } else {
+ gpiod_err(desc,
+ "%s: desc->flags is not set to FLAG_PASS_THROUGH\n",
+ __func__);
+ return -EIO;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(gpiod_direction_pass_through);
+
/**
* gpiod_set_debounce - sets @debounce time for a GPIO
* @desc: descriptor of the GPIO for which to set debounce time
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index a7e49fef73d4..b143ee47870a 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -210,6 +210,7 @@ struct gpio_desc {
#define FLAG_IS_OUT 1
#define FLAG_EXPORT 2 /* protected by sysfs_lock */
#define FLAG_SYSFS 3 /* exported via /sys/class/gpio/control */
+#define FLAG_PASS_THROUGH 4 /*Gpio is passthrough type*/
#define FLAG_ACTIVE_LOW 6 /* value has active low */
#define FLAG_OPEN_DRAIN 7 /* Gpio is open drain type */
#define FLAG_OPEN_SOURCE 8 /* Gpio is open source type */
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 21ddbe440030..f0231dd09a2c 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -99,6 +99,7 @@ void devm_gpiod_put_array(struct device *dev, struct gpio_descs *descs);
int gpiod_get_direction(struct gpio_desc *desc);
int gpiod_direction_input(struct gpio_desc *desc);
int gpiod_direction_output(struct gpio_desc *desc, int value);
+int gpiod_direction_pass_through(struct gpio_desc *desc);
int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
/* Value get/set from non-sleeping context */
@@ -314,6 +315,12 @@ static inline int gpiod_direction_output(struct gpio_desc *desc, int value)
WARN_ON(1);
return -ENOSYS;
}
+static inline int gpiod_direction_pass_through(struct gpio_desc *desc)
+{
+ /* GPIO can never have been requested */
+ WARN_ON(1);
+ return -ENOSYS;
+}
static inline int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
{
/* GPIO can never have been requested */
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index 6c0680641108..b2cdea81d6c9 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -124,6 +124,7 @@ enum pin_config_param {
PIN_CONFIG_SLEW_RATE,
PIN_CONFIG_SKEW_DELAY,
PIN_CONFIG_PERSIST_STATE,
+ PIN_CONFIG_PASS_THROUGH,
PIN_CONFIG_END = 0x7F,
PIN_CONFIG_MAX = 0xFF,
};
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 1bf6e6df084b..44d9876b4aa2 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -62,6 +62,7 @@ struct gpioline_info {
#define GPIOHANDLE_REQUEST_ACTIVE_LOW (1UL << 2)
#define GPIOHANDLE_REQUEST_OPEN_DRAIN (1UL << 3)
#define GPIOHANDLE_REQUEST_OPEN_SOURCE (1UL << 4)
+#define GPIOHANDLE_REQUEST_PASS_THROUGH (1UL << 5)
/**
* struct gpiohandle_request - Information about a GPIO handle request
--
diff --git a/aclocal.m4 b/aclocal.m4
index b0db596..a5e28ad 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -911,7 +911,7 @@ AS_VAR_IF([$1], [""], [$5], [$4])dnl
# generated from the m4 files accompanying Automake X.Y.
# (This private macro should not be called outside this file.)
AC_DEFUN([AM_AUTOMAKE_VERSION],
-[am__api_version='1.15'
+[am__api_version='1.16'
dnl Some users find AM_AUTOMAKE_VERSION and mistake it for a way to
dnl require some minimum version. Point them to the right macro.
m4_if([$1], [1.15], [],
diff --git a/configure b/configure
index 3d44cd1..d218338 100755
--- a/configure
+++ b/configure
@@ -2477,7 +2477,7 @@ ac_configure="$SHELL $ac_aux_dir/configure" # Please don't use this var.
-am__api_version='1.15'
+am__api_version='1.16'
# Find a good install program. We prefer a C program (faster),
# so one script is as good as another. But avoid the broken or
diff --git a/include/gpiod.h b/include/gpiod.h
index ccff977..0f935e6 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -121,6 +121,7 @@ typedef void (*gpiod_ctxless_set_value_cb)(void *);
* @param offset The offset of the GPIO line.
* @param value New value (0 or 1).
* @param active_low The active state of this line - true if low.
+ * @param pass_through The pass-through state of the lines - true if enabled.
* @param consumer Name of the consumer.
* @param cb Optional callback function that will be called right after setting
* the value. Users can use this, for example, to pause the execution
@@ -129,7 +130,7 @@ typedef void (*gpiod_ctxless_set_value_cb)(void *);
* @return 0 if the operation succeeds, -1 on error.
*/
int gpiod_ctxless_set_value(const char *device, unsigned int offset, int value,
- bool active_low, const char *consumer,
+ bool active_low, bool pass_through, const char *consumer,
gpiod_ctxless_set_value_cb cb,
void *data) GPIOD_API;
@@ -140,6 +141,7 @@ int gpiod_ctxless_set_value(const char *device, unsigned int offset, int value,
* @param values Array of integers containing new values.
* @param num_lines Number of lines, must be > 0.
* @param active_low The active state of the lines - true if low.
+ * @param pass_through The pass-through state of the lines - true if enabled.
* @param consumer Name of the consumer.
* @param cb Optional callback function that will be called right after setting
* all values. Works the same as in ::gpiod_ctxless_set_value.
@@ -149,7 +151,7 @@ int gpiod_ctxless_set_value(const char *device, unsigned int offset, int value,
int gpiod_ctxless_set_value_multiple(const char *device,
const unsigned int *offsets,
const int *values, unsigned int num_lines,
- bool active_low, const char *consumer,
+ bool active_low, bool pass_through, const char *consumer,
gpiod_ctxless_set_value_cb cb,
void *data) GPIOD_API;
@@ -766,6 +768,8 @@ enum {
/**< Request the line(s) for reading the GPIO line state. */
GPIOD_LINE_REQUEST_DIRECTION_OUTPUT,
/**< Request the line(s) for setting the GPIO line state. */
+ GPIOD_LINE_REQUEST_DIRECTION_PASS_THROUGH,
+ /**< Request the line(s) for setting the GPIO line state. */
GPIOD_LINE_REQUEST_EVENT_FALLING_EDGE,
/**< Monitor both types of events. */
GPIOD_LINE_REQUEST_EVENT_RISING_EDGE,
@@ -784,6 +788,8 @@ enum {
/**< The line is an open-source port. */
GPIOD_LINE_REQUEST_FLAG_ACTIVE_LOW = GPIOD_BIT(2),
/**< The active state of the line is low (high is the default). */
+ GPIOD_LINE_REQUEST_FLAG_PASS_THROUGH = GPIOD_BIT(5),
+ /**< The line is a pass-through port*/
};
/**
diff --git a/src/lib/core.c b/src/lib/core.c
index 4f273e3..74139a0 100644
--- a/src/lib/core.c
+++ b/src/lib/core.c
@@ -617,7 +617,8 @@ static bool line_request_is_direction(int request)
{
return request == GPIOD_LINE_REQUEST_DIRECTION_AS_IS ||
request == GPIOD_LINE_REQUEST_DIRECTION_INPUT ||
- request == GPIOD_LINE_REQUEST_DIRECTION_OUTPUT;
+ request == GPIOD_LINE_REQUEST_DIRECTION_OUTPUT ||
+ request == GPIOD_LINE_REQUEST_DIRECTION_PASS_THROUGH;
}
static bool line_request_is_events(int request)
diff --git a/src/lib/ctxless.c b/src/lib/ctxless.c
index 0009504..c48e739 100644
--- a/src/lib/ctxless.c
+++ b/src/lib/ctxless.c
@@ -76,17 +76,17 @@ int gpiod_ctxless_get_value_multiple(const char *device,
}
int gpiod_ctxless_set_value(const char *device, unsigned int offset, int value,
- bool active_low, const char *consumer,
+ bool active_low, bool pass_through, const char *consumer,
gpiod_ctxless_set_value_cb cb, void *data)
{
return gpiod_ctxless_set_value_multiple(device, &offset, &value, 1,
- active_low, consumer, cb, data);
+ active_low, pass_through, consumer, cb, data);
}
int gpiod_ctxless_set_value_multiple(const char *device,
const unsigned int *offsets,
const int *values, unsigned int num_lines,
- bool active_low, const char *consumer,
+ bool active_low, bool pass_through, const char *consumer,
gpiod_ctxless_set_value_cb cb, void *data)
{
struct gpiod_line_bulk bulk;
@@ -117,6 +117,7 @@ int gpiod_ctxless_set_value_multiple(const char *device,
}
flags = active_low ? GPIOD_LINE_REQUEST_FLAG_ACTIVE_LOW : 0;
+ flags |= pass_through ? GPIOD_LINE_REQUEST_FLAG_PASS_THROUGH : 0;
status = gpiod_line_request_bulk_output_flags(&bulk, consumer,
flags, values);
diff --git a/src/tools/gpioset.c b/src/tools/gpioset.c
index fb012fa..d5f0b77 100644
--- a/src/tools/gpioset.c
+++ b/src/tools/gpioset.c
@@ -22,7 +22,8 @@
static const struct option longopts[] = {
{ "help", no_argument, NULL, 'h' },
{ "version", no_argument, NULL, 'v' },
- { "active-low", no_argument, NULL, 'l' },
+ { "active-low", no_argument, NULL, 'l' },
+ { "pass-through", no_argument, NULL, 'p' },
{ "mode", required_argument, NULL, 'm' },
{ "sec", required_argument, NULL, 's' },
{ "usec", required_argument, NULL, 'u' },
@@ -30,7 +31,7 @@ static const struct option longopts[] = {
{ GETOPT_NULL_LONGOPT },
};
-static const char *const shortopts = "+hvlm:s:u:b";
+static const char *const shortopts = "+hvlpm:s:u:b";
static void print_help(void)
{
@@ -40,8 +41,9 @@ static void print_help(void)
printf("\n");
printf("Options:\n");
printf(" -h, --help:\t\tdisplay this message and exit\n");
- printf(" -v, --version:\tdisplay the version and exit\n");
printf(" -l, --active-low:\tset the line active state to low\n");
+ printf(" -v, --version:\tdisplay the version and exit\n");
+ printf(" -p, --pass-through:\tset it to pass through mode\n");
printf(" -m, --mode=[exit|wait|time|signal] (defaults to 'exit'):\n");
printf(" tell the program what to do after setting values\n");
printf(" -s, --sec=SEC:\tspecify the number of seconds to wait (only valid for --mode=time)\n");
@@ -179,6 +181,7 @@ int main(int argc, char **argv)
int *values, status, optc, opti;
struct callback_data cbdata;
bool active_low = false;
+ bool pass_through = false;
char *device, *end;
memset(&cbdata, 0, sizeof(cbdata));
@@ -197,6 +200,8 @@ int main(int argc, char **argv)
return EXIT_SUCCESS;
case 'l':
active_low = true;
+ case 'p':
+ pass_through = true;
break;
case 'm':
mode = parse_mode(optarg);
@@ -263,7 +268,7 @@ int main(int argc, char **argv)
}
status = gpiod_ctxless_set_value_multiple(device, offsets, values,
- num_lines, active_low,
+ num_lines, active_low, pass_through,
"gpioset", mode->callback,
&cbdata);
if (status < 0)
diff --git a/tests/tests-ctxless.c b/tests/tests-ctxless.c
index ea9403d..228c49d 100644
--- a/tests/tests-ctxless.c
+++ b/tests/tests-ctxless.c
@@ -20,7 +20,7 @@ static void ctxless_set_get_value(void)
TEST_ASSERT_EQ(ret, 0);
ret = gpiod_ctxless_set_value(test_chip_name(0), 3, 1,
- false, TEST_CONSUMER, NULL, NULL);
+ false, false, TEST_CONSUMER, NULL, NULL);
TEST_ASSERT_RET_OK(ret);
ret = gpiod_ctxless_get_value(test_chip_name(0), 3,
Thanks,
Kwin.
-----Original Message-----
From: Linus Walleij [mailto:linus.walleij@linaro.org]
Sent: Monday, January 28, 2019 9:25 PM
To: Wang, Kuiying <kuiying.wang@intel.com>
Cc: Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Thomas Petazzoni <thomas.petazzoni@bootlin.com>; open list:GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>; Andrew Geissler <geissonator@gmail.com>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Mauery, Vernon <vernon.mauery@intel.com>; Feist, James <james.feist@intel.com>; Yoo, Jae Hyun <jae.hyun.yoo@intel.com>; Nguyen, Hai V <hai.v.nguyen@intel.com>; Khetan, Sharad <sharad.khetan@intel.com>
Subject: Re: Enable buttons GPIO passthrough
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
Hi Kwin, thanks for your patch! On Thu, Jan 31, 2019 at 4:11 PM Wang, Kuiying <kuiying.wang@intel.com> wrote: > Below patch is based gpio character device and extend .set_config() property to > support pass-through. This is starting to look good! > And I plan to add passthrough setting for gpioset of libgpiod, patch is attached. > If you agree I will upstream it together w/ this pass-through patch. Bartosz is the maintainer of libgpiod so he can answer about that. I'm not sure why it needs to be a userspace thing, can you explain this? Why can you not just set up passthrough in the device tree file? It seems like a dangerous thing for userspace to deal with. > BTW: I have 2 questions hope could get your answer: > 1. why we have to switch to gpio character device and what's > the advantage of it? - We get a proper topology with local numberspace per gpiochip instead of an unmaintainable large global numberspace with numbers that are magic and different on every system. - Discovery mechanism, you can inspect the available GPIOs by gpiochip and name. - When a process using a character device crashes, it will release its resources and release the GPIO lines it is holding. When using sysfs, if a process that exported a few GPIOs crashes, sometimes things are left in an undefined state, never cleaned up and the best way to get control over the machine again is reboot. All of the above issue of the sysfs ABI were pretty serious. > 2. As you mentioned " sysfs ABI is deprecated since > over three years ", why sysfs ABI is still in use and > most of cases are still using sysfs ABI? What are you basing this statement on? It is deprecated, and people have to actively choose to compile it into their kernel. Big distributions like Fedora does not support the sysfs ABI and disable it in their kernels. If others enable it, please tell then they are wrong and the GPIO maintainer want them to stop using it. > And when sysfs ABI is removed totally? Around the year 2020. Maybe not even then, if people are still using it. However the GPIO character device is design to last ~100 years so I think it will win in the end. I have patience. > Please share all the related information (including web page/video/PPT/etc.) ASAP. :) I have some presentation here: https://dflund.se/~triad/papers/GPIO-for-Engineers-and-Makers.pdf > +static int aspeed_gpio_pass_through(struct gpio_chip *chip, unsigned int offset, > + unsigned long usecs) > +{ > + printk("kwin::aspeed_gpio_pass_through is called"); > + return 0; > +} I guess you want to actually implement this too :D > 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); > - > + printk("kwin:: aspeed_gpio_set_config"); Please remove debug prints when submitting the final patch. BTW pr_info() is better for this kind of stuff. > + else if (param == PIN_CONFIG_PASS_THROUGH) > + return aspeed_gpio_pass_through((chip, offset, arg); Yes this is good. > @@ -579,6 +579,8 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip) > set_bit(FLAG_OPEN_DRAIN, &desc->flags); > if (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE) > set_bit(FLAG_OPEN_SOURCE, &desc->flags); > + if (lflags & GPIOHANDLE_REQUEST_PASS_THROUGH) > + set_bit(FLAG_PASS_THROUGH, &desc->flags); This is good if we support this on the character device. > @@ -598,6 +600,10 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip) > ret = gpiod_direction_input(desc); > if (ret) > goto out_free_descs; > + } else if (lflags & GPIOHANDLE_REQUEST_PASS_THROUGH) { > + ret = gpiod_direction_pass_through(desc); > + if (ret) > + goto out_free_descs; OK looks good too. > +/** > + * gpiod_direction_pass_through - set the GPIO direction to pass-through > + * @desc: GPIO to set to pass-through > + * > + * Set the direction of the passed GPIO to passthrough. > + * > + * Return 0 in case of success, else an error code. > + */ > +int gpiod_direction_pass_through(struct gpio_desc *desc) > +{ > + struct gpio_chip *gc; > + printk("kwin:: gpiod_direction_pass_through"); > + VALIDATE_DESC(desc); > + /* GPIOs used for IRQs shall not be set as pass-through */ > + if (test_bit(FLAG_USED_AS_IRQ, &desc->flags)) { > + gpiod_err(desc, > + "%s: tried to set a GPIO tied to an IRQ as pass-through\n", > + __func__); > + return -EIO; > + } > + gc = desc->gdev->chip; > + if (test_bit(FLAG_PASS_THROUGH, &desc->flags)) { > + gpio_set_drive_single_ended(gc, gpio_chip_hwgpio(desc), > + PIN_CONFIG_PASS_THROUGH); > + } else { > + gpiod_err(desc, > + "%s: desc->flags is not set to FLAG_PASS_THROUGH\n", > + __func__); > + return -EIO; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(gpiod_direction_pass_through); I think you should skip this. I don't think users in the kernel need to dynamically set this up. It should be done as a flag in either device trees, machine tables or from explicit request from the character device ioctl(). > #define FLAG_SYSFS 3 /* exported via /sys/class/gpio/control */ > +#define FLAG_PASS_THROUGH 4 /*Gpio is passthrough type*/ OK good. > @@ -124,6 +124,7 @@ enum pin_config_param { > PIN_CONFIG_SLEW_RATE, > PIN_CONFIG_SKEW_DELAY, > PIN_CONFIG_PERSIST_STATE, > + PIN_CONFIG_PASS_THROUGH, Good. > diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h > index 1bf6e6df084b..44d9876b4aa2 100644 > --- a/include/uapi/linux/gpio.h > +++ b/include/uapi/linux/gpio.h > @@ -62,6 +62,7 @@ struct gpioline_info { > #define GPIOHANDLE_REQUEST_ACTIVE_LOW (1UL << 2) > #define GPIOHANDLE_REQUEST_OPEN_DRAIN (1UL << 3) > #define GPIOHANDLE_REQUEST_OPEN_SOURCE (1UL << 4) > +#define GPIOHANDLE_REQUEST_PASS_THROUGH (1UL << 5) Seems right, if we should support userspace. Things are missing from this patch set: - Adding the flag to include/linux/gpio/machine.h so we can set this up from machine descriptor tables. - Adding the flag to include/dt-bindings/gpio/gpio.h so we can set this from the device tree. Also code for handling both above ways of setting it up. And I think what you want to do with Aspeed is to set this up using device tree and not in userspace, unless you can explain some reason why you wouldn't do that and instead do this complex and dangerous system operation in userspace. Definately you want the system default to be set up from device tree and then that should support passthrough, right? I can't comment much on libgpiod, we'll see if we want this for userspace first. Yours, Linus Walleij
Hi Kwin, On Fri, 8 Feb 2019, at 22:31, Linus Walleij wrote: > > +static int aspeed_gpio_pass_through(struct gpio_chip *chip, unsigned int offset, > > + unsigned long usecs) > > +{ > > + printk("kwin::aspeed_gpio_pass_through is called"); > > + return 0; > > +} > > I guess you want to actually implement this too :D Something to keep in mind is that the ASPEED pinctrl already supports configuring the passthrough bits. What we don't want is to cause some inconsistent state between the GPIO and pinmux subsystems by just whacking the bits directly. Looks like we should be able to make it work with PIN_MAP_MUX_GROUP() and using some custom name for the state (maybe "passthrough")? It's a half-baked thought though so I'm not sure if it's feasible let alone acceptable, but hopefully Linus can comment. Andrew
On Mon, Feb 11, 2019 at 5:54 AM Andrew Jeffery <andrew@aj.id.au> wrote: > On Fri, 8 Feb 2019, at 22:31, Linus Walleij wrote: > > > +static int aspeed_gpio_pass_through(struct gpio_chip *chip, unsigned int offset, > > > + unsigned long usecs) > > > +{ > > > + printk("kwin::aspeed_gpio_pass_through is called"); > > > + return 0; > > > +} > > > > I guess you want to actually implement this too :D > > Something to keep in mind is that the ASPEED pinctrl already supports configuring the > passthrough bits. What we don't want is to cause some inconsistent state between > the GPIO and pinmux subsystems by just whacking the bits directly. > > Looks like we should be able to make it work with PIN_MAP_MUX_GROUP() and using > some custom name for the state (maybe "passthrough")? It's a half-baked thought > though so I'm not sure if it's feasible let alone acceptable, but hopefully Linus can > comment. Since its a real odd thing this "passthrough" I would certainly prefer if you could keep it as a custom thing on the pin control side of things. Then I suppose that you set it up using the device tree and all is fine? What I'd like to hear from Kwin is a convincing story why Aspeed SoCs need to do this from userspace, as that seems to be his goal. Yours, Linus Walleij
Hi Linus, User space supporting is necessary in OpenBMC. OpenBMC has to support IPMI command(user space network command) to dynamically disable/enable all the buttons in the front panel of a server, which need pass-through to support be disabled/enabled dynamically in the user space. Once pass-through of power/reset button is enabled, IPMI command cannot handle the power any more. So correct logic should be that pass-through of buttons is enabled by default, once OpenBMC got the related ipmi command (like power on/off/reset/disable front panel command), disable pass-through firstly, do the corresponding operation and then enable pass-through back. Thanks, Kwin. -----Original Message----- From: Linus Walleij [mailto:linus.walleij@linaro.org] Sent: Monday, February 11, 2019 4:08 PM To: Andrew Jeffery <andrew@aj.id.au> Cc: Wang, Kuiying <kuiying.wang@intel.com>; Joel Stanley <joel@jms.id.au>; Thomas Petazzoni <thomas.petazzoni@bootlin.com>; open list:GPIO SUBSYSTEM <linux-gpio@vger.kernel.org>; Andrew Geissler <geissonator@gmail.com>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Mauery, Vernon <vernon.mauery@intel.com>; Feist, James <james.feist@intel.com>; Yoo, Jae Hyun <jae.hyun.yoo@intel.com>; Nguyen, Hai V <hai.v.nguyen@intel.com>; Khetan, Sharad <sharad.khetan@intel.com>; Benjamin Herrenschmidt <benh@kernel.crashing.org> Subject: Re: Enable buttons GPIO passthrough On Mon, Feb 11, 2019 at 5:54 AM Andrew Jeffery <andrew@aj.id.au> wrote: > On Fri, 8 Feb 2019, at 22:31, Linus Walleij wrote: > > > +static int aspeed_gpio_pass_through(struct gpio_chip *chip, unsigned int offset, > > > + unsigned long usecs) { > > > + printk("kwin::aspeed_gpio_pass_through is called"); > > > + return 0; > > > +} > > > > I guess you want to actually implement this too :D > > Something to keep in mind is that the ASPEED pinctrl already supports > configuring the passthrough bits. What we don't want is to cause some > inconsistent state between the GPIO and pinmux subsystems by just whacking the bits directly. > > Looks like we should be able to make it work with PIN_MAP_MUX_GROUP() > and using some custom name for the state (maybe "passthrough")? It's a > half-baked thought though so I'm not sure if it's feasible let alone > acceptable, but hopefully Linus can comment. Since its a real odd thing this "passthrough" I would certainly prefer if you could keep it as a custom thing on the pin control side of things. Then I suppose that you set it up using the device tree and all is fine? What I'd like to hear from Kwin is a convincing story why Aspeed SoCs need to do this from userspace, as that seems to be his goal. Yours, Linus Walleij
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index f2062546250c..e94ee86820d3 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -4,6 +4,12 @@ menu "Misc devices" +config GPIO_PASS_THROUGH + tristate "GPIO Pass Through" + depends on (ARCH_ASPEED || COMPILE_TEST) + help + Enable this for buttons GPIO pass through. + config SENSORS_LIS3LV02D tristate depends on INPUT diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index bb89694e6b4b..13b8b8edbb70 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -61,3 +61,4 @@ obj-$(CONFIG_ASPEED_LPC_SIO) += aspeed-lpc-sio.o obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o obj-$(CONFIG_OCXL) += ocxl/ obj-$(CONFIG_MISC_RTSX) += cardreader/ +obj-$(CONFIG_GPIO_PASS_THROUGH) += gpio-passthrough.o diff --git a/drivers/misc/gpio-passthrough.c b/drivers/misc/gpio-passthrough.c new file mode 100644 index 000000000000..0126fc08ae55 --- /dev/null +++ b/drivers/misc/gpio-passthrough.c @@ -0,0 +1,260 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018 Intel Corporation +*/ + +#include "gpio-passthrough.h" + +struct aspeed_gpio_pass_through_dev { + struct miscdevice miscdev; + unsigned int addr; + unsigned int size; +}; + +static struct aspeed_gpio_pass_through_dev ast_cdev_gpio_pass_through; + +static long ast_passthru_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) +{ + long ret = 0; + struct passthru_ioctl_data passthru_data; + + if (cmd == GPIO_IOC_PASSTHRU) + { + if (copy_from_user(&passthru_data, + (void __user*)arg, sizeof(passthru_data))) + return -EFAULT; + if (passthru_data.idx >= GPIO_PASSTHRU_MAX) + return -EINVAL; + + switch (passthru_data.cmd) { + case SET_GPIO_PASSTHRU_ENABLE: + ast_set_passthru_enable(passthru_data.idx, + passthru_data.data); + break; + case GET_GPIO_PASSTHRU_ENABLE: + passthru_data.data = ast_get_passthru_enable(passthru_data.idx); + if (copy_to_user((void __user*)arg, &passthru_data, + sizeof(passthru_data))) + ret = -EFAULT; + break; + + case SET_GPIO_PASSTHRU_OUT: + ast_set_passthru_out(passthru_data.idx, passthru_data.data); + break; + + default: + ret = -EINVAL; + break; + } + } + return ret; + +} + +static int ast_passthru_open(struct inode *inode, struct file *filp) +{ + return container_of(filp->private_data, + struct aspeed_gpio_pass_through_dev, miscdev); +} + +static const struct file_operations ast_gpio_pth_fops = { + .owner = THIS_MODULE, + .llseek = no_llseek, + .unlocked_ioctl = ast_passthru_ioctl, + .open = ast_passthru_open, +}; + +static struct miscdevice ast_gpio_pth_miscdev = { + .minor = MISC_DYNAMIC_MINOR, + .name = GPIO_PASS_THROUGH_NAME, + .fops = &ast_gpio_pth_fops, +}; + +static u32 ast_scu_base = IO_ADDRESS(AST_SCU_BASE); + +static inline u32 ast_scu_read(u32 reg) +{ + return readl((void *)(ast_scu_base + reg)); +} + +static inline void ast_scu_write(u32 val, u32 reg) +{ +#ifdef CONFIG_AST_SCU_LOCK + writel(SCU_PROTECT_UNLOCK, (void *)(ast_scu_base + AST_SCU_PROTECT)); + writel(val, (void *)(ast_scu_base + reg)); + writel(0x000000AA, (void *)(ast_scu_base + AST_SCU_PROTECT)); +#else + writel(SCU_PROTECT_UNLOCK, (void *)(ast_scu_base + AST_SCU_PROTECT)); + writel(val, (void *)(ast_scu_base + reg)); +#endif +} + +static int gpio_pass_through_probe(struct platform_device *pdev) +{ + struct aspeed_gpio_pass_through_dev *gpio_pth_dev = &ast_cdev_gpio_pass_through; + struct device *dev = &pdev->dev; + struct resource *rc; + + dev_set_drvdata(&pdev->dev, gpio_pth_dev); + rc = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!rc) { + dev_err(dev, "Fail to platform_get_resource\n"); + return -ENXIO; + } + gpio_pth_dev->addr = rc->start; + gpio_pth_dev->size = resource_size(rc); + gpio_pth_dev->miscdev = ast_gpio_pth_miscdev; + ast_passthru_init(); + printk("GPIO PASS THROUGH DRIVER is loaded \n"); + return misc_register(&gpio_pth_dev->miscdev); +} + +static int gpio_pass_through_remove(struct platform_device *pdev) +{ + struct aspeed_gpio_pass_through_dev *gpio_pth_dev = + dev_get_drvdata(&pdev->dev); + misc_deregister(&gpio_pth_dev->miscdev); + printk("GPIO PASS THROUGH DRIVER is removing \n"); + + return 0; +} + +static struct platform_driver gpio_pass_through_driver = { + .probe = gpio_pass_through_probe, + .remove = gpio_pass_through_remove, + .driver = { + .name = "gpio-pass-through", + .owner = THIS_MODULE, + + }, +}; + +/* GPIOE group only */ +struct gpio_passthru { + u32 passthru_mask; + u16 pin_in; + u16 pin_out; +}; + +static struct gpio_passthru passthru_settings[GPIO_PASSTHRU_MAX] = { + [GPIO_PASSTHRU0] = { + .passthru_mask = (1 << 12), /* SCU8C[12] */ + .pin_in = PGPIO_PIN(GPIOE, 0), + .pin_out = PGPIO_PIN(GPIOE, 1), + }, + + [GPIO_PASSTHRU1] = { + .passthru_mask = (1 << 13), /* SCU8C[13] */ + .pin_in = PGPIO_PIN(GPIOE, 2), + .pin_out = PGPIO_PIN(GPIOE, 3), + }, + + [GPIO_PASSTHRU2] = { + .passthru_mask = (1 << 14), /* SCU8C[14] */ + .pin_in = PGPIO_PIN(GPIOE, 4), + .pin_out = PGPIO_PIN(GPIOE, 5), + }, + + [GPIO_PASSTHRU3] = { + .passthru_mask = (1 << 15), /* SCU8C[15] */ + .pin_in = PGPIO_PIN(GPIOE, 6), + .pin_out = PGPIO_PIN(GPIOE, 7), + }, +}; + +static void ast_set_passthru_enable( + unsigned short idx, unsigned int enable) +{ + u32 val; + unsigned long flags; + struct gpio_passthru *passthru = &passthru_settings[idx]; + + local_irq_save(flags); + + val = ast_scu_read(AST_SCU_FUN_PIN_CTRL4); + if (enable) + val |= (passthru->passthru_mask); + else + val &= ~(passthru->passthru_mask); + ast_scu_write(val, AST_SCU_FUN_PIN_CTRL4); + + local_irq_restore(flags); +} + +static unsigned int ast_get_passthru_enable(unsigned short idx) +{ + unsigned int enable; + unsigned long flags; + struct gpio_passthru *passthru = &passthru_settings[idx]; + + local_irq_save(flags); + + enable = (ast_scu_read(AST_SCU_FUN_PIN_CTRL4) & passthru->passthru_mask) != 0 ? 1 : 0; + + local_irq_restore(flags); + + return enable; +} + +static void ast_set_passthru_out( + unsigned short idx, unsigned int val) +{ + unsigned long flags; + struct gpio_passthru *passthru = &passthru_settings[idx]; + + local_irq_save(flags); + + /* Disable PASSTHRU */ + val = ast_scu_read(AST_SCU_FUN_PIN_CTRL4); + val &= ~(passthru->passthru_mask); + ast_scu_write(val, AST_SCU_FUN_PIN_CTRL4); + + local_irq_restore(flags); +} + +static void ast_passthru_init(void) +{ + int i; + u32 val; + unsigned long flags; + struct gpio_passthru *passthru; + + local_irq_save(flags); + + /* 1. Enable GPIOE pin mode, SCU80[16:23] = 00 */ + ast_scu_write(ast_scu_read(AST_SCU_FUN_PIN_CTRL1) & (~0x00FF0000), + AST_SCU_FUN_PIN_CTRL1); + + /* 2. Enable them by setting SCU8C[12:15] */ + for (i = 0; i < GPIO_PASSTHRU_MAX; i++) { + passthru = &passthru_settings[i]; + + val = ast_scu_read(AST_SCU_FUN_PIN_CTRL4); + val |= passthru->passthru_mask; + ast_scu_write(val, AST_SCU_FUN_PIN_CTRL4); + } + + /************************************************************** + * 3. Disable HWTrap for GPIOE pass-through mode + * + * Hardware strap register (SCU70) programming method. + * #Write '1' to SCU70 can set the specific bit with value '1' + * Write '0' has no effect. + * #Write '1' to SCU7C can clear the specific bit of SCU70 to + * value '0'. Write '0' has no effect. + **************************************************************/ + if (ast_scu_read(AST_SCU_HW_STRAP1) & (0x1 << 22)) + ast_scu_write((0x1 << 22), AST_SCU_REVISION_ID); + + local_irq_restore(flags); + + printk("HW_STRAP1 = 0x%08X\n", ast_scu_read(AST_SCU_HW_STRAP1)); +} + +module_platform_driver(gpio_pass_through_driver); + +MODULE_AUTHOR("Kuiying Wang <kuiying.wang@intel.com>"); +MODULE_DESCRIPTION("GPIO Pass Through Control Driver for all buttons like Power/Reset/ID button"); +MODULE_LICENSE("GPL v2"); + diff --git a/drivers/misc/gpio-passthrough.h b/drivers/misc/gpio-passthrough.h new file mode 100644 index 000000000000..a7274b8ab31e --- /dev/null +++ b/drivers/misc/gpio-passthrough.h @@ -0,0 +1,60 @@ +#ifndef __GPIO_PASS_THROUGH_H__ +#define __GPIO_PASS_THROUGH_H__ + +#include <linux/kernel.h> +#include <linux/miscdevice.h> +#include <linux/uaccess.h> +#include <linux/module.h> +#include <linux/of_platform.h> +#include <linux/mm.h> +#include <asm/io.h> + +#define PGPIO_PIN(PORT, PIN) (((PORT) << 3) | ((PIN) & 0x07)) +#define GPIOE 4 +#define AST_SCU_BASE 0x1E6E2000 /* SCU */ +#define AST_SCU_PROTECT 0x00 /* protection key register */ +#define SCU_PROTECT_UNLOCK 0x1688A8A8 +#define AST_SCU_FUN_PIN_CTRL4 0x8C /* Multi-function Pin Control#4*/ +#define AST_SCU_FUN_PIN_CTRL1 0x80 /* Multi-function Pin Control#1*/ +#define AST_SCU_HW_STRAP1 0x70 /* hardware strapping register */ +#define AST_SCU_REVISION_ID 0x7C /* Silicon revision ID register */ +#define GPIO_PASS_THROUGH_NAME "gpiopassthrough" +#define IO_ADDRESS(x) (x) + +enum GPIO_PASSTHRU_INDEX { + GPIO_PASSTHRU0 = 0, /* GPIOE0 -> GPIOE1 */ + GPIO_PASSTHRU1, /* GPIOE2 -> GPIOE3 */ + GPIO_PASSTHRU2, /* GPIOE4 -> GPIOE5 */ + GPIO_PASSTHRU3, /* GPIOE6 -> GPIOE7 */ + + GPIO_PASSTHRU_MAX +}; + +enum GPIO_PASSTHRU_CMD { + SET_GPIO_PASSTHRU_ENABLE = 0, + GET_GPIO_PASSTHRU_ENABLE, + GET_GPIO_PASSTHRU_IN, + SET_GPIO_PASSTHRU_OUT, /* !!! The PASSTHRU will be disabled !!! */ +}; + +struct passthru_ioctl_data { + unsigned short cmd; + unsigned short idx; + unsigned int data; +}; + +static void ast_set_passthru_enable( + unsigned short idx, unsigned int enable); +static unsigned int ast_get_passthru_enable(unsigned short idx); +static void ast_set_passthru_out( + unsigned short idx, unsigned int val); +static void ast_passthru_init(void); +static inline u32 ast_scu_read(u32 reg); +static inline void ast_scu_write(u32 val, u32 reg); + +/* IOCTL */ +#define GPIO_IOC_BASE 'G' +#define GPIO_IOC_PASSTHRU _IOWR(GPIO_IOC_BASE, 1, struct passthru_ioctl_data) + + +#endif