RE: Enable buttons GPIO passthrough

["Wang, Kuiying" <kuiying.wang@intel.com>]

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