* [PATCH v4 0/4] gpio: aspeed: Fixes and support for sharing with co-processor
From: Linus Walleij @ 2018-07-02 14:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629041119.5132-1-benh@kernel.crashing.org>
On Fri, Jun 29, 2018 at 6:11 AM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> This series cleans up the register accessors a bit, adds missing ones,
> fixes the access to the write latch and finally adds an interface
> that a co-processor driver can use to change the owner of some of
> the GPIO lines and arbitrate access to shared GPIO banks.
>
> v2: - Address Joel comments
> v3: - Adds Reviewed-by tags
> v4: - Document the reason for including gpiolob.h
> - Expose GPIO offsets & bit number so coprocessor firmware doesn't
> have to hard-code them (allows multi-system generic firwmare
> that gets "configure" by the copro driver)
I've applied the v4 to the "ib-apeed" driver and merged that for
devel, I will push to linux-next once the build servers confirm
they are happy as well.
The immutable branch is here:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-aspeed
Yours,
Linus Walleij
^ permalink raw reply
* [RFC PATCH 01/14] devres: Add devm_of_iomap()
From: Benjamin Herrenschmidt @ 2018-06-30 1:06 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAHp75VfshFLuvOSB4deALX=EUf2Z_RW4Ct-K5cMsAj4yifF-cg@mail.gmail.com>
On Fri, 2018-06-29 at 23:27 +0300, Andy Shevchenko wrote:
> On Fri, Jun 29, 2018 at 12:14 PM, Linus Walleij
> <linus.walleij@linaro.org> wrote:
>
> > I wonder if it is easy to find these cases and replace them with
> > this neat function...
>
> Would be reasonable easy by using coccinelle.
For the obvious ones yes. A lot of the existing users of of_iomap
however don't do the request_region, and while they probably should and
should use the new accessor, this can't be done blindly without
testing, because there are many old things around that have broken
memory region tracking and that will fail..
I plan to do a sweep through some of my old powermac/powerpc stuff one
of these days and do some conversions.
Cheers,
Ben.
^ permalink raw reply
* [RFC PATCH 01/14] devres: Add devm_of_iomap()
From: Andy Shevchenko @ 2018-06-29 20:27 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CACRpkdbhxjahYaVnQN0xqD0NLPouO8VCW8w-GZD0sbLFtFtY5g@mail.gmail.com>
On Fri, Jun 29, 2018 at 12:14 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> I wonder if it is easy to find these cases and replace them with
> this neat function...
Would be reasonable easy by using coccinelle.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [RFC PATCH 01/14] devres: Add devm_of_iomap()
From: Linus Walleij @ 2018-06-29 9:14 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180622043134.18238-2-benh@kernel.crashing.org>
On Fri, Jun 22, 2018 at 6:31 AM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> There are still quite a few cases where a device might want
> to get to a different node of the device-tree, obtain the
> resources and map them.
>
> We have of_iomap() and of_io_request_and_map() but they both
> have shortcomings, such as not returning the size of the
> resource found (which can be useful) and not being "managed".
>
> This adds a devm_of_iomap() that provides all of these and
> should probably replace uses of the above in most drivers.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Ugh I just feel I have seen homecooked solutions to this problem
a few times :/
I wonder if it is easy to find these cases and replace them with
this neat function...
Thanks for doing this.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v4 4/4] gpio: aspeed: Add interfaces for co-processor to grab GPIOs
From: Benjamin Herrenschmidt @ 2018-06-29 4:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629041119.5132-1-benh@kernel.crashing.org>
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>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
---
drivers/gpio/gpio-aspeed.c | 251 +++++++++++++++++++++++++++++++++---
include/linux/gpio/aspeed.h | 15 +++
2 files changed, 246 insertions(+), 20 deletions(-)
create mode 100644 include/linux/gpio/aspeed.h
diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index b3968f66b1d2..1e00f4045f9d 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -12,6 +12,7 @@
#include <asm/div64.h>
#include <linux/clk.h>
#include <linux/gpio/driver.h>
+#include <linux/gpio/aspeed.h>
#include <linux/hashtable.h>
#include <linux/init.h>
#include <linux/io.h>
@@ -22,6 +23,15 @@
#include <linux/spinlock.h>
#include <linux/string.h>
+/*
+ * These two headers aren't meant to be used by GPIO drivers. We need
+ * them in order to access gpio_chip_hwgpio() which we need to implement
+ * the aspeed specific API which allows the coprocessor to request
+ * access to some GPIOs and to arbitrate between coprocessor and ARM.
+ */
+#include <linux/gpio/consumer.h>
+#include "gpiolib.h"
+
struct aspeed_bank_props {
unsigned int bank;
u32 input;
@@ -56,6 +66,7 @@ struct aspeed_gpio {
struct clk *clk;
u32 *dcache;
+ u8 *cf_copro_bankmap;
};
struct aspeed_gpio_bank {
@@ -83,6 +94,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,
@@ -323,6 +337,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);
@@ -356,11 +414,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);
}
@@ -368,7 +430,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))
@@ -376,8 +440,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);
@@ -389,7 +458,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))
@@ -397,10 +468,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;
@@ -430,24 +506,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;
}
@@ -458,17 +533,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);
}
@@ -479,15 +560,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)
@@ -496,6 +579,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);
}
@@ -520,9 +605,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;
@@ -548,6 +634,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);
@@ -564,6 +651,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);
@@ -658,11 +747,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)
@@ -671,6 +763,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;
@@ -766,6 +861,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);
@@ -912,6 +1010,111 @@ 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
+ * @vreg_offset: If non-NULL, returns the value register offset in the GPIO space
+ * @dreg_offset: If non-NULL, returns the data latch register offset in the GPIO space
+ * @bit: If non-NULL, returns the bit number of the GPIO in the registers
+ */
+int aspeed_gpio_copro_grab_gpio(struct gpio_desc *desc,
+ u16 *vreg_offset, u16 *dreg_offset, u8 *bit)
+{
+ 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);
+
+ if (vreg_offset)
+ *vreg_offset = bank->val_regs;
+ if (dreg_offset)
+ *dreg_offset = bank->rdata_reg;
+ if (bit)
+ *bit = GPIO_OFFSET(offset);
+ 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:
@@ -1002,10 +1205,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..1bfb3cdc86d0
--- /dev/null
+++ b/include/linux/gpio/aspeed.h
@@ -0,0 +1,15 @@
+#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,
+ u16 *vreg_offset, u16 *dreg_offset, u8 *bit);
+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 */
--
2.17.1
^ permalink raw reply related
* [PATCH v4 3/4] gpio: aspeed: Add command source registers
From: Benjamin Herrenschmidt @ 2018-06-29 4:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629041119.5132-1-benh@kernel.crashing.org>
This adds the definitions for the command source registers
and a helper to set them.
Those registers allow to control which bus master on the
SoC is allowed to modify a given bank of GPIOs and will
be used by subsequent patches.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/gpio/gpio-aspeed.c | 54 ++++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index a5ded50c6db0..b3968f66b1d2 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -66,6 +66,7 @@ struct aspeed_gpio_bank {
uint16_t irq_regs;
uint16_t debounce_regs;
uint16_t tolerance_regs;
+ uint16_t cmdsrc_regs;
const char names[4][3];
};
@@ -89,6 +90,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
.irq_regs = 0x0008,
.debounce_regs = 0x0040,
.tolerance_regs = 0x001c,
+ .cmdsrc_regs = 0x0060,
.names = { "A", "B", "C", "D" },
},
{
@@ -97,6 +99,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
.irq_regs = 0x0028,
.debounce_regs = 0x0048,
.tolerance_regs = 0x003c,
+ .cmdsrc_regs = 0x0068,
.names = { "E", "F", "G", "H" },
},
{
@@ -105,6 +108,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
.irq_regs = 0x0098,
.debounce_regs = 0x00b0,
.tolerance_regs = 0x00ac,
+ .cmdsrc_regs = 0x0090,
.names = { "I", "J", "K", "L" },
},
{
@@ -113,6 +117,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
.irq_regs = 0x00e8,
.debounce_regs = 0x0100,
.tolerance_regs = 0x00fc,
+ .cmdsrc_regs = 0x00e0,
.names = { "M", "N", "O", "P" },
},
{
@@ -121,6 +126,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
.irq_regs = 0x0118,
.debounce_regs = 0x0130,
.tolerance_regs = 0x012c,
+ .cmdsrc_regs = 0x0110,
.names = { "Q", "R", "S", "T" },
},
{
@@ -129,6 +135,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
.irq_regs = 0x0148,
.debounce_regs = 0x0160,
.tolerance_regs = 0x015c,
+ .cmdsrc_regs = 0x0140,
.names = { "U", "V", "W", "X" },
},
{
@@ -137,6 +144,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
.irq_regs = 0x0178,
.debounce_regs = 0x0190,
.tolerance_regs = 0x018c,
+ .cmdsrc_regs = 0x0170,
.names = { "Y", "Z", "AA", "AB" },
},
{
@@ -145,6 +153,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
.irq_regs = 0x01a8,
.debounce_regs = 0x01c0,
.tolerance_regs = 0x01bc,
+ .cmdsrc_regs = 0x01a0,
.names = { "AC", "", "", "" },
},
};
@@ -161,6 +170,8 @@ enum aspeed_gpio_reg {
reg_debounce_sel1,
reg_debounce_sel2,
reg_tolerance,
+ reg_cmdsrc0,
+ reg_cmdsrc1,
};
#define GPIO_VAL_VALUE 0x00
@@ -175,6 +186,13 @@ enum aspeed_gpio_reg {
#define GPIO_DEBOUNCE_SEL1 0x00
#define GPIO_DEBOUNCE_SEL2 0x04
+#define GPIO_CMDSRC_0 0x00
+#define GPIO_CMDSRC_1 0x04
+#define GPIO_CMDSRC_ARM 0
+#define GPIO_CMDSRC_LPC 1
+#define GPIO_CMDSRC_COLDFIRE 2
+#define GPIO_CMDSRC_RESERVED 3
+
/* This will be resolved at compile time */
static inline void __iomem *bank_reg(struct aspeed_gpio *gpio,
const struct aspeed_gpio_bank *bank,
@@ -203,6 +221,10 @@ static inline void __iomem *bank_reg(struct aspeed_gpio *gpio,
return gpio->base + bank->debounce_regs + GPIO_DEBOUNCE_SEL2;
case reg_tolerance:
return gpio->base + bank->tolerance_regs;
+ case reg_cmdsrc0:
+ return gpio->base + bank->cmdsrc_regs + GPIO_CMDSRC_0;
+ case reg_cmdsrc1:
+ return gpio->base + bank->cmdsrc_regs + GPIO_CMDSRC_1;
}
BUG_ON(1);
}
@@ -269,6 +291,38 @@ static inline bool have_output(struct aspeed_gpio *gpio, unsigned int offset)
return !props || (props->output & GPIO_BIT(offset));
}
+static void aspeed_gpio_change_cmd_source(struct aspeed_gpio *gpio,
+ const struct aspeed_gpio_bank *bank,
+ int bindex, int cmdsrc)
+{
+ void __iomem *c0 = bank_reg(gpio, bank, reg_cmdsrc0);
+ void __iomem *c1 = bank_reg(gpio, bank, reg_cmdsrc1);
+ u32 bit, reg;
+
+ /*
+ * Each register controls 4 banks, so take the bottom 2
+ * bits of the bank index, and use them to select the
+ * right control bit (0, 8, 16 or 24).
+ */
+ bit = BIT((bindex & 3) << 3);
+
+ /* Source 1 first to avoid illegal 11 combination */
+ reg = ioread32(c1);
+ if (cmdsrc & 2)
+ reg |= bit;
+ else
+ reg &= ~bit;
+ iowrite32(reg, c1);
+
+ /* Then Source 0 */
+ reg = ioread32(c0);
+ if (cmdsrc & 1)
+ reg |= bit;
+ else
+ reg &= ~bit;
+ iowrite32(reg, c0);
+}
+
static int aspeed_gpio_get(struct gpio_chip *gc, unsigned int offset)
{
struct aspeed_gpio *gpio = gpiochip_get_data(gc);
--
2.17.1
^ permalink raw reply related
* [PATCH v4 2/4] gpio: aspeed: Add "Read Data" register to read the write latch
From: Benjamin Herrenschmidt @ 2018-06-29 4:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629041119.5132-1-benh@kernel.crashing.org>
The Aspeed GPIO hardware has a quirk: the value register, for an
output GPIO, doesn't contain the last value written (the write
latch content) but the sampled input value.
This means that when reading back shortly after writing, you can
get an incorrect value as the input value is delayed by a few
synchronizers.
The HW supports a separate read-only register "Data Read Register"
which allows you to read the write latch instead.
This adds the definition for it, and uses it for the initial
population of the GPIO value cache. It will be used more in
subsequent patches.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/gpio/gpio-aspeed.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index c9baeeb7f0cc..a5ded50c6db0 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -59,18 +59,33 @@ struct aspeed_gpio {
};
struct aspeed_gpio_bank {
- uint16_t val_regs;
+ uint16_t val_regs; /* +0: Rd: read input value, Wr: set write latch
+ * +4: Rd/Wr: Direction (0=in, 1=out)
+ */
+ uint16_t rdata_reg; /* Rd: read write latch, Wr: <none> */
uint16_t irq_regs;
uint16_t debounce_regs;
uint16_t tolerance_regs;
const char names[4][3];
};
+/*
+ * Note: The "value" register returns the input value sampled on the
+ * line even when the GPIO is configured as an output. Since
+ * that input goes through synchronizers, writing, then reading
+ * back may not return the written value right away.
+ *
+ * The "rdata" register returns the content of the write latch
+ * and thus can be used to read back what was last written
+ * reliably.
+ */
+
static const int debounce_timers[4] = { 0x00, 0x50, 0x54, 0x58 };
static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
{
.val_regs = 0x0000,
+ .rdata_reg = 0x00c0,
.irq_regs = 0x0008,
.debounce_regs = 0x0040,
.tolerance_regs = 0x001c,
@@ -78,6 +93,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
},
{
.val_regs = 0x0020,
+ .rdata_reg = 0x00c4,
.irq_regs = 0x0028,
.debounce_regs = 0x0048,
.tolerance_regs = 0x003c,
@@ -85,6 +101,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
},
{
.val_regs = 0x0070,
+ .rdata_reg = 0x00c8,
.irq_regs = 0x0098,
.debounce_regs = 0x00b0,
.tolerance_regs = 0x00ac,
@@ -92,6 +109,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
},
{
.val_regs = 0x0078,
+ .rdata_reg = 0x00cc,
.irq_regs = 0x00e8,
.debounce_regs = 0x0100,
.tolerance_regs = 0x00fc,
@@ -99,6 +117,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
},
{
.val_regs = 0x0080,
+ .rdata_reg = 0x00d0,
.irq_regs = 0x0118,
.debounce_regs = 0x0130,
.tolerance_regs = 0x012c,
@@ -106,6 +125,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
},
{
.val_regs = 0x0088,
+ .rdata_reg = 0x00d4,
.irq_regs = 0x0148,
.debounce_regs = 0x0160,
.tolerance_regs = 0x015c,
@@ -113,6 +133,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
},
{
.val_regs = 0x01E0,
+ .rdata_reg = 0x00d8,
.irq_regs = 0x0178,
.debounce_regs = 0x0190,
.tolerance_regs = 0x018c,
@@ -120,6 +141,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
},
{
.val_regs = 0x01e8,
+ .rdata_reg = 0x00dc,
.irq_regs = 0x01a8,
.debounce_regs = 0x01c0,
.tolerance_regs = 0x01bc,
@@ -129,6 +151,7 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
enum aspeed_gpio_reg {
reg_val,
+ reg_rdata,
reg_dir,
reg_irq_enable,
reg_irq_type0,
@@ -160,6 +183,8 @@ static inline void __iomem *bank_reg(struct aspeed_gpio *gpio,
switch (reg) {
case reg_val:
return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
case reg_dir:
return gpio->base + bank->val_regs + GPIO_VAL_DIR;
case reg_irq_enable:
@@ -925,7 +950,7 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
/* Populate it with initial values read from the HW */
for (i = 0; i < banks; i++) {
- void __iomem *addr = bank_reg(gpio, &aspeed_gpio_banks[i], reg_val);
+ void __iomem *addr = bank_reg(gpio, &aspeed_gpio_banks[i], reg_rdata);
gpio->dcache[i] = ioread32(addr);
}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 1/4] gpio: aspeed: Rework register type accessors
From: Benjamin Herrenschmidt @ 2018-06-29 4:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629041119.5132-1-benh@kernel.crashing.org>
Use a single accessor function for all register types instead
of several spread around. This will make it easier/cleaner
to introduce new registers and keep the mechanism in one
place.
The big switch/case is optimized at compile time since the
switch value is a constant.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/gpio/gpio-aspeed.c | 118 ++++++++++++++++++++++---------------
1 file changed, 69 insertions(+), 49 deletions(-)
diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index b31ae16170e7..c9baeeb7f0cc 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -127,12 +127,21 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
},
};
-#define GPIO_BANK(x) ((x) >> 5)
-#define GPIO_OFFSET(x) ((x) & 0x1f)
-#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+enum aspeed_gpio_reg {
+ reg_val,
+ reg_dir,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+ reg_debounce_sel1,
+ reg_debounce_sel2,
+ reg_tolerance,
+};
-#define GPIO_DATA 0x00
-#define GPIO_DIR 0x04
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_VAL_DIR 0x04
#define GPIO_IRQ_ENABLE 0x00
#define GPIO_IRQ_TYPE0 0x04
@@ -143,6 +152,40 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
#define GPIO_DEBOUNCE_SEL1 0x00
#define GPIO_DEBOUNCE_SEL2 0x04
+/* This will be resolved at compile time */
+static inline void __iomem *bank_reg(struct aspeed_gpio *gpio,
+ const struct aspeed_gpio_bank *bank,
+ const enum aspeed_gpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_dir:
+ return gpio->base + bank->val_regs + GPIO_VAL_DIR;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ case reg_debounce_sel1:
+ return gpio->base + bank->debounce_regs + GPIO_DEBOUNCE_SEL1;
+ case reg_debounce_sel2:
+ return gpio->base + bank->debounce_regs + GPIO_DEBOUNCE_SEL2;
+ case reg_tolerance:
+ return gpio->base + bank->tolerance_regs;
+ }
+ BUG_ON(1);
+}
+
+#define GPIO_BANK(x) ((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
#define _GPIO_SET_DEBOUNCE(t, o, i) ((!!((t) & BIT(i))) << GPIO_OFFSET(o))
#define GPIO_SET_DEBOUNCE1(t, o) _GPIO_SET_DEBOUNCE(t, o, 1)
#define GPIO_SET_DEBOUNCE2(t, o) _GPIO_SET_DEBOUNCE(t, o, 0)
@@ -201,27 +244,12 @@ static inline bool have_output(struct aspeed_gpio *gpio, unsigned int offset)
return !props || (props->output & GPIO_BIT(offset));
}
-static void __iomem *bank_val_reg(struct aspeed_gpio *gpio,
- const struct aspeed_gpio_bank *bank,
- unsigned int reg)
-{
- return gpio->base + bank->val_regs + reg;
-}
-
-static void __iomem *bank_irq_reg(struct aspeed_gpio *gpio,
- const struct aspeed_gpio_bank *bank,
- unsigned int reg)
-{
- return gpio->base + bank->irq_regs + reg;
-}
-
static int aspeed_gpio_get(struct gpio_chip *gc, unsigned int offset)
{
struct aspeed_gpio *gpio = gpiochip_get_data(gc);
const struct aspeed_gpio_bank *bank = to_bank(offset);
- return !!(ioread32(bank_val_reg(gpio, bank, GPIO_DATA))
- & GPIO_BIT(offset));
+ return !!(ioread32(bank_reg(gpio, bank, reg_val)) & GPIO_BIT(offset));
}
static void __aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
@@ -232,7 +260,7 @@ static void __aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
void __iomem *addr;
u32 reg;
- addr = bank_val_reg(gpio, bank, GPIO_DATA);
+ addr = bank_reg(gpio, bank, reg_val);
reg = gpio->dcache[GPIO_BANK(offset)];
if (val)
@@ -269,8 +297,8 @@ static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
spin_lock_irqsave(&gpio->lock, flags);
- reg = ioread32(bank_val_reg(gpio, bank, GPIO_DIR));
- iowrite32(reg & ~GPIO_BIT(offset), bank_val_reg(gpio, bank, GPIO_DIR));
+ reg = ioread32(bank_reg(gpio, bank, reg_dir));
+ iowrite32(reg & ~GPIO_BIT(offset), bank_reg(gpio, bank, reg_dir));
spin_unlock_irqrestore(&gpio->lock, flags);
@@ -291,8 +319,8 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
spin_lock_irqsave(&gpio->lock, flags);
__aspeed_gpio_set(gc, offset, val);
- reg = ioread32(bank_val_reg(gpio, bank, GPIO_DIR));
- iowrite32(reg | GPIO_BIT(offset), bank_val_reg(gpio, bank, GPIO_DIR));
+ reg = ioread32(bank_reg(gpio, bank, reg_dir));
+ iowrite32(reg | GPIO_BIT(offset), bank_reg(gpio, bank, reg_dir));
spin_unlock_irqrestore(&gpio->lock, flags);
@@ -314,7 +342,7 @@ static int aspeed_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
spin_lock_irqsave(&gpio->lock, flags);
- val = ioread32(bank_val_reg(gpio, bank, GPIO_DIR)) & GPIO_BIT(offset);
+ val = ioread32(bank_reg(gpio, bank, reg_dir)) & GPIO_BIT(offset);
spin_unlock_irqrestore(&gpio->lock, flags);
@@ -358,7 +386,7 @@ static void aspeed_gpio_irq_ack(struct irq_data *d)
if (rc)
return;
- status_addr = bank_irq_reg(gpio, bank, GPIO_IRQ_STATUS);
+ status_addr = bank_reg(gpio, bank, reg_irq_status);
spin_lock_irqsave(&gpio->lock, flags);
iowrite32(bit, status_addr);
@@ -378,7 +406,7 @@ static void aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
if (rc)
return;
- addr = bank_irq_reg(gpio, bank, GPIO_IRQ_ENABLE);
+ addr = bank_reg(gpio, bank, reg_irq_enable);
spin_lock_irqsave(&gpio->lock, flags);
@@ -442,17 +470,17 @@ static int aspeed_gpio_set_type(struct irq_data *d, unsigned int type)
spin_lock_irqsave(&gpio->lock, flags);
- addr = bank_irq_reg(gpio, bank, GPIO_IRQ_TYPE0);
+ addr = bank_reg(gpio, bank, reg_irq_type0);
reg = ioread32(addr);
reg = (reg & ~bit) | type0;
iowrite32(reg, addr);
- addr = bank_irq_reg(gpio, bank, GPIO_IRQ_TYPE1);
+ addr = bank_reg(gpio, bank, reg_irq_type1);
reg = ioread32(addr);
reg = (reg & ~bit) | type1;
iowrite32(reg, addr);
- addr = bank_irq_reg(gpio, bank, GPIO_IRQ_TYPE2);
+ addr = bank_reg(gpio, bank, reg_irq_type2);
reg = ioread32(addr);
reg = (reg & ~bit) | type2;
iowrite32(reg, addr);
@@ -477,7 +505,7 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc)
for (i = 0; i < ARRAY_SIZE(aspeed_gpio_banks); i++) {
const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
- reg = ioread32(bank_irq_reg(data, bank, GPIO_IRQ_STATUS));
+ reg = ioread32(bank_reg(data, bank, reg_irq_status));
for_each_set_bit(p, ®, 32) {
girq = irq_find_mapping(gc->irq.domain, i * 32 + p);
@@ -549,21 +577,21 @@ static int aspeed_gpio_reset_tolerance(struct gpio_chip *chip,
unsigned int offset, bool enable)
{
struct aspeed_gpio *gpio = gpiochip_get_data(chip);
- const struct aspeed_gpio_bank *bank;
unsigned long flags;
+ void __iomem *treg;
u32 val;
- bank = to_bank(offset);
+ treg = bank_reg(gpio, to_bank(offset), reg_tolerance);
spin_lock_irqsave(&gpio->lock, flags);
- val = readl(gpio->base + bank->tolerance_regs);
+ val = readl(treg);
if (enable)
val |= GPIO_BIT(offset);
else
val &= ~GPIO_BIT(offset);
- writel(val, gpio->base + bank->tolerance_regs);
+ writel(val, treg);
spin_unlock_irqrestore(&gpio->lock, flags);
return 0;
@@ -582,13 +610,6 @@ static void aspeed_gpio_free(struct gpio_chip *chip, unsigned int offset)
pinctrl_gpio_free(chip->base + offset);
}
-static inline void __iomem *bank_debounce_reg(struct aspeed_gpio *gpio,
- const struct aspeed_gpio_bank *bank,
- unsigned int reg)
-{
- return gpio->base + bank->debounce_regs + reg;
-}
-
static int usecs_to_cycles(struct aspeed_gpio *gpio, unsigned long usecs,
u32 *cycles)
{
@@ -666,11 +687,11 @@ static void configure_timer(struct aspeed_gpio *gpio, unsigned int offset,
void __iomem *addr;
u32 val;
- addr = bank_debounce_reg(gpio, bank, GPIO_DEBOUNCE_SEL1);
+ addr = bank_reg(gpio, bank, reg_debounce_sel1);
val = ioread32(addr);
iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE1(timer, offset), addr);
- addr = bank_debounce_reg(gpio, bank, GPIO_DEBOUNCE_SEL2);
+ addr = bank_reg(gpio, bank, reg_debounce_sel2);
val = ioread32(addr);
iowrite32((val & ~mask) | GPIO_SET_DEBOUNCE2(timer, offset), addr);
}
@@ -904,9 +925,8 @@ static int __init aspeed_gpio_probe(struct platform_device *pdev)
/* Populate it with initial values read from the HW */
for (i = 0; i < banks; i++) {
- const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i];
- gpio->dcache[i] = ioread32(gpio->base + bank->val_regs +
- GPIO_DATA);
+ void __iomem *addr = bank_reg(gpio, &aspeed_gpio_banks[i], reg_val);
+ gpio->dcache[i] = ioread32(addr);
}
rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
--
2.17.1
^ permalink raw reply related
* [PATCH v4 0/4] gpio: aspeed: Fixes and support for sharing with co-processor
From: Benjamin Herrenschmidt @ 2018-06-29 4:11 UTC (permalink / raw)
To: linux-aspeed
This series cleans up the register accessors a bit, adds missing ones,
fixes the access to the write latch and finally adds an interface
that a co-processor driver can use to change the owner of some of
the GPIO lines and arbitrate access to shared GPIO banks.
v2: - Address Joel comments
v3: - Adds Reviewed-by tags
v4: - Document the reason for including gpiolob.h
- Expose GPIO offsets & bit number so coprocessor firmware doesn't
have to hard-code them (allows multi-system generic firwmare
that gets "configure" by the copro driver)
Please consider putting this in a topic branch so I can pull it
back into dependent series and point the reviewers to dependencies
^ permalink raw reply
* [PATCH 7/7] arm: dts: aspeed: Enable vhub on port A of AST2500 EVB
From: Benjamin Herrenschmidt @ 2018-06-29 3:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629035106.27181-1-benh@kernel.crashing.org>
This is an eval board, it makes sense to enable many
functions by default. This changes the device-tree to
set port A to be a USB device and leave port B as a
host, along with a little comment explaining how to
change it.
(the vhub device can only exist on port A on this SoC)
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/arm/boot/dts/aspeed-ast2500-evb.dts | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
index 2bff1b253842..2375449c02d0 100644
--- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
@@ -80,7 +80,13 @@
};
};
-&ehci0 {
+/*
+ * Enable port A as device (via the virtual hub) and port B as
+ * host by default on the eval board. This can be easily changed
+ * by replacing the override below with &ehci0 { ... } to enable
+ * host on both ports.
+ */
+&vhub {
status = "okay";
};
--
2.17.1
^ permalink raw reply related
* [PATCH 6/7] arm: configs: Add USB gadget to Aspeed G5 defconfig
From: Benjamin Herrenschmidt @ 2018-06-29 3:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629035106.27181-1-benh@kernel.crashing.org>
Now that the vhub driver is upstream and the device-trees
updated, let's enable this by default.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/arm/configs/aspeed_g5_defconfig | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index 8c7ea033cdc2..38e9b2d43df3 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -140,6 +140,23 @@ CONFIG_USB_DYNAMIC_MINORS=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_EHCI_ROOT_HUB_TT=y
CONFIG_USB_EHCI_HCD_PLATFORM=y
+CONFIG_USB_GADGET=y
+CONFIG_U_SERIAL_CONSOLE=y
+CONFIG_USB_ASPEED_VHUB=y
+CONFIG_USB_CONFIGFS=y
+CONFIG_USB_CONFIGFS_SERIAL=y
+CONFIG_USB_CONFIGFS_ACM=y
+CONFIG_USB_CONFIGFS_OBEX=y
+CONFIG_USB_CONFIGFS_NCM=y
+CONFIG_USB_CONFIGFS_ECM=y
+CONFIG_USB_CONFIGFS_ECM_SUBSET=y
+CONFIG_USB_CONFIGFS_RNDIS=y
+CONFIG_USB_CONFIGFS_EEM=y
+CONFIG_USB_CONFIGFS_MASS_STORAGE=y
+CONFIG_USB_CONFIGFS_F_LB_SS=y
+CONFIG_USB_CONFIGFS_F_FS=y
+CONFIG_USB_CONFIGFS_F_HID=y
+CONFIG_USB_CONFIGFS_F_PRINTER=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=y
CONFIG_LEDS_CLASS_FLASH=y
--
2.17.1
^ permalink raw reply related
* [PATCH 5/7] arm: configs: Add USB gadget to Aspeed G4 defconfig
From: Benjamin Herrenschmidt @ 2018-06-29 3:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629035106.27181-1-benh@kernel.crashing.org>
Now that the vhub driver is upstream and the device-trees
updated, let's enable this by default.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/arm/configs/aspeed_g4_defconfig | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm/configs/aspeed_g4_defconfig b/arch/arm/configs/aspeed_g4_defconfig
index 95946dee9c77..be714ea088ed 100644
--- a/arch/arm/configs/aspeed_g4_defconfig
+++ b/arch/arm/configs/aspeed_g4_defconfig
@@ -138,6 +138,23 @@ CONFIG_USB_DYNAMIC_MINORS=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_EHCI_ROOT_HUB_TT=y
CONFIG_USB_EHCI_HCD_PLATFORM=y
+CONFIG_USB_GADGET=y
+CONFIG_U_SERIAL_CONSOLE=y
+CONFIG_USB_ASPEED_VHUB=y
+CONFIG_USB_CONFIGFS=y
+CONFIG_USB_CONFIGFS_SERIAL=y
+CONFIG_USB_CONFIGFS_ACM=y
+CONFIG_USB_CONFIGFS_OBEX=y
+CONFIG_USB_CONFIGFS_NCM=y
+CONFIG_USB_CONFIGFS_ECM=y
+CONFIG_USB_CONFIGFS_ECM_SUBSET=y
+CONFIG_USB_CONFIGFS_RNDIS=y
+CONFIG_USB_CONFIGFS_EEM=y
+CONFIG_USB_CONFIGFS_MASS_STORAGE=y
+CONFIG_USB_CONFIGFS_F_LB_SS=y
+CONFIG_USB_CONFIGFS_F_FS=y
+CONFIG_USB_CONFIGFS_F_HID=y
+CONFIG_USB_CONFIGFS_F_PRINTER=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=y
CONFIG_LEDS_CLASS_FLASH=y
--
2.17.1
^ permalink raw reply related
* [PATCH 4/7] arm: dts: aspeed: Add Aspeed 54 USB Virtual Hub
From: Benjamin Herrenschmidt @ 2018-06-29 3:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629035106.27181-1-benh@kernel.crashing.org>
This adds the (disabled by default) device node for the
Aspeed virtual hub,a long with clocks and pinmux.
This also adds the missing pinmux definition for it
(the kernel driver already knows about it).
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/arm/boot/dts/aspeed-g5.dtsi | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index dfdc239b86f6..6274d3eaf374 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -176,6 +176,16 @@
*/
};
+ vhub: usb-vhub at 1e6a0000 {
+ compatible = "aspeed,ast2500-usb-vhub";
+ reg = <0x1e6a0000 0x300>;
+ interrupts = <5>;
+ clocks = <&syscon ASPEED_CLK_GATE_USBPORT1CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb2ad_default>;
+ status = "disabled";
+ };
+
apb {
compatible = "simple-bus";
#address-cells = <1>;
@@ -1425,6 +1435,11 @@
groups = "USB2AH";
};
+ pinctrl_usb2ad_default: usb2ad_default {
+ function = "USB2AD";
+ groups = "USB2AD";
+ };
+
pinctrl_usb11bhid_default: usb11bhid_default {
function = "USB11BHID";
groups = "USB11BHID";
--
2.17.1
^ permalink raw reply related
* [PATCH 3/7] arm: dts: aspeed: Add Aspeed G4 USB Virtual Hub
From: Benjamin Herrenschmidt @ 2018-06-29 3:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629035106.27181-1-benh@kernel.crashing.org>
This adds the (disabled by default) device node for the
Aspeed virtual hub,a long with clocks and pinmux.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/arm/boot/dts/aspeed-g4.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index 1d7ffa9fdb11..54524564037c 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -131,6 +131,16 @@
*/
};
+ vhub: usb-vhub at 1e6a0000 {
+ compatible = "aspeed,ast2400-usb-vhub";
+ reg = <0x1e6a0000 0x300>;
+ interrupts = <5>;
+ clocks = <&syscon ASPEED_CLK_GATE_USBPORT1CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb2d_default>;
+ status = "disabled";
+ };
+
apb {
compatible = "simple-bus";
#address-cells = <1>;
--
2.17.1
^ permalink raw reply related
* [PATCH 2/7] arm: dts: aspeed: Add aspeed G5 USB host pinmux
From: Benjamin Herrenschmidt @ 2018-06-29 3:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629035106.27181-1-benh@kernel.crashing.org>
Set the default pinmux for EHCIs so boards don't have to do
it an document why it is not set for UHCI.
Remove the properties from the AST2500 EVB board which are
now redundant
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/arm/boot/dts/aspeed-ast2500-evb.dts | 6 ------
arch/arm/boot/dts/aspeed-g5.dtsi | 8 ++++++++
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
index ede11c597673..2bff1b253842 100644
--- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
@@ -82,18 +82,12 @@
&ehci0 {
status = "okay";
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_usb2ah_default>;
};
&ehci1 {
status = "okay";
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_usb2bh_default>;
};
&uhci {
status = "okay";
-
- /* No pinctrl, this follows the above EHCI settings */
};
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 17f2714d18a7..dfdc239b86f6 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -148,6 +148,8 @@
reg = <0x1e6a1000 0x100>;
interrupts = <5>;
clocks = <&syscon ASPEED_CLK_GATE_USBPORT1CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb2ah_default>;
status = "disabled";
};
@@ -156,6 +158,8 @@
reg = <0x1e6a3000 0x100>;
interrupts = <13>;
clocks = <&syscon ASPEED_CLK_GATE_USBPORT2CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb2bh_default>;
status = "disabled";
};
@@ -166,6 +170,10 @@
#ports = <2>;
clocks = <&syscon ASPEED_CLK_GATE_USBUHCICLK>;
status = "disabled";
+ /*
+ * No default pinmux, it will follow EHCI, use an explicit pinmux
+ * override if you don't enable EHCI
+ */
};
apb {
--
2.17.1
^ permalink raw reply related
* [PATCH 1/7] arm: dts: aspeed: Add aspeed G4 USB pinmux
From: Benjamin Herrenschmidt @ 2018-06-29 3:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180629035106.27181-1-benh@kernel.crashing.org>
Set the default pinmux for EHCI so boards don't have to do
it an document why it is not set for UHCI.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/arm/boot/dts/aspeed-g4.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index 75df1573380e..1d7ffa9fdb11 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -113,6 +113,8 @@
reg = <0x1e6a1000 0x100>;
interrupts = <5>;
clocks = <&syscon ASPEED_CLK_GATE_USBPORT1CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb2h_default>;
status = "disabled";
};
@@ -123,6 +125,10 @@
#ports = <3>;
clocks = <&syscon ASPEED_CLK_GATE_USBUHCICLK>;
status = "disabled";
+ /*
+ * No default pinmux, it will follow EHCI, use an explicit pinmux
+ * override if you don't enable EHCI
+ */
};
apb {
--
2.17.1
^ permalink raw reply related
* [PATCH 0/7] arm: Aspeed DT & config updates for USB
From: Benjamin Herrenschmidt @ 2018-06-29 3:50 UTC (permalink / raw)
To: linux-aspeed
This updates the device-tree of the Aspeed SoC to better define
the pinmux for the USB host and to add the missing "Virtual Hub"
USB device.
We also enable USB device in the defconfigs and configure a
port as device on the evaluation board.
^ permalink raw reply
* [PATCH 4/4] gpio: aspeed: Add interfaces for co-processor to grab GPIOs
From: Benjamin Herrenschmidt @ 2018-06-28 23:53 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CACRpkdYjUX4JtEW0+0Gxjfp5gSNKp8PM7CPY_nf=nQb=rfHRBQ@mail.gmail.com>
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.
^ permalink raw reply
* [PATCH] clk: aspeed: Support HPLL strapping on ast2400
From: Joel Stanley @ 2018-06-28 23:15 UTC (permalink / raw)
To: linux-aspeed
The HPLL can be configured through a register (SCU24), however some
platforms chose to configure it through the strapping settings and do
not use the register. This was not noticed as the logic for bit 18 in
SCU24 was confused: set means programmed, but the driver read it as set
means strapped.
This gives us the correct HPLL value on Palmetto systems, from which
most of the peripheral clocks are generated.
Fixes: 5eda5d79e4be ("clk: Add clock driver for ASPEED BMC SoCs")
Cc: stable at vger.kernel.org # v4.15
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/clk/clk-aspeed.c | 42 +++++++++++++++++++++++++++-------------
1 file changed, 29 insertions(+), 13 deletions(-)
diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
index 38b366b00c57..2ef4ad7bdbdc 100644
--- a/drivers/clk/clk-aspeed.c
+++ b/drivers/clk/clk-aspeed.c
@@ -24,7 +24,7 @@
#define ASPEED_MPLL_PARAM 0x20
#define ASPEED_HPLL_PARAM 0x24
#define AST2500_HPLL_BYPASS_EN BIT(20)
-#define AST2400_HPLL_STRAPPED BIT(18)
+#define AST2400_HPLL_PROGRAMMED BIT(18)
#define AST2400_HPLL_BYPASS_EN BIT(17)
#define ASPEED_MISC_CTRL 0x2c
#define UART_DIV13_EN BIT(12)
@@ -565,29 +565,45 @@ builtin_platform_driver(aspeed_clk_driver);
static void __init aspeed_ast2400_cc(struct regmap *map)
{
struct clk_hw *hw;
- u32 val, freq, div;
+ u32 val, div, clkin, hpll;
+ const u16 hpll_rates[][4] = {
+ {384, 360, 336, 408},
+ {400, 375, 350, 425},
+ };
+ int rate;
/*
* CLKIN is the crystal oscillator, 24, 48 or 25MHz selected by
* strapping
*/
regmap_read(map, ASPEED_STRAP, &val);
- if (val & CLKIN_25MHZ_EN)
- freq = 25000000;
- else if (val & AST2400_CLK_SOURCE_SEL)
- freq = 48000000;
- else
- freq = 24000000;
- hw = clk_hw_register_fixed_rate(NULL, "clkin", NULL, 0, freq);
- pr_debug("clkin @%u MHz\n", freq / 1000000);
+ rate = (val >> 8) & 3;
+ if (val & CLKIN_25MHZ_EN) {
+ clkin = 25000000;
+ hpll = hpll_rates[1][rate];
+ } else if (val & AST2400_CLK_SOURCE_SEL) {
+ clkin = 48000000;
+ hpll = hpll_rates[0][rate];
+ } else {
+ clkin = 24000000;
+ hpll = hpll_rates[0][rate];
+ }
+ hw = clk_hw_register_fixed_rate(NULL, "clkin", NULL, 0, clkin);
+ pr_debug("clkin @%u MHz\n", clkin / 1000000);
/*
* High-speed PLL clock derived from the crystal. This the CPU clock,
- * and we assume that it is enabled
+ * and we assume that it is enabled. It can be configured through the
+ * HPLL_PARAM register, or set to a specified frequency by strapping.
*/
regmap_read(map, ASPEED_HPLL_PARAM, &val);
- WARN(val & AST2400_HPLL_STRAPPED, "hpll is strapped not configured");
- aspeed_clk_data->hws[ASPEED_CLK_HPLL] = aspeed_ast2400_calc_pll("hpll", val);
+ if (val & AST2400_HPLL_PROGRAMMED)
+ hw = aspeed_ast2400_calc_pll("hpll", val);
+ else
+ hw = clk_hw_register_fixed_rate(NULL, "hpll", "clkin", 0,
+ hpll * 1000000);
+
+ aspeed_clk_data->hws[ASPEED_CLK_HPLL] = hw;
/*
* Strap bits 11:10 define the CPU/AHB clock frequency ratio (aka HCLK)
--
2.17.1
^ permalink raw reply related
* [PATCH] pinctrl: aspeed: Fix documentation
From: Linus Walleij @ 2018-06-28 14:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180618124506.12961-1-joel@jms.id.au>
On Mon, Jun 18, 2018 at 2:45 PM Joel Stanley <joel@jms.id.au> wrote:
> Fixes these warnings:
>
> pinctrl-aspeed.c:112: warning: Function parameter or member 'map' not
> described in 'aspeed_sig_desc_eval'
> pinctrl-aspeed.c:112: warning: Excess function parameter 'regmap'
> description in 'aspeed_sig_desc_eval'
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Patch applied with Andrew's ACK.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 4/4] gpio: aspeed: Add interfaces for co-processor to grab GPIOs
From: Linus Walleij @ 2018-06-28 13:42 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <480dd5a586acfaaeddcd099e3f1854f27fa44522.camel@kernel.crashing.org>
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
^ permalink raw reply
* [PATCH v3 4/4] gpio: aspeed: Add interfaces for co-processor to grab GPIOs
From: Benjamin Herrenschmidt @ 2018-06-28 6:33 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180622020448.6102-5-benh@kernel.crashing.org>
On Fri, 2018-06-22 at 12:04 +1000, Benjamin Herrenschmidt 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).
At the risk of offending people even more, I'm going to respin
this with another extra Aspeed specific interface to retrieve
a given GPIO registers and bit number.
So far, I've made the corprocessor code system specific, effectively
hard wiring in the binary the above. But I found a way to make it
more flexible without losing too much performance, thus drastically
simplifying deployment for us since every machine out there seems
to be wiring these things differently.
However, for that to work, after "grabbing" the GPIOs, the coprocessor
driver will need to give the data and write data latch registers and
bit numbers to the microcode.
I'll respin a v4 with that added interface in the form of a separate
patch at the end of the series.
Cheers,
Ben.
^ permalink raw reply
* [PATCH 12/14] fsi: master-ast-cf: Add new FSI master using Aspeed ColdFire
From: Joel Stanley @ 2018-06-28 5:03 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-13-benh@kernel.crashing.org>
Hi Ben,
On 27 June 2018 at 08:56, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> The Aspeed AST2x00 can contain a ColdFire v1 coprocessor which
> is currently unused on OpenPower systems.
>
> This adds an alternative to the fsi-master-gpio driver that
> uses that coprocessor instead of bit banging from the ARM
> core itself. The end result is about 4 times faster.
>
> The firmware for the coprocessor and its source code can be
> found at https://github.com/ozbenh/cf-fsi and is system specific.
>
> Currently tested on Romulus and Palmetto systems.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Nice work. I gave this a spin on Romulus and it looked good.
If you run it through sparse there are a bunch of things to fix. I've
also got some comments below.
> --- /dev/null
> +++ b/drivers/fsi/cf-fsi-fw.h
> @@ -0,0 +1,131 @@
Copyright?
> +#ifndef __CF_FSI_FW_H
> +#define __CF_FSI_FW_H
> +
> +/*
> + * uCode file layout
> + *
> + * 0000...03ff : m68k exception vectors
> + * 0400...04ff : Header info & boot config block
> + * 0500....... : Code & stack
> + */
> diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c
> new file mode 100644
> index 000000000000..6b17f27c27f6
> --- /dev/null
> +++ b/drivers/fsi/fsi-master-ast-cf.c
> @@ -0,0 +1,1376 @@
> +// SPDX-License-Identifier: GPL-2.0
Normally 2+ for new IBM code? You also need something like this on the
next line:
// Copyright 2018 IBM Corp
> +static int read_copro_response(struct fsi_master_acf *master, uint8_t size,
> + __be32 *response, u8 *tag)
> +{
> + u8 rtag = readb(master->sram + STAT_RTAG);
> + u8 rcrc = readb(master->sram + STAT_RCRC);
> + __be32 rdata = 0;
> + u32 crc;
> + u8 ack;
> +
> + *tag = ack = rtag & 3;
> +
> + /* we have a whole message now; check CRC */
> + crc = crc4(0, 1, 1);
> + crc = crc4(crc, rtag, 4);
> + if (ack == FSI_RESP_ACK && size) {
> + rdata = readl(master->sram + RSP_DATA);
> + crc = crc4(crc, be32_to_cpu(rdata), 32);
> + if (response)
> + *response = rdata;
> + }
> + crc = crc4(crc, rcrc, 4);
> +
> + trace_fsi_master_acf_copro_response(master, rtag, rcrc, rdata, crc == 0);
> +
> + if (crc) {
> + /*
> + * Check if it's all 1's or all 0's, that probably means
> + * the host is off
> + */
> + if ((rtag == 0xf && rcrc == 0xf) || (rtag == 0 && rcrc == 0))
> + return -ENODEV;
> + dev_dbg(master->dev, "Bad response CRC !\n");
> + return -EAGAIN;
> + }
> + return 0;
> +}
> +
> +static int send_term(struct fsi_master_acf *master, uint8_t slave)
> +{
> + struct fsi_msg cmd;
> + uint8_t tag;
> + int rc;
> +
> + build_term_command(&cmd, slave);
> +
> + rc = send_request(master, &cmd, true);
> + if (rc) {
> + dev_warn(master->dev, "Error %d sending term\n", rc);
> + return rc;
> + }
> +
> + rc = read_copro_response(master, 0, NULL, &tag);
> + if (rc < 0) {
> + dev_err(master->dev,
> + "TERM failed; lost communication with slave\n");
> + return -EIO;
> + } else if (tag != FSI_RESP_ACK) {
> + dev_err(master->dev, "TERM failed; response %d\n", tag);
> + return -EIO;
> + }
> + return 0;
> +}
> +
> +static void dump_trace(struct fsi_master_acf *master)
> +{
> + char trbuf[52];
I was checking that this was big enough.
52 = 16 * 3 + '\n' + \0' = 50?
Looks to be okay.
> + char *p;
> + int i;
> +
> + dev_dbg(master->dev,
> + "CMDSTAT:%08x RTAG=%02x RCRC=%02x RDATA=%02x #INT=%08x\n",
> + be32_to_cpu(readl(master->sram + CMD_STAT_REG)),
> + readb(master->sram + STAT_RTAG),
> + readb(master->sram + STAT_RCRC),
> + be32_to_cpu(readl(master->sram + RSP_DATA)),
> + be32_to_cpu(readl(master->sram + INT_CNT)));
> +
> + for (i = 0; i < 512; i++) {
> + uint8_t v;
> + if ((i % 16) == 0)
> + p = trbuf;
> + v = readb(master->sram + TRACEBUF + i);
> + p += sprintf(p, "%02x ", v);
> + if (((i % 16) == 15) || v == TR_END)
> + dev_dbg(master->dev, "%s\n", trbuf);
> + if (v == TR_END)
> + break;
> + }
> +}
> +
> +static int handle_response(struct fsi_master_acf *master,
> + uint8_t slave, uint8_t size, void *data)
> +{
> + int busy_count = 0, rc;
> + int crc_err_retries = 0;
> + struct fsi_msg cmd;
> + __be32 response;
> + uint8_t tag;
> +retry:
> + rc = read_copro_response(master, size, &response, &tag);
> +
> + /* Handle retries on CRC errors */
> + if (rc == -EAGAIN) {
> + /* Too many retries ? */
> + if (crc_err_retries++ > FSI_CRC_ERR_RETRIES) {
> + /*
> + * Pass it up as a -EIO otherwise upper level will retry
> + * the whole command which isn't what we want here.
> + */
> + rc = -EIO;
> + goto bail;
> + }
> + trace_fsi_master_acf_crc_rsp_error(master, crc_err_retries);
> + if (master->trace_enabled)
> + dump_trace(master);
> + rc = clock_zeros(master, FSI_MASTER_EPOLL_CLOCKS);
> + if (rc) {
> + dev_warn(master->dev,
> + "Error %d clocking zeros for E_POLL\n", rc);
> + return rc;
> + }
> + build_epoll_command(&cmd, slave);
> + rc = send_request(master, &cmd, size == 0);
> + if (rc) {
> + dev_warn(master->dev, "Error %d sending E_POLL\n", rc);
> + return -EIO;
> + }
> + goto retry;
> + }
> + if (rc)
> + return rc;
> +
> + switch (tag) {
> + case FSI_RESP_ACK:
> + if (size && data) {
> + if (size == 4)
> + *(__be32 *)data = response;
> + else if (size == 2)
> + *(__be16 *)data = response;
> + else
> + *(u8 *)data = response;
Response is a u32, the idea here is to discard the top two or three byes?
> +
> +static int fsi_master_acf_setup(struct fsi_master_acf *master)
> +{
> + int timeout, rc;
> + u32 val;
> +
> +
> + /* Wait for status register to indicate command completion
> + * which signals the initialization is complete
> + */
> + for (timeout = 0; timeout < 10; timeout++) {
> + val = readb(master->sram + CF_STARTED);
> + if (val)
> + break;
> + msleep(1);
> + };
drivers/fsi/fsi-master-ast-cf.c:920:2-3: Unneeded semicolon
> + if (!val) {
> + dev_err(master->dev, "Coprocessor startup timeout !\n");
> + rc = -ENODEV;
> + goto err;
> + }
> +
> + /* Configure echo & send delay */
> + writeb(master->t_send_delay, master->sram + SEND_DLY_REG);
> + writeb(master->t_echo_delay, master->sram + ECHO_DLY_REG);
> +
> + /* Enable SW interrupt to copro if any */
> + if (master->cvic) {
> + rc = copro_enable_sw_irq(master);
> + if (rc)
> + goto err;
> + }
> + return 0;
> + err:
> + /* An error occurred, don't leave the coprocessor running */
> + reset_cf(master);
> +
> + /* Release the GPIOs */
> + release_copro_gpios(master);
> +
> + return rc;
> +}
> +static int fsi_master_acf_gpio_request(void *data)
> +{
> + struct fsi_master_acf *master = data;
> + int timeout;
> + u8 val;
> +
> + /* Note: This doesn't require holding out mutex */
> +
> + /* Write reqest */
> + writeb(ARB_ARM_REQ, master->sram + ARB_REG);
> +
> + /* Read back to avoid ordering issue */
> + (void)readb(master->sram + ARB_REG);
> +
> + /*
> + * There is a race (which does happen at boot time) when we get an
> + * arbitration request as we are either about to or just starting
> + * the coprocessor.
> + *
> + * To handle it, we first check if we are running. If not yet we
> + * check whether the copro is started in the SCU.
> + *
> + * If it's not started, we can basically just assume we have arbitration
> + * and return. Otherwise, we wait normally expecting for the arbitration
> + * to eventually complete.
> + */
> + if (readl(master->sram + CF_STARTED) == 0) {
> + unsigned int reg = 0;
> +
> + regmap_read(master->scu, SCU_COPRO_CTRL, ®);
> + if (!reg & SCU_COPRO_CLK_EN)
Is this correct? Looks like it might be missing some ( )
> + return 0;
> + }
> +
> + /* Ring doorbell if any */
> + if (master->cvic)
> + writel(0x2, master->cvic + CVIC_TRIG_REG);
> +
> + for (timeout = 0; timeout < 10000; timeout++) {
> + val = readb(master->sram + ARB_REG);
> + if (val != ARB_ARM_REQ)
> + break;
> + udelay(1);
> + }
> +
> + /* If it failed, override anyway */
> + if (val != ARB_ARM_ACK)
> + dev_warn(master->dev, "GPIO request arbitration timeout\n");
> +
> + return 0;
> +}
> +
^ permalink raw reply
* [PATCH 14/14] arm: dts: OpenPower Palmetto system can use coprocessor for FSI
From: Joel Stanley @ 2018-06-28 4:13 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-15-benh@kernel.crashing.org>
On 27 June 2018 at 08:56, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> This switches away from userspace bitbanging to kernel FSI
> using the coprocessor.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
As with the other patch, I will take this through the ASPEED SoC tree
once we've got acks on the bindings.
Cheers,
Joel
^ permalink raw reply
* [PATCH 13/14] arm: dts: OpenPower Romulus system can use coprocessor for FSI
From: Joel Stanley @ 2018-06-28 4:12 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20180626232605.13420-14-benh@kernel.crashing.org>
On 27 June 2018 at 08:56, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
I will take this through the ASPEED SoC tree once we've got acks on
the bindings.
Cheers,
Joel
> ---
> arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> index 347938673c83..070b8f8f1d62 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> @@ -37,6 +37,11 @@
> compatible = "shared-dma-pool";
> reusable;
> };
> +
> + coldfire_memory: codefire_memory at 9ef00000 {
> + reg = <0x9ef00000 0x00100000>;
> + no-map;
> + };
> };
>
> leds {
> @@ -56,10 +61,16 @@
> };
>
> fsi: gpio-fsi {
> - compatible = "fsi-master-gpio", "fsi-master";
> + compatible = "ibm,fsi-master-ast2500-cf", "fsi-master";
> #address-cells = <2>;
> #size-cells = <0>;
> - no-gpio-delays;
> +
> + memory-region = <&coldfire_memory>;
> + sram = <&sram>;
> + cvic = <&cvic>;
> +
> + fw-name = "romulus";
> + fw-platform-sig = <0x526d>;
>
> clock-gpios = <&gpio ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
> data-gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_HIGH>;
> --
> 2.17.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox