diff mbox series

[4/4] gpio: aspeed: Add interfaces for co-processor to grab GPIOs

Message ID 20180612001043.9327-5-benh@kernel.crashing.org
State Not Applicable, archived
Headers show
Series gpio: aspeed: Fixes and support for sharing with co-processor | expand

Commit Message

Benjamin Herrenschmidt June 12, 2018, 12:10 a.m. UTC 
On the Aspeed chip, the GPIOs can be under control of the ARM
chip or of the ColdFire coprocessor. (There's a third command
source, the LPC bus, which we don't use or support yet).

The control of which master is allowed to modify a given
GPIO is per-bank (8 GPIOs).

Unfortunately, systems already exist for which we want to
use GPIOs of both sources in the same bank.

This provides an API exported by the gpio-aspeed driver
that an aspeed coprocessor driver can use to "grab" some
GPIOs for use by the coprocessor, and allow the coprocessor
driver to provide callbacks for arbitrating access.

Once at least one GPIO of a given bank has been "grabbed"
by the coprocessor, the entire bank is marked as being
under coprocessor control. It's command source is switched
to the coprocessor.

If the ARM then tries to write to a GPIO in such a marked bank,
the provided callbacks are used to request access from the
coprocessor driver, which is responsible to doing whatever
is necessary to "pause" the coprocessor or prevent it from
trying to use the GPIOs while the ARM is doing its accesses.

During that time, the command source for the bank is temporarily
switched back to the ARM.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/gpio/gpio-aspeed.c  | 235 +++++++++++++++++++++++++++++++++---
 include/linux/gpio/aspeed.h |  14 +++
 2 files changed, 229 insertions(+), 20 deletions(-)
 create mode 100644 include/linux/gpio/aspeed.h

Comments

Joel Stanley June 12, 2018, 5:37 a.m. UTC | #1 
On 12 June 2018 at 09:40, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On the Aspeed chip, the GPIOs can be under control of the ARM
> chip or of the ColdFire coprocessor. (There's a third command
> source, the LPC bus, which we don't use or support yet).
>
> The control of which master is allowed to modify a given
> GPIO is per-bank (8 GPIOs).
>
> Unfortunately, systems already exist for which we want to
> use GPIOs of both sources in the same bank.
>
> This provides an API exported by the gpio-aspeed driver
> that an aspeed coprocessor driver can use to "grab" some
> GPIOs for use by the coprocessor, and allow the coprocessor
> driver to provide callbacks for arbitrating access.
>
> Once at least one GPIO of a given bank has been "grabbed"
> by the coprocessor, the entire bank is marked as being
> under coprocessor control. It's command source is switched
> to the coprocessor.
>
> If the ARM then tries to write to a GPIO in such a marked bank,
> the provided callbacks are used to request access from the
> coprocessor driver, which is responsible to doing whatever
> is necessary to "pause" the coprocessor or prevent it from
> trying to use the GPIOs while the ARM is doing its accesses.
>
> During that time, the command source for the bank is temporarily
> switched back to the ARM.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Looks okay to me. Just need to deal with the memory allocated.

> +
> +/**
> + * aspeed_gpio_copro_grab_gpio - Mark a GPIO used by the coprocessor. The entire
> + *                               bank gets marked and any access from the ARM will
> + *                               result in handshaking via callbacks.
> + * @desc: The GPIO to be marked
> + */
> +int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc)
> +{
> +       struct gpio_chip *chip = gpiod_to_chip(desc);
> +       struct aspeed_gpio *gpio = gpiochip_get_data(chip);
> +       int rc = 0, bindex, offset = gpio_chip_hwgpio(desc);
> +       const struct aspeed_gpio_bank *bank = to_bank(offset);
> +       unsigned long flags;
> +
> +       if (!gpio->cf_copro_bankmap)
> +               gpio->cf_copro_bankmap = kzalloc(gpio->config->nr_gpios >> 3, GFP_KERNEL);

Someone should free this.

> +       if (!gpio->cf_copro_bankmap)
> +               return -ENOMEM;
> +       if (offset < 0 || offset > gpio->config->nr_gpios)
> +               return -EINVAL;
> +       bindex = offset >> 3;
> +
> +       spin_lock_irqsave(&gpio->lock, flags);
> +
> +       /* Sanity check, this shouldn't happen */
> +       if (gpio->cf_copro_bankmap[bindex] == 0xff) {
> +               rc = -EIO;
> +               goto bail;
> +       }
> +       gpio->cf_copro_bankmap[bindex]++;
> +
> +       /* Switch command source */
> +       if (gpio->cf_copro_bankmap[bindex] == 1)
> +               aspeed_gpio_change_cmd_source(gpio, bank, bindex,
> +                                             GPIO_CMDSRC_COLDFIRE);
> +
> + bail:
> +       spin_unlock_irqrestore(&gpio->lock, flags);
> +       return rc;
> +}
> +EXPORT_SYMBOL_GPL(aspeed_gpio_copro_grab_gpio);
Benjamin Herrenschmidt June 12, 2018, 7:17 a.m. UTC | #2 
On Tue, 2018-06-12 at 15:07 +0930, Joel Stanley wrote:
> On 12 June 2018 at 09:40, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > On the Aspeed chip, the GPIOs can be under control of the ARM
> > chip or of the ColdFire coprocessor. (There's a third command
> > source, the LPC bus, which we don't use or support yet).
> > 
> > The control of which master is allowed to modify a given
> > GPIO is per-bank (8 GPIOs).
> > 
> > Unfortunately, systems already exist for which we want to
> > use GPIOs of both sources in the same bank.
> > 
> > This provides an API exported by the gpio-aspeed driver
> > that an aspeed coprocessor driver can use to "grab" some
> > GPIOs for use by the coprocessor, and allow the coprocessor
> > driver to provide callbacks for arbitrating access.
> > 
> > Once at least one GPIO of a given bank has been "grabbed"
> > by the coprocessor, the entire bank is marked as being
> > under coprocessor control. It's command source is switched
> > to the coprocessor.
> > 
> > If the ARM then tries to write to a GPIO in such a marked bank,
> > the provided callbacks are used to request access from the
> > coprocessor driver, which is responsible to doing whatever
> > is necessary to "pause" the coprocessor or prevent it from
> > trying to use the GPIOs while the ARM is doing its accesses.
> > 
> > During that time, the command source for the bank is temporarily
> > switched back to the ARM.
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Looks okay to me. Just need to deal with the memory allocated.
> 
> > +
> > +/**
> > + * aspeed_gpio_copro_grab_gpio - Mark a GPIO used by the coprocessor. The entire
> > + *                               bank gets marked and any access from the ARM will
> > + *                               result in handshaking via callbacks.
> > + * @desc: The GPIO to be marked
> > + */
> > +int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc)
> > +{
> > +       struct gpio_chip *chip = gpiod_to_chip(desc);
> > +       struct aspeed_gpio *gpio = gpiochip_get_data(chip);
> > +       int rc = 0, bindex, offset = gpio_chip_hwgpio(desc);
> > +       const struct aspeed_gpio_bank *bank = to_bank(offset);
> > +       unsigned long flags;
> > +
> > +       if (!gpio->cf_copro_bankmap)
> > +               gpio->cf_copro_bankmap = kzalloc(gpio->config->nr_gpios >> 3, GFP_KERNEL);
> 
> Someone should free this.

Right, I'll probably just make it a devm_kzalloc

> > +       if (!gpio->cf_copro_bankmap)
> > +               return -ENOMEM;
> > +       if (offset < 0 || offset > gpio->config->nr_gpios)
> > +               return -EINVAL;
> > +       bindex = offset >> 3;
> > +
> > +       spin_lock_irqsave(&gpio->lock, flags);
> > +
> > +       /* Sanity check, this shouldn't happen */
> > +       if (gpio->cf_copro_bankmap[bindex] == 0xff) {
> > +               rc = -EIO;
> > +               goto bail;
> > +       }
> > +       gpio->cf_copro_bankmap[bindex]++;
> > +
> > +       /* Switch command source */
> > +       if (gpio->cf_copro_bankmap[bindex] == 1)
> > +               aspeed_gpio_change_cmd_source(gpio, bank, bindex,
> > +                                             GPIO_CMDSRC_COLDFIRE);
> > +
> > + bail:
> > +       spin_unlock_irqrestore(&gpio->lock, flags);
> > +       return rc;
> > +}
> > +EXPORT_SYMBOL_GPL(aspeed_gpio_copro_grab_gpio);
Linus Walleij June 14, 2018, 8:59 a.m. UTC | #3 
On Tue, Jun 12, 2018 at 2:10 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:

> On the Aspeed chip, the GPIOs can be under control of the ARM
> chip or of the ColdFire coprocessor. (There's a third command
> source, the LPC bus, which we don't use or support yet).
>
> The control of which master is allowed to modify a given
> GPIO is per-bank (8 GPIOs).
>
> Unfortunately, systems already exist for which we want to
> use GPIOs of both sources in the same bank.
>
> This provides an API exported by the gpio-aspeed driver
> that an aspeed coprocessor driver can use to "grab" some
> GPIOs for use by the coprocessor, and allow the coprocessor
> driver to provide callbacks for arbitrating access.
>
> Once at least one GPIO of a given bank has been "grabbed"
> by the coprocessor, the entire bank is marked as being
> under coprocessor control. It's command source is switched
> to the coprocessor.
>
> If the ARM then tries to write to a GPIO in such a marked bank,
> the provided callbacks are used to request access from the
> coprocessor driver, which is responsible to doing whatever
> is necessary to "pause" the coprocessor or prevent it from
> trying to use the GPIOs while the ARM is doing its accesses.
>
> During that time, the command source for the bank is temporarily
> switched back to the ARM.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Overall looks fine!

You definately need something like this for handling this special case.

> +#include <linux/gpio/consumer.h>

Why do you need this?

I don't see that you use any functions from it.

> +#include "gpiolib.h"

I'm not so happy about this either, what is this needed for?

It seems to me you can remove both includes, but admittedly I miss
fine details all the time.

Yours,
Linus Walleij
Benjamin Herrenschmidt June 14, 2018, 11:40 p.m. UTC | #4 
On Thu, 2018-06-14 at 10:59 +0200, Linus Walleij wrote:
> 
> Overall looks fine!
> 
> You definately need something like this for handling this special case.
> 
> > +#include <linux/gpio/consumer.h>
> 
> Why do you need this?
> 
> I don't see that you use any functions from it.
> 
> > +#include "gpiolib.h"
> 
> I'm not so happy about this either, what is this needed for?
> 
> It seems to me you can remove both includes, but admittedly I miss
> fine details all the time.

I wish I could :-) This is the main wart in there to be honest.

consumer.h is needed to build gpiolib.h

gpiolib.h is needed for gpio_chip_hwgpio() which needs the definition
of gpio_desc, etc...

I didn't find a simple way out if it... the "API" I expose to the copro
driver takes a gpio_desc which is the clean thing to do but I can't get
to the underlying GPIO number without gpiolib.

Cheers,
Ben.
Benjamin Herrenschmidt June 15, 2018, 6:04 a.m. UTC | #5 
> > +
> > +/**
> > + * aspeed_gpio_copro_grab_gpio - Mark a GPIO used by the coprocessor. The entire
> > + *                               bank gets marked and any access from the ARM will
> > + *                               result in handshaking via callbacks.
> > + * @desc: The GPIO to be marked
> > + */
> > +int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc)
> > +{
> > +       struct gpio_chip *chip = gpiod_to_chip(desc);
> > +       struct aspeed_gpio *gpio = gpiochip_get_data(chip);
> > +       int rc = 0, bindex, offset = gpio_chip_hwgpio(desc);
> > +       const struct aspeed_gpio_bank *bank = to_bank(offset);
> > +       unsigned long flags;
> > +
> > +       if (!gpio->cf_copro_bankmap)
> > +               gpio->cf_copro_bankmap = kzalloc(gpio->config->nr_gpios >> 3, GFP_KERNEL);
> 
> Someone should free this.

Actually no. The driver doesn't have a remove(), so it doesn't.

Cheers,
Ben.
Linus Walleij June 28, 2018, 1:42 p.m. UTC | #6 
On Fri, Jun 15, 2018 at 1:41 AM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Thu, 2018-06-14 at 10:59 +0200, Linus Walleij wrote:
> >
> > Overall looks fine!
> >
> > You definately need something like this for handling this special case.
> >
> > > +#include <linux/gpio/consumer.h>
> >
> > Why do you need this?
> >
> > I don't see that you use any functions from it.
> >
> > > +#include "gpiolib.h"
> >
> > I'm not so happy about this either, what is this needed for?
> >
> > It seems to me you can remove both includes, but admittedly I miss
> > fine details all the time.
>
> I wish I could :-) This is the main wart in there to be honest.
>
> consumer.h is needed to build gpiolib.h
>
> gpiolib.h is needed for gpio_chip_hwgpio() which needs the definition
> of gpio_desc, etc...

Aha I see, you need that because while normally the gpiolib
does the conversion of desc -> offset this is needed here
in a driver.

Maybe we should just name it gpiod_to_offset() or something
and make it a public API.

But this is actually OK, you definately need it here, toss in some
comment or so about what is going on and why this is included
so we know in the future.

Yours,
Linus Walleij
Benjamin Herrenschmidt June 28, 2018, 11:53 p.m. UTC | #7 
On Thu, 2018-06-28 at 15:42 +0200, Linus Walleij wrote:
> 
> Aha I see, you need that because while normally the gpiolib
> does the conversion of desc -> offset this is needed here
> in a driver.
> 
> Maybe we should just name it gpiod_to_offset() or something
> and make it a public API.
>
> But this is actually OK, you definately need it here, toss in some
> comment or so about what is going on and why this is included
> so we know in the future.

Ok, I'll spin a v4 with the interface addition I need to pass the
offsets to the coprocessor and will add that comment some time today.

Cheers,
Ben.
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index 06510634ddd6..323b885dd019 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -12,6 +12,8 @@ 
 #include <asm/div64.h>
 #include <linux/clk.h>
 #include <linux/gpio/driver.h>
+#include <linux/gpio/aspeed.h>
+#include <linux/gpio/consumer.h>
 #include <linux/hashtable.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -22,6 +24,8 @@ 
 #include <linux/spinlock.h>
 #include <linux/string.h>
 
+#include "gpiolib.h"
+
 struct aspeed_bank_props {
 	unsigned int bank;
 	u32 input;
@@ -56,6 +60,7 @@  struct aspeed_gpio {
 	struct clk *clk;
 
 	u32 *dcache;
+	u8 *cf_copro_bankmap;
 };
 
 struct aspeed_gpio_bank {
@@ -72,6 +77,9 @@  struct aspeed_gpio_bank {
 
 static const int debounce_timers[4] = { 0x00, 0x50, 0x54, 0x58 };
 
+static const struct aspeed_gpio_copro_ops *copro_ops;
+static void *copro_data;
+
 static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
 	{
 		.val_regs = 0x0000,
@@ -306,6 +314,50 @@  static void aspeed_gpio_change_cmd_source(struct aspeed_gpio *gpio,
 	iowrite32(reg, c0);
 }
 
+static bool aspeed_gpio_copro_request(struct aspeed_gpio *gpio,
+				      unsigned int offset)
+{
+	const struct aspeed_gpio_bank *bank = to_bank(offset);
+
+	if (!copro_ops || !gpio->cf_copro_bankmap)
+		return false;
+	if (!gpio->cf_copro_bankmap[offset >> 3])
+		return false;
+	if (!copro_ops->request_access)
+		return false;
+
+	/* Pause the coprocessor */
+	copro_ops->request_access(copro_data);
+
+	/* Change command source back to ARM */
+	aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3, GPIO_CMDSRC_ARM);
+
+	/* Update cache */
+	gpio->dcache[GPIO_BANK(offset)] = ioread32(bank_reg(gpio, bank, reg_rdata));
+
+	return true;
+}
+
+static void aspeed_gpio_copro_release(struct aspeed_gpio *gpio,
+				      unsigned int offset)
+{
+	const struct aspeed_gpio_bank *bank = to_bank(offset);
+
+	if (!copro_ops || !gpio->cf_copro_bankmap)
+		return;
+	if (!gpio->cf_copro_bankmap[offset >> 3])
+		return;
+	if (!copro_ops->release_access)
+		return;
+
+	/* Change command source back to ColdFire */
+	aspeed_gpio_change_cmd_source(gpio, bank, offset >> 3,
+				      GPIO_CMDSRC_COLDFIRE);
+
+	/* Restart the coprocessor */
+	copro_ops->release_access(copro_data);
+}
+
 static int aspeed_gpio_get(struct gpio_chip *gc, unsigned int offset)
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
@@ -339,11 +391,15 @@  static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
 {
 	struct aspeed_gpio *gpio = gpiochip_get_data(gc);
 	unsigned long flags;
+	bool copro;
 
 	spin_lock_irqsave(&gpio->lock, flags);
+	copro = aspeed_gpio_copro_request(gpio, offset);
 
 	__aspeed_gpio_set(gc, offset, val);
 
+	if (copro)
+		aspeed_gpio_copro_release(gpio, offset);
 	spin_unlock_irqrestore(&gpio->lock, flags);
 }
 
@@ -351,7 +407,9 @@  static int aspeed_gpio_dir_in(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;
 
 	if (!have_input(gpio, offset))
@@ -359,8 +417,13 @@  static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
 
 	spin_lock_irqsave(&gpio->lock, flags);
 
-	reg = ioread32(bank_reg(gpio, bank, reg_dir));
-	iowrite32(reg & ~GPIO_BIT(offset), bank_reg(gpio, bank, reg_dir));
+	reg = ioread32(addr);
+	reg &= ~GPIO_BIT(offset);
+
+	copro = aspeed_gpio_copro_request(gpio, offset);
+	iowrite32(reg, addr);
+	if (copro)
+		aspeed_gpio_copro_release(gpio, offset);
 
 	spin_unlock_irqrestore(&gpio->lock, flags);
 
@@ -372,7 +435,9 @@  static int aspeed_gpio_dir_out(struct gpio_chip *gc,
 {
 	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;
 
 	if (!have_output(gpio, offset))
@@ -380,10 +445,15 @@  static int aspeed_gpio_dir_out(struct gpio_chip *gc,
 
 	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);
-	reg = ioread32(bank_reg(gpio, bank, reg_dir));
-	iowrite32(reg | GPIO_BIT(offset), bank_reg(gpio, bank, reg_dir));
+	iowrite32(reg, addr);
 
+	if (copro)
+		aspeed_gpio_copro_release(gpio, offset);
 	spin_unlock_irqrestore(&gpio->lock, flags);
 
 	return 0;
@@ -413,24 +483,23 @@  static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 }
 
 static inline int irqd_to_aspeed_gpio_data(struct irq_data *d,
-		struct aspeed_gpio **gpio,
-		const struct aspeed_gpio_bank **bank,
-		u32 *bit)
+					   struct aspeed_gpio **gpio,
+					   const struct aspeed_gpio_bank **bank,
+					   u32 *bit, int *offset)
 {
-	int offset;
 	struct aspeed_gpio *internal;
 
-	offset = irqd_to_hwirq(d);
+	*offset = irqd_to_hwirq(d);
 
 	internal = irq_data_get_irq_chip_data(d);
 
 	/* This might be a bit of a questionable place to check */
-	if (!have_irq(internal, offset))
+	if (!have_irq(internal, *offset))
 		return -ENOTSUPP;
 
 	*gpio = internal;
-	*bank = to_bank(offset);
-	*bit = GPIO_BIT(offset);
+	*bank = to_bank(*offset);
+	*bit = GPIO_BIT(*offset);
 
 	return 0;
 }
@@ -441,17 +510,23 @@  static void aspeed_gpio_irq_ack(struct irq_data *d)
 	struct aspeed_gpio *gpio;
 	unsigned long flags;
 	void __iomem *status_addr;
+	int rc, offset;
+	bool copro;
 	u32 bit;
-	int rc;
 
-	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit);
+	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
 	if (rc)
 		return;
 
 	status_addr = bank_reg(gpio, bank, reg_irq_status);
 
 	spin_lock_irqsave(&gpio->lock, flags);
+	copro = aspeed_gpio_copro_request(gpio, offset);
+
 	iowrite32(bit, status_addr);
+
+	if (copro)
+		aspeed_gpio_copro_release(gpio, offset);
 	spin_unlock_irqrestore(&gpio->lock, flags);
 }
 
@@ -462,15 +537,17 @@  static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
 	unsigned long flags;
 	u32 reg, bit;
 	void __iomem *addr;
-	int rc;
+	int rc, offset;
+	bool copro;
 
-	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit);
+	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
 	if (rc)
 		return;
 
 	addr = bank_reg(gpio, bank, reg_irq_enable);
 
 	spin_lock_irqsave(&gpio->lock, flags);
+	copro = aspeed_gpio_copro_request(gpio, offset);
 
 	reg = ioread32(addr);
 	if (set)
@@ -479,6 +556,8 @@  static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
 		reg &= bit;
 	iowrite32(reg, addr);
 
+	if (copro)
+		aspeed_gpio_copro_release(gpio, offset);
 	spin_unlock_irqrestore(&gpio->lock, flags);
 }
 
@@ -503,9 +582,10 @@  static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
 	struct aspeed_gpio *gpio;
 	unsigned long flags;
 	void __iomem *addr;
-	int rc;
+	int rc, offset;
+	bool copro;
 
-	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit);
+	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit, &offset);
 	if (rc)
 		return -EINVAL;
 
@@ -528,6 +608,7 @@  static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
 	}
 
 	spin_lock_irqsave(&gpio->lock, flags);
+	copro = aspeed_gpio_copro_request(gpio, offset);
 
 	addr = bank_reg(gpio, bank, reg_irq_type0);
 	reg = ioread32(addr);
@@ -544,6 +625,8 @@  static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
 	reg = (reg & ~bit) | type2;
 	iowrite32(reg, addr);
 
+	if (copro)
+		aspeed_gpio_copro_release(gpio, offset);
 	spin_unlock_irqrestore(&gpio->lock, flags);
 
 	irq_set_handler_locked(d, handler);
@@ -638,11 +721,14 @@  static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip,
 	struct aspeed_gpio *gpio = gpiochip_get_data(chip);
 	unsigned long flags;
 	void __iomem *treg;
+	bool copro;
 	u32 val;
 
 	treg = bank_reg(gpio, to_bank(offset), reg_tolerance);
 
 	spin_lock_irqsave(&gpio->lock, flags);
+	copro = aspeed_gpio_copro_request(gpio, offset);
+
 	val = readl(treg);
 
 	if (enable)
@@ -651,6 +737,9 @@  static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip,
 		val &= ~GPIO_BIT(offset);
 
 	writel(val, treg);
+
+	if (copro)
+		aspeed_gpio_copro_release(gpio, offset);
 	spin_unlock_irqrestore(&gpio->lock, flags);
 
 	return 0;
@@ -746,6 +835,9 @@  static void configure_timer(struct aspeed_gpio *gpio, unsigned int offset,
 	void __iomem *addr;
 	u32 val;
 
+	/* Note: Debounce timer isn't under control of the command
+	 * source registers, so no need to sync with the coprocessor
+	 */
 	addr = bank_reg(gpio, bank, reg_debounce_sel1);
 	val = ioread32(addr);
 	iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE1(timer, offset), addr);
@@ -892,6 +984,101 @@  static int aspeed_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
 	return -ENOTSUPP;
 }
 
+/**
+ * aspeed_gpio_copro_set_ops - Sets the callbacks used for handhsaking with
+ *                             the coprocessor for shared GPIO banks
+ * @ops: The callbacks
+ * @data: Pointer passed back to the callbacks
+ */
+int aspeed_gpio_copro_set_ops(const struct aspeed_gpio_copro_ops *ops, void *data)
+{
+	copro_data = data;
+	copro_ops = ops;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(aspeed_gpio_copro_set_ops);
+
+/**
+ * aspeed_gpio_copro_grab_gpio - Mark a GPIO used by the coprocessor. The entire
+ *                               bank gets marked and any access from the ARM will
+ *                               result in handshaking via callbacks.
+ * @desc: The GPIO to be marked
+ */
+int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc)
+{
+	struct gpio_chip *chip = gpiod_to_chip(desc);
+	struct aspeed_gpio *gpio = gpiochip_get_data(chip);
+	int rc = 0, bindex, offset = gpio_chip_hwgpio(desc);
+	const struct aspeed_gpio_bank *bank = to_bank(offset);
+	unsigned long flags;
+
+	if (!gpio->cf_copro_bankmap)
+		gpio->cf_copro_bankmap = kzalloc(gpio->config->nr_gpios >> 3, GFP_KERNEL);
+	if (!gpio->cf_copro_bankmap)
+		return -ENOMEM;
+	if (offset < 0 || offset > gpio->config->nr_gpios)
+		return -EINVAL;
+	bindex = offset >> 3;
+
+	spin_lock_irqsave(&gpio->lock, flags);
+
+	/* Sanity check, this shouldn't happen */
+	if (gpio->cf_copro_bankmap[bindex] == 0xff) {
+		rc = -EIO;
+		goto bail;
+	}
+	gpio->cf_copro_bankmap[bindex]++;
+
+	/* Switch command source */
+	if (gpio->cf_copro_bankmap[bindex] == 1)
+		aspeed_gpio_change_cmd_source(gpio, bank, bindex,
+					      GPIO_CMDSRC_COLDFIRE);
+
+ bail:
+	spin_unlock_irqrestore(&gpio->lock, flags);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(aspeed_gpio_copro_grab_gpio);
+
+/**
+ * aspeed_gpio_copro_release_gpio - Unmark a GPIO used by the coprocessor.
+ * @desc: The GPIO to be marked
+ */
+int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc)
+{
+	struct gpio_chip *chip = gpiod_to_chip(desc);
+	struct aspeed_gpio *gpio = gpiochip_get_data(chip);
+	int rc = 0, bindex, offset = gpio_chip_hwgpio(desc);
+	const struct aspeed_gpio_bank *bank = to_bank(offset);
+	unsigned long flags;
+
+	if (!gpio->cf_copro_bankmap)
+		return -ENXIO;
+
+	if (offset < 0 || offset > gpio->config->nr_gpios)
+		return -EINVAL;
+	bindex = offset >> 3;
+
+	spin_lock_irqsave(&gpio->lock, flags);
+
+	/* Sanity check, this shouldn't happen */
+	if (gpio->cf_copro_bankmap[bindex] == 0) {
+		rc = -EIO;
+		goto bail;
+	}
+	gpio->cf_copro_bankmap[bindex]--;
+
+	/* Switch command source */
+	if (gpio->cf_copro_bankmap[bindex] == 0)
+		aspeed_gpio_change_cmd_source(gpio, bank, bindex,
+					      GPIO_CMDSRC_ARM);
+ bail:
+	spin_unlock_irqrestore(&gpio->lock, flags);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(aspeed_gpio_copro_release_gpio);
+
 /*
  * Any banks not specified in a struct aspeed_bank_props array are assumed to
  * have the properties:
@@ -982,10 +1169,18 @@  static int __init aspeed_gpio_probe(struct platform_device *pdev)
 	if (!gpio->dcache)
 		return -ENOMEM;
 
-	/* Populate it with initial values read from the HW */
+	/*
+	 * Populate it with initial values read from the HW and switch
+	 * all command sources to the ARM by default
+	 */
 	for (i = 0; i < banks; i++) {
-		void __iomem *addr = bank_reg(gpio, &aspeed_gpio_banks[i], reg_rdata);
+		const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
+		void __iomem *addr = bank_reg(gpio, bank, reg_rdata);
 		gpio->dcache[i] = ioread32(addr);
+		aspeed_gpio_change_cmd_source(gpio, bank, 0, GPIO_CMDSRC_ARM);
+		aspeed_gpio_change_cmd_source(gpio, bank, 1, GPIO_CMDSRC_ARM);
+		aspeed_gpio_change_cmd_source(gpio, bank, 2, GPIO_CMDSRC_ARM);
+		aspeed_gpio_change_cmd_source(gpio, bank, 3, GPIO_CMDSRC_ARM);
 	}
 
 	rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
diff --git a/include/linux/gpio/aspeed.h b/include/linux/gpio/aspeed.h
new file mode 100644
index 000000000000..b6871c9b71f7
--- /dev/null
+++ b/include/linux/gpio/aspeed.h
@@ -0,0 +1,14 @@ 
+#ifndef __GPIO_ASPEED_H
+#define __GPIO_ASPEED_H
+
+struct aspeed_gpio_copro_ops {
+	int (*request_access)(void *data);
+	int (*release_access)(void *data);
+};
+
+int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc);
+int aspeed_gpio_copro_release_gpio(struct gpio_desc *desc);
+int aspeed_gpio_copro_set_ops(const struct aspeed_gpio_copro_ops *ops, void *data);
+
+
+#endif /* __GPIO_ASPEED_H */